[PATCH v2 0/4] usb: dwc2: Make dwc2 endianness agnostic
This series contains patches which are make dwc2 core endianness agnostic Changes from v1: Rebased to latest balbi/next. Changes from v0: Moved dwc2_check_core_endianness() call after devm_ioremap_resource() to avoid ioread32() call on null pointer. Gevorg Sahakyan (4): usb: dwc2: Move dwc2_readl/writel functions after hsotg structure usb: dwc2: Modify dwc2_readl/writel functions prototype usb: dwc2: replace ioread32/iowrite32_rep with dwc2_readl/writel_rep usb: dwc2: Make dwc2_readl/writel functions endianness-agnostic. drivers/usb/dwc2/core.c | 241 ++-- drivers/usb/dwc2/core.h | 115 +- drivers/usb/dwc2/core_intr.c | 118 +- drivers/usb/dwc2/debugfs.c | 55 +++-- drivers/usb/dwc2/gadget.c| 514 +-- drivers/usb/dwc2/hcd.c | 461 +++--- drivers/usb/dwc2/hcd.h | 10 +- drivers/usb/dwc2/hcd_ddma.c | 10 +- drivers/usb/dwc2/hcd_intr.c | 96 drivers/usb/dwc2/hcd_queue.c | 10 +- drivers/usb/dwc2/params.c| 20 +- drivers/usb/dwc2/platform.c | 19 ++ 12 files changed, 843 insertions(+), 826 deletions(-) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] usb: dwc2: Move dwc2_readl/writel functions after hsotg structure
Moved dwc2_readl/writel functions after hsotg declaration for adding hsotg structure to dwc2_readl/writel function prototypes. Signed-off-by: Gevorg Sahakyan --- drivers/usb/dwc2/core.h | 108 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 4a56ac772a3c..a645508fffe0 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -65,60 +65,6 @@ DWC2_TRACE_SCHEDULER_VB(pr_fmt("%s: SCH: " fmt),\ dev_name(hsotg->dev), ##__VA_ARGS__) -#ifdef CONFIG_MIPS -/* - * There are some MIPS machines that can run in either big-endian - * or little-endian mode and that use the dwc2 register without - * a byteswap in both ways. - * Unlike other architectures, MIPS apparently does not require a - * barrier before the __raw_writel() to synchronize with DMA but does - * require the barrier after the __raw_writel() to serialize a set of - * writes. This set of operations was added specifically for MIPS and - * should only be used there. - */ -static inline u32 dwc2_readl(const void __iomem *addr) -{ - u32 value = __raw_readl(addr); - - /* In order to preserve endianness __raw_* operation is used. Therefore -* a barrier is needed to ensure IO access is not re-ordered across -* reads or writes -*/ - mb(); - return value; -} - -static inline void dwc2_writel(u32 value, void __iomem *addr) -{ - __raw_writel(value, addr); - - /* -* In order to preserve endianness __raw_* operation is used. Therefore -* a barrier is needed to ensure IO access is not re-ordered across -* reads or writes -*/ - mb(); -#ifdef DWC2_LOG_WRITES - pr_info("INFO:: wrote %08x to %p\n", value, addr); -#endif -} -#else -/* Normal architectures just use readl/write */ -static inline u32 dwc2_readl(const void __iomem *addr) -{ - return readl(addr); -} - -static inline void dwc2_writel(u32 value, void __iomem *addr) -{ - writel(value, addr); - -#ifdef DWC2_LOG_WRITES - pr_info("info:: wrote %08x to %p\n", value, addr); -#endif -} -#endif - /* Maximum number of Endpoints/HostChannels */ #define MAX_EPS_CHANNELS 16 @@ -1212,6 +1158,60 @@ struct dwc2_hsotg { #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */ }; +#ifdef CONFIG_MIPS +/* + * There are some MIPS machines that can run in either big-endian + * or little-endian mode and that use the dwc2 register without + * a byteswap in both ways. + * Unlike other architectures, MIPS apparently does not require a + * barrier before the __raw_writel() to synchronize with DMA but does + * require the barrier after the __raw_writel() to serialize a set of + * writes. This set of operations was added specifically for MIPS and + * should only be used there. + */ +static inline u32 dwc2_readl(const void __iomem *addr) +{ + u32 value = __raw_readl(addr); + + /* In order to preserve endianness __raw_* operation is used. Therefore +* a barrier is needed to ensure IO access is not re-ordered across +* reads or writes +*/ + mb(); + return value; +} + +static inline void dwc2_writel(u32 value, void __iomem *addr) +{ + __raw_writel(value, addr); + + /* +* In order to preserve endianness __raw_* operation is used. Therefore +* a barrier is needed to ensure IO access is not re-ordered across +* reads or writes +*/ + mb(); +#ifdef DWC2_LOG_WRITES + pr_info("INFO:: wrote %08x to %p\n", value, addr); +#endif +} +#else +/* Normal architectures just use readl/write */ +static inline u32 dwc2_readl(const void __iomem *addr) +{ + return readl(addr); +} + +static inline void dwc2_writel(u32 value, void __iomem *addr) +{ + writel(value, addr); + +#ifdef DWC2_LOG_WRITES + pr_info("info:: wrote %08x to %p\n", value, addr); +#endif +} +#endif + /* Reasons for halting a host channel */ enum dwc2_halt_status { DWC2_HC_XFER_NO_HALT_STATUS, -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] usb: dwc2: replace ioread32/iowrite32_rep with dwc2_readl/writel_rep
dwc2_readl_rep/dwc2_writel_rep functions using readl/writel in a loop. Signed-off-by: Gevorg Sahakyan --- drivers/usb/dwc2/core.h | 61 ++- drivers/usb/dwc2/gadget.c | 6 ++--- 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 364ba9cf993e..dcc1bf633439 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -1158,60 +1158,45 @@ struct dwc2_hsotg { #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */ }; -#ifdef CONFIG_MIPS -/* - * There are some MIPS machines that can run in either big-endian - * or little-endian mode and that use the dwc2 register without - * a byteswap in both ways. - * Unlike other architectures, MIPS apparently does not require a - * barrier before the __raw_writel() to synchronize with DMA but does - * require the barrier after the __raw_writel() to serialize a set of - * writes. This set of operations was added specifically for MIPS and - * should only be used there. - */ +/* Normal architectures just use readl/write */ static inline u32 dwc2_readl(struct dwc2_hsotg *hsotg, u32 offset) { - u32 value = __raw_readl(hsotg->regs + offset); - - /* In order to preserve endianness __raw_* operation is used. Therefore -* a barrier is needed to ensure IO access is not re-ordered across -* reads or writes -*/ - mb(); - return value; + return readl(hsotg->regs + offset); } static inline void dwc2_writel(struct dwc2_hsotg *hsotg, u32 value, u32 offset) { - __raw_writel(value, hsotg->regs + offset); - - /* -* In order to preserve endianness __raw_* operation is used. Therefore -* a barrier is needed to ensure IO access is not re-ordered across -* reads or writes -*/ - mb(); + writel(value, hsotg->regs + offset); + #ifdef DWC2_LOG_WRITES - pr_info("INFO:: wrote %08x to %p\n", value, hsotg->regs + offset); + pr_info("info:: wrote %08x to %p\n", value, hsotg->regs + offset); #endif } -#else -/* Normal architectures just use readl/write */ -static inline u32 dwc2_readl(struct dwc2_hsotg *hsotg, u32 offset) +static inline void dwc2_readl_rep(struct dwc2_hsotg *hsotg, u32 offset, + void *buffer, unsigned int count) { - return readl(hsotg->regs + offset); + if (count) { + u32 *buf = buffer; + + do { + u32 x = dwc2_readl(hsotg, offset); + *buf++ = x; + } while (--count); + } } -static inline void dwc2_writel(struct dwc2_hsotg *hsotg, u32 value, u32 offset) +static inline void dwc2_writel_rep(struct dwc2_hsotg *hsotg, u32 offset, + const void *buffer, unsigned int count) { - writel(value, hsotg->regs + offset); + if (count) { + const u32 *buf = buffer; -#ifdef DWC2_LOG_WRITES - pr_info("info:: wrote %08x to %p\n", value, hsotg->regs + offset); -#endif + do { + dwc2_writel(hsotg, *buf++, offset); + } while (--count); + } } -#endif /* Reasons for halting a host channel */ enum dwc2_halt_status { diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 77f592c5e01e..9cb5e3aba1d5 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -599,7 +599,7 @@ static int dwc2_hsotg_write_fifo(struct dwc2_hsotg *hsotg, to_write = DIV_ROUND_UP(to_write, 4); data = hs_req->req.buf + buf_pos; - iowrite32_rep(hsotg->regs + EPFIFO(hs_ep->index), data, to_write); + dwc2_writel_rep(hsotg, EPFIFO(hs_ep->index), data, to_write); return (to_write >= can_write) ? -ENOSPC : 0; } @@ -2163,8 +2163,8 @@ static void dwc2_hsotg_rx_data(struct dwc2_hsotg *hsotg, int ep_idx, int size) * note, we might over-write the buffer end by 3 bytes depending on * alignment of the data. */ - ioread32_rep(hsotg->regs + EPFIFO(ep_idx), -hs_req->req.buf + read_ptr, to_read); + dwc2_readl_rep(hsotg, EPFIFO(ep_idx), + hs_req->req.buf + read_ptr, to_read); } /** -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] usb: dwc2: Make dwc2_readl/writel functions endianness-agnostic.
Declared dwc2_check_core_endianness() function for dynamicly check core endianness. Added needs_byte_swap flag to hsotg structure, and depending on flag swap value inside dwc2_readl/writel functions. Signed-off-by: Gevorg Sahakyan --- drivers/usb/dwc2/core.h | 21 +++-- drivers/usb/dwc2/platform.c | 19 +++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index dcc1bf633439..ce61d0af14a0 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -96,6 +96,13 @@ static const char * const dwc2_hsotg_supply_names[] = { */ #define EP0_MPS_LIMIT 64 +#define swap32(x) (\ + {typeof(x) x_ = (x); \ + (((u32)(x_) << 24) & (u32)0xFF00) | \ + (((u32)(x_) << 8) & (u32)0x00FF) | \ + (((u32)(x_) >> 8) & (u32)0xFF00) | \ + (((u32)(x_) >> 24) & (u32)0x00FF); }) + struct dwc2_hsotg; struct dwc2_hsotg_req; @@ -1045,6 +1052,7 @@ struct dwc2_hsotg { struct dentry *debug_root; struct debugfs_regset32 *regset; + bool needs_byte_swap; /* DWC OTG HW Release versions */ #define DWC2_CORE_REV_2_71a0x4f54271a @@ -1161,12 +1169,21 @@ struct dwc2_hsotg { /* Normal architectures just use readl/write */ static inline u32 dwc2_readl(struct dwc2_hsotg *hsotg, u32 offset) { - return readl(hsotg->regs + offset); + u32 val; + + val = readl(hsotg->regs + offset); + if (hsotg->needs_byte_swap) + return swap32(val); + else + return val; } static inline void dwc2_writel(struct dwc2_hsotg *hsotg, u32 value, u32 offset) { - writel(value, hsotg->regs + offset); + if (hsotg->needs_byte_swap) + writel(swap32(value), hsotg->regs + offset); + else + writel(value, hsotg->regs + offset); #ifdef DWC2_LOG_WRITES pr_info("info:: wrote %08x to %p\n", value, hsotg->regs + offset); diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index 4c0819554bcd..9a53a58e676e 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -352,6 +352,23 @@ static void dwc2_driver_shutdown(struct platform_device *dev) disable_irq(hsotg->irq); } +/** + * dwc2_check_core_endianness() - Returns true if core and AHB have + * opposite endianness. + * @hsotg: Programming view of the DWC_otg controller. + */ +static bool dwc2_check_core_endianness(struct dwc2_hsotg *hsotg) +{ + u32 snpsid; + + snpsid = ioread32(hsotg->regs + GSNPSID); + if ((snpsid & GSNPSID_ID_MASK) == DWC2_OTG_ID || + (snpsid & GSNPSID_ID_MASK) == DWC2_FS_IOT_ID || + (snpsid & GSNPSID_ID_MASK) == DWC2_HS_IOT_ID) + return false; + return true; +} + /** * dwc2_driver_probe() - Called when the DWC_otg core is bound to the DWC_otg * driver @@ -395,6 +412,8 @@ static int dwc2_driver_probe(struct platform_device *dev) dev_dbg(&dev->dev, "mapped PA %08lx to VA %p\n", (unsigned long)res->start, hsotg->regs); + hsotg->needs_byte_swap = dwc2_check_core_endianness(hsotg); + retval = dwc2_lowlevel_hw_init(hsotg); if (retval) return retval; -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] usb: dwc2: Make dwc2 endianness agnostic
Hi Filipe, This patch series changing dwc2_readl/dwc2_writel functions prototypes. To avoid multiple re-basing, preferable to merge this series to your next branch as soon as possible. Do you have any objections? Thanks, Minas Acked-by: Minas Harutyunyan On 5/23/2018 11:54 AM, Gevorg Sahakyan wrote: > This series contains patches which are make dwc2 core endianness agnostic > > Changes from v1: > > Rebased to latest balbi/next. > > Changes from v0: > > Moved dwc2_check_core_endianness() call after devm_ioremap_resource() > to avoid ioread32() call on null pointer. > > > > Gevorg Sahakyan (4): >usb: dwc2: Move dwc2_readl/writel functions after hsotg structure >usb: dwc2: Modify dwc2_readl/writel functions prototype >usb: dwc2: replace ioread32/iowrite32_rep with dwc2_readl/writel_rep >usb: dwc2: Make dwc2_readl/writel functions endianness-agnostic. > > drivers/usb/dwc2/core.c | 241 ++-- > drivers/usb/dwc2/core.h | 115 +- > drivers/usb/dwc2/core_intr.c | 118 +- > drivers/usb/dwc2/debugfs.c | 55 +++-- > drivers/usb/dwc2/gadget.c| 514 > +-- > drivers/usb/dwc2/hcd.c | 461 +++--- > drivers/usb/dwc2/hcd.h | 10 +- > drivers/usb/dwc2/hcd_ddma.c | 10 +- > drivers/usb/dwc2/hcd_intr.c | 96 > drivers/usb/dwc2/hcd_queue.c | 10 +- > drivers/usb/dwc2/params.c| 20 +- > drivers/usb/dwc2/platform.c | 19 ++ > 12 files changed, 843 insertions(+), 826 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[next, PATCH 2/6] usb: mtu3: fix uncontinuous SeqN issue after disable EP
Reset EP when disable it to reset data toggle for U2 EP, and SeqN, flow control status etc for U3 EP, this can avoid issue of uncontinuous SeqN Signed-off-by: Chunfeng Yun --- drivers/usb/mtu3/mtu3_core.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c index 65ff53a..279f9cd 100644 --- a/drivers/usb/mtu3/mtu3_core.c +++ b/drivers/usb/mtu3/mtu3_core.c @@ -195,6 +195,16 @@ static void mtu3_intr_enable(struct mtu3 *mtu) mtu3_writel(mbase, U3D_DEV_LINK_INTR_ENABLE, SSUSB_DEV_SPEED_CHG_INTR); } +/* reset: u2 - data toggle, u3 - SeqN, flow control status etc */ +static void mtu3_ep_reset(struct mtu3_ep *mep) +{ + struct mtu3 *mtu = mep->mtu; + u32 rst_bit = EP_RST(mep->is_in, mep->epnum); + + mtu3_setbits(mtu->mac_base, U3D_EP_RST, rst_bit); + mtu3_clrbits(mtu->mac_base, U3D_EP_RST, rst_bit); +} + /* set/clear the stall and toggle bits for non-ep0 */ void mtu3_ep_stall_set(struct mtu3_ep *mep, bool set) { @@ -220,8 +230,7 @@ void mtu3_ep_stall_set(struct mtu3_ep *mep, bool set) } if (!set) { - mtu3_setbits(mbase, U3D_EP_RST, EP_RST(mep->is_in, epnum)); - mtu3_clrbits(mbase, U3D_EP_RST, EP_RST(mep->is_in, epnum)); + mtu3_ep_reset(mep); mep->flags &= ~MTU3_EP_STALL; } else { mep->flags |= MTU3_EP_STALL; @@ -400,6 +409,7 @@ void mtu3_deconfig_ep(struct mtu3 *mtu, struct mtu3_ep *mep) mtu3_setbits(mbase, U3D_QIECR0, QMU_RX_DONE_INT(epnum)); } + mtu3_ep_reset(mep); ep_fifo_free(mep); dev_dbg(mtu->dev, "%s: %s\n", __func__, mep->name); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[next, PATCH 5/6] usb: mtu3: reset gadget when VBUS_FALL interrupt arises
When VBUS_FALL interrupt arises, it means U3 device is disconnected with host, so need reset status of gadget Signed-off-by: Chunfeng Yun --- drivers/usb/mtu3/mtu3_core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c index 279f9cd..eecfd06 100644 --- a/drivers/usb/mtu3/mtu3_core.c +++ b/drivers/usb/mtu3/mtu3_core.c @@ -668,8 +668,10 @@ static irqreturn_t mtu3_u3_ltssm_isr(struct mtu3 *mtu) if (ltssm & (HOT_RST_INTR | WARM_RST_INTR)) mtu3_gadget_reset(mtu); - if (ltssm & VBUS_FALL_INTR) + if (ltssm & VBUS_FALL_INTR) { mtu3_ss_func_set(mtu, false); + mtu3_gadget_reset(mtu); + } if (ltssm & VBUS_RISE_INTR) mtu3_ss_func_set(mtu, true); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[next, PATCH 6/6] usb: mtu3: fix warning of sleep in atomic context in notifier callback
The notifier callbacks of extcon are called in atomic context, but the callbacks will call regulator_enable()/regulator_disable() which may sleep caused by mutex, so use work queue to call the sleep functions. Signed-off-by: Chunfeng Yun --- drivers/usb/mtu3/mtu3.h| 11 ++- drivers/usb/mtu3/mtu3_dr.c | 44 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h index a56fee0..87823ac 100644 --- a/drivers/usb/mtu3/mtu3.h +++ b/drivers/usb/mtu3/mtu3.h @@ -196,7 +196,12 @@ struct mtu3_gpd_ring { * @vbus: vbus 5V used by host mode * @edev: external connector used to detect vbus and iddig changes * @vbus_nb: notifier for vbus detection -* @vbus_nb: notifier for iddig(idpin) detection +* @vbus_work : work of vbus detection notifier, used to avoid sleep in +* notifier callback which is atomic context +* @vbus_event : event of vbus detecion notifier +* @id_nb : notifier for iddig(idpin) detection +* @id_work : work of iddig detection notifier +* @id_event : event of iddig detecion notifier * @is_u3_drd: whether port0 supports usb3.0 dual-role device or not * @manual_drd_enabled: it's true when supports dual-role device by debugfs * to switch host/device modes depending on user input. @@ -205,7 +210,11 @@ struct otg_switch_mtk { struct regulator *vbus; struct extcon_dev *edev; struct notifier_block vbus_nb; + struct work_struct vbus_work; + unsigned long vbus_event; struct notifier_block id_nb; + struct work_struct id_work; + unsigned long id_event; bool is_u3_drd; bool manual_drd_enabled; }; diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c index 80083e0..8c3bbf7 100644 --- a/drivers/usb/mtu3/mtu3_dr.c +++ b/drivers/usb/mtu3/mtu3_dr.c @@ -174,16 +174,40 @@ static void ssusb_set_mailbox(struct otg_switch_mtk *otg_sx, } } -static int ssusb_id_notifier(struct notifier_block *nb, - unsigned long event, void *ptr) +static void ssusb_id_work(struct work_struct *work) { struct otg_switch_mtk *otg_sx = - container_of(nb, struct otg_switch_mtk, id_nb); + container_of(work, struct otg_switch_mtk, id_work); - if (event) + if (otg_sx->id_event) ssusb_set_mailbox(otg_sx, MTU3_ID_GROUND); else ssusb_set_mailbox(otg_sx, MTU3_ID_FLOAT); +} + +static void ssusb_vbus_work(struct work_struct *work) +{ + struct otg_switch_mtk *otg_sx = + container_of(work, struct otg_switch_mtk, vbus_work); + + if (otg_sx->vbus_event) + ssusb_set_mailbox(otg_sx, MTU3_VBUS_VALID); + else + ssusb_set_mailbox(otg_sx, MTU3_VBUS_OFF); +} + +/* + * @ssusb_id_notifier is called in atomic context, but @ssusb_set_mailbox + * may sleep, so use work queue here + */ +static int ssusb_id_notifier(struct notifier_block *nb, + unsigned long event, void *ptr) +{ + struct otg_switch_mtk *otg_sx = + container_of(nb, struct otg_switch_mtk, id_nb); + + otg_sx->id_event = event; + schedule_work(&otg_sx->id_work); return NOTIFY_DONE; } @@ -194,10 +218,8 @@ static int ssusb_vbus_notifier(struct notifier_block *nb, struct otg_switch_mtk *otg_sx = container_of(nb, struct otg_switch_mtk, vbus_nb); - if (event) - ssusb_set_mailbox(otg_sx, MTU3_VBUS_VALID); - else - ssusb_set_mailbox(otg_sx, MTU3_VBUS_OFF); + otg_sx->vbus_event = event; + schedule_work(&otg_sx->vbus_work); return NOTIFY_DONE; } @@ -398,6 +420,9 @@ int ssusb_otg_switch_init(struct ssusb_mtk *ssusb) { struct otg_switch_mtk *otg_sx = &ssusb->otg_switch; + INIT_WORK(&otg_sx->id_work, ssusb_id_work); + INIT_WORK(&otg_sx->vbus_work, ssusb_vbus_work); + if (otg_sx->manual_drd_enabled) ssusb_debugfs_init(ssusb); else @@ -412,4 +437,7 @@ void ssusb_otg_switch_exit(struct ssusb_mtk *ssusb) if (otg_sx->manual_drd_enabled) ssusb_debugfs_exit(ssusb); + + cancel_work_sync(&otg_sx->id_work); + cancel_work_sync(&otg_sx->vbus_work); } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[next, PATCH 4/6] usb: mtu3: avoid sleep in atomic context when enter test mode
Use readl_poll_timeout_atomic() instead of readl_poll_timeout() in atomic context Signed-off-by: Chunfeng Yun --- drivers/usb/mtu3/mtu3_gadget_ep0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/mtu3/mtu3_gadget_ep0.c b/drivers/usb/mtu3/mtu3_gadget_ep0.c index 0d2b1cf..25216e7 100644 --- a/drivers/usb/mtu3/mtu3_gadget_ep0.c +++ b/drivers/usb/mtu3/mtu3_gadget_ep0.c @@ -299,7 +299,7 @@ static int handle_test_mode(struct mtu3 *mtu, struct usb_ctrlrequest *setup) mtu3_writel(mbase, U3D_EP0CSR, value | EP0_SETUPPKTRDY | EP0_DATAEND); /* wait for ACK status sent by host */ - readl_poll_timeout(mbase + U3D_EP0CSR, value, + readl_poll_timeout_atomic(mbase + U3D_EP0CSR, value, !(value & EP0_DATAEND), 100, 5000); mtu3_writel(mbase, U3D_USB2_TEST_MODE, mtu->test_mode_nr); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[next, PATCH 3/6] usb: mtu3: clear test_mode flag when reset
Clear test_mode flag when the gadget is reset by host, otherwise will affect the next test item. Signed-off-by: Chunfeng Yun --- drivers/usb/mtu3/mtu3_gadget.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c index de0de01..5c60a8c 100644 --- a/drivers/usb/mtu3/mtu3_gadget.c +++ b/drivers/usb/mtu3/mtu3_gadget.c @@ -719,4 +719,5 @@ void mtu3_gadget_reset(struct mtu3 *mtu) mtu->u1_enable = 0; mtu->u2_enable = 0; mtu->delayed_status = false; + mtu->test_mode = false; } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[next, PATCH 1/6] usb: mtu3: re-enable controller to accept LPM request after LPM resume
After the controller receives a LPM request, it will reject the LPM request, and need software to re-enable it after LPM resume if the controller doesn't remote wakeup from L1 automatically Signed-off-by: Chunfeng Yun --- drivers/usb/mtu3/mtu3_core.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c index b1b99a8..65ff53a 100644 --- a/drivers/usb/mtu3/mtu3_core.c +++ b/drivers/usb/mtu3/mtu3_core.c @@ -176,7 +176,7 @@ static void mtu3_intr_enable(struct mtu3 *mtu) mtu3_writel(mbase, U3D_LV1IESR, value); /* Enable U2 common USB interrupts */ - value = SUSPEND_INTR | RESUME_INTR | RESET_INTR; + value = SUSPEND_INTR | RESUME_INTR | RESET_INTR | LPM_RESUME_INTR; mtu3_writel(mbase, U3D_COMMON_USB_INTR_ENABLE, value); if (mtu->is_u3_ip) { @@ -692,6 +692,12 @@ static irqreturn_t mtu3_u2_common_isr(struct mtu3 *mtu) if (u2comm & RESET_INTR) mtu3_gadget_reset(mtu); + if (u2comm & LPM_RESUME_INTR) { + if (!(mtu3_readl(mbase, U3D_POWER_MANAGEMENT) & LPM_HRWE)) + mtu3_setbits(mbase, U3D_USB20_MISC_CONTROL, +LPM_U3_ACK_EN); + } + return IRQ_HANDLED; } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] usbip: dynamically allocate idev by nports found in sysfs
As the amount of available ports varies by the kernels build configuration. To remove the limitation of the fixed 128 ports we allocate the amount of idevs by using the number we get from the kernel. Signed-off-by: Michael Grzeschik --- v1 -> v2: - reworked memory allocation into one calloc call - added error path on allocation failure v2 -> v3: - moved check for available nports to beginning of function tools/usb/usbip/libsrc/vhci_driver.c | 24 ++-- tools/usb/usbip/libsrc/vhci_driver.h | 3 +-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index c9c81614a66ad..c5db1be784bab 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -242,13 +242,25 @@ static int read_record(int rhport, char *host, unsigned long host_len, int usbip_vhci_driver_open(void) { + int nports = get_nports(); + + if (nports <= 0) { + err("no available ports"); + return -1; + } + udev_context = udev_new(); if (!udev_context) { err("udev_new failed"); return -1; } - vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver)); + vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver) + + nports * sizeof(struct usbip_imported_device)); + if (!vhci_driver) { + err("vhci_driver allocation failed"); + return -1; + } /* will be freed in usbip_driver_close() */ vhci_driver->hc_device = @@ -260,17 +272,9 @@ int usbip_vhci_driver_open(void) goto err; } - vhci_driver->nports = get_nports(); + vhci_driver->nports = nports; dbg("available ports: %d", vhci_driver->nports); - if (vhci_driver->nports <= 0) { - err("no available ports"); - goto err; - } else if (vhci_driver->nports > MAXNPORT) { - err("port number exceeds %d", MAXNPORT); - goto err; - } - vhci_driver->ncontrollers = get_ncontrollers(); dbg("available controllers: %d", vhci_driver->ncontrollers); diff --git a/tools/usb/usbip/libsrc/vhci_driver.h b/tools/usb/usbip/libsrc/vhci_driver.h index 418b404d51210..6c9aca2167051 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.h +++ b/tools/usb/usbip/libsrc/vhci_driver.h @@ -13,7 +13,6 @@ #define USBIP_VHCI_BUS_TYPE "platform" #define USBIP_VHCI_DEVICE_NAME "vhci_hcd.0" -#define MAXNPORT 128 enum hub_speed { HUB_SPEED_HIGH = 0, @@ -41,7 +40,7 @@ struct usbip_vhci_driver { int ncontrollers; int nports; - struct usbip_imported_device idev[MAXNPORT]; + struct usbip_imported_device idev[]; }; -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for USB HCD instead of tasklets
On 2018-05-22 15:14:17 [-0400], Alan Stern wrote: > On Tue, 22 May 2018, Sebastian Andrzej Siewior wrote: > > > On 2018-05-07 11:37:29 [-0400], Alan Stern wrote: > > > > As far as I understand it there should be no deadlock. Without the > > > > local_irq_save() things should not deadlock because the HCD invokes USB > > > > driver's (usb-storage for instance) ->complete callback always in the > > > > same way. If you mix the usb-driver with two different HCDs (say ehci > > > > and xhci) then lockdep would complain. However, the locks which were > > > > allocated by usb-storage for the ehci driver would never be used in the > > > > context of the xhci driver. So lockdep would report a deacklock but > > > > there wouldn't be one (again, if I got the piece right). > > > > > > That argument would be correct if the completion routines were the only > > > places where the higher-level drivers (such as usb-storage or usbhid) > > > acquired their locks. But we can't depend on this being true; you > > > would have to audit each USB driver to make sure. > > > > an entry point for IRQ usage outside of the driver would be the usage of > > hrtimer. > > Sorry, I don't understand that sentence at all. And I don't see how it > could be relevant to the point I was trying to make. > > Consider, for example, drivers/hid/usbhid/hid-core.c. In that file, > hid_io_error() is called by hid_irq_in(), which is an URB completion > handler. hid_io_error() acquires usbhid_lock. Therefore it would be > necessary to audit the usbhid driver to see whether interrupts are > enabled or disabled any place where usbhid_lock is acquired. And in > fact, hid_ctrl() (another completion handler) calls > spin_lock(&usbhid->lock) -- this could cause problems for you. And > usbhid->lock is acquired in other places, ones that are not inside > completion handlers. > > None of this has anything to do with IRQ usage or hrtimers. Yeah, I get what you mean. > > You mean the "Reserved Bandwidth Transfers:" paragraph? > > Paragraphs (plural). Three paragraphs, to be precise. > > > > It's possible to rewrite the HCDs not to rely on this (I did exactly > > > that for ehci-hcd), but it is a nontrivial job. > > > > are you referring to commit 9118f9eb4f1e ("USB: EHCI: improve interrupt > > qh unlink")? > > That one, plus: > > 46c73d1d3ebc ("USB: EHCI: handle isochronous underruns with tasklets") > e4e18cbd52c8 ("USB: EHCI: code rearrangement in iso_stream_schedule()") > 24f531371de1 ("USB: EHCI: accept very late isochronous URBs") > 35371e4fbc3e ("USB: EHCI: improve ehci_endpoint_disable") > > Not all parts of all those commits were relevant, but as far as I > recall, they each contributed something. And I may have omitted > one or two commits by mistake. Thank you, let me look at those so I can see what is needed… > Alan Stern Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/12] usb: usbtmc: Support Read Status Byte with SRQ per file handle
Am Montag, den 21.05.2018, 21:00 + schrieb guido@kiener- muenchen.de: > > I looked for a race here, but I do not find a race between open and release, > since a refcount of "file_data->data->kref" is always hold by > usbtmc_probe/disconnect. > > However I see a race between usbtmc_open and usbtmc_disconnect. Are these > callback functions called mutual exclusive? No, they are not. > I'm not sure, but if not, then I think we have the same problem in > usb-skeleton.c In usb-skeleton.c a race exists. You are right. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/12] usb: usbtmc: Add ioctls for trigger, EOM bit and TermChar
Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener: > + retval = usb_bulk_msg(data->usb_dev, > + usb_sndbulkpipe(data->usb_dev, > + data->bulk_out), > + buffer, USBTMC_HEADER_SIZE, > + &actual, file_data->timeout); > + > + /* Store bTag (in case we need to abort) */ > + data->bTag_last_write = data->bTag; > + > + /* Increment bTag -- and increment again if zero */ > + data->bTag++; > + if (!data->bTag) > + data->bTag++; > + Independent of whether this needs to be split up, do you really want to do this regardless of usb_bulk_msg() returning an error? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/12] usb: usbtmc: Add ioctl USBTMC488_IOCTL_WAIT_SRQ
Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener: > +static int usbtmc488_ioctl_wait_srq(struct usbtmc_file_data *file_data, > + unsigned int __user *arg) > +{ > + struct usbtmc_device_data *data = file_data->data; > + struct device *dev = &data->intf->dev; > + int rv; > + unsigned int timeout; > + unsigned long expire; > + > + if (!data->iin_ep_present) { > + dev_dbg(dev, "no interrupt endpoint present\n"); > + return -EFAULT; > + } > + > + if (get_user(timeout, arg)) > + return -EFAULT; > + > + expire = msecs_to_jiffies(timeout); > + > + mutex_unlock(&data->io_mutex); There is such a thing as threads sharing file descriptors. That leads to the question what happens to the mutex if this ioctl() is called multiple times. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/12] usb: usbtmc: Fix ioctl USBTMC_IOCTL_CLEAR
Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener: > Insert a sleep of 50 ms between subsequent CHECK_CLEAR_STATUS > control requests to avoid stressing the instrument with repeated > requests. > > Some instruments need time to cleanup internal I/O buffers. > Polling and repeated requests slow down the response time of > devices. The connection between the patch and the description is loose. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dwc2: gadget: Fix ISOC IN DDMA PID bitfield value calculation
PID bitfield in descriptor should be set based on particular request length, not based on EP's mc value. PID value can't be set to 0 even request length is 0. Signed-off-by: Minas Harutyunyan --- drivers/usb/dwc2/gadget.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 6c32bf26e48e..9c79919a0a49 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -808,6 +808,7 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep, u32 index; u32 maxsize = 0; u32 mask = 0; + u8 pid = 0; maxsize = dwc2_gadget_get_desc_params(hs_ep, &mask); if (len > maxsize) { @@ -853,7 +854,11 @@ static int dwc2_gadget_fill_isoc_desc(struct dwc2_hsotg_ep *hs_ep, ((len << DEV_DMA_NBYTES_SHIFT) & mask)); if (hs_ep->dir_in) { - desc->status |= ((hs_ep->mc << DEV_DMA_ISOC_PID_SHIFT) & + if (len) + pid = DIV_ROUND_UP(len, hs_ep->ep.maxpacket); + else + pid = 1; + desc->status |= ((pid << DEV_DMA_ISOC_PID_SHIFT) & DEV_DMA_ISOC_PID_MASK) | ((len % hs_ep->ep.maxpacket) ? DEV_DMA_SHORT : 0) | -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc2: Fix host exit from hibernation flow.
In case when a hub is connected to DWC2 host auto suspend occurs and host goes to hibernation. When any device connected to hub host hibernation exiting incorrectly. - Added dwc2_hcd_rem_wakeup() function call to exit from suspend state by remote wakeup. - Increase timeout value for port suspend bit to be set. Signed-off-by: Artur Petrosyan --- drivers/usb/dwc2/hcd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 1faefea16cec..736dcc816abf 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -5434,7 +5434,7 @@ int dwc2_host_enter_hibernation(struct dwc2_hsotg *hsotg) dwc2_writel(hprt0, hsotg->regs + HPRT0); /* Wait for the HPRT0.PrtSusp register field to be set */ - if (dwc2_hsotg_wait_bit_set(hsotg, HPRT0, HPRT0_SUSP, 300)) + if (dwc2_hsotg_wait_bit_set(hsotg, HPRT0, HPRT0_SUSP, 3000)) dev_warn(hsotg->dev, "Suspend wasn't generated\n"); /* @@ -5615,6 +5615,8 @@ int dwc2_host_exit_hibernation(struct dwc2_hsotg *hsotg, int rem_wakeup, return ret; } + dwc2_hcd_rem_wakeup(hsotg); + hsotg->hibernated = 0; hsotg->bus_suspended = 0; hsotg->lx_state = DWC2_L0; -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dwc2: Regression on 96Boards Hikey due to enabling power down
Hi Mani, Could you please test the patch ([PATCH] usb: dwc2: Fix host exit from hibernation flow.) which should fix issue seen on your setup and provide feedback. Meantime we will debug issue reported by John Stultz, which is another case/scenario. Regards, Artur On 5/22/2018 22:45, Manivannan Sadhasivam wrote: > + John Stultz. > > John submitted a patch recently to temporarily fix the issue by disabling the > power down feature in Hikey. > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2018_5_21_730&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=6LhexoM7WPYJCGKz9Ht2lFT0FTJGYReuH-DLADH3jTk&s=T5DKo2-Jt1vCbo1Tq0MsuktMrpJ-GnxKCigx58tKqrw&e= > > John, I have listed some of my findings to this thread which will help > debugging the issue. > > Thanks, > Mani > > On Thu, May 17, 2018 at 04:04:01PM +0530, Manivannan Sadhasivam wrote: >> Hi Artur, >> >> Thanks for the reply! >> >> On Thu, May 17, 2018 at 09:10:06AM +, Artur Petrosyan wrote: >>> Hi Mani, >>> >>> We need some detailed information to perform debugging. >>> >>> 1. Could you please share the documentation of "96Boards HiKey" board, at >>> least dwc core configuration parameters. Or dump of GHWCFG1-4. >> >> You can find the HiKey documentation here: >> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_96boards_documentation_tree_master_consumer_hikey_hardware-2Ddocs&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=6LhexoM7WPYJCGKz9Ht2lFT0FTJGYReuH-DLADH3jTk&s=jpHs7p6qQrZZ70jXyaj4fon8SE7y6N4rds4ZmuXr-8s&e= >> >> GHWCFG register dump: >> >> GHWCFG1 = 0x >> GHWCFG2 = 0x23affc70 >> GHWCFG3 = 0x0780d4e8 >> GHWCFG4 = 0xfff00060 >> >>> 2. Could you please share with us full debug log of dwc2 loading and plug >>> the USB device. >> >> Here is the full kernel log from the boot till USB device gets plugged in: >> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.ubuntu.com_p_3bZwWtk8wD_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=6LhexoM7WPYJCGKz9Ht2lFT0FTJGYReuH-DLADH3jTk&s=e2qpTQMTCiMabfKTLZiHML--9dJd_iEbSpnbO8oVruE&e= >> >>> 3. From short debug log seen that Host exited from hibernation after USB >>> device plugged to the port. Do you mean that enumeration process didn't >>> start? If not, could you please dump registers after "dwc2 f72c.usb: >>> Host hibernation restore complete" >> >> Yeah, I guess the enumeration process didn't happen at all. Here is the >> register dump after plugging the device: >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.ubuntu.com_p_3vxdkFGgp6_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=6LhexoM7WPYJCGKz9Ht2lFT0FTJGYReuH-DLADH3jTk&s=scjqjTPnWj7MbUktAI5S6zao5g7KXG4gDfg-wNC1zfc&e= >> >>> 4. In which mode had the dwc2 been built. In host only mode or DRD mode? >> >> DWC2 in HiKey is built in Dual Role Device (DRD) mode. >> >>> 5. And you mention that USB controller's DP/DM out is connected to a switch >>> which switches between a USB type C port and a HUB (USB2513B). Is the >>> device connected to USB type C port or to HUB (USB2513B) ? Switching >>> connection done before dwc2 load or after? >>> >> >> The USB device is connected to one of the ports from HUB and there is one >> gpio which is used for switching the USB ports between Type-C and HUB. By >> default, this gpio is unused and it pulled low, which means HUB will get >> selected before dwc2 gets loaded. >> >> Hope this information will help debugging the issue. >> >> Thanks, >> Mani >> >>> Looking forward to your reply. >>> >>> Regards, >>> Artur >>> >>> >>> -Original Message- >>> From: Manivannan Sadhasivam [mailto:manivannan.sadhasi...@linaro.org] >>> Sent: Wednesday, May 16, 2018 17:23 >>> To: linux-usb@vger.kernel.org >>> Cc: john.y...@synopsys.com; mvar...@synopsys.com; >>> arthur.petros...@synopsys.com; grigor.tovmas...@synopsys.com; >>> felipe.ba...@linux.intel.com >>> Subject: usb: dwc2: Regression on 96Boards Hikey due to enabling power down >>> >>> Hello, >>> >>> Commit 03ea6d6e9e1ff1b0222eb723eee5990d3511cc4d introduced the powerdown >>> feature in USB DWC2 driver which stops USB from working on 96Boards HiKey >>> board. >>> >>> During bootup, USB host controller goes into hibernation state and when any >>> USB device is plugged onto the port, the host finishes executing the GPWRDN >>> interrupt handler but still it didn't wake up. >>> >>> Below is the dmesg log after plugging a device onto USB port: >>> >>> [ 30.763414] dwc2 f72c.usb: dwc2_handle_gpwrdn_intr: >>> dwc2_handle_gpwrdwn_intr called gpwrdn= 081411bb >>> [ 30.763458] dwc2 f72c.usb: dwc2_handle_gpwrdn_intr: GPWRDN_LNSTSCHG >>> [ 30.763492] dwc2 f72c.usb: dwc2_host_exit_hibernation: called with >>> rem_wakeup = 1 reset = 0 >>> [ 30.763623] dwc2 f72c.usb: dwc2_restore_essential_regs: restoring >>> essential regs >>> [ 30.76
Re: [Query] checking hub port status while USB 2.0 port is resuming.
On Wed, 23 May 2018, Anshuman Gupta wrote: > On Tue, May 22, 2018 at 11:54:19AM -0400, Alan Stern wrote: > > On Tue, 22 May 2018, Anshuman Gupta wrote: > > > > > On Tue, May 22, 2018 at 09:53:09AM -0400, Alan Stern wrote: > > > > On Tue, 22 May 2018, Anshuman Gupta wrote: > > > > > > > Thanks Alan for your quick response. > > > > > Hi, > > > > > > > > > > When a xhci USB2.0 port is resuming and waiting for resume signaling, > > > > > completion of > > > > > USB_RESUME_TIMEOUT, it just reports the port status as > > > > > USB_PORT_STAT_SUSPEND, > > > > > and let the usbcore to check the port status again. > > > > > > > > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/host/xhci-hub.c#L960 > > > > > > > > > > > > > > > USB core just checks the port status in usb_port_resume function > > > > > and if port status is in suspended state, it clears the port suspend > > > > > feature. > > > > > > > > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3441 > > > > > > > > > > But when system resumes due to a USB remote wakeup event from suspend > > > > > state, > > > > > and port which causes remote wakeup is still waiting for resume > > > > > signaling, > > > > > usb_port_resume function in this particular case clears the port > > > > > suspend feature, > > > > > and wait for 40ms again(USB_RESUME_TIMEOUT). > > > > > > > > > > Query regarding above case: > > > > > Here in this case USB core will miss the wake up event from the port > > > > > which causes > > > > > the remote wake. > > > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3444 > > > > > > > > What do you mean? You said above: "system resumes due to a USB remote > > > > wakeup event". The fact that the system is resuming indicates that it > > > > did _not_ miss the wakeup event. > > > > > > > What was i meaning, if system come out of suspend due to a key pressed on > > > usb keyboard. > > > For the above mention case, it will not raise any pm_wakeup_event so the > > > notification > > > of the pm_wakeup_event for usb keyboard will get missed. > > > > Okay, let's say you're right. What difference does it make if the > > pm_wakeup_event gets lost? Will that cause a problem? > There is no as such problem for usb core but on chrome-os we want to > increment > the wakeup-count for the usb device which is causing wakeup from suspend. > > So to address above mention case, is it appropriate that hub_port_status URB > completion > can wait for port to be resume, if the particular port was resuming at that > instant? I'm not sure what you're asking. If the port's suspend feature is set, of course the kernel has to clear the feature and wait for the port to resume. If the suspend feature isn't set then the pm_wakeup_event will be sent, which is what you want. Have I misunderstood something? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
On Wed, 23 May 2018, Nicolas Boichat wrote: > The "old" enumeration scheme is considerably faster (it takes > ~294ms instead of ~439ms to get the descriptor). > > It is currently only possible to use the old scheme globally > (/sys/module/usbcore/parameters/old_scheme_first), which is not > desirable as the new scheme was introduced to increase compatibility > with more devices. > > However, in our case, we care about time-to-active for a specific > USB device (which we make the firmware for), on a specific port > (that is pogo-pin based: not a standard USB port). This new > sysfs option makes it possible to use the old scheme on a single > port only. > > Signed-off-by: Nicolas Boichat > --- > > There are other "quirks" that we could add to reduce further > enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY > to 10ms instead of 50ms as used currently), but the logic is quite > similar, so it'd be good to have this reviewed first. I'm not opposed to the idea in principle, although I don't like your implementation because it breaks the original old_scheme_first parameter. Let's see what some other people think. Yours is a rather special case, because you know exactly what device will be attached to a specific port. Still, I can see that sort of thing happening in constrained and special-purpose settings. How do you arrange to set the new quirk before the device is discovered? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] usb: typec: fusb302: Fix debugfs issue
Removing the "fusb302" debugfs directory when unloading the driver. That allows the driver to be loaded more then ones. The directory will not get actually removed until it is empty, so only after the last instance has been removed. This fixes an issue where the driver can't be re-loaded if it has been unloaded as the "fusb302" debugfs directory already exists. Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging") Signed-off-by: Heikki Krogerus --- drivers/usb/typec/fusb302/fusb302.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c index eba6bb890b17..9c1eba9ea004 100644 --- a/drivers/usb/typec/fusb302/fusb302.c +++ b/drivers/usb/typec/fusb302/fusb302.c @@ -234,6 +234,7 @@ static int fusb302_debugfs_init(struct fusb302_chip *chip) static void fusb302_debugfs_exit(struct fusb302_chip *chip) { debugfs_remove(chip->dentry); + debugfs_remove(rootdir); } #else -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control
Trying to determine the USB port type with this mux is very difficult. To simplify the situation, always allowing user control, even if the port is USB Type-C port. Signed-off-by: Heikki Krogerus --- .../usb/roles/intel-xhci-usb-role-switch.c| 21 +-- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c index 30b07ea3a3c6..3f14153d753f 100644 --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c @@ -39,20 +39,6 @@ struct intel_xhci_usb_data { void __iomem *base; }; -struct intel_xhci_acpi_match { - const char *hid; - int hrv; -}; - -/* - * ACPI IDs for PMICs which do not support separate data and power role - * detection (USB ACA detection for micro USB OTG), we allow userspace to - * change the role manually on these. - */ -static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = { - { "INT33F4", 3 }, /* X-Powers AXP288 PMIC */ -}; - static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role) { struct intel_xhci_usb_data *data = dev_get_drvdata(dev); @@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct device *dev) static struct usb_role_switch_desc sw_desc = { .set = intel_xhci_usb_set_role, .get = intel_xhci_usb_get_role, + .allow_userspace_control = true, }; static int intel_xhci_usb_probe(struct platform_device *pdev) @@ -146,7 +133,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct intel_xhci_usb_data *data; struct resource *res; - int i; data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); if (!data) @@ -159,11 +145,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev) if (!data->base) return -ENOMEM; - for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++) - if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1", -allow_userspace_ctrl_ids[i].hrv)) - sw_desc.allow_userspace_control = true; - platform_set_drvdata(pdev, data); data->role_sw = usb_role_switch_register(dev, &sw_desc); -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] platform: x86: intel_cht_int33fe: Fix dependencies
The driver will not probe unless bq24190 is loaded, so making it a dependency. Signed-off-by: Heikki Krogerus Cc: Wolfram Sang Cc: Darren Hart Cc: Andy Shevchenko --- drivers/i2c/busses/Kconfig | 3 +-- drivers/platform/x86/Kconfig | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 99edffae27f6..4f8df2ec87b1 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -202,8 +202,7 @@ config I2C_CHT_WC Note this controller is hooked up to a TI bq24292i charger-IC, combined with a FUSB302 Type-C port-controller as such it is advised - to also select CONFIG_CHARGER_BQ24190=m and CONFIG_TYPEC_FUSB302=m - (the fusb302 driver currently is in drivers/staging). + to also select CONFIG_TYPEC_FUSB302=m. config I2C_NFORCE2 tristate "Nvidia nForce2, nForce3 and nForce4" diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 566644bb496a..f27cb186437d 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -866,6 +866,7 @@ config ACPI_CMPC config INTEL_CHT_INT33FE tristate "Intel Cherry Trail ACPI INT33FE Driver" depends on X86 && ACPI && I2C && REGULATOR + depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m) ---help--- This driver add support for the INT33FE ACPI device found on some Intel Cherry Trail devices. @@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE i2c drivers for these chips can bind to the them. If you enable this driver it is advised to also select - CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and - CONFIG_BATTERY_MAX17042=m. + CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m. config INTEL_INT0002_VGPIO tristate "Intel ACPI INT0002 Virtual GPIO driver" -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] [PATCH 0/4] usb: typec: fixes for Cherry Trails
Hi, This is second version of the remaining patches that fix various problems I encountered while testing my USB Type-C Alternate Mode patches with GPD Win board (Intel Cherry Trail based). In this version I've addressed the problems pointed out by Hans and Guenter. Link to the original version: https://lkml.org/lkml/2018/4/30/350 Heikki Krogerus (3): usb: roles: intel_xhci: Always allow user control platform: x86: intel_cht_int33fe: Fix dependencies usb: typec: fusb302: Fix debugfs issue drivers/i2c/busses/Kconfig| 3 +-- drivers/platform/x86/Kconfig | 4 ++-- .../usb/roles/intel-xhci-usb-role-switch.c| 21 +-- drivers/usb/typec/fusb302/fusb302.c | 1 + 4 files changed, 5 insertions(+), 24 deletions(-) -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
On Wed, May 23, 2018 at 1:52 AM, Yoshihiro Shimoda wrote: > Hi Rob, > > Thank you for the review! > >> From: Rob Herring, Sent: Wednesday, May 23, 2018 2:13 AM >> >> On Tue, May 22, 2018 at 09:01:07PM +0900, Yoshihiro Shimoda wrote: >> > This patch adds role switch support for R-Car SoCs into the USB 3.0 >> > peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0 >> > dual-role device controller which has the USB 3.0 xHCI host and >> > Renesas USB 3.0 peripheral. >> > >> > Unfortunately, the mode change register contains the USB 3.0 peripheral >> > controller side only. So, the USB 3.0 peripheral driver (renesas_usb3) >> > manages this register now. However, in peripheral mode, the host >> > should stop. Also the host hardware needs to reinitialize its own >> > registers when the mode changes from peripheral to host mode. >> > Otherwise, the host cannot work correctly (e.g. detect a device as >> > high-speed). >> > >> > To achieve this by a driver, this role switch driver manages >> > the mode change register and attach/release the xhci-plat driver. >> > >> > Signed-off-by: Yoshihiro Shimoda >> > --- >> > .../devicetree/bindings/usb/renesas_usb3.txt | 15 >> >> Please split bindings to a separate patch. > > Oops. I got it. > >> > drivers/usb/gadget/udc/Kconfig | 1 + >> > drivers/usb/gadget/udc/renesas_usb3.c | 82 >> > ++ >> > 3 files changed, 98 insertions(+) >> > >> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt >> b/Documentation/devicetree/bindings/usb/renesas_usb3.txt >> > index 2c071bb5..f6105aa 100644 >> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt >> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt >> > @@ -19,6 +19,9 @@ Required properties: >> > Optional properties: >> >- phys: phandle + phy specifier pair >> >- phy-names: must be "usb" >> > + - The connection to a usb3.0 host node needs by using OF graph bindings >> > for >> > +usb role switch. >> > + - port@0 = USB3.0 host port. >> >> On the host side, this might conflict with the USB connector binding. >> >> I would either make sure this can work with the connector binding by >> having 2 endpoints on the HS or SS port or just use the 'companion' >> property defined in usb-generic.txt. > > I don't understand the first one now... This means the renesas_usb3 should > follow > USB connector binding and have 2 endpoints for the usb role switch to avoid > the conflict like below? > - port1@0: Super Speed (SS), present in SS capable connectors (from > usb-connector.txt). > - port1@1: USB3.0 host port. I'm confused, SS and USB3.0 are the same essentially. It would be: port@1/endpoint@0: SS host port port@1/endpoint@1: SS device port > About the 'companion' in usb-generic.txt, the property intends to be used for > EHCI or host side > like the commit log [1]. If there is accept to use 'companion' for this > patch, I think it will > be simple to achieve this role switch feature. However, in last month, I > submitted a similar patch [2] > that has "renesas,host" property, but I got reply from Andy [3] and Heikki > [4]. So, I'm > trying to improve the device connection framework [5] now. I think this case is rare enough that we don't need a general solution using OF graph, so I'm fine with a simple, single property to link the 2 nodes. Either reusing "companion" or "renesas,host" is fine by me. Rob > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/usb/generic.txt?h=v4.17-rc6&id=5095cb89c62acc78b4cfaeb9a4072979d010510a > > [2] > https://www.spinics.net/lists/linux-usb/msg167773.html > > [3] > https://www.spinics.net/lists/linux-usb/msg167780.html > > [4] > https://www.spinics.net/lists/linux-usb/msg167806.html > > [5] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/device_connection.rst > > Best regards, > Yoshihiro Shimoda > >> Rob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: uvc: configfs: Move function to avoid forward declaration
The to_f_uvc_opts() function is forward-declared without needing to, as its definition can simply be moved up in the file. Fix it. Signed-off-by: Laurent Pinchart --- drivers/usb/gadget/function/uvc_configfs.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index c9b8cc4aae5a..b51f0d278826 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -31,7 +31,11 @@ static struct configfs_attribute prefix##attr_##cname = { \ .show = prefix##cname##_show, \ } -static inline struct f_uvc_opts *to_f_uvc_opts(struct config_item *item); +static inline struct f_uvc_opts *to_f_uvc_opts(struct config_item *item) +{ + return container_of(to_config_group(item), struct f_uvc_opts, + func_inst.group); +} /* control/header/ */ DECLARE_UVC_HEADER_DESCRIPTOR(1); @@ -2105,12 +2109,6 @@ static const struct config_item_type uvcg_streaming_grp_type = { .ct_owner = THIS_MODULE, }; -static inline struct f_uvc_opts *to_f_uvc_opts(struct config_item *item) -{ - return container_of(to_config_group(item), struct f_uvc_opts, - func_inst.group); -} - static void uvc_attr_release(struct config_item *item) { struct f_uvc_opts *opts = to_f_uvc_opts(item); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] usb: typec: fusb302: Fix debugfs issue
Hello! On 05/23/2018 05:37 PM, Heikki Krogerus wrote: > Removing the "fusb302" debugfs directory when unloading > the driver. That allows the driver to be loaded more then > ones. The directory will not get actually removed until it s/ones/once/? > is empty, so only after the last instance has been removed. > > This fixes an issue where the driver can't be re-loaded if > it has been unloaded as the "fusb302" debugfs directory > already exists. > > Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging") > Signed-off-by: Heikki Krogerus [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control
On 05/23/2018 05:37 PM, Heikki Krogerus wrote: > Trying to determine the USB port type with this mux is very > difficult. To simplify the situation, always allowing user s/allowing/allow/? Else the statement doesn't parse for me. :-) > control, even if the port is USB Type-C port. > > Signed-off-by: Heikki Krogerus [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/12] usb: usbtmc: Add ioctl USBTMC488_IOCTL_WAIT_SRQ
On Wed, May 23, 2018 at 02:08:27PM +0200, Oliver Neukum wrote: > Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener: > > +static int usbtmc488_ioctl_wait_srq(struct usbtmc_file_data *file_data, > > + unsigned int __user *arg) > > +{ > > + struct usbtmc_device_data *data = file_data->data; > > + struct device *dev = &data->intf->dev; > > + int rv; > > + unsigned int timeout; > > + unsigned long expire; > > + > > + if (!data->iin_ep_present) { > > + dev_dbg(dev, "no interrupt endpoint present\n"); > > + return -EFAULT; > > + } > > + > > + if (get_user(timeout, arg)) > > + return -EFAULT; > > + > > + expire = msecs_to_jiffies(timeout); > > + > > + mutex_unlock(&data->io_mutex); > > There is such a thing as threads sharing file descriptors. > That leads to the question what happens to the mutex if this > ioctl() is called multiple times. Processes can share file descriptors as well, no need to mess with a thread :) good catch. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
On Wed, May 23, 2018 at 10:03:55AM -0400, Alan Stern wrote: > On Wed, 23 May 2018, Nicolas Boichat wrote: > > > The "old" enumeration scheme is considerably faster (it takes > > ~294ms instead of ~439ms to get the descriptor). > > > > It is currently only possible to use the old scheme globally > > (/sys/module/usbcore/parameters/old_scheme_first), which is not > > desirable as the new scheme was introduced to increase compatibility > > with more devices. > > > > However, in our case, we care about time-to-active for a specific > > USB device (which we make the firmware for), on a specific port > > (that is pogo-pin based: not a standard USB port). This new > > sysfs option makes it possible to use the old scheme on a single > > port only. > > > > Signed-off-by: Nicolas Boichat > > --- > > > > There are other "quirks" that we could add to reduce further > > enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY > > to 10ms instead of 50ms as used currently), but the logic is quite > > similar, so it'd be good to have this reviewed first. > > I'm not opposed to the idea in principle, although I don't like your > implementation because it breaks the original old_scheme_first > parameter. > > Let's see what some other people think. > > Yours is a rather special case, because you know exactly what device > will be attached to a specific port. Still, I can see that sort of > thing happening in constrained and special-purpose settings. > > How do you arrange to set the new quirk before the device is > discovered? Yeah, this last question is what I had when looking at this. Or does it not matter at first boot and only matters for wake-up? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usbip: dynamically allocate idev by nports found in sysfs
On 05/23/2018 03:22 AM, Michael Grzeschik wrote: > As the amount of available ports varies by the kernels build > configuration. To remove the limitation of the fixed 128 ports > we allocate the amount of idevs by using the number we get > from the kernel. > > Signed-off-by: Michael Grzeschik > --- > v1 -> v2: - reworked memory allocation into one calloc call > - added error path on allocation failure > v2 -> v3: - moved check for available nports to beginning of function > Hmm. With this patch I see a segfault when I run usbip port command. I think this patch is incomplete and more changes are needed to the code that references the idev array. I can't take this patch. thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] xhci: Add quirk to zero 64bit registers on Renesas PCIe controllers
On 21/05/18 09:23, Mathias Nyman wrote: > On 18.05.2018 19:29, Marc Zyngier wrote: >> Some Renesas controllers get into a weird state if they are reset while >> programmed with 64bit addresses (they will preserve the top half of the >> address in internal, non visible registers). >> >> You end up with half the address coming from the kernel, and the other >> half coming from the firmware. > > Should those registers be zeroed in resume from hibernate where we also > reset the host controller? That's a very good point. Given that this controller also has the XHCI_RESET_ON_RESUME quick, it is always treated as coming out of hibernate (how this thing ended up in production is beyond me). I'll place another stick of dynamite at that level too. >> Also, changing the programming leads to extra accesses even if the >> controller is supposed to be halted. The controller ends up with a fatal >> fault, and is then ripe for being properly reset. On the flip side, >> this is completely unsafe if the defvice isn't behind an IOMMU, so >> we have to make sure that this is the case. Can you say "broken"? >> >> This is an alternative method to the one introduced in 8466489ef5ba >> ("xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue"), >> which will subsequently be removed. >> >> Signed-off-by: Marc Zyngier >> --- >> drivers/usb/host/xhci-pci.c | 8 -- >> drivers/usb/host/xhci.c | 59 >> + >> drivers/usb/host/xhci.h | 1 + >> 3 files changed, 66 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c >> index 85ffda85f8ab..e0a0a12871e2 100644 >> --- a/drivers/usb/host/xhci-pci.c >> +++ b/drivers/usb/host/xhci-pci.c >> @@ -196,11 +196,15 @@ static void xhci_pci_quirks(struct device *dev, struct >> xhci_hcd *xhci) >> xhci->quirks |= XHCI_BROKEN_STREAMS; >> } >> if (pdev->vendor == PCI_VENDOR_ID_RENESAS && >> -pdev->device == 0x0014) >> +pdev->device == 0x0014) { >> xhci->quirks |= XHCI_TRUST_TX_LENGTH; >> +xhci->quirks |= XHCI_ZERO_64B_REGS; >> +} >> if (pdev->vendor == PCI_VENDOR_ID_RENESAS && >> -pdev->device == 0x0015) >> +pdev->device == 0x0015) { >> xhci->quirks |= XHCI_RESET_ON_RESUME; >> +xhci->quirks |= XHCI_ZERO_64B_REGS; >> +} >> if (pdev->vendor == PCI_VENDOR_ID_VIA) >> xhci->quirks |= XHCI_RESET_ON_RESUME; >> >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c >> index 8dba26d3de07..07272d1ce32a 100644 >> --- a/drivers/usb/host/xhci.c >> +++ b/drivers/usb/host/xhci.c >> @@ -4921,6 +4921,65 @@ int xhci_gen_setup(struct usb_hcd *hcd, >> xhci_get_quirks_t get_quirks) >> if (retval) >> return retval; >> >> +/* >> + * Some Renesas controllers get into a weird state if they are >> + * reset while programmed with 64bit addresses (they will preserve >> + * the top half of the address in internal, non visible >> + * registers). You end up with half the address coming from the >> + * kernel, and the other half coming from the firmware. Also, >> + * changing the programming leads to extra accesses even if the >> + * controller is supposed to be halted. The controller ends up with >> + * a fatal fault, and is then ripe for being properly reset. >> + * >> + * Special care is taken to only apply this if the device is behind >> + * an iommu. Doing anything when there is no iommu is definitely >> + * unsafe... >> + */ >> +if ((xhci->quirks & XHCI_ZERO_64B_REGS) && dev->iommu_group) { >> +u64 val; >> +int i; >> + >> +xhci_info(xhci, >> + "Zeroing 64bit base registers, expecting fault\n"); >> + >> +/* Clear HSEIE so that faults do not get signaled */ >> +val = readl(&xhci->op_regs->command); >> +val &= ~CMD_HSEIE; >> +writel(val, &xhci->op_regs->command); >> + >> +/* Clear HSE (aka FATAL) */ >> +val = readl(&xhci->op_regs->status); >> +val |= STS_FATAL; >> +writel(val, &xhci->op_regs->status); >> + >> +/* Now zero the registers, and brace for impact */ >> +val = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr); >> +if (upper_32_bits(val)) >> +xhci_write_64(xhci, 0, &xhci->op_regs->dcbaa_ptr); >> +val = xhci_read_64(xhci, &xhci->op_regs->cmd_ring); >> +if (upper_32_bits(val)) >> +xhci_write_64(xhci, 0, &xhci->op_regs->cmd_ring); >> + >> +for (i = 0; i < HCS_MAX_INTRS(xhci->hcs_params1); i++) { >> +struct xhci_intr_reg __iomem *ir; >> + >> +ir = &xhci->run_regs->ir_set[i]; >> +val = xhci_read_64(xhci, &ir->erst_base); >> +
[PATCH v3 2/3] xhci: Add quirk to zero 64bit registers on Renesas PCIe controllers
Some Renesas controllers get into a weird state if they are reset while programmed with 64bit addresses (they will preserve the top half of the address in internal, non visible registers). You end up with half the address coming from the kernel, and the other half coming from the firmware. Also, changing the programming leads to extra accesses even if the controller is supposed to be halted. The controller ends up with a fatal fault, and is then ripe for being properly reset. On the flip side, this is completely unsafe if the defvice isn't behind an IOMMU, so we have to make sure that this is the case. Can you say "broken"? This is an alternative method to the one introduced in 8466489ef5ba ("xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue"), which will subsequently be removed. Tested-by: Domenico Andreoli Signed-off-by: Marc Zyngier --- drivers/usb/host/xhci-pci.c | 8 -- drivers/usb/host/xhci.c | 65 + drivers/usb/host/xhci.h | 1 + 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 85ffda85f8ab..e0a0a12871e2 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -196,11 +196,15 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) xhci->quirks |= XHCI_BROKEN_STREAMS; } if (pdev->vendor == PCI_VENDOR_ID_RENESAS && - pdev->device == 0x0014) + pdev->device == 0x0014) { xhci->quirks |= XHCI_TRUST_TX_LENGTH; + xhci->quirks |= XHCI_ZERO_64B_REGS; + } if (pdev->vendor == PCI_VENDOR_ID_RENESAS && - pdev->device == 0x0015) + pdev->device == 0x0015) { xhci->quirks |= XHCI_RESET_ON_RESUME; + xhci->quirks |= XHCI_ZERO_64B_REGS; + } if (pdev->vendor == PCI_VENDOR_ID_VIA) xhci->quirks |= XHCI_RESET_ON_RESUME; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 8dba26d3de07..4d9437279681 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -209,6 +209,68 @@ int xhci_reset(struct xhci_hcd *xhci) return ret; } +static void xhci_zero_64b_regs(struct xhci_hcd *xhci) +{ + struct device *dev = xhci_to_hcd(xhci)->self.sysdev; + int err, i; + u64 val; + + /* +* Some Renesas controllers get into a weird state if they are +* reset while programmed with 64bit addresses (they will preserve +* the top half of the address in internal, non visible +* registers). You end up with half the address coming from the +* kernel, and the other half coming from the firmware. Also, +* changing the programming leads to extra accesses even if the +* controller is supposed to be halted. The controller ends up with +* a fatal fault, and is then ripe for being properly reset. +* +* Special care is taken to only apply this if the device is behind +* an iommu. Doing anything when there is no iommu is definitely +* unsafe... +*/ + if (!(xhci->quirks & XHCI_ZERO_64B_REGS) || !dev->iommu_group) + return; + + xhci_info(xhci, "Zeroing 64bit base registers, expecting fault\n"); + + /* Clear HSEIE so that faults do not get signaled */ + val = readl(&xhci->op_regs->command); + val &= ~CMD_HSEIE; + writel(val, &xhci->op_regs->command); + + /* Clear HSE (aka FATAL) */ + val = readl(&xhci->op_regs->status); + val |= STS_FATAL; + writel(val, &xhci->op_regs->status); + + /* Now zero the registers, and brace for impact */ + val = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr); + if (upper_32_bits(val)) + xhci_write_64(xhci, 0, &xhci->op_regs->dcbaa_ptr); + val = xhci_read_64(xhci, &xhci->op_regs->cmd_ring); + if (upper_32_bits(val)) + xhci_write_64(xhci, 0, &xhci->op_regs->cmd_ring); + + for (i = 0; i < HCS_MAX_INTRS(xhci->hcs_params1); i++) { + struct xhci_intr_reg __iomem *ir; + + ir = &xhci->run_regs->ir_set[i]; + val = xhci_read_64(xhci, &ir->erst_base); + if (upper_32_bits(val)) + xhci_write_64(xhci, 0, &ir->erst_base); + val= xhci_read_64(xhci, &ir->erst_dequeue); + if (upper_32_bits(val)) + xhci_write_64(xhci, 0, &ir->erst_dequeue); + } + + /* Wait for the fault to appear. It will be cleared on reset */ + err = xhci_handshake(&xhci->op_regs->status, +STS_FATAL, STS_FATAL, +XHCI_MAX_HALT_USEC); + if (!err) + xhci_info(xhci, "Fault detected\n"); +} #ifdef CONFIG_USB_PCI /* @@ -1004,6 +1066,7 @@ int xhci_resume(struct xhci_hcd *xhci, bo
[PATCH v3 1/3] xhci: Allow more than 32 quirks
We now have 32 different quirks, and the field that holds them is full. Let's bump it up to the next stage so that we can handle some more... The type is now an unsigned long long, which is 64bit on most architectures. We take this opportunity to change the quirks from using (1 << x) to BIT_ULL(x). Tested-by: Domenico Andreoli Signed-off-by: Marc Zyngier --- drivers/usb/host/xhci.c | 6 ++--- drivers/usb/host/xhci.h | 66 - 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 711da3306b14..8dba26d3de07 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -33,8 +33,8 @@ static int link_quirk; module_param(link_quirk, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(link_quirk, "Don't clear the chain bit on a link TRB"); -static unsigned int quirks; -module_param(quirks, uint, S_IRUGO); +static unsigned long long quirks; +module_param(quirks, ullong, S_IRUGO); MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default"); /* TODO: copied from ehci-hcd.c - can this be refactored? */ @@ -4963,7 +4963,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) return retval; xhci_dbg(xhci, "Called HCD init\n"); - xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%08x\n", + xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%016llx\n", xhci->hcc_params, xhci->hci_version, xhci->quirks); return 0; diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 6dfc4867dbcf..e322afd3f08b 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1787,12 +1787,12 @@ struct xhci_hcd { #define XHCI_STATE_DYING (1 << 0) #define XHCI_STATE_HALTED (1 << 1) #define XHCI_STATE_REMOVING(1 << 2) - unsigned intquirks; -#defineXHCI_LINK_TRB_QUIRK (1 << 0) -#define XHCI_RESET_EP_QUIRK(1 << 1) -#define XHCI_NEC_HOST (1 << 2) -#define XHCI_AMD_PLL_FIX (1 << 3) -#define XHCI_SPURIOUS_SUCCESS (1 << 4) + unsigned long long quirks; +#defineXHCI_LINK_TRB_QUIRK BIT_ULL(0) +#define XHCI_RESET_EP_QUIRKBIT_ULL(1) +#define XHCI_NEC_HOST BIT_ULL(2) +#define XHCI_AMD_PLL_FIX BIT_ULL(3) +#define XHCI_SPURIOUS_SUCCESS BIT_ULL(4) /* * Certain Intel host controllers have a limit to the number of endpoint * contexts they can handle. Ideally, they would signal that they can't handle @@ -1802,35 +1802,35 @@ struct xhci_hcd { * commands, reset device commands, disable slot commands, and address device * commands. */ -#define XHCI_EP_LIMIT_QUIRK(1 << 5) -#define XHCI_BROKEN_MSI(1 << 6) -#define XHCI_RESET_ON_RESUME (1 << 7) -#defineXHCI_SW_BW_CHECKING (1 << 8) -#define XHCI_AMD_0x96_HOST (1 << 9) -#define XHCI_TRUST_TX_LENGTH (1 << 10) -#define XHCI_LPM_SUPPORT (1 << 11) -#define XHCI_INTEL_HOST(1 << 12) -#define XHCI_SPURIOUS_REBOOT (1 << 13) -#define XHCI_COMP_MODE_QUIRK (1 << 14) -#define XHCI_AVOID_BEI (1 << 15) -#define XHCI_PLAT (1 << 16) -#define XHCI_SLOW_SUSPEND (1 << 17) -#define XHCI_SPURIOUS_WAKEUP (1 << 18) +#define XHCI_EP_LIMIT_QUIRKBIT_ULL(5) +#define XHCI_BROKEN_MSIBIT_ULL(6) +#define XHCI_RESET_ON_RESUME BIT_ULL(7) +#defineXHCI_SW_BW_CHECKING BIT_ULL(8) +#define XHCI_AMD_0x96_HOST BIT_ULL(9) +#define XHCI_TRUST_TX_LENGTH BIT_ULL(10) +#define XHCI_LPM_SUPPORT BIT_ULL(11) +#define XHCI_INTEL_HOSTBIT_ULL(12) +#define XHCI_SPURIOUS_REBOOT BIT_ULL(13) +#define XHCI_COMP_MODE_QUIRK BIT_ULL(14) +#define XHCI_AVOID_BEI BIT_ULL(15) +#define XHCI_PLAT BIT_ULL(16) +#define XHCI_SLOW_SUSPEND BIT_ULL(17) +#define XHCI_SPURIOUS_WAKEUP BIT_ULL(18) /* For controllers with a broken beyond repair streams implementation */ -#define XHCI_BROKEN_STREAMS(1 << 19) -#define XHCI_PME_STUCK_QUIRK (1 << 20) -#define XHCI_MTK_HOST (1 << 21) -#define XHCI_SSIC_PORT_UNUSED (1 << 22) -#define XHCI_NO_64BIT_SUPPORT (1 << 23) -#define XHCI_MISSING_CAS (1 << 24) +#define XHCI_BROKEN_STREAMSBIT_ULL(19) +#define XHCI_PME_STUCK_QUIRK BIT_ULL(20) +#define XHCI_MTK_HOST BIT_ULL(21) +#define XHCI_SSIC_PORT_UNUSED BIT_ULL(22) +#define XHCI_NO_64BIT_SUPPORT BIT_ULL(23) +#define XHCI_MISSING_CAS BIT_ULL(24) /* For controller with a broken Port Disable implementation */ -#define XHCI_BROKEN_PORT_PED (1 << 25) -#define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26) -#define XHCI_U2_DISABLE_WAKE (1 << 27) -#define XHCI_ASMEDIA_MODIFY_FLOWCONTROL(1 << 28) -#define XHCI_HW_LPM_DISABLE(1 << 29) -#define XHCI_SUSPEND_DELAY (1 << 30) -#define XHCI_INTEL_USB_ROLE_SW (1 << 31) +#define XHCI_BROKEN_PORT_PED BIT_ULL(25) +#define XHCI_LIMIT_ENDPOINT_IN
[PATCH v3 0/3] Revised Renesas uPD72020x workaround for 32bit DMA issue
Back around the 4.13 timeframe, we tried to address a rather bad issue with the Renesas uPD72020x USB3 controller family. They have trouble with the programming of the base addresses which tend to stick on XHCI reset. This makes transitionning from 64bit to 32bit addresses completely unsafe. This was observed on a fairly popular arm64 platform (AMD Opteron 1100, which has all of its memory above 4GB). The fix was to perform a PCI reset of the device, but we have had multiple reports that this generated regressions (the controller not being usable after the patch was applied). This series reverts the problematic patch, and tries to address it in a more constrained way. If the controller is behind an IOMMU (and only in that case), we zero its base addresses before reseting it. In the affected configuration, this has the effect of putting the device in a state where the XHCI reset will be effective. It must be noted that in the absence of an IOMMU (and maybe even in its presence), this device is completely unsafe, and may silently corrupt memory. * From v1: - Fixed stupid hunk misplacement, now properly moved back to patch 2 - Converted all quirks to BIT_ULL() * From v2: - Moved zeroing code to its own function - Also perform the zeroing on hibernate Marc Zyngier (3): xhci: Allow more than 32 quirks xhci: Add quirk to zero 64bit registers on Renesas PCIe controllers Revert "xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue" drivers/usb/host/pci-quirks.c | 20 drivers/usb/host/pci-quirks.h | 1 - drivers/usb/host/xhci-pci.c | 15 - drivers/usb/host/xhci.c | 71 +-- drivers/usb/host/xhci.h | 67 5 files changed, 108 insertions(+), 66 deletions(-) -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/3] Revert "xhci: Reset Renesas uPD72020x USB controller for 32-bit DMA issue"
This reverts commit 8466489ef5ba48272ba4fa4ea9f8f403306de4c7. Now that we can properly reset the uPD72020x without a hard PCI reset, let's get rid of the existing quirks. Tested-by: Domenico Andreoli Signed-off-by: Marc Zyngier --- drivers/usb/host/pci-quirks.c | 20 drivers/usb/host/pci-quirks.h | 1 - drivers/usb/host/xhci-pci.c | 7 --- 3 files changed, 28 deletions(-) diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index 67ad4bb6919a..3625a5c1a41b 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -1268,23 +1268,3 @@ static void quirk_usb_early_handoff(struct pci_dev *pdev) } DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_SERIAL_USB, 8, quirk_usb_early_handoff); - -bool usb_xhci_needs_pci_reset(struct pci_dev *pdev) -{ - /* -* Our dear uPD72020{1,2} friend only partially resets when -* asked to via the XHCI interface, and may end up doing DMA -* at the wrong addresses, as it keeps the top 32bit of some -* addresses from its previous programming under obscure -* circumstances. -* Give it a good wack at probe time. Unfortunately, this -* needs to happen before we've had a chance to discover any -* quirk, or the system will be in a rather bad state. -*/ - if (pdev->vendor == PCI_VENDOR_ID_RENESAS && - (pdev->device == 0x0014 || pdev->device == 0x0015)) - return true; - - return false; -} -EXPORT_SYMBOL_GPL(usb_xhci_needs_pci_reset); diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h index 4ca0d9b7e463..63c633077d9e 100644 --- a/drivers/usb/host/pci-quirks.h +++ b/drivers/usb/host/pci-quirks.h @@ -16,7 +16,6 @@ void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev); void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev); void usb_disable_xhci_ports(struct pci_dev *xhci_pdev); void sb800_prefetch(struct device *dev, int on); -bool usb_xhci_needs_pci_reset(struct pci_dev *pdev); bool usb_amd_pt_check_port(struct device *device, int port); #else struct pci_dev; diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index e0a0a12871e2..6372edf339d9 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -288,13 +288,6 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) driver = (struct hc_driver *)id->driver_data; - /* For some HW implementation, a XHCI reset is just not enough... */ - if (usb_xhci_needs_pci_reset(dev)) { - dev_info(&dev->dev, "Resetting\n"); - if (pci_reset_function_locked(dev)) - dev_warn(&dev->dev, "Reset failed"); - } - /* Prevent runtime suspending between USB-2 and USB-3 initialization */ pm_runtime_get_noresume(&dev->dev); -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control
Hi, On 23-05-18 16:37, Heikki Krogerus wrote: Trying to determine the USB port type with this mux is very difficult. To simplify the situation, always allowing user control, even if the port is USB Type-C port. Signed-off-by: Heikki Krogerus --- .../usb/roles/intel-xhci-usb-role-switch.c| 21 +-- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c index 30b07ea3a3c6..3f14153d753f 100644 --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c @@ -39,20 +39,6 @@ struct intel_xhci_usb_data { void __iomem *base; }; -struct intel_xhci_acpi_match { - const char *hid; - int hrv; -}; - -/* - * ACPI IDs for PMICs which do not support separate data and power role - * detection (USB ACA detection for micro USB OTG), we allow userspace to - * change the role manually on these. - */ -static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = { - { "INT33F4", 3 }, /* X-Powers AXP288 PMIC */ -}; - static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role) { struct intel_xhci_usb_data *data = dev_get_drvdata(dev); @@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct device *dev) static struct usb_role_switch_desc sw_desc = { .set = intel_xhci_usb_set_role, .get = intel_xhci_usb_get_role, + .allow_userspace_control = true, }; static int intel_xhci_usb_probe(struct platform_device *pdev) I hate to be pedantic here, but the sw_desc can and should be made const now, with that fixed the entire series is: Reviewed-by: Hans de Goede Regards, Hans @@ -146,7 +133,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct intel_xhci_usb_data *data; struct resource *res; - int i; data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); if (!data) @@ -159,11 +145,6 @@ static int intel_xhci_usb_probe(struct platform_device *pdev) if (!data->base) return -ENOMEM; - for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++) - if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1", -allow_userspace_ctrl_ids[i].hrv)) - sw_desc.allow_userspace_control = true; - platform_set_drvdata(pdev, data); data->role_sw = usb_role_switch_register(dev, &sw_desc); -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: f_uvc: Expose configuration name through video node
From: Kieran Bingham When utilising multiple instantiations of a UVC gadget on a composite device, there is no clear method to link a particular configuration to its respective video node. Provide a means for identifying the correct video node by exposing the name of the function configuration through sysfs. Signed-off-by: Kieran Bingham --- drivers/usb/gadget/function/f_uvc.c | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 1affa8e3a974..f326d1707633 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -421,10 +421,21 @@ uvc_function_disconnect(struct uvc_device *uvc) * USB probe and disconnect */ +static ssize_t config_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct uvc_device *uvc = dev_get_drvdata(dev); + + return sprintf(buf, "%s\n", uvc->func.fi->group.cg_item.ci_name); +} + +static DEVICE_ATTR_RO(config); + static int uvc_register_video(struct uvc_device *uvc) { struct usb_composite_dev *cdev = uvc->func.config->cdev; + int ret; /* TODO reference counting. */ uvc->vdev.v4l2_dev = &uvc->v4l2_dev; @@ -437,7 +448,17 @@ uvc_register_video(struct uvc_device *uvc) video_set_drvdata(&uvc->vdev, uvc); - return video_register_device(&uvc->vdev, VFL_TYPE_GRABBER, -1); + ret = video_register_device(&uvc->vdev, VFL_TYPE_GRABBER, -1); + if (ret < 0) + return ret; + + ret = device_create_file(&uvc->vdev.dev, &dev_attr_config); + if (ret < 0) { + video_unregister_device(&uvc->vdev); + return ret; + } + + return 0; } #define UVC_COPY_DESCRIPTOR(mem, dst, desc) \ @@ -875,6 +896,7 @@ static void uvc_unbind(struct usb_configuration *c, struct usb_function *f) INFO(cdev, "%s\n", __func__); + device_remove_file(&uvc->vdev.dev, &dev_attr_config); video_unregister_device(&uvc->vdev); v4l2_device_unregister(&uvc->v4l2_dev); -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usbip: dynamically allocate idev by nports found in sysfs
On Wed, May 23, 2018 at 10:44:57AM -0600, Shuah Khan wrote: > On 05/23/2018 03:22 AM, Michael Grzeschik wrote: > > As the amount of available ports varies by the kernels build > > configuration. To remove the limitation of the fixed 128 ports > > we allocate the amount of idevs by using the number we get > > from the kernel. > > > > Signed-off-by: Michael Grzeschik > > --- > > v1 -> v2: - reworked memory allocation into one calloc call > > - added error path on allocation failure > > v2 -> v3: - moved check for available nports to beginning of function > > > > Hmm. With this patch I see a segfault when I run usbip port command. > I think this patch is incomplete and more changes are needed to the > code that references the idev array. > > I can't take this patch. I will test it again tomorrow. Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | signature.asc Description: PGP signature
Re: usb HC busted?
Hi Mathias, On Fri, May 18, 2018 at 04:19:02PM +0300, Mathias Nyman wrote: > On 18.05.2018 16:04, Sudip Mukherjee wrote: > > Hi Mathias, > > > > On Fri, May 18, 2018 at 03:55:04PM +0300, Mathias Nyman wrote: > > > Hi, > > > > > > Looks like event for Transfer block (TRB) at 0x32a21060 was never > > > completed, > > > or at least not handled by xhci driver. > > > (either the event was never issued by hw, or something got messed up in > > > the driver along the way) > > > > > > HC doesn't look busted, it continues sending transfer completions events. > > > it is already at event 0x32a211d0, which is 23 TRBS later. (one TRB is > > > 0x10) > > > > > > This small log sinppet doesnt' say much about the reasons. > > > > > > Can you enable tracing for xhci and send me the output. > > > > I saw in another thread that you will ask for the trace log and so I > > was making a kernel with tracer enabled. I was also thinking of getting > > the logs from usbmon. Will that be of any help? > > > > Not sure yet, in this case i think traces are most interesting, then dynamic > debug messages > (which spams dmesg and affects timing) and last usbmon. We have finally reproduced the error while traces were on. The trace and the relevant part of the dmesg (when the error starts) are in: https://drive.google.com/open?id=1PbcYwL1a9ndtHw1MNjE6uVqb0fFX9jV8 Will request you to have a look and suggest what might be going wrong here. -- Regards Sudip -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
On Thu, May 24, 2018 at 12:39 AM, Greg Kroah-Hartman wrote: > On Wed, May 23, 2018 at 10:03:55AM -0400, Alan Stern wrote: >> On Wed, 23 May 2018, Nicolas Boichat wrote: >> >> > The "old" enumeration scheme is considerably faster (it takes >> > ~294ms instead of ~439ms to get the descriptor). >> > >> > It is currently only possible to use the old scheme globally >> > (/sys/module/usbcore/parameters/old_scheme_first), which is not >> > desirable as the new scheme was introduced to increase compatibility >> > with more devices. >> > >> > However, in our case, we care about time-to-active for a specific >> > USB device (which we make the firmware for), on a specific port >> > (that is pogo-pin based: not a standard USB port). This new >> > sysfs option makes it possible to use the old scheme on a single >> > port only. >> > >> > Signed-off-by: Nicolas Boichat >> > --- >> > >> > There are other "quirks" that we could add to reduce further >> > enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY >> > to 10ms instead of 50ms as used currently), but the logic is quite >> > similar, so it'd be good to have this reviewed first. >> >> I'm not opposed to the idea in principle, although I don't like your >> implementation because it breaks the original old_scheme_first >> parameter. I don't think it breaks the original parameter? I mean, /sys/module/usbcore/parameters/old_scheme_first is still a global default, while bit 0 of /sys/bus/usb/devices/x/y/z/quirks becomes a port-specific override. >> Let's see what some other people think. >> >> Yours is a rather special case, because you know exactly what device >> will be attached to a specific port. Still, I can see that sort of >> thing happening in constrained and special-purpose settings. >> >> How do you arrange to set the new quirk before the device is >> discovered? > > Yeah, this last question is what I had when looking at this. Or does it > not matter at first boot and only matters for wake-up? It does not matter on boot, we have plenty of time to enumerate the device. We use USB (auto-)suspend and remote wake, so no re-enumeration there either. It only matters on unplug/replug where the device needs to be re-enumerated. Somewhere in an init script, we would do this (we know in advance that usb1 port2 is the bus/port where we have our pogo-pin USB interface, so we can hard-code the path): echo 1 > /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/quirks We could try to add ACPI support (just like connect_type), but we don't strictly need it for our application. Thanks, -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Documentation On Gadget Drivers (g_ffs)
All that is in tree is ./Documentation/usb/functionfs.txt which I can not get the information I need from. I happened to find https://events.static.linuxfound.org/sites/events/files/slides/LinuxConNA-Make-your-own-USB-gadget-Andrzej.Pietrasiewicz.pdf, but the commands within did not work and I do not know how to troubleshoot. I am trying to work on getting libusbg or libusbgx on my device. Is that the recommended method? When using gadget drivers, how is the device-capable port specified if there is more than one? Cheers, R0b0t1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control
On Wed, May 23, 2018 at 08:03:43PM +0200, Hans de Goede wrote: > Hi, > > On 23-05-18 16:37, Heikki Krogerus wrote: > > Trying to determine the USB port type with this mux is very > > difficult. To simplify the situation, always allowing user > > control, even if the port is USB Type-C port. > > > > Signed-off-by: Heikki Krogerus > > --- > > .../usb/roles/intel-xhci-usb-role-switch.c| 21 +-- > > 1 file changed, 1 insertion(+), 20 deletions(-) > > > > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c > > b/drivers/usb/roles/intel-xhci-usb-role-switch.c > > index 30b07ea3a3c6..3f14153d753f 100644 > > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c > > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c > > @@ -39,20 +39,6 @@ struct intel_xhci_usb_data { > > void __iomem *base; > > }; > > -struct intel_xhci_acpi_match { > > - const char *hid; > > - int hrv; > > -}; > > - > > -/* > > - * ACPI IDs for PMICs which do not support separate data and power role > > - * detection (USB ACA detection for micro USB OTG), we allow userspace to > > - * change the role manually on these. > > - */ > > -static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = { > > - { "INT33F4", 3 }, /* X-Powers AXP288 PMIC */ > > -}; > > - > > static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role) > > { > > struct intel_xhci_usb_data *data = dev_get_drvdata(dev); > > @@ -139,6 +125,7 @@ static enum usb_role intel_xhci_usb_get_role(struct > > device *dev) > > static struct usb_role_switch_desc sw_desc = { > > .set = intel_xhci_usb_set_role, > > .get = intel_xhci_usb_get_role, > > + .allow_userspace_control = true, > > }; > > static int intel_xhci_usb_probe(struct platform_device *pdev) > > > I hate to be pedantic here, but the sw_desc can and should be made > const now, with that fixed the entire series is: Sure, I'll fix that. > Reviewed-by: Hans de Goede Thanks Hans. I'll send the v3. I need to add one more fix to the series. I got a report where somebody hit the issue related to the runtime PM we talked about a while back: If xHCI is suspended, we fail to program the mux registers as they are part of xHCI MMIO, so I will add a patch to this series where I enable runtime PM here to fix that problem. Cheers, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] usb: roles: intel_xhci: Always allow user control
On Wed, May 23, 2018 at 07:10:58PM +0300, Sergei Shtylyov wrote: > On 05/23/2018 05:37 PM, Heikki Krogerus wrote: > > > Trying to determine the USB port type with this mux is very > > difficult. To simplify the situation, always allowing user > >s/allowing/allow/? Else the statement doesn't parse for me. :-) Thanks, I'll fix that, and the other patch too. Cheers, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html