Re: [PATCH drivers\usb\serial\pl2303.c & pl2303.h] Prolific's PL2303G: new USB to UART chip
Hi Greg, The patch file: diffpl2303.patch is attached.. Please you kindly check... Thanks! Charles Charles Yeh 於 2018年11月20日 週二 下午5:35寫道: > > Hi Greg, >Try again...no problem... > > make one patch for all of the files modified, <<---OK. > > proper changelog text and a signed-off-by line <<-- I will study > Documentation/SubmittingPatches again. > > use tabs, not spaces <<---OK. > > Greg KH 於 2018年11月20日 週二 下午5:28寫道: > > > > On Tue, Nov 20, 2018 at 05:21:04PM +0800, Charles Yeh wrote: > > > Hi Johan, > > > After read Documentation\process\submitting-patches.rst, > > > I have write some describe in attach file : "diffpl2303c.patch" > > > "diffpl2303h.patch" > > > > > > If this file does not meet the file requirements, please let me > > > know how to do it. > > > > This is close, but not quite there yet. > > > > You need to make one patch for all of the files modified, look at all of > > the patches on the mailing list for examples of this. > > > > Also, you need a proper changelog text and a signed-off-by line, the > > file Documentation/SubmittingPatches in the kernel source tree will > > describe all of this and how to do it. > > > > And finally, your patch needs to properly use tabs, not spaces, look at > > the diff you generated for examples of how the lines do not align > > properly now. > > > > Can you try again please? > > > > thanks, > > > > greg k-h diffpl2303.patch Description: Binary data
RE: [RFC PATCH v2 05/15] usb:cdns3: Added DRD support
>> Patch adds supports for detecting Host/Device mode. >> Controller has additional OTG register that allow >> implement even whole OTG functionality. >> At this moment patch adds support only for detecting >> the appropriate mode based on strap pins and ID pin. >> >> Signed-off-by: Pawel Laszczak >> --- >> drivers/usb/cdns3/Makefile | 2 +- >> drivers/usb/cdns3/core.c | 27 +++-- >> drivers/usb/cdns3/drd.c| 229 + >> drivers/usb/cdns3/drd.h| 122 >> 4 files changed, 372 insertions(+), 8 deletions(-) >> create mode 100644 drivers/usb/cdns3/drd.c >> create mode 100644 drivers/usb/cdns3/drd.h >> >> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile >> index 02d25b23c5d3..e779b2a2f8eb 100644 >> --- a/drivers/usb/cdns3/Makefile >> +++ b/drivers/usb/cdns3/Makefile >> @@ -1,5 +1,5 @@ >> obj-$(CONFIG_USB_CDNS3)+= cdns3.o >> obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci.o >> >> -cdns3-y:= core.o >> +cdns3-y:= core.o drd.o >> cdns3-pci-y:= cdns3-pci-wrap.o >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c >> index f9055d4da67f..dbee4325da7f 100644 >> --- a/drivers/usb/cdns3/core.c >> +++ b/drivers/usb/cdns3/core.c >> @@ -17,6 +17,7 @@ >> >> #include "gadget.h" >> #include "core.h" >> +#include "drd.h" >> >> static inline struct cdns3_role_driver >> *cdns3_get_current_role_driver(struct cdns3 *cdns) >> { >> @@ -57,8 +58,10 @@ static inline void cdns3_role_stop(struct cdns3 *cdns) >> static enum cdns3_roles cdns3_get_role(struct cdns3 *cdns) >> { >> if (cdns->roles[CDNS3_ROLE_HOST] && cdns->roles[CDNS3_ROLE_GADGET]) { >> - //TODO: implements selecting device/host mode >> - return CDNS3_ROLE_HOST; >> + if (cdns3_is_host(cdns)) >> + return CDNS3_ROLE_HOST; >> + if (cdns3_is_device(cdns)) >> + return CDNS3_ROLE_GADGET; >> } >> return cdns->roles[CDNS3_ROLE_HOST] >> ? CDNS3_ROLE_HOST >> @@ -124,6 +127,12 @@ static irqreturn_t cdns3_irq(int irq, void *data) >> struct cdns3 *cdns = data; >> irqreturn_t ret = IRQ_NONE; >> >> + if (cdns->dr_mode == USB_DR_MODE_OTG) { >> + ret = cdns3_drd_irq(cdns); >> + if (ret == IRQ_HANDLED) >> + return ret; >> + } >> + >> /* Handle device/host interrupt */ >> if (cdns->role != CDNS3_ROLE_END) >> ret = cdns3_get_current_role_driver(cdns)->irq(cdns); >> @@ -176,11 +185,8 @@ static void cdns3_role_switch(struct work_struct *work) >> >> cdns = container_of(work, struct cdns3, role_switch_wq); >> >> - //TODO: implements this functions. >> - //host = cdns3_is_host(cdns); >> - //device = cdns3_is_device(cdns); >> - host = 1; >> - device = 0; >> + host = cdns3_is_host(cdns); >> + device = cdns3_is_device(cdns); >> >> if (host) >> role = CDNS3_ROLE_HOST; >> @@ -194,6 +200,12 @@ static void cdns3_role_switch(struct work_struct *work) >> pm_runtime_get_sync(cdns->dev); >> cdns3_role_stop(cdns); >> >> + if (cdns->desired_dr_mode != cdns->current_dr_mode) { >> + cdns3_drd_update_mode(cdns); >> + host = cdns3_is_host(cdns); >> + device = cdns3_is_device(cdns); >> + } >> + >> if (host) { >> if (cdns->roles[CDNS3_ROLE_HOST]) >> cdns3_do_role_switch(cdns, CDNS3_ROLE_HOST); >> @@ -287,6 +299,7 @@ static int cdns3_probe(struct platform_device *pdev) >> if (ret) >> goto err2; >> >> + ret = cdns3_drd_init(cdns); >> if (ret) >> goto err2; >> >> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c >> new file mode 100644 >> index ..ac741c80e776 >> --- /dev/null >> +++ b/drivers/usb/cdns3/drd.c >> @@ -0,0 +1,229 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Cadence USBSS DRD Driver. >> + * >> + * Copyright (C) 2018 Cadence. >> + * >> + * Author: Pawel Laszczak > + * >> + */ >> +#include >> +#include >> +#include >> +#include >> + >> +#include "gadget.h" >> +#include "drd.h" >> + >> +/** >> + * cdns3_set_mode - change mode of OTG Core >> + * @cdns: pointer to context structure >> + * @mode: selected mode from cdns_role >> + */ >> +void cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode) >> +{ >> + u32 reg; >> + >> + cdns->current_dr_mode = mode; >> + switch (mode) { >> + case USB_DR_MODE_PERIPHERAL: >> + dev_info(cdns->dev, "Set controller to Gadget mode\n"); >> + writel(OTGCMD_DEV_BUS_REQ | OTGCMD_OTG_DIS, >> + &cdns->otg_regs->cmd); >> + break;
RE: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
> >> Hi Roger >> >> >On 18/11/18 12:09, Pawel Laszczak wrote: >> >> Patch adds core.c and core.h file that implements initialization >> >> of platform driver and adds function responsible for selecting, >> >> switching and running appropriate Device/Host mode. >> >> >> >> Signed-off-by: Pawel Laszczak >> >> --- >> >> drivers/usb/cdns3/Makefile | 2 + >> >> drivers/usb/cdns3/core.c | 413 + >> >> drivers/usb/cdns3/core.h | 100 + >> >> 3 files changed, 515 insertions(+) >> >> create mode 100644 drivers/usb/cdns3/core.c >> >> create mode 100644 drivers/usb/cdns3/core.h >> >> >> >> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile >> >> index dcdd62003c6a..02d25b23c5d3 100644 >> >> --- a/drivers/usb/cdns3/Makefile >> >> +++ b/drivers/usb/cdns3/Makefile >> >> @@ -1,3 +1,5 @@ >> >> +obj-$(CONFIG_USB_CDNS3) += cdns3.o >> >> obj-$(CONFIG_USB_CDNS3_PCI_WRAP)+= cdns3-pci.o >> >> >> >> +cdns3-y := core.o >> >> cdns3-pci-y := cdns3-pci-wrap.o >> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c >> >> new file mode 100644 >> >> index ..f9055d4da67f >> >> --- /dev/null >> >> +++ b/drivers/usb/cdns3/core.c >> >> @@ -0,0 +1,413 @@ >> >> +// SPDX-License-Identifier: GPL-2.0 >> >> +/* >> >> + * Cadence USBSS DRD Driver. >> >> + * >> >> + * Copyright (C) 2018 Cadence. >> >> + * >> >> + * Author: Peter Chen >> >> + * Pawel Laszczak >> >> + */ >> >> + >> >> +#include >> >> +#include >> >> +#include >> >> +#include >> >> +#include >> >> +#include >> >> + >> >> +#include "gadget.h" >> >> +#include "core.h" >> >> + >> >> +static inline struct cdns3_role_driver >> >> *cdns3_get_current_role_driver(struct cdns3 *cdns) >> >> +{ >> >> +WARN_ON(cdns->role >= CDNS3_ROLE_END || !cdns->roles[cdns->role]); >> >> +return cdns->roles[cdns->role]; >> >> +} >> >> + >> >> +static inline int cdns3_role_start(struct cdns3 *cdns, enum cdns3_roles >> >> role) >> >> +{ >> >> +int ret; >> >> + >> >> +if (role >= CDNS3_ROLE_END) >> > >> >WARN_ON()? >> I agree. >> > >> >> +return 0; >> >> + >> >> +if (!cdns->roles[role]) >> >> +return -ENXIO; >> >> + >> >> +mutex_lock(&cdns->mutex); >> >> +cdns->role = role; >> >> +ret = cdns->roles[role]->start(cdns); >> >> +mutex_unlock(&cdns->mutex); >> >> +return ret; >> >> +} >> >> + >> >> +static inline void cdns3_role_stop(struct cdns3 *cdns) >> >> +{ >> >> +enum cdns3_roles role = cdns->role; >> >> + >> >> +if (role == CDNS3_ROLE_END) >> > >> >WARN_ON(role >= CNDS3_ROLE_END) ? >> I agree >> > >> >> +return; >> >> + >> >> +mutex_lock(&cdns->mutex); >> >> +cdns->roles[role]->stop(cdns); >> >> +cdns->role = CDNS3_ROLE_END; >> > >> >Why change the role here? You are just stopping the role not changing it. >> >I think cdns->role should remain unchanged, so we can call >> >cdns3_role_start() >> >if required without error. >> >> This line is unnecessary. >> >> > >> >> +mutex_unlock(&cdns->mutex); >> >> +} >> >> + >> >> +static enum cdns3_roles cdns3_get_role(struct cdns3 *cdns) >> >> +{ >> >> +if (cdns->roles[CDNS3_ROLE_HOST] && cdns->roles[CDNS3_ROLE_GADGET]) { >> >> +//TODO: implements selecting device/host mode >> >> +return CDNS3_ROLE_HOST; >> >> +} >> >> +return cdns->roles[CDNS3_ROLE_HOST] >> >> +? CDNS3_ROLE_HOST >> >> +: CDNS3_ROLE_GADGET; >> > >> >Why not just >> > return cdns->role; >> > >> >I'm wondering if we really need this function >> >> TODO will look likie: >> if (cdns3_is_host(cdns)) >> return CDNS3_ROLE_HOST; >> if (cdns3_is_device(cdns)) >> return CDNS3_ROLE_GADGET; >> >> Function selects initial role. Before invoking it the role is unknown. >> I think that function name should be changed because current name can be >> misleading. >> >> I will change it to cdns3_get_initial_role. >> . >> >> +} >> > >> >> + >> >> +/** >> >> + * cdns3_core_init_role - initialize role of operation >> >> + * @cdns: Pointer to cdns3 structure >> >> + * >> >> + * Returns 0 on success otherwise negative errno >> >> + */ >> >> +static int cdns3_core_init_role(struct cdns3 *cdns) >> >> +{ >> >> +struct device *dev = cdns->dev; >> >> +enum usb_dr_mode dr_mode; >> >> + >> >> +dr_mode = usb_get_dr_mode(dev); >> >> +cdns->role = CDNS3_ROLE_END; >> >> + >> >> +/* >> >> + * If driver can't read mode by means of usb_get_dr_mdoe function >> >> then >> >> + * chooses mode according with Kernel configuration. This setting >> >> + * can be restricted later depending on strap pin configuration. >> >> + */ >> >> +if (dr_mode == USB_DR_MODE_UNKNOWN) { >> >> +if (IS_ENABLED(CONFIG_USB_CDNS3_HOST) && >> >> +IS_ENABLED(CONF
Re: usb: thoughts of adding more support for FT232H
On 12/5/18 11:17 PM, Anatolij Gustschin wrote: Hi, On Wed, 5 Dec 2018 22:10:40 +0800 Song Qiang songqiang1304...@gmail.com wrote: ... I've been developing some iio device drivers and found that some people would like to test their devices with a qemu system which requires an i2c or spi port on our development hosts. Usually this is achieved with a DLN-2 adapter, while this is a bit difficult for me because it costs ~175$ in my country. Then I found that FTDI's FT232H supports both these two modes and costs only less than 5$ but without full support in kernel. The ftdi-sio driver supports FT232H only as a serial converter. So I'm planning to write a mfd driver for it supports both these three modes, here are my thoughts: There already has been a discussion [1] about adding an MFD driver for FT232H, since the operating modes are mutually exclusive (and bus pins shared between different modes), the MFD approach doesn't seem to be a good fit. - This device cannot support these three modes together because they share some common pins, so I'm planning to add a sysfs entry 'current_mode' for selecting which mode the device should be working on. - This device is in uart mode on reset, so default mode would be reset, too. This also helps for people only want to use this as a serial converter feels nothing has happened (compatible). - I was trying to reuse the ftdi-sio driver but it seems like mfd can only register platform devices, while this is a usb driver. I may have to copy some functions from this driver. Would you share any ideas? I'd appreciate it. There is a patch series [2] adding an interface driver for FT232H- based adapter devices, it already supports adding custom MPSSE based SPI busses with SPI slaves for a custom USB PID. It already supports adding custom CBUS-/MPSSE-GPIO adapters for user-defined USB PID. Adding I2C driver/adapter support should be easy, too. Maybe you can re-use it. Thanks, Anatolij [1] https://patchwork.kernel.org/patch/9828985 [2] https://patchwork.kernel.org/project/linux-usb/list/?series=48255 Hi Anatolij, Johan, Great work you've done! While there still some thing confusing me. Patch series [2] added new custom PIDs to distinguish the mode this device should be in when powered on, is this right? Since USB has a convention for all the VIDs and PIDs, is this really a good approach to use some us-defined PIDs? In the discussion [1] #4, Johan said that mfd is not suitable for this situation because 'all drivers for these devices be able to retrieve the current mode during probe and only bind when the mode matches'. I think this is saying that we can only register these devices(i2c, spi, gpio) when we plug it in, but FT232H's functions are surely mutually exclusive, so can't we dynamically register these devices in userspace? I mean through a sysfs interface, and through the implementation functions of this interface, we can try to use mfd_add_devices() and mfd_remove_devices() to unload one function(like uart) and load it as another device like a spi adapter. Is there any side effects of doing this in this way? And also my English may be not very well so please correct me if I'm understanding anything wrong. :) yours, Song
Re: [PATCH] USB: qcaux: Add Motorola modem UARTs
On Wed, Dec 05, 2018 at 05:54:07PM -0800, Tony Lindgren wrote: > Hi, > > * Johan Hovold [181205 06:17]: > > On Sun, Dec 02, 2018 at 05:34:24PM -0800, Tony Lindgren wrote: > > > On Motorola Mapphone devices such as Droid 4 there are five USB ports > > > that do not use the same layout as Gobi 1K/2K/etc devices listed in > > > qcserial.c. So we should use qcaux.c or option.c as noted by > > > Dan Williams . > > > > > > The ff/ff/ff interfaces seem to always be UARTs on Motorola devices. > > > And we should not add interfaces with 0x0a class (CDC Data) as they > > > are part of a multi-interface function like for example interface > > > 0x22b8:0x4281 as noted by Bjørn Mork . > > > > Can you post the output of usb-devices (or lsusb -v) for these three > > devices (PIDs)? > > Here's two out of three for you to look at. They're all listed in > drivers/usb/serial/mdm6600.c in at least the Motorola Mapphone > Android kernels, see for example the LineageOS kernel at [0] if > you want to look at the USB serial driver. > > I don't have a device with 9600 with 0x2e0a id. > > [0] > https://github.com/LineageOS/android_kernel_motorola_omap4-common/blob/cm-14.1/drivers/usb/serial/mdm6600.c Thanks for the pointer. > > 8< - > Bus 001 Device 002: ID 22b8:4281 > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 2.00 > bDeviceClass0 > bDeviceSubClass 0 > bDeviceProtocol 0 > bMaxPacketSize064 > idVendor 0x22b8 > idProduct 0x4281 This PID is not included in your patch however. > bcdDevice0.00 > iManufacturer 1 Motorola, Incorporated > iProduct2 Flash MZ600 > iSerial 0 > bNumConfigurations 1 > Configuration Descriptor: > bLength 9 > bDescriptorType 2 > wTotalLength 0x0020 > bNumInterfaces 1 > bConfigurationValue 1 > iConfiguration 3 Motorola Configuration > bmAttributes 0xe0 > Self Powered > Remote Wakeup > MaxPower 500mA > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber0 > bAlternateSetting 0 > bNumEndpoints 2 > bInterfaceClass10 > bInterfaceSubClass 0 > bInterfaceProtocol252 And wouldn't match on ff/ff/ff in any case. > iInterface 5 Motorola Flash > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x81 EP 1 IN > bmAttributes2 > Transfer TypeBulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 0 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x01 EP 1 OUT > bmAttributes2 > Transfer TypeBulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 0 > Device Status: 0x > (Bus Powered) > > Bus 003 Device 109: ID 22b8:900e > Device Descriptor: > bLength18 > bDescriptorType 1 > bcdUSB 2.00 > bDeviceClass0 > bDeviceSubClass 0 > bDeviceProtocol 0 > bMaxPacketSize064 > idVendor 0x22b8 > idProduct 0x900e > bcdDevice0.00 > iManufacturer 1 Motorola, Incorporated > iProduct2 Flash MZ600 > iSerial 0 > bNumConfigurations 1 > Configuration Descriptor: > bLength 9 > bDescriptorType 2 > wTotalLength 0x0020 > bNumInterfaces 1 > bConfigurationValue 1 > iConfiguration 3 Motorola Configuration > bmAttributes 0xe0 > Self Powered > Remote Wakeup > MaxPower 500mA > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber0 > bAlternateSetting 0 > bNumEndpoints 2 > bInterfaceClass 255 > bInterfaceSubClass255 > bInterfaceProtocol255 > iInterface 0 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x81 EP 1 IN > bmAttributes2 > Transfer TypeBulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInter
Re: [PATCH v3 2/2] usb: dwc3: gadget: Report isoc transfer frame number
Hi Felipe, On 12/5/2018 1:15 AM, Felipe Balbi wrote: > Hi, > > Thinh Nguyen writes: >> Implement the new frame_number API to report the isochronous interval >> frame number. This patch checks and reports the interval in which the >> isoc transfer was transmitted or received via the Isoc-First TRB SOF >> number field. >> >> Signed-off-by: Thinh Nguyen >> --- >> Change in v3: >> - Implement the change with the redefined frame_number meaning >> Change in v2: >> - Capture frame number at request cleanup >> >> drivers/usb/dwc3/core.h | 1 + >> drivers/usb/dwc3/gadget.c | 13 + >> 2 files changed, 14 insertions(+) >> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index ed0359d1216d..2c9f7a93147a 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -777,6 +777,7 @@ enum dwc3_link_state { >> #define DWC3_TRB_CTRL_ISP_IMI BIT(10) >> #define DWC3_TRB_CTRL_IOC BIT(11) >> #define DWC3_TRB_CTRL_SID_SOFN(n) (((n) & 0x) << 14) >> +#define DWC3_TRB_CTRL_GET_SID_SOFN(n) (((n) & (0x << 14)) >> 14) >> >> #define DWC3_TRBCTL_TYPE(n) ((n) & (0x3f << 4)) >> #define DWC3_TRBCTL_NORMAL DWC3_TRB_CTRL_TRBCTL(1) >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index 679c12e14522..2de563124fc1 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -2254,6 +2254,19 @@ static int >> dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, >> if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO)) >> trb->ctrl &= ~DWC3_TRB_CTRL_HWO; >> >> +/* >> + * For isochronous transfers, the first TRB in a service interval must >> + * have the Isoc-First type. Track and report its interval frame number. >> + */ >> +if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && >> +(trb->ctrl & DWC3_TRBCTL_ISOCHRONOUS_FIRST)) { >> +unsigned int frame_number; >> + >> +frame_number = DWC3_TRB_CTRL_GET_SID_SOFN(trb->ctrl); >> +frame_number &= ~(dep->interval - 1); >> +req->request.frame_number = frame_number; >> +} > could you refresh my memory on how this is going to be used? > Sure. This informs the upper layer driver what interval the isoc request went out. The users can use this information for applications that require synchronization with the host. Thinh
[PATCH] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
Current matching rules ensure that the voltage range of selected Source Capability is entirely within the range defined in one of the Sink Capabilities. This is reasonable but not practical because Sink may not support wide range of voltage when sinking power while Source could advertise its capabilities in raletively wider range. For example, a Source PDO advertising 3.3V-11V@3A (9V Prog of Fixed Nominal Voltage) will not be selected if the Sink requires 5V-12V@3A PPS power. However, the Sink could work well if the requested voltage range in RDOs is 5V-11V@3A. To improve the usability, change the matching rules to what listed below: a. The Source PDO is selectable if any portion of the voltage range overlaps one of the Sink PDO's voltage range. b. The maximum operational voltage will be the lower one between the selected Source PDO and the matching Sink PDO. c. The maximum power will be the maximum operational voltage times the maximum current defined in the selected Source PDO d. Select the Source PDO with the highest maximum power Signed-off-by: Kyle Tso --- drivers/usb/typec/tcpm/tcpm.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 3620efee2688..3001df7bd602 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -2213,7 +2213,8 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) unsigned int i, j, max_mw = 0, max_mv = 0; unsigned int min_src_mv, max_src_mv, src_ma, src_mw; unsigned int min_snk_mv, max_snk_mv; - u32 pdo; + unsigned int max_op_mv; + u32 pdo, src, snk; unsigned int src_pdo = 0, snk_pdo = 0; /* @@ -2263,16 +2264,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) continue; } - if (max_src_mv <= max_snk_mv && - min_src_mv >= min_snk_mv) { + if (min_src_mv <= max_snk_mv && + max_src_mv >= min_snk_mv) { + max_op_mv = min(max_src_mv, max_snk_mv); + src_mw = (max_op_mv * src_ma) / 1000; /* Prefer higher voltages if available */ if ((src_mw == max_mw && -min_src_mv > max_mv) || +max_op_mv > max_mv) || src_mw > max_mw) { src_pdo = i; snk_pdo = j; max_mw = src_mw; - max_mv = max_src_mv; + max_mv = max_op_mv; } } } @@ -2285,14 +2288,16 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) } if (src_pdo) { - pdo = port->source_caps[src_pdo]; - - port->pps_data.min_volt = pdo_pps_apdo_min_voltage(pdo); - port->pps_data.max_volt = pdo_pps_apdo_max_voltage(pdo); - port->pps_data.max_curr = - min_pps_apdo_current(pdo, port->snk_pdo[snk_pdo]); + src = port->source_caps[src_pdo]; + snk = port->snk_pdo[snk_pdo]; + + port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src), + pdo_pps_apdo_min_voltage(snk)); + port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src), + pdo_pps_apdo_max_voltage(snk)); + port->pps_data.max_curr = min_pps_apdo_current(src, snk); port->pps_data.out_volt = - min(pdo_pps_apdo_max_voltage(pdo), port->pps_data.out_volt); + min(port->pps_data.max_volt, port->pps_data.out_volt); port->pps_data.op_curr = min(port->pps_data.max_curr, port->pps_data.op_curr); } -- 2.20.0.rc2.403.gdbc3b29805-goog
Re: [PATCH] USB: qcaux: Add Motorola modem UARTs
Hi, * Johan Hovold [181205 06:17]: > On Sun, Dec 02, 2018 at 05:34:24PM -0800, Tony Lindgren wrote: > > On Motorola Mapphone devices such as Droid 4 there are five USB ports > > that do not use the same layout as Gobi 1K/2K/etc devices listed in > > qcserial.c. So we should use qcaux.c or option.c as noted by > > Dan Williams . > > > > The ff/ff/ff interfaces seem to always be UARTs on Motorola devices. > > And we should not add interfaces with 0x0a class (CDC Data) as they > > are part of a multi-interface function like for example interface > > 0x22b8:0x4281 as noted by Bjørn Mork . > > Can you post the output of usb-devices (or lsusb -v) for these three > devices (PIDs)? Here's two out of three for you to look at. They're all listed in drivers/usb/serial/mdm6600.c in at least the Motorola Mapphone Android kernels, see for example the LineageOS kernel at [0] if you want to look at the USB serial driver. I don't have a device with 9600 with 0x2e0a id. Regards, Tony [0] https://github.com/LineageOS/android_kernel_motorola_omap4-common/blob/cm-14.1/drivers/usb/serial/mdm6600.c 8< - Bus 001 Device 002: ID 22b8:4281 Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x22b8 idProduct 0x4281 bcdDevice0.00 iManufacturer 1 Motorola, Incorporated iProduct2 Flash MZ600 iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 0x0020 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 3 Motorola Configuration bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass10 bInterfaceSubClass 0 bInterfaceProtocol252 iInterface 5 Motorola Flash Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x01 EP 1 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 Device Status: 0x (Bus Powered) Bus 003 Device 109: ID 22b8:900e Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x22b8 idProduct 0x900e bcdDevice0.00 iManufacturer 1 Motorola, Incorporated iProduct2 Flash MZ600 iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 0x0020 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 3 Motorola Configuration bmAttributes 0xe0 Self Powered Remote Wakeup MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 255 bInterfaceSubClass255 bInterfaceProtocol255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 32 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x01 EP 1 OUT bmAttributes2 Transfer TypeBulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 32 Device Status: 0x (Bus Powered)
RE: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
+ Tomek Klimek >> > + >> > +static inline void cdns3_role_stop(struct cdns3 *cdns) >> > +{ >> > + enum cdns3_roles role = cdns->role; >> > + >> > + if (role == CDNS3_ROLE_END) >> >> WARN_ON(role >= CNDS3_ROLE_END) ? >> >> > + return; >> > + >> > + mutex_lock(&cdns->mutex); >> > + cdns->roles[role]->stop(cdns); >> > + cdns->role = CDNS3_ROLE_END; >> >> Why change the role here? You are just stopping the role not changing it. >> I think cdns->role should remain unchanged, so we can call cdns3_role_start() >> if required without error. >> > >The current version of this IP has some issues to detect vbus status correctly, >we have to force vbus status accordingly, so we need a status to indicate >vbus disconnection, and add some code to let controller know vbus >removal, in that case, the controller's state machine can be correct. >So, we increase one role 'CDNS3_ROLE_END' to for this purpose. Hi, Tomek do you have any comment for this. We have in RTL whole OTG machine and we can read all states. From otg specification we have in otg_state such bits: 5:3 host_otg_state "Current state of the OTG Host FSM. 3'b000 : H_IDLE 3'b001 : H_VBUS_ON 3'b010 : H_VBUS_FAILED 3'b011 : H_OTG_HOST_MODE 3'b100 : H_HOST_MODE 3'b101 : H_SWITCH_TO_DEVICE 3'b110 : H_A_SUSPEND 3'b111 : H_WAIT_VBUS_FALL" RO 0x0 2:0 dev_otg_state "Current state of the OTG Device FSM. 3'b000 : DEV_IDLE 3'b001 : DEV_MODE 3'b010 : DEV_SRP 3'b011 : DEV_WAIT_VBUS_FALL 3'b100 : DEV_SWITCH_TO_HOST 3'b101 : DEV_WAIT_FOR_CONN" > >CDNS3_ROLE_GADGET: gadget mode and VBUS on >CDNS3_ROLE_HOST: host mode and VBUS on >CDNS3_ROLE_END: VBUS off, eg either host or device cable on the port. > >So, we may start role from CDNS3_ROLE_END at probe when nothing is connected, >and need to set role as CDNS3_ROLE_END at ->stop for further handling at >role switch routine. > >> > + mutex_unlock(&cdns->mutex); >> > +} >> > + >> > +static enum cdns3_roles cdns3_get_role(struct cdns3 *cdns) >> > +{ >> > + if (cdns->roles[CDNS3_ROLE_HOST] && cdns->roles[CDNS3_ROLE_GADGET]) { >> > + //TODO: implements selecting device/host mode >> > + return CDNS3_ROLE_HOST; >> > + } >> > + return cdns->roles[CDNS3_ROLE_HOST] >> > + ? CDNS3_ROLE_HOST >> > + : CDNS3_ROLE_GADGET; Cheers Pawel,
RE: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
Hi + Tomek >> > + * Cadence USBSS DRD Driver. >> > + * >> > + * Copyright (C) 2018 Cadence. >> > + * >> > + * Author: Peter Chen >> > + * Pawel Laszczak >> > + */ >> > + >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > +#include >> > + >> > +#include "gadget.h" >> > +#include "core.h" >> > + >> > +static inline struct cdns3_role_driver >> > *cdns3_get_current_role_driver(struct cdns3 *cdns) >> > +{ >> > + WARN_ON(cdns->role >= CDNS3_ROLE_END || !cdns->roles[cdns->role]); >> > + return cdns->roles[cdns->role]; >> > +} >> > + >> > +static inline int cdns3_role_start(struct cdns3 *cdns, enum cdns3_roles >> > role) >> > +{ >> > + int ret; >> > + >> > + if (role >= CDNS3_ROLE_END) >> >> WARN_ON()? >> >> > + return 0; >> > + >> > + if (!cdns->roles[role]) >> > + return -ENXIO; >> > + >> > + mutex_lock(&cdns->mutex); >> > + cdns->role = role; >> > + ret = cdns->roles[role]->start(cdns); >> > + mutex_unlock(&cdns->mutex); >> > + return ret; >> > +} >> > + >> > +static inline void cdns3_role_stop(struct cdns3 *cdns) >> > +{ >> > + enum cdns3_roles role = cdns->role; >> > + >> > + if (role == CDNS3_ROLE_END) >> >> WARN_ON(role >= CNDS3_ROLE_END) ? >> >> > + return; >> > + >> > + mutex_lock(&cdns->mutex); >> > + cdns->roles[role]->stop(cdns); >> > + cdns->role = CDNS3_ROLE_END; >> >> Why change the role here? You are just stopping the role not changing it. >> I think cdns->role should remain unchanged, so we can call cdns3_role_start() >> if required without error. >> > >The current version of this IP has some issues to detect vbus status correctly, >we have to force vbus status accordingly, so we need a status to indicate >vbus disconnection, and add some code to let controller know vbus >removal, in that case, the controller's state machine can be correct. >So, we increase one role 'CDNS3_ROLE_END' to for this purpose. > >CDNS3_ROLE_GADGET: gadget mode and VBUS on >CDNS3_ROLE_HOST: host mode and VBUS on >CDNS3_ROLE_END: VBUS off, eg either host or device cable on the port. > >So, we may start role from CDNS3_ROLE_END at probe when nothing is connected, >and need to set role as CDNS3_ROLE_END at ->stop for further handling at >role switch routine. > >> > + mutex_unlock(&cdns->mutex); >> > +} >> > + >> > +static enum cdns3_roles cdns3_get_role(struct cdns3 *cdns) >> > +{ >> > + if (cdns->roles[CDNS3_ROLE_HOST] && cdns->roles[CDNS3_ROLE_GADGET]) { >> > + //TODO: implements selecting device/host mode >> > + return CDNS3_ROLE_HOST; >> > + } >> > + return cdns->roles[CDNS3_ROLE_HOST] >> > + ? CDNS3_ROLE_HOST >> > + : CDNS3_ROLE_GADGET; >> >> Why not just >> return cdns->role; >> >> I'm wondering if we really need this function. > >cdns->role gets from cdns3_get_role, and this API tells role at the runtime. >If both roles are supported, the role is decided by external >conditions, eg, vbus/id >or external connector. If only single role is supported, only one role >structure >is allocated, cdns->roles[CDNS3_ROLE_HOST] or cdns->roles[CDNS3_ROLE_GADGET] > >> > +} >> >> > + >> > +/** >> > + * cdns3_core_init_role - initialize role of operation >> > + * @cdns: Pointer to cdns3 structure >> > + * >> > + * Returns 0 on success otherwise negative errno >> > + */ >> > +static int cdns3_core_init_role(struct cdns3 *cdns) >> > +{ >> > + struct device *dev = cdns->dev; >> > + enum usb_dr_mode dr_mode; >> > + >> > + dr_mode = usb_get_dr_mode(dev); >> > + cdns->role = CDNS3_ROLE_END; >> > + >> > + /* >> > + * If driver can't read mode by means of usb_get_dr_mdoe function >> > then >> > + * chooses mode according with Kernel configuration. This setting >> > + * can be restricted later depending on strap pin configuration. >> > + */ >> > + if (dr_mode == USB_DR_MODE_UNKNOWN) { >> > + if (IS_ENABLED(CONFIG_USB_CDNS3_HOST) && >> > + IS_ENABLED(CONFIG_USB_CDNS3_GADGET)) >> > + dr_mode = USB_DR_MODE_OTG; >> > + else if (IS_ENABLED(CONFIG_USB_CDNS3_HOST)) >> > + dr_mode = USB_DR_MODE_HOST; >> > + else if (IS_ENABLED(CONFIG_USB_CDNS3_GADGET)) >> > + dr_mode = USB_DR_MODE_PERIPHERAL; >> > + } >> > + >> > + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { >> > + //TODO: implements host initialization >> >> /* TODO: Add host role */ ? >> >> > + } >> > + >> > + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) >> > { >> > + //TODO: implements device initialization >> >> /* TODO: Add device role */ ? >> > >Yes, it needs to allocate cdns->roles[CDNS3_ROLE_HOST] and >cdns->roles[CDNS3_ROLE_GADGET]. > >> > + } >> > + >> > + if (!cdns->roles[CDNS3_ROLE_HOST] && >> >
RE: [PATCH v7 05/10] usb: dwc3: make controller clear transfer resources after complete
HI Felipe, >-Original Message- >From: Felipe Balbi [mailto:ba...@kernel.org] >Sent: Wednesday, December 05, 2018 2:32 PM >To: Anurag Kumar Vulisha ; Greg Kroah-Hartman >; Shuah Khan ; Alan Stern >; Johan Hovold ; Jaejoong Kim >; Benjamin Herrenschmidt ; >Roger Quadros ; Manu Gautam ; >martin.peter...@oracle.com; Bart Van Assche ; Mike >Christie ; Matthew Wilcox ; Colin Ian >King >Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; >v.anuragku...@gmail.com; Thinh Nguyen ; Tejas Joglekar >; Ajay Yugalkishore Pandey ; >Anurag Kumar Vulisha >Subject: Re: [PATCH v7 05/10] usb: dwc3: make controller clear transfer >resources >after complete > > >Hi, > >Anurag Kumar Vulisha writes: >> @@ -2487,6 +2497,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, >> } >> >> switch (event->endpoint_event) { >> +case DWC3_DEPEVT_XFERCOMPLETE: >> +if (!dep->stream_capable) >> +break; >> +dep->flags &= ~DWC3_EP_TRANSFER_STARTED; >> +/* Fall Through */ > >instead, let's add a proper handler for this: > >dwc3_gadget_endpoint_transfer_complete(dep, event); > >That handler should be "self-sufficient". IOW, we shouldn't have this >fall through here. This means that the other patch where you modify >dwc3_gadget_transfer_in_progress() shouldn't be necessary, since that >event shouldn't run for stream capable endpoints. > Thanks for your suggestion, will implement the changes as said and send the patches >While rewriting the patches, please rebase on my testing/next as I have >applied a few of the patches in this series. Okay , will rebase on top of testing/next and send the patches Best Regards, Anurag Kumar Vulisha
RE: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields
Hi Felipe, >-Original Message- >From: Felipe Balbi [mailto:ba...@kernel.org] >Sent: Wednesday, December 05, 2018 2:38 PM >To: Anurag Kumar Vulisha ; Greg Kroah-Hartman >; Shuah Khan ; Alan Stern >; Johan Hovold ; Jaejoong Kim >; Benjamin Herrenschmidt ; >Roger Quadros ; Manu Gautam ; >martin.peter...@oracle.com; Bart Van Assche ; Mike >Christie ; Matthew Wilcox ; Colin Ian >King >Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; >v.anuragku...@gmail.com; Thinh Nguyen ; Tejas Joglekar >; Ajay Yugalkishore Pandey ; >Anurag Kumar Vulisha >Subject: Re: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both >event->status >and TRB->ctrl fields > > >Hi, > >Anurag Kumar Vulisha writes: >> The present code in dwc3_gadget_ep_reclaim_completed_trb() will check >> for IOC/LST bit in the event->status and returns if IOC/LST bit is >> set. This logic doesn't work if multiple TRBs are queued per >> request and the IOC/LST bit is set on the last TRB of that request. >> Consider an example where a queued request has multiple queued TRBs >> and IOC/LST bit is set only for the last TRB. In this case, the Core >> generates XferComplete/XferInProgress events only for the last TRB >> (since IOC/LST are set only for the last TRB). As per the logic in >> dwc3_gadget_ep_reclaim_completed_trb() event->status is checked for >> IOC/LST bit and returns on the first TRB. This makes the remaining >> TRBs left unhandled. >> To aviod this, changed the code to check for IOC/LST bits in both >> event->status & TRB->ctrl. This patch does the same. >> >> Signed-off-by: Anurag Kumar Vulisha >> Reviewed-by: Thinh Nguyen >> Tested-by: Tejas Joglekar >> --- >> Changes in v7: >> 1. None >> >> Changes in v6: >> 1. None >> >> Changes in v5: >> 1. None >> >> Changes in v4: >> 1. None >> >> Changes in v3: >> 1. None >> >> Changes in v2: >> 1. None >> --- >> drivers/usb/dwc3/gadget.c | 7 ++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index 9ddc9fd..216179e 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -2286,7 +2286,12 @@ static int >dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, >> if (event->status & DEPEVT_STATUS_SHORT && !chain) >> return 1; >> >> -if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST)) >> +if ((event->status & DEPEVT_STATUS_IOC) && >> +(trb->ctrl & DWC3_TRB_CTRL_IOC)) >> +return 1; > >this shouldn't be necessary. According to databook, event->status >contains the bits from the completed TRB. Which means that >event->status & IOC will always be equal to trb->ctrl & IOC. > Thanks for reviewing this patch. Lets consider an example where a request has num_sgs > 0 and each sg is mapped to a TRB and the last TRB has the IOC bit set. Once the controller is done with the transfer, it generates XferInProgress for the last TRB (since IOC bit is set). As a part of trb reclaim process dwc3_gadget_ep_reclaim_trb_sg() calls dwc3_gadget_ep_reclaim_completed_trb() for req->num_sgs times. Since the event already has the IOC bit set, the loop is exited from the loop at the very first TRB and the remaining TRBs (mapped to the sglist) are left unhandled. To avoid this we modified the code to exit only if both TRB & event has the IOC bit set. >Can you further describe the situation here? Perhaps share tracepoints >exposing the problem? Sure, will capture the traces and reply back Thanks, Anurag Kumar Vulisha
RE: [PATCH v3 1/5] usb: split code locating ACPI companion into port and device
>On Wed, Nov 21, 2018 at 03:50:16PM -0800, Rajat Jain wrote: >> From: Dmitry Torokhov >> >> In preparation for handling embedded USB devices let's split >> usb_acpi_find_companion() into usb_acpi_find_companion_for_device() >> and usb_acpi_find_companion_for_port(). >> >> Signed-off-by: Dmitry Torokhov >> Signed-off-by: Rajat Jain > >Acked-by: Greg Kroah-Hartman Tested-by: Sukumar Ghorai
RE: [PATCH v3 2/5] usb: assign ACPI companions for embedded USB devices
>On Wed, Nov 21, 2018 at 03:50:17PM -0800, Rajat Jain wrote: >> From: Dmitry Torokhov >> >> USB devices permanently connected to USB ports may be described in >> ACPI tables and share ACPI devices with ports they are connected to. >> See [1] for details. >> >> This will allow us to describe sideband resources for devices, such >> as, for example, hard reset line for BT USB controllers. >> >> [1] >> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/othe >> r-acpi-namespace-objects#acpi-namespace-hierarchy-and-adr-for- >embedded >> -usb-devices >> >> Signed-off-by: Dmitry Torokhov >> Signed-off-by: Rajat Jain (changed how we get the >> usb_port) > >Acked-by: Greg Kroah-Hartman Tested-by: Sukumar Ghorai
[PATCH] usb: host: isp1362-hcd: convert to DEFINE_SHOW_ATTRIBUTE
Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code. Signed-off-by: Yangtao Li --- drivers/usb/host/isp1362-hcd.c | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/usb/host/isp1362-hcd.c b/drivers/usb/host/isp1362-hcd.c index b21c386e6a46..28bf8bfb091e 100644 --- a/drivers/usb/host/isp1362-hcd.c +++ b/drivers/usb/host/isp1362-hcd.c @@ -2159,25 +2159,15 @@ static int isp1362_show(struct seq_file *s, void *unused) return 0; } - -static int isp1362_open(struct inode *inode, struct file *file) -{ - return single_open(file, isp1362_show, inode); -} - -static const struct file_operations debug_ops = { - .open = isp1362_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; +DEFINE_SHOW_ATTRIBUTE(isp1362); /* expect just one isp1362_hcd per system */ static void create_debug_file(struct isp1362_hcd *isp1362_hcd) { isp1362_hcd->debug_file = debugfs_create_file("isp1362", S_IRUGO, usb_debug_root, - isp1362_hcd, &debug_ops); + isp1362_hcd, + &isp1362_fops); } static void remove_debug_file(struct isp1362_hcd *isp1362_hcd) -- 2.17.0
[PATCH] USB: gadget: udc: s3c2410_udc: convert to DEFINE_SHOW_ATTRIBUTE
Use DEFINE_SHOW_ATTRIBUTE macro to simplify the code. Signed-off-by: Yangtao Li --- drivers/usb/gadget/udc/s3c2410_udc.c | 17 ++--- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c index 8bf5ad7a59ad..af3e63316ace 100644 --- a/drivers/usb/gadget/udc/s3c2410_udc.c +++ b/drivers/usb/gadget/udc/s3c2410_udc.c @@ -119,7 +119,7 @@ static void dprintk(int level, const char *fmt, ...) } #endif -static int s3c2410_udc_debugfs_seq_show(struct seq_file *m, void *p) +static int s3c2410_udc_debugfs_show(struct seq_file *m, void *p) { u32 addr_reg, pwr_reg, ep_int_reg, usb_int_reg; u32 ep_int_en_reg, usb_int_en_reg, ep0_csr; @@ -168,20 +168,7 @@ static int s3c2410_udc_debugfs_seq_show(struct seq_file *m, void *p) return 0; } - -static int s3c2410_udc_debugfs_fops_open(struct inode *inode, -struct file *file) -{ - return single_open(file, s3c2410_udc_debugfs_seq_show, NULL); -} - -static const struct file_operations s3c2410_udc_debugfs_fops = { - .open = s3c2410_udc_debugfs_fops_open, - .read = seq_read, - .llseek = seq_lseek, - .release= single_release, - .owner = THIS_MODULE, -}; +DEFINE_SHOW_ATTRIBUTE(s3c2410_udc_debugfs); /* io macros */ -- 2.17.0
RE: [PATCH v7 01/10] usb: gadget: udc: Add timer support for usb requests
Hi Alan, >-Original Message- >From: Alan Stern [mailto:st...@rowland.harvard.edu] >Sent: Wednesday, December 05, 2018 12:59 AM >To: Anurag Kumar Vulisha >Cc: Felipe Balbi ; Greg Kroah-Hartman >; Shuah Khan ; Johan Hovold >; Jaejoong Kim ; Benjamin >Herrenschmidt ; Roger Quadros ; Manu >Gautam ; martin.peter...@oracle.com; Bart Van >Assche ; Mike Christie ; Matthew >Wilcox ; Colin Ian King ; linux- >u...@vger.kernel.org; linux-ker...@vger.kernel.org; v.anuragku...@gmail.com; >Thinh Nguyen ; Tejas Joglekar >; Ajay Yugalkishore Pandey >Subject: RE: [PATCH v7 01/10] usb: gadget: udc: Add timer support for usb >requests > >On Tue, 4 Dec 2018, Anurag Kumar Vulisha wrote: > >> >Is there any way for the device controller driver to detect the problem >> >without >relying >> >on a timeout? >> > >> >In fact, is this problem a known bug in the existing device controller >> >hardware? >Have >> >the controller manufacturers published a suggested scheme for working >> >around it? >> > >> >> Yes, this problem is mentioned in dwc3 controller data book and the >> workaround >> mentioned is the same that we are doing , to implement timeout and if no >> valid >> stream event is found , after timeout issue end transfer followed by start >> transfer. > >Okay. But this implies that the problem is confined to DWC3 and >doesn't affect other types of controllers, which would mean modifying >the UDC core would be inappropriate. > Both host & gadget are equally responsible for this issue and it may potentially occur with other controllers also(which are incapable of handling this situation) . Because of this reason I would not call this issue as a dwc3 alone bug. One thing is that, with dwc3 the issue is easily reproduced. Because of these reasons I feel that it would be better to have a generic udc timers rather than having driver specific workaround. We had this same discussion earlier in this mailing list https://lkml.org/lkml/2018/10/9/638 >Does the data book suggest a value for the timeout? > No, the databook doesn't mention about the timeout value >> >At this point, it seems that the generic approach will be messier than >> >having every >> >controller driver implement its own fix. At least, that's how it appears >> >to me. > >(Especially if dwc3 is the only driver affected.) > As discussed above, the issue may happen with other gadgets too. As I got divide opinions on this implementation and both the implementations looks fine to me, I am little confused on which should be implemented. @Felipe: Do you agree with Alan's implementation? Please let us know your suggestion on this. >> With the dequeue approach there is an advantage(not related to this issue) >> that the >> class driver can have control of the timed out transfer whether to requeue >> it or >consider >> it as an error (based on implementation). This approach gives more >> flexibility to the >class >> drivers. This may be out of context of this issue but wanted to point out >> that we >may lose >> this advantage on switching to older implementation. > >Class drivers can cancel and requeue requests at any time. There's no >extra flexibility. > I agree with you, but the class drivers have to implement their own logic instead of having a generic logic to implement the timeouts. >> >Ideally it would not be necessary to rely on a timeout at all. >> > >> >Also, maintainers dislike module parameters. It would be better not to add >> >one. >> >> Okay. I would be happy if any alternative for this issue is present but >> unfortunately >> I am not able to figure out any alternative other than timers. If not >module_params() >> we can add an configfs entry in stream gadget to update the timeout. Please >provide >> your opinion on this approach. > >Since the purpose of the timeout is to detect a deadlock caused by a >hardware bug, I suggest a fixed and relatively short timeout value such >as one second. Cancelling and requeuing a few requests at 1-second >intervals shouldn't add very much overhead. > Thanks for suggesting. 1 sec timeout should be fair enough. I will change this in next series Best Regards, Anurag Kumar Vulisha
Re: usb: thoughts of adding more support for FT232H
On Wed, Dec 05, 2018 at 04:17:18PM +0100, Anatolij Gustschin wrote: > Hi, > > On Wed, 5 Dec 2018 22:10:40 +0800 > Song Qiang songqiang1304...@gmail.com wrote: > ... > >I've been developing some iio device drivers and found that some people > >would like to test their devices with a qemu system which requires an > >i2c or spi port on our development hosts. Usually this is achieved with > >a DLN-2 adapter, while this is a bit difficult for me because it costs > >~175$ in my country. Then I found that FTDI's FT232H supports both these > >two modes and costs only less than 5$ but without full support in kernel. > >The ftdi-sio driver supports FT232H only as a serial converter. > >So I'm planning to write a mfd driver for it supports both these three > >modes, here are my thoughts: > > There already has been a discussion [1] about adding an MFD driver for > FT232H, since the operating modes are mutually exclusive (and bus pins > shared between different modes), the MFD approach doesn't seem to be > a good fit. > > > - This device cannot support these three modes together because they > > share some common pins, so I'm planning to add a sysfs entry > > 'current_mode' for selecting which mode the device should be working > > on. > > - This device is in uart mode on reset, so default mode would be reset, > > too. This also helps for people only want to use this as a serial > > converter feels nothing has happened (compatible). > > - I was trying to reuse the ftdi-sio driver but it seems like mfd can > > only register platform devices, while this is a usb driver. I may > > have to copy some functions from this driver. > > > >Would you share any ideas? I'd appreciate it. > > There is a patch series [2] adding an interface driver for FT232H- > based adapter devices, it already supports adding custom MPSSE based > SPI busses with SPI slaves for a custom USB PID. It already supports > adding custom CBUS-/MPSSE-GPIO adapters for user-defined USB PID. > Adding I2C driver/adapter support should be easy, too. Maybe you can > re-use it. > [1] https://patchwork.kernel.org/patch/9828985 > [2] https://patchwork.kernel.org/project/linux-usb/list/?series=48255 I was going to refer to these series, but Anatolij beat me to it (haven't looked at [2] myself yet, though). Thanks, Johan
[PATCH 1/3] usb: dwc3: gadget: Fix OTG events when gadget driver isn't loaded
On v3.10a in dual-role mode, if port is in device mode and gadget driver isn't loaded, the OTG event interrupts don't come through. It seems that if the core is configured to be OTG2.0 only, then we can't leave the DCFG.DEVSPD at Super-speed (default) if we expect OTG to work properly. It must be set to High-speed. Fix this issue by configuring DCFG.DEVSPD to the supported maximum speed at gadget init. Device tree still needs to provide correct supported maximum speed for this to work. This issue wasn't present on v2.40a but is seen on v3.10a. It doesn't cause any side effects on v2.40a. Signed-off-by: Roger Quadros Signed-off-by: Sekhar Nori --- drivers/usb/dwc3/gadget.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 679c12e..79120c0 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -3240,6 +3240,8 @@ int dwc3_gadget_init(struct dwc3 *dwc) goto err4; } + dwc3_gadget_set_speed(&dwc->gadget, dwc->maximum_speed); + return 0; err4: -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH 3/3] usb: dwc3: keystone: Add support for ti,am654-dwc3
The AM654 SoC contains a DWC3 controller with TI specific wrapper. Add support for that. Unlike the Keystone 2 case, for AM654 We don't need to process any IRQs for basic USB operation. Signed-off-by: Roger Quadros --- drivers/usb/dwc3/Kconfig | 6 +++--- drivers/usb/dwc3/dwc3-keystone.c | 11 ++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index 1a0404f..a83a84f 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -86,11 +86,11 @@ config USB_DWC3_HAPS platform, please say 'Y' or 'M' here. config USB_DWC3_KEYSTONE - tristate "Texas Instruments Keystone2 Platforms" - depends on ARCH_KEYSTONE || COMPILE_TEST + tristate "Texas Instruments Keystone2/3 Platforms" + depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST default USB_DWC3 help - Support of USB2/3 functionality in TI Keystone2 platforms. + Support of USB2/3 functionality in TI Keystone2/3 platforms. Say 'Y' or 'M' here if you have one such device config USB_DWC3_OF_SIMPLE diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c index 193a9a8..cbee5fb 100644 --- a/drivers/usb/dwc3/dwc3-keystone.c +++ b/drivers/usb/dwc3/dwc3-keystone.c @@ -106,6 +106,10 @@ static int kdwc3_probe(struct platform_device *pdev) goto err_irq; } + /* IRQ processing not required currently for AM65 */ + if (of_device_is_compatible(node, "ti,am654-dwc3")) + goto skip_irq; + irq = platform_get_irq(pdev, 0); if (irq < 0) { dev_err(&pdev->dev, "missing irq\n"); @@ -123,6 +127,7 @@ static int kdwc3_probe(struct platform_device *pdev) kdwc3_enable_irqs(kdwc); +skip_irq: error = of_platform_populate(node, NULL, NULL, dev); if (error) { dev_err(&pdev->dev, "failed to create dwc3 core\n"); @@ -152,8 +157,11 @@ static int kdwc3_remove_core(struct device *dev, void *c) static int kdwc3_remove(struct platform_device *pdev) { struct dwc3_keystone *kdwc = platform_get_drvdata(pdev); + struct device_node *node = pdev->dev.of_node; + + if (!of_device_is_compatible(node, "ti,am654-dwc3")) + kdwc3_disable_irqs(kdwc); - kdwc3_disable_irqs(kdwc); device_for_each_child(&pdev->dev, NULL, kdwc3_remove_core); pm_runtime_put_sync(kdwc->dev); pm_runtime_disable(kdwc->dev); @@ -165,6 +173,7 @@ static int kdwc3_remove(struct platform_device *pdev) static const struct of_device_id kdwc3_of_match[] = { { .compatible = "ti,keystone-dwc3", }, + { .compatible = "ti,am654-dwc3" }, {}, }; MODULE_DEVICE_TABLE(of, kdwc3_of_match); -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH 0/3] usb: dwc3: Add support for AM654 USB
Misspelled Felipe's ID. Will resend this series. On 05/12/18 17:14, Roger Quadros wrote: > Hi Felipe, > > TI's AM654 USB SoC has 2 instances of the DWC3 controller. > This series adds AM654 USB wrapper support to the keystone-usb driver. > > cheers, > -roger > > Roger Quadros (3): > usb: dwc3: gadget: Fix OTG events when gadget driver isn't loaded > dt-bindings: usb: keystone-usb: Add ti,am654-dwc3 support > usb: dwc3: keystone: Add support for ti,am654-dwc3 > > Documentation/devicetree/bindings/usb/keystone-usb.txt | 4 ++-- > drivers/usb/dwc3/Kconfig | 6 +++--- > drivers/usb/dwc3/dwc3-keystone.c | 11 ++- > drivers/usb/dwc3/gadget.c | 2 ++ > 4 files changed, 17 insertions(+), 6 deletions(-) > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: usb: thoughts of adding more support for FT232H
Hi, On Wed, 5 Dec 2018 22:10:40 +0800 Song Qiang songqiang1304...@gmail.com wrote: ... >I've been developing some iio device drivers and found that some people >would like to test their devices with a qemu system which requires an >i2c or spi port on our development hosts. Usually this is achieved with >a DLN-2 adapter, while this is a bit difficult for me because it costs >~175$ in my country. Then I found that FTDI's FT232H supports both these >two modes and costs only less than 5$ but without full support in kernel. >The ftdi-sio driver supports FT232H only as a serial converter. >So I'm planning to write a mfd driver for it supports both these three >modes, here are my thoughts: There already has been a discussion [1] about adding an MFD driver for FT232H, since the operating modes are mutually exclusive (and bus pins shared between different modes), the MFD approach doesn't seem to be a good fit. > - This device cannot support these three modes together because they > share some common pins, so I'm planning to add a sysfs entry > 'current_mode' for selecting which mode the device should be working > on. > - This device is in uart mode on reset, so default mode would be reset, > too. This also helps for people only want to use this as a serial > converter feels nothing has happened (compatible). > - I was trying to reuse the ftdi-sio driver but it seems like mfd can > only register platform devices, while this is a usb driver. I may > have to copy some functions from this driver. > >Would you share any ideas? I'd appreciate it. There is a patch series [2] adding an interface driver for FT232H- based adapter devices, it already supports adding custom MPSSE based SPI busses with SPI slaves for a custom USB PID. It already supports adding custom CBUS-/MPSSE-GPIO adapters for user-defined USB PID. Adding I2C driver/adapter support should be easy, too. Maybe you can re-use it. Thanks, Anatolij [1] https://patchwork.kernel.org/patch/9828985 [2] https://patchwork.kernel.org/project/linux-usb/list/?series=48255
[PATCH 0/3] usb: dwc3: Add support for AM654 USB
Hi Felipe, TI's AM654 USB SoC has 2 instances of the DWC3 controller. This series adds AM654 USB wrapper support to the keystone-usb driver. cheers, -roger Roger Quadros (3): usb: dwc3: gadget: Fix OTG events when gadget driver isn't loaded dt-bindings: usb: keystone-usb: Add ti,am654-dwc3 support usb: dwc3: keystone: Add support for ti,am654-dwc3 Documentation/devicetree/bindings/usb/keystone-usb.txt | 4 ++-- drivers/usb/dwc3/Kconfig | 6 +++--- drivers/usb/dwc3/dwc3-keystone.c | 11 ++- drivers/usb/dwc3/gadget.c | 2 ++ 4 files changed, 17 insertions(+), 6 deletions(-) -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH 2/3] dt-bindings: usb: keystone-usb: Add ti,am654-dwc3 support
The AM654 SoC from TI contains a DWC3 controller. Add support for it. Signed-off-by: Roger Quadros --- Documentation/devicetree/bindings/usb/keystone-usb.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt index f96e09f..fa29ff8 100644 --- a/Documentation/devicetree/bindings/usb/keystone-usb.txt +++ b/Documentation/devicetree/bindings/usb/keystone-usb.txt @@ -3,7 +3,7 @@ TI Keystone Soc USB Controller DWC3 GLUE Required properties: - - compatible: should be "ti,keystone-dwc3". + - compatible: should be "ti,keystone-dwc3" or "ti,am654-dwc3". - #address-cells, #size-cells : should be '1' if the device has sub-nodes with 'reg' property. - reg : Address and length of the register set for the USB subsystem on @@ -21,7 +21,7 @@ SoCs only: - clock-names: Must be "usb". -The following are mandatory properties for Keystone 2 66AK2G SoCs only: +The following are mandatory properties for 66AK2G and AM654: - power-domains: Should contain a phandle to a PM domain provider node and an args specifier containing the USB device id -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH 0/3] usb: dwc3: Add support for AM654 USB
Hi Felipe, TI's AM654 USB SoC has 2 instances of the DWC3 controller. This series adds AM654 USB wrapper support to the keystone-usb driver. cheers, -roger Roger Quadros (3): usb: dwc3: gadget: Fix OTG events when gadget driver isn't loaded dt-bindings: usb: keystone-usb: Add ti,am654-dwc3 support usb: dwc3: keystone: Add support for ti,am654-dwc3 Documentation/devicetree/bindings/usb/keystone-usb.txt | 4 ++-- drivers/usb/dwc3/Kconfig | 6 +++--- drivers/usb/dwc3/dwc3-keystone.c | 11 ++- drivers/usb/dwc3/gadget.c | 2 ++ 4 files changed, 17 insertions(+), 6 deletions(-) -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH 2/3] dt-bindings: usb: keystone-usb: Add ti,am654-dwc3 support
The AM654 SoC from TI contains a DWC3 controller. Add support for it. Signed-off-by: Roger Quadros --- Documentation/devicetree/bindings/usb/keystone-usb.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/keystone-usb.txt b/Documentation/devicetree/bindings/usb/keystone-usb.txt index f96e09f..fa29ff8 100644 --- a/Documentation/devicetree/bindings/usb/keystone-usb.txt +++ b/Documentation/devicetree/bindings/usb/keystone-usb.txt @@ -3,7 +3,7 @@ TI Keystone Soc USB Controller DWC3 GLUE Required properties: - - compatible: should be "ti,keystone-dwc3". + - compatible: should be "ti,keystone-dwc3" or "ti,am654-dwc3". - #address-cells, #size-cells : should be '1' if the device has sub-nodes with 'reg' property. - reg : Address and length of the register set for the USB subsystem on @@ -21,7 +21,7 @@ SoCs only: - clock-names: Must be "usb". -The following are mandatory properties for Keystone 2 66AK2G SoCs only: +The following are mandatory properties for 66AK2G and AM654: - power-domains: Should contain a phandle to a PM domain provider node and an args specifier containing the USB device id -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH 3/3] usb: dwc3: keystone: Add support for ti,am654-dwc3
The AM654 SoC contains a DWC3 controller with TI specific wrapper. Add support for that. Unlike the Keystone 2 case, for AM654 We don't need to process any IRQs for basic USB operation. Signed-off-by: Roger Quadros --- drivers/usb/dwc3/Kconfig | 6 +++--- drivers/usb/dwc3/dwc3-keystone.c | 11 ++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index 1a0404f..a83a84f 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -86,11 +86,11 @@ config USB_DWC3_HAPS platform, please say 'Y' or 'M' here. config USB_DWC3_KEYSTONE - tristate "Texas Instruments Keystone2 Platforms" - depends on ARCH_KEYSTONE || COMPILE_TEST + tristate "Texas Instruments Keystone2/3 Platforms" + depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST default USB_DWC3 help - Support of USB2/3 functionality in TI Keystone2 platforms. + Support of USB2/3 functionality in TI Keystone2/3 platforms. Say 'Y' or 'M' here if you have one such device config USB_DWC3_OF_SIMPLE diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c index 193a9a8..cbee5fb 100644 --- a/drivers/usb/dwc3/dwc3-keystone.c +++ b/drivers/usb/dwc3/dwc3-keystone.c @@ -106,6 +106,10 @@ static int kdwc3_probe(struct platform_device *pdev) goto err_irq; } + /* IRQ processing not required currently for AM65 */ + if (of_device_is_compatible(node, "ti,am654-dwc3")) + goto skip_irq; + irq = platform_get_irq(pdev, 0); if (irq < 0) { dev_err(&pdev->dev, "missing irq\n"); @@ -123,6 +127,7 @@ static int kdwc3_probe(struct platform_device *pdev) kdwc3_enable_irqs(kdwc); +skip_irq: error = of_platform_populate(node, NULL, NULL, dev); if (error) { dev_err(&pdev->dev, "failed to create dwc3 core\n"); @@ -152,8 +157,11 @@ static int kdwc3_remove_core(struct device *dev, void *c) static int kdwc3_remove(struct platform_device *pdev) { struct dwc3_keystone *kdwc = platform_get_drvdata(pdev); + struct device_node *node = pdev->dev.of_node; + + if (!of_device_is_compatible(node, "ti,am654-dwc3")) + kdwc3_disable_irqs(kdwc); - kdwc3_disable_irqs(kdwc); device_for_each_child(&pdev->dev, NULL, kdwc3_remove_core); pm_runtime_put_sync(kdwc->dev); pm_runtime_disable(kdwc->dev); @@ -165,6 +173,7 @@ static int kdwc3_remove(struct platform_device *pdev) static const struct of_device_id kdwc3_of_match[] = { { .compatible = "ti,keystone-dwc3", }, + { .compatible = "ti,am654-dwc3" }, {}, }; MODULE_DEVICE_TABLE(of, kdwc3_of_match); -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH 1/3] usb: dwc3: gadget: Fix OTG events when gadget driver isn't loaded
On v3.10a in dual-role mode, if port is in device mode and gadget driver isn't loaded, the OTG event interrupts don't come through. It seems that if the core is configured to be OTG2.0 only, then we can't leave the DCFG.DEVSPD at Super-speed (default) if we expect OTG to work properly. It must be set to High-speed. Fix this issue by configuring DCFG.DEVSPD to the supported maximum speed at gadget init. Device tree still needs to provide correct supported maximum speed for this to work. This issue wasn't present on v2.40a but is seen on v3.10a. It doesn't cause any side effects on v2.40a. Signed-off-by: Roger Quadros Signed-off-by: Sekhar Nori --- drivers/usb/dwc3/gadget.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 679c12e..79120c0 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -3240,6 +3240,8 @@ int dwc3_gadget_init(struct dwc3 *dwc) goto err4; } + dwc3_gadget_set_speed(&dwc->gadget, dwc->maximum_speed); + return 0; err4: -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[balbi-usb:testing/next 50/50] drivers/usb/dwc3/./trace.h:239:2: note: in expansion of macro 'TP_printk'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next head: ad7b607f82731eec3ed17d9d22764eb6f09609f9 commit: ad7b607f82731eec3ed17d9d22764eb6f09609f9 [50/50] usb: dwc3: trace: add missing break statement to make compiler happy config: i386-randconfig-x006-12051027 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: git checkout ad7b607f82731eec3ed17d9d22764eb6f09609f9 # save the attached .config to linux build tree make ARCH=i386 Note: it may well be a FALSE warning. FWIW you are at least aware of it now. http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings All warnings (new ones prefixed by >>): In file included from include/trace/define_trace.h:96:0, from drivers/usb/dwc3/trace.h:342, from drivers/usb/dwc3/trace.c:11: drivers/usb/dwc3/./trace.h: In function 'trace_raw_output_dwc3_log_trb': >> include/trace/trace_events.h:360:2: warning: 's' may be used uninitialized >> in this function [-Wmaybe-uninitialized] trace_seq_printf(s, print); \ ^~~~ drivers/usb/dwc3/./trace.h:241:11: note: 's' was declared here ({char *s; ^ include/trace/trace_events.h:360:22: note: in definition of macro 'DECLARE_EVENT_CLASS' trace_seq_printf(s, print); \ ^ >> drivers/usb/dwc3/./trace.h:239:2: note: in expansion of macro 'TP_printk' TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x (%c%c%c%c:%c%c:%s)", ^ -- In file included from include/trace/define_trace.h:96:0, from drivers/usb//dwc3/trace.h:342, from drivers/usb//dwc3/trace.c:11: drivers/usb//dwc3/./trace.h: In function 'trace_raw_output_dwc3_log_trb': >> include/trace/trace_events.h:360:2: warning: 's' may be used uninitialized >> in this function [-Wmaybe-uninitialized] trace_seq_printf(s, print); \ ^~~~ drivers/usb//dwc3/./trace.h:241:11: note: 's' was declared here ({char *s; ^ include/trace/trace_events.h:360:22: note: in definition of macro 'DECLARE_EVENT_CLASS' trace_seq_printf(s, print); \ ^ drivers/usb//dwc3/./trace.h:239:2: note: in expansion of macro 'TP_printk' TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x (%c%c%c%c:%c%c:%s)", ^ vim +/TP_printk +239 drivers/usb/dwc3/./trace.h 2c4cbe6e5a Felipe Balbi2014-04-30 215 2c4cbe6e5a Felipe Balbi2014-04-30 216 DECLARE_EVENT_CLASS(dwc3_log_trb, 2c4cbe6e5a Felipe Balbi2014-04-30 217 TP_PROTO(struct dwc3_ep *dep, struct dwc3_trb *trb), 2c4cbe6e5a Felipe Balbi2014-04-30 218 TP_ARGS(dep, trb), 2c4cbe6e5a Felipe Balbi2014-04-30 219 TP_STRUCT__entry( e42f09b85f Felipe Balbi2017-04-28 220 __string(name, dep->name) 2c4cbe6e5a Felipe Balbi2014-04-30 221 __field(struct dwc3_trb *, trb) 68d34c8a74 Felipe Balbi2016-05-30 222 __field(u32, allocated) 68d34c8a74 Felipe Balbi2016-05-30 223 __field(u32, queued) 4ac4fc9322 Felipe Balbi2014-09-17 224 __field(u32, bpl) 4ac4fc9322 Felipe Balbi2014-09-17 225 __field(u32, bph) 4ac4fc9322 Felipe Balbi2014-09-17 226 __field(u32, size) 4ac4fc9322 Felipe Balbi2014-09-17 227 __field(u32, ctrl) fa8d965d73 Felipe Balbi2016-09-28 228 __field(u32, type) 2c4cbe6e5a Felipe Balbi2014-04-30 229 ), 2c4cbe6e5a Felipe Balbi2014-04-30 230 TP_fast_assign( e42f09b85f Felipe Balbi2017-04-28 231 __assign_str(name, dep->name); 2c4cbe6e5a Felipe Balbi2014-04-30 232 __entry->trb = trb; 4ac4fc9322 Felipe Balbi2014-09-17 233 __entry->bpl = trb->bpl; 4ac4fc9322 Felipe Balbi2014-09-17 234 __entry->bph = trb->bph; 4ac4fc9322 Felipe Balbi2014-09-17 235 __entry->size = trb->size; 4ac4fc9322 Felipe Balbi2014-09-17 236 __entry->ctrl = trb->ctrl; fa8d965d73 Felipe Balbi2016-09-28 237 __entry->type = usb_endpoint_type(dep->endpoint.desc); 2c4cbe6e5a Felipe Balbi2014-04-30 238 ), 0bd0f6d201 Felipe Balbi2018-03-26 @239 TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x (%c%c%c%c:%c%c:%s)", 0bd0f6d201 Felipe Balbi2018-03-26 240 __get_str(name), __entry->trb, __entry->bph, __entry->bpl, fa8d965d73 Felipe Balbi2016-09-28 @241 ({char *s; fa8d965d73 Felipe Balbi2016-09-28 242 int pcm = ((__entry->size >> 24) & 3) + 1; fa8d965d73 Felipe Balbi2016-09-28 243 switch (__entry->type) { fa8d965d73 Felipe Balbi2016-09-28 244 case USB_ENDPOINT_XFER_INT: fa8d965d73 Felipe Balbi2016-09-28 245 case USB_ENDPOINT_XFER_ISOC: fa8d965d73 Felipe Balbi2016-09-28
usb: thoughts of adding more support for FT232H
Hi Johan, I've been developing some iio device drivers and found that some people would like to test their devices with a qemu system which requires an i2c or spi port on our development hosts. Usually this is achieved with a DLN-2 adapter, while this is a bit difficult for me because it costs ~175$ in my country. Then I found that FTDI's FT232H supports both these two modes and costs only less than 5$ but without full support in kernel. The ftdi-sio driver supports FT232H only as a serial converter. So I'm planning to write a mfd driver for it supports both these three modes, here are my thoughts: - This device cannot support these three modes together because they share some common pins, so I'm planning to add a sysfs entry 'current_mode' for selecting which mode the device should be working on. - This device is in uart mode on reset, so default mode would be reset, too. This also helps for people only want to use this as a serial converter feels nothing has happened (compatible). - I was trying to reuse the ftdi-sio driver but it seems like mfd can only register platform devices, while this is a usb driver. I may have to copy some functions from this driver. Would you share any ideas? I'd appreciate it. yours, Song Qiang
Re: [PATCH] usb: dwc2: Revert "usb: dwc2: Disable all EP's on disconnect"
Hi, On 12/4/2018 5:29 PM, Dan Carpenter wrote: > On Tue, Dec 04, 2018 at 12:34:08PM +, Minas Harutyunyan wrote: >> @@ -3185,12 +3183,13 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg) >> hsotg->connected = 0; >> hsotg->test_mode = 0; >> >> - /* all endpoints should be shutdown */ >> for (ep = 0; ep < hsotg->num_of_eps; ep++) { >> if (hsotg->eps_in[ep]) >> - dwc2_hsotg_ep_disable(&hsotg->eps_in[ep]->ep); >> + kill_all_requests(hsotg, hsotg->eps_in[ep], >> + -ESHUTDOWN); >> if (hsotg->eps_out[ep]) >> - dwc2_hsotg_ep_disable(&hsotg->eps_out[ep]->ep); >> + kill_all_requests(hsotg, hsotg->eps_out[ep], >> + -ESHUTDOWN); > > > Should this part be in a separate patch? > > I'm not trying to be rhetorical at all. I literally don't know the > code very well. Hopefully the full commit message will explain it. > Actually, this fragment of patch revert changes from V2 and keep untouched dwc2_hsotg_disconnect() function. >> } >> >> call_gadget(hsotg, disconnect); >> @@ -3234,6 +3233,8 @@ static void dwc2_hsotg_irq_fifoempty(struct >> dwc2_hsotg *hsotg, bool periodic) >> GINTSTS_PTXFEMP | \ >> GINTSTS_RXFLVL) >> >> +static int dwc2_hsotg_ep_disable(struct usb_ep *ep); >> + >>/** >> * dwc2_hsotg_core_init - issue softreset to the core >> * @hsotg: The device state >> @@ -3258,12 +3259,14 @@ void dwc2_hsotg_core_init_disconnected(struct >> dwc2_hsotg *hsotg, >> return; >> } else { >> /* all endpoints should be shutdown */ >> + spin_unlock(&hsotg->lock); >> for (ep = 1; ep < hsotg->num_of_eps; ep++) { >> if (hsotg->eps_in[ep]) >> >> dwc2_hsotg_ep_disable(&hsotg->eps_in[ep]->ep); >> if (hsotg->eps_out[ep]) >> >> dwc2_hsotg_ep_disable(&hsotg->eps_out[ep]->ep); >> } >> + spin_lock(&hsotg->lock); >> } >> >> /* > > The idea here is that this is the only caller which is holding the > lock and we drop it here and take it again inside dwc2_hsotg_ep_disable(). > I don't know the code very well and can't totally swear that this > doesn't introduce a small race condition... > Above fragment of patch allow to keep untouched dwc2_hsotg_ep_disble() function also, without changing spin_lock/_unlock stuff inside function. My approach here minimally update code to add any races. Just in dwc2_hsotg_core_init_disconnected() function on USB reset interrupt perform disabling all EP's. Because on USB reset interrupt, called from interrupt handler with acquired lock and dwc2_hsotg_ep_disble() function (without changes) acquire lock, just need to unlock lock to avoid any troubles. > Another option would be to introduce a new function which takes the lock > and change all the other callers instead. To me that would be easier to > review... See below for how it might look: > > regards, > dan carpenter > > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > index 94f3ba995580..b17a5dbefd5f 100644 > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -3166,6 +3166,7 @@ static void kill_all_requests(struct dwc2_hsotg *hsotg, > } > > static int dwc2_hsotg_ep_disable(struct usb_ep *ep); > +static int dwc2_hsotg_ep_disable_lock(struct usb_ep *ep); > > /** >* dwc2_hsotg_disconnect - disconnect service > @@ -3188,9 +3189,9 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg) > /* all endpoints should be shutdown */ > for (ep = 0; ep < hsotg->num_of_eps; ep++) { > if (hsotg->eps_in[ep]) > - dwc2_hsotg_ep_disable(&hsotg->eps_in[ep]->ep); > + dwc2_hsotg_ep_disable_lock(&hsotg->eps_in[ep]->ep); > if (hsotg->eps_out[ep]) > - dwc2_hsotg_ep_disable(&hsotg->eps_out[ep]->ep); > + dwc2_hsotg_ep_disable_lock(&hsotg->eps_out[ep]->ep); > } > > call_gadget(hsotg, disconnect); > @@ -4069,10 +4070,8 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep) > struct dwc2_hsotg *hsotg = hs_ep->parent; > int dir_in = hs_ep->dir_in; > int index = hs_ep->index; > - unsigned long flags; > u32 epctrl_reg; > u32 ctrl; > - int locked; > > dev_dbg(hsotg->dev, "%s(ep %p)\n", __func__, ep); > > @@ -4088,10 +4087,6 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep) > > epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index); > > - locked = spin_is_locked(&hsotg->lock); > - if (!locked) > - spin_lock_irqsave(&hsotg->lock, flags); > - > ct
[PATCH 1/2] xhci: workaround CSS timeout on AMD SNPS 3.0 xHC
From: Sandeep Singh Occasionally AMD SNPS 3.0 xHC does not respond to CSS when set, also it does not flag anything on SRE and HCE to point the internal xHC errors on USBSTS register. This stalls the entire system wide suspend and there is no point in stalling just because of xHC CSS is not responding. To work around this problem, if the xHC does not flag anything on SRE and HCE, we can skip the CSS timeout and allow the system to continue the suspend. Once the system resume happens we can internally reset the controller using XHCI_RESET_ON_RESUME quirk Signed-off-by: Shyam Sundar S K Signed-off-by: Sandeep Singh cc: Nehal Shah Cc: Tested-by: Kai-Heng Feng Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-pci.c | 4 drivers/usb/host/xhci.c | 26 ++ drivers/usb/host/xhci.h | 3 +++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index a951526..a9ec705 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -139,6 +139,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) pdev->device == 0x43bb)) xhci->quirks |= XHCI_SUSPEND_DELAY; + if (pdev->vendor == PCI_VENDOR_ID_AMD && + (pdev->device == 0x15e0 || pdev->device == 0x15e1)) + xhci->quirks |= XHCI_SNPS_BROKEN_SUSPEND; + if (pdev->vendor == PCI_VENDOR_ID_AMD) xhci->quirks |= XHCI_TRUST_TX_LENGTH; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index c928dbb..c20b85e 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -968,6 +968,7 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup) unsigned intdelay = XHCI_MAX_HALT_USEC; struct usb_hcd *hcd = xhci_to_hcd(xhci); u32 command; + u32 res; if (!hcd->state) return 0; @@ -1021,11 +1022,28 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup) command = readl(&xhci->op_regs->command); command |= CMD_CSS; writel(command, &xhci->op_regs->command); + xhci->broken_suspend = 0; if (xhci_handshake(&xhci->op_regs->status, STS_SAVE, 0, 10 * 1000)) { - xhci_warn(xhci, "WARN: xHC save state timeout\n"); - spin_unlock_irq(&xhci->lock); - return -ETIMEDOUT; + /* +* AMD SNPS xHC 3.0 occasionally does not clear the +* SSS bit of USBSTS and when driver tries to poll +* to see if the xHC clears BIT(8) which never happens +* and driver assumes that controller is not responding +* and times out. To workaround this, its good to check +* if SRE and HCE bits are not set (as per xhci +* Section 5.4.2) and bypass the timeout. +*/ + res = readl(&xhci->op_regs->status); + if ((xhci->quirks & XHCI_SNPS_BROKEN_SUSPEND) && + (((res & STS_SRE) == 0) && + ((res & STS_HCE) == 0))) { + xhci->broken_suspend = 1; + } else { + xhci_warn(xhci, "WARN: xHC save state timeout\n"); + spin_unlock_irq(&xhci->lock); + return -ETIMEDOUT; + } } spin_unlock_irq(&xhci->lock); @@ -1078,7 +1096,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags); spin_lock_irq(&xhci->lock); - if (xhci->quirks & XHCI_RESET_ON_RESUME) + if ((xhci->quirks & XHCI_RESET_ON_RESUME) || xhci->broken_suspend) hibernated = true; if (!hibernated) { diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 260b259..c3515ba 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1850,6 +1850,7 @@ struct xhci_hcd { #define XHCI_ZERO_64B_REGS BIT_ULL(32) #define XHCI_DEFAULT_PM_RUNTIME_ALLOW BIT_ULL(33) #define XHCI_RESET_PLL_ON_DISCONNECT BIT_ULL(34) +#define XHCI_SNPS_BROKEN_SUSPENDBIT_ULL(35) unsigned intnum_active_eps; unsigned intlimit_active_eps; @@ -1879,6 +1880,8 @@ struct xhci_hcd { void*dbc; /* platform-specific data -- must come last */ unsigned long priv[0] __aligned(sizeof(s64)); + /* Broken Suspend flag for SNPS Suspend resume issue */ + u8 broken_suspend; }; /* Platform specific overrides to generic XHCI hc_driver ops */ -- 2.7.4
[PATCH 0/2] xhci fixes for usb-linus
Hi Greg A couple patches for 4.20. Finally found a reason why devices attached after a high bandwidth USB3 isoc device may fail to enumerate with bandwidth error. Second patch is a quirk for AMD SNPS host specific suspend issue. -Mathias Mathias Nyman (1): xhci: Prevent U1/U2 link pm states if exit latency is too long Sandeep Singh (1): xhci: workaround CSS timeout on AMD SNPS 3.0 xHC drivers/usb/host/xhci-pci.c | 4 drivers/usb/host/xhci.c | 42 ++ drivers/usb/host/xhci.h | 3 +++ 3 files changed, 45 insertions(+), 4 deletions(-) -- 2.7.4
[PATCH 2/2] xhci: Prevent U1/U2 link pm states if exit latency is too long
Don't allow USB3 U1 or U2 if the latency to wake up from the U-state reaches the service interval for a periodic endpoint. This is according to xhci 1.1 specification section 4.23.5.2 extra note: "Software shall ensure that a device is prevented from entering a U-state where its worst case exit latency approaches the ESIT." Allowing too long exit latencies for periodic endpoint confuses xHC internal scheduling, and new devices may fail to enumerate with a "Not enough bandwidth for new device state" error from the host. Cc: Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index c20b85e..dae3be1 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -4514,6 +4514,14 @@ static u16 xhci_calculate_u1_timeout(struct xhci_hcd *xhci, { unsigned long long timeout_ns; + /* Prevent U1 if service interval is shorter than U1 exit latency */ + if (usb_endpoint_xfer_int(desc) || usb_endpoint_xfer_isoc(desc)) { + if (xhci_service_interval_to_ns(desc) <= udev->u1_params.mel) { + dev_dbg(&udev->dev, "Disable U1, ESIT shorter than exit latency\n"); + return USB3_LPM_DISABLED; + } + } + if (xhci->quirks & XHCI_INTEL_HOST) timeout_ns = xhci_calculate_intel_u1_timeout(udev, desc); else @@ -4570,6 +4578,14 @@ static u16 xhci_calculate_u2_timeout(struct xhci_hcd *xhci, { unsigned long long timeout_ns; + /* Prevent U2 if service interval is shorter than U2 exit latency */ + if (usb_endpoint_xfer_int(desc) || usb_endpoint_xfer_isoc(desc)) { + if (xhci_service_interval_to_ns(desc) <= udev->u2_params.mel) { + dev_dbg(&udev->dev, "Disable U2, ESIT shorter than exit latency\n"); + return USB3_LPM_DISABLED; + } + } + if (xhci->quirks & XHCI_INTEL_HOST) timeout_ns = xhci_calculate_intel_u2_timeout(udev, desc); else -- 2.7.4
[balbi-usb:testing/next 50/50] drivers/usb/gadget/function/f_fs.c:2198:9: error: too few arguments to function 'ffs_do_single_desc'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next head: 3591a2f3900e0f93dbd246d979ddd939251cac3e commit: 3591a2f3900e0f93dbd246d979ddd939251cac3e [50/50] usb: gadget: f_fs: add the support for alternate interface setting config: arm-omap2plus_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 3591a2f3900e0f93dbd246d979ddd939251cac3e # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=arm All errors (new ones prefixed by >>): drivers/usb/gadget/function/f_fs.c: In function 'ffs_do_descs_alt_intf': >> drivers/usb/gadget/function/f_fs.c:2198:9: error: too few arguments to >> function 'ffs_do_single_desc' ret = ffs_do_single_desc(data, len, entity, priv); ^~ drivers/usb/gadget/function/f_fs.c:1966:25: note: declared here static int __must_check ffs_do_single_desc(char *data, unsigned len, ^~ vim +/ffs_do_single_desc +2198 drivers/usb/gadget/function/f_fs.c 2148 2149 static int do_function_enable_interface(enum ffs_entity_type type, u8 *valuep, 2150 struct usb_descriptor_header *desc, 2151 void *priv); 2152 2153 static int do_function_disable_interface(enum ffs_entity_type type, u8 *valuep, 2154 struct usb_descriptor_header *desc, 2155 void *priv); 2156 2157 static int __must_check ffs_do_descs_alt_intf(unsigned int count, char *data, 2158 unsigned int len, unsigned int intf, 2159 unsigned int alt, void *priv) 2160 { 2161 const unsigned int _len = len; 2162 unsigned long num = 0; 2163 struct usb_descriptor_header *_ds; 2164 struct usb_interface_descriptor *idecs; 2165 ffs_entity_callback entity = NULL; 2166 2167 ENTER(); 2168 2169 for (;;) { 2170 int ret; 2171 2172 if (num == count) 2173 data = NULL; 2174 2175 if (!data) { 2176 pr_vdebug("%s Exiting. No more Data\n", __func__); 2177 return _len - len; 2178 } 2179 2180 _ds = (void *)data; 2181 if (_ds->bDescriptorType == USB_DT_INTERFACE) { 2182 idecs = (void *)_ds; 2183 2184 /*Check the interface no to deal with */ 2185 if (idecs->bInterfaceNumber == intf) { 2186 if (idecs->bAlternateSetting == alt) 2187 entity = do_function_enable_interface; 2188 else 2189 entity = do_function_disable_interface; 2190 } else if (entity && 2191 (idecs->bInterfaceNumber != intf)) { 2192 pr_vdebug( 2193 "%s Exiting.Moved past the interface no\n", 2194 __func__); 2195 return _len - len; 2196 } 2197 } > 2198 ret = ffs_do_single_desc(data, len, entity, priv); 2199 if (unlikely(ret < 0)) { 2200 pr_err("%s Exiting with err %d\n", __func__, ret); 2201 return ret; 2202 } 2203 2204 len -= ret; 2205 data += ret; 2206 ++num; 2207 } 2208 } 2209 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
XHCI DbC (debug port) problems
I'm assisting in debugging what appears to be a race condition in I2C code [0] on Intel ComputeStick STK1A32SC [1] devices. As part of that effort we were originally using an external USB<>RS232 adapter and "console=ttyUSB0,115200n8" but that can be extremely slow (connected to a real UART port) and the debug messages seem to prevent the issue occuring. We decided to switch to use the XHCI debug port and ordered a set of USB3 A<>A debug crossover cables [2]. We are following the kernel admin guide's USB3 debug port document [3] We've hit problems in getting the DbC port to work correctly and need some advice as to whether there is a kernel issue here? The target is using custom-built 4.20-rc4 kernels with all required modules built in. The original issue was first detected in Debian kernels v4.18.*, but didn't occur in v4.9. The ComputeStick devices have 2 USB ports, 1 is USB3, which the cable connects to. The host is using v4.18.18 (Fedora) and has usb_debug module loaded. Initially the host reported: usb 2-6: new SuperSpeed Gen 1 USB device number 9 using xhci_hcd usb 2-6: LPM exit latency is zeroed, disabling LPM. usb 2-6: language id specifier not provided by device, defaulting to English usb 2-6: New USB device found, idVendor=1d6b, idProduct=0011, bcdDevice= 0.10 usb 2-6: New USB device strings: Mfr=1, Product=2, SerialNumber=3 usb 2-6: rejected 1 configuration due to insufficient available bus power usb 2-6: no configuration chosen from 1 choice mtp-probe[32428]: checking bus 2, device 9: "/sys/devices/pci:00/:00:14.0/usb2/2-6" mtp-probe[32428]: bus: 2, device: 9 was not an MTP device fwupd[2590]: failed to add USB device 1d6b:0011: 1d6b:0011 is not supported: USB error on device 1d6b:0011 : Entity not found [-5] Due to "rejected 1 configuration due to insufficient available bus power" we then enabled the workaround: # echo 1 > /sys/bus/usb/devices/.../bConfigurationValue That got us further but then hits repeated -EPROTO errors: usb 2-6: no configuration chosen from 1 choice usb 2-6: new config #1 exceeds power limit by 660mA usb_debug 2-6:1.0: xhci_dbc converter detected usb 2-6: xhci_dbc converter now attached to ttyUSB0 xhci_dbc ttyUSB0: usb_serial_generic_write_bulk_callback - nonzero urb status: -71 The last line is repeated constantly several times each second. The first thing that has confused us is where is the host getting the "idProduct=0011" from? Looking at the xhci-dbgcap.h the only USB ID is #define DBC_PRODUCT_ID 0x0010 /* device 0010 */ and xhci-dbgcap.c seems to write it without change: dev_info = cpu_to_le32((DBC_DEVICE_REV << 16) | DBC_PRODUCT_ID); usb_serial does declare both 0x0010 and 0x0011 as aliases but that doesn't explain why the target is reported as 0x0011. We're wondering if the warning: "usb_serial_generic_write_bulk_callback - nonzero urb status: -71" is a regression or bug, rather than us missing something, since we've tried to do the same operation between two Fedora 4.18.18 PCs and got exactly the same results. Secondly, the "new config #1 exceeds power limit by 660mA" report, if we calculate correctly, suggests that bMaxPower being sent by the target is 900mA + 660mA = 1560mA, but cannot find in the code where bMaxPower is being set, if any. How is this set by the target? Any advice or experience as to what we might be missing? [0] https://bugs.freedesktop.org/show_bug.cgi?id=108714 [1] https://ark.intel.com/products/91064/Intel-Compute-Stick-STK1A32SC [2] https://www.datapro.net/products/usb-3-0-super-speed-a-a-debugging-cable.html [3] https://www.kernel.org/doc/html/v4.14/driver-api/usb/usb3-debug-port.html
Re: [PATCH v3 4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups
On Wed, Dec 5, 2018 at 5:57 AM PETER CHEN wrote: > So, there are two examples at binding-doc, one for normal, one for HSIC? > Fabio, do you > mean that? If DT maintainer agrees it too, I will add it. Yes, I think it will makes things clearer. Thanks
Re: [PATCH v1] usb: dwc3: trace: Refactor nested switch to make compiler happy
Andy Shevchenko writes: > On Wed, Dec 05, 2018 at 11:18:45AM +0200, Andy Shevchenko wrote: >> On Wed, Dec 05, 2018 at 11:10:46AM +0200, Felipe Balbi wrote: >> > Andy Shevchenko writes: >> > >> > > The missed break statement in the outer switch makes the code fall >> > > through >> > > always and thus always same value will be printed. >> > > >> > > Besides that, compiler warns about missed fall through marker: >> > > >> > > drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’: >> > > drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall >> > > through [-Wimplicit-fallthrough=] >> > > switch (pcm) { >> > > ^~ > >> > easier to add "break" here, no? That would be the minimal fix. >> >> No. Then you would need to add same default to the inner switch. > > Ah, you meant that pcm would be never outside of the given cases. > Yes, that's fine then, consider my patch as a bugreport. updated locally: From ad7b607f82731eec3ed17d9d22764eb6f09609f9 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 3 Dec 2018 11:28:47 +0200 Subject: [PATCH] usb: dwc3: trace: add missing break statement to make compiler happy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The missed break statement in the outer switch makes the code fall through always and thus always same value will be printed. Besides that, compiler warns about missed fall through marker: drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’: drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through [-Wimplicit-fallthrough=] switch (pcm) { ^~ Add the missing break statement to work correctly without compilation warnings. Fixes: fa8d965d736b ("usb: dwc3: trace: pretty print high-bandwidth transfers too") Cc: Felipe Balbi Signed-off-by: Andy Shevchenko Signed-off-by: Felipe Balbi --- drivers/usb/dwc3/trace.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h index 50fb6f2d92dd..36e5a4795fc8 100644 --- a/drivers/usb/dwc3/trace.h +++ b/drivers/usb/dwc3/trace.h @@ -254,6 +254,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb, s = "3x "; break; } + break; default: s = ""; } s; }), -- 2.19.2 -- balbi signature.asc Description: PGP signature
Re: [PATCH] USB: serial: console: fix reported terminal settings
On Wed, Dec 05, 2018 at 11:18:50AM +0100, Greg Kroah-Hartman wrote: > On Wed, Dec 05, 2018 at 11:10:20AM +0100, Johan Hovold wrote: > > On Wed, Dec 05, 2018 at 11:05:45AM +0100, Greg Kroah-Hartman wrote: > > > On Wed, Dec 05, 2018 at 10:50:49AM +0100, Johan Hovold wrote: > > > > > > Note that the changes to tty are trivial; I'm just renaming and > > > > exporting an existing helper. > > > > > > Yeah, I have no objections to them. We can take them in my usb tree. > > > > Thanks. Can I add your ack to the patch as well? > > Please do: > > Acked-by: Greg Kroah-Hartman Thanks, now applied. Johan
Re: USB driver resets
On 05/12/2018 09:44, Mika Westerberg wrote: On Tue, Dec 04, 2018 at 04:02:56PM +, Richard van der Hoff wrote: Sorry. The system is a Dell XPS13 9350, with an Intel DSL6340 Thunderbolt 3 bridge. The dock is a Plugable UD-CA1 [2]. Also if you have dual boot or similar, do you experience similar issue in Windows side? I do have dual-boot, though I very rarely boot into Windows. Since the issue is somewhat intermittent (for some reason it seems to happen particularly when I'm in a video conference...) I wouldn't have a huge amount of confidence of being able to reproduce it under Windows. Thanks again. [1] https://bugzilla.kernel.org/show_bug.cgi?id=201853 [2] https://plugable.com/products/ud-ca1/ This seems to be normal USB-C dock (not TBT). Yes, that's my understanding too. In the logs it looks like at some point the USB link just goes down which makes the host side to hot-remove xHCI and the intermediate PCIe ports. The UD-CA1 dock site you linked above mentions connection issues with some systems so I'm suspecting it is an issue with the dock itself not with the kernel. It would be good if you could try to reproduce on Windows side as well, basically doing the same things you do in Linux (assuming it is possible) and see if it triggers. OK, will give it a go. It'll probably take me a few days. Thanks for your help.
Re: [PATCH v3 2/3] spi: add FTDI MPSSE SPI controller driver
Hi Mark, On Tue, 27 Nov 2018 18:54:37 +0100 Anatolij Gustschin ag...@denx.de wrote: >Add SPI bus controller driver for FTDI MPSSE mode. This driver >is supposed to be used together with the FT232H interface driver >for FPGA configuration in drivers/usb/misc/ft232h-intf.c which >adds an mpsse spi platform device describing USB SPI bus with >attached SPI slave devices. I suppose this patch should go via Greg's usb tree as part of the series, once acked. I've addressed comments in v3, is it okay now? Thanks, Anatolij
Re: [PATCH] USB: serial: console: fix reported terminal settings
On Wed, Dec 05, 2018 at 11:10:20AM +0100, Johan Hovold wrote: > On Wed, Dec 05, 2018 at 11:05:45AM +0100, Greg Kroah-Hartman wrote: > > On Wed, Dec 05, 2018 at 10:50:49AM +0100, Johan Hovold wrote: > > > > Note that the changes to tty are trivial; I'm just renaming and > > > exporting an existing helper. > > > > Yeah, I have no objections to them. We can take them in my usb tree. > > Thanks. Can I add your ack to the patch as well? Please do: Acked-by: Greg Kroah-Hartman
Re: [PATCH] USB: serial: console: fix reported terminal settings
On 12/4/18 6:31 PM, Johan Hovold wrote: On Tue, Dec 04, 2018 at 05:15:18PM +0100, Greg Kroah-Hartman wrote: On Tue, Dec 04, 2018 at 05:00:36PM +0100, Johan Hovold wrote: The USB-serial console implementation has never reported the actual terminal settings used. Despite storing the corresponding cflags in its struct console, this was never honoured on later tty open() where the tty termios would be left initialised to the driver defaults. Unlike the serial console implementation, the USB-serial code calls subdriver open() already at console setup. While calling set_termios() before open() looks like it could work for some USB-serial drivers, others definitely do not expect this, so modelling this after serial core is going to be intrusive, if at all possible. Instead, use a (renamed) tty helper to save the termios data used at console setup, so that the tty termios reflects the actual terminal settings after a subsequent tty open(). Note that the calls to tty_init_termios() (tty_driver_install()) and tty_save_termios() are serialised using the disconnect mutex. This specifically fixes a regression that was triggered by a recent change adding software flow control to the pl2303 driver: a getty trying to disable flow control while leaving the baud rate unchanged would now also set the baud rate to the driver default (prior to the flow-control change this had been a noop). Fixes: 7041d9c3f01b ("USB: serial: pl2303: add support for tx xon/xoff flow control") Cc: stable# 4.18 Reported-by: Jarkko Nikula Cc: Florian Zumbiehl Signed-off-by: Johan Hovold --- drivers/tty/tty_io.c | 11 +-- drivers/usb/serial/console.c | 2 +- include/linux/tty.h | 1 + 3 files changed, 11 insertions(+), 3 deletions(-) Ah, messy :) Want me to take this through my tty tree? If you prefer. I was planning on including this in a USB-serial pull request for -rc6 since it fixes a user-reported regression, but perhaps taking this through your tty-linus branch (which already holds a console fix) is easier/faster. We should wait for Jarkko to confirm that this fixes the problem he reported first, though. Great, this fixed the issue for both pl2303 based adapters I reported. Tested on top of 0072a0c14d5b ("Merge tag 'media/v4.20-4' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media") Tested-by: Jarkko Nikula
Re: [PATCH] USB: serial: console: fix reported terminal settings
On Wed, Dec 05, 2018 at 11:05:45AM +0100, Greg Kroah-Hartman wrote: > On Wed, Dec 05, 2018 at 10:50:49AM +0100, Johan Hovold wrote: > > Note that the changes to tty are trivial; I'm just renaming and > > exporting an existing helper. > > Yeah, I have no objections to them. We can take them in my usb tree. Thanks. Can I add your ack to the patch as well? Johan
Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare|Meetup|Brio|Rally
On Tue, Dec 04, 2018 at 03:38:25PM -0500, Kyle Williams wrote: > Description: Some USB device / host controller combinations seem to have > problems with Link Power management. In particular it is described that > the combination of certain Logitech devices and other powered media > devices such as the Atrus device causes 'not enough bandwidth for > new device state'error. > > This patch creates quirk entries for the tested Logitech device > indicating LPM should remain disabled for the device. > > Signed-off-by: Kyle Williams > --- > drivers/usb/core/quirks.c | 16 > 1 file changed, 16 insertions(+) Along with what Alan said, this patch was sent in html mode, making it impossible to apply, and it was rejected by the mailing lists. Please fix up your email client before resending patches. Or just use 'git send-email'. thanks, greg k-h
[PATCH AUTOSEL 4.19 069/123] USB: omap_udc: fix crashes on probe error and module removal
From: Aaro Koskinen [ Upstream commit 99f700366fcea1aa2fa3c49c99f371670c3c62f8 ] We currently crash if usb_add_gadget_udc_release() fails, since the udc->done is not initialized until in the remove function. Furthermore, on module removal the udc data is accessed although the release function is already triggered by usb_del_gadget_udc() early in the function. Fix by rewriting the release and remove functions, basically moving all the cleanup into the release function, and doing the completion only in the module removal case. The patch fixes omap_udc module probe with a failing gadged, and also allows the removal of omap_udc. Tested by running "modprobe omap_udc; modprobe -r omap_udc" in a loop. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 50 --- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 1c77218c82af..240ccba44592 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2593,9 +2593,22 @@ omap_ep_setup(char *name, u8 addr, u8 type, static void omap_udc_release(struct device *dev) { - complete(udc->done); + pullup_disable(udc); + if (!IS_ERR_OR_NULL(udc->transceiver)) { + usb_put_phy(udc->transceiver); + udc->transceiver = NULL; + } + omap_writew(0, UDC_SYSCON1); + remove_proc_file(); + if (udc->dc_clk) { + if (udc->clk_requested) + omap_udc_enable_clock(0); + clk_put(udc->hhc_clk); + clk_put(udc->dc_clk); + } + if (udc->done) + complete(udc->done); kfree(udc); - udc = NULL; } static int @@ -2900,12 +2913,8 @@ static int omap_udc_probe(struct platform_device *pdev) } create_proc_file(); - status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, - omap_udc_release); - if (!status) - return 0; - - remove_proc_file(); + return usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, + omap_udc_release); cleanup1: kfree(udc); @@ -2932,36 +2941,15 @@ static int omap_udc_remove(struct platform_device *pdev) { DECLARE_COMPLETION_ONSTACK(done); - if (!udc) - return -ENODEV; - - usb_del_gadget_udc(&udc->gadget); - if (udc->driver) - return -EBUSY; - udc->done = &done; - pullup_disable(udc); - if (!IS_ERR_OR_NULL(udc->transceiver)) { - usb_put_phy(udc->transceiver); - udc->transceiver = NULL; - } - omap_writew(0, UDC_SYSCON1); - - remove_proc_file(); + usb_del_gadget_udc(&udc->gadget); - if (udc->dc_clk) { - if (udc->clk_requested) - omap_udc_enable_clock(0); - clk_put(udc->hhc_clk); - clk_put(udc->dc_clk); - } + wait_for_completion(&done); release_mem_region(pdev->resource[0].start, pdev->resource[0].end - pdev->resource[0].start + 1); - wait_for_completion(&done); - return 0; } -- 2.17.1
[PATCH AUTOSEL 4.19 070/123] USB: omap_udc: fix omap_udc_start() on 15xx machines
From: Aaro Koskinen [ Upstream commit 6ca6695f576b8453fe68865e84d25946d63b10ad ] On OMAP 15xx machines there are no transceivers, and omap_udc_start() always fails as it forgot to adjust the default return value. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 240ccba44592..33250e569af8 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2041,7 +2041,7 @@ static inline int machine_without_vbus_sense(void) static int omap_udc_start(struct usb_gadget *g, struct usb_gadget_driver *driver) { - int status = -ENODEV; + int status; struct omap_ep *ep; unsigned long flags; @@ -2079,6 +2079,7 @@ static int omap_udc_start(struct usb_gadget *g, goto done; } } else { + status = 0; if (can_pullup(udc)) pullup_enable(udc); else -- 2.17.1
[PATCH AUTOSEL 4.19 068/123] USB: omap_udc: use devm_request_irq()
From: Aaro Koskinen [ Upstream commit 286afdde1640d8ea8916a0f05e811441fbbf4b9d ] The current code fails to release the third irq on the error path (observed by reading the code), and we get also multiple WARNs with failing gadget drivers due to duplicate IRQ releases. Fix by using devm_request_irq(). Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 37 +-- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 3a16431da321..1c77218c82af 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2867,8 +2867,8 @@ static int omap_udc_probe(struct platform_device *pdev) udc->clr_halt = UDC_RESET_EP; /* USB general purpose IRQ: ep0, state changes, dma, etc */ - status = request_irq(pdev->resource[1].start, omap_udc_irq, - 0, driver_name, udc); + status = devm_request_irq(&pdev->dev, pdev->resource[1].start, + omap_udc_irq, 0, driver_name, udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[1].start, status); @@ -2876,20 +2876,20 @@ static int omap_udc_probe(struct platform_device *pdev) } /* USB "non-iso" IRQ (PIO for all but ep0) */ - status = request_irq(pdev->resource[2].start, omap_udc_pio_irq, - 0, "omap_udc pio", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[2].start, + omap_udc_pio_irq, 0, "omap_udc pio", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[2].start, status); - goto cleanup2; + goto cleanup1; } #ifdef USE_ISO - status = request_irq(pdev->resource[3].start, omap_udc_iso_irq, - 0, "omap_udc iso", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[3].start, + omap_udc_iso_irq, 0, "omap_udc iso", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[3].start, status); - goto cleanup3; + goto cleanup1; } #endif if (cpu_is_omap16xx() || cpu_is_omap7xx()) { @@ -2902,22 +2902,11 @@ static int omap_udc_probe(struct platform_device *pdev) create_proc_file(); status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, omap_udc_release); - if (status) - goto cleanup4; - - return 0; + if (!status) + return 0; -cleanup4: remove_proc_file(); -#ifdef USE_ISO -cleanup3: - free_irq(pdev->resource[2].start, udc); -#endif - -cleanup2: - free_irq(pdev->resource[1].start, udc); - cleanup1: kfree(udc); udc = NULL; @@ -2961,12 +2950,6 @@ static int omap_udc_remove(struct platform_device *pdev) remove_proc_file(); -#ifdef USE_ISO - free_irq(pdev->resource[3].start, udc); -#endif - free_irq(pdev->resource[2].start, udc); - free_irq(pdev->resource[1].start, udc); - if (udc->dc_clk) { if (udc->clk_requested) omap_udc_enable_clock(0); -- 2.17.1
[PATCH AUTOSEL 4.19 072/123] USB: omap_udc: fix rejection of out transfers when DMA is used
From: Aaro Koskinen [ Upstream commit 069caf5950dfa75d0526cd89c439ff9d9d3136d8 ] Commit 387f869d2579 ("usb: gadget: u_ether: conditionally align transfer size") started aligning transfer size only if requested, breaking omap_udc DMA mode. Set quirk_ep_out_aligned_size to restore the old behaviour. Fixes: 387f869d2579 ("usb: gadget: u_ether: conditionally align transfer size") Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 9b23e04c8f02..fcf13ef33b31 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2642,6 +2642,7 @@ omap_udc_setup(struct platform_device *odev, struct usb_phy *xceiv) udc->gadget.speed = USB_SPEED_UNKNOWN; udc->gadget.max_speed = USB_SPEED_FULL; udc->gadget.name = driver_name; + udc->gadget.quirk_ep_out_aligned_size = 1; udc->transceiver = xceiv; /* ep0 is special; put it right after the SETUP buffer */ -- 2.17.1
[PATCH AUTOSEL 4.19 071/123] USB: omap_udc: fix USB gadget functionality on Palm Tungsten E
From: Aaro Koskinen [ Upstream commit 2c2322fbcab8102b8cadc09d66714700a2da42c2 ] On Palm TE nothing happens when you try to use gadget drivers and plug the USB cable. Fix by adding the board to the vbus sense quirk list. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 33250e569af8..9b23e04c8f02 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2033,6 +2033,7 @@ static inline int machine_without_vbus_sense(void) { return machine_is_omap_innovator() || machine_is_omap_osk() + || machine_is_omap_palmte() || machine_is_sx1() /* No known omap7xx boards with vbus sense */ || cpu_is_omap7xx(); -- 2.17.1
[PATCH AUTOSEL 4.19 083/123] usbnet: ipheth: fix potential recvmsg bug and recvmsg bug 2
From: Bernd Eckstein <3erndeckst...@gmail.com> [ Upstream commit 45611c61dd503454b2edae00aabe1e429ec49ebe ] The bug is not easily reproducable, as it may occur very infrequently (we had machines with 20minutes heavy downloading before it occurred) However, on a virual machine (VMWare on Windows 10 host) it occurred pretty frequently (1-2 seconds after a speedtest was started) dev->tx_skb mab be freed via dev_kfree_skb_irq on a callback before it is set. This causes the following problems: - double free of the skb or potential memory leak - in dmesg: 'recvmsg bug' and 'recvmsg bug 2' and eventually general protection fault Example dmesg output: [ 134.841986] [ cut here ] [ 134.841987] recvmsg bug: copied 9C24A555 seq 9C24B557 rcvnxt 9C25A6B3 fl 0 [ 134.841993] WARNING: CPU: 7 PID: 2629 at /build/linux-hwe-On9fm7/linux-hwe-4.15.0/net/ipv4/tcp.c:1865 tcp_recvmsg+0x44d/0xab0 [ 134.841994] Modules linked in: ipheth(OE) kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 crypto_simd glue_helper cryptd vmw_balloon intel_rapl_perf joydev input_leds serio_raw vmw_vsock_vmci_transport vsock shpchp i2c_piix4 mac_hid binfmt_misc vmw_vmci parport_pc ppdev lp parport autofs4 vmw_pvscsi vmxnet3 hid_generic usbhid hid vmwgfx ttm drm_kms_helper syscopyarea sysfillrect mptspi mptscsih sysimgblt ahci psmouse fb_sys_fops pata_acpi mptbase libahci e1000 drm scsi_transport_spi [ 134.842046] CPU: 7 PID: 2629 Comm: python Tainted: GW OE 4.15.0-34-generic #37~16.04.1-Ubuntu [ 134.842046] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017 [ 134.842048] RIP: 0010:tcp_recvmsg+0x44d/0xab0 [ 134.842048] RSP: 0018:a6630422bcc8 EFLAGS: 00010286 [ 134.842049] RAX: RBX: 997616f4f200 RCX: 0006 [ 134.842049] RDX: 0007 RSI: 0082 RDI: 9976257d6490 [ 134.842050] RBP: a6630422bd98 R08: 0001 R09: 0004bba4 [ 134.842050] R10: 01e00c6f R11: 0004bba4 R12: 99760dee3000 [ 134.842051] R13: R14: 99760dee3514 R15: [ 134.842051] FS: 7fe332347700() GS:9976257c() knlGS: [ 134.842052] CS: 0010 DS: ES: CR0: 80050033 [ 134.842053] CR2: 01e41000 CR3: 00020e9b4006 CR4: 003606e0 [ 134.842055] DR0: DR1: DR2: [ 134.842055] DR3: DR6: fffe0ff0 DR7: 0400 [ 134.842057] Call Trace: [ 134.842060] ? aa_sk_perm+0x53/0x1a0 [ 134.842064] inet_recvmsg+0x51/0xc0 [ 134.842066] sock_recvmsg+0x43/0x50 [ 134.842070] SYSC_recvfrom+0xe4/0x160 [ 134.842072] ? __schedule+0x3de/0x8b0 [ 134.842075] ? ktime_get_ts64+0x4c/0xf0 [ 134.842079] SyS_recvfrom+0xe/0x10 [ 134.842082] do_syscall_64+0x73/0x130 [ 134.842086] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 [ 134.842086] RIP: 0033:0x7fe331f5a81d [ 134.842088] RSP: 002b:7ffe8da98398 EFLAGS: 0246 ORIG_RAX: 002d [ 134.842090] RAX: ffda RBX: RCX: 7fe331f5a81d [ 134.842094] RDX: 03fb RSI: 01e00874 RDI: 0003 [ 134.842095] RBP: 7fe32f642c70 R08: R09: [ 134.842097] R10: R11: 0246 R12: 7fe332347698 [ 134.842099] R13: 01b7e0a0 R14: 01e00874 R15: [ 134.842103] Code: 24 fd ff ff e9 cc fe ff ff 48 89 d8 41 8b 8c 24 10 05 00 00 44 8b 45 80 48 c7 c7 08 bd 59 8b 48 89 85 68 ff ff ff e8 b3 c4 7d ff <0f> 0b 48 8b 85 68 ff ff ff e9 e9 fe ff ff 41 8b 8c 24 10 05 00 [ 134.842126] ---[ end trace b7138fc08c83147f ]--- [ 134.842144] general protection fault: [#1] SMP PTI [ 134.842145] Modules linked in: ipheth(OE) kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 crypto_simd glue_helper cryptd vmw_balloon intel_rapl_perf joydev input_leds serio_raw vmw_vsock_vmci_transport vsock shpchp i2c_piix4 mac_hid binfmt_misc vmw_vmci parport_pc ppdev lp parport autofs4 vmw_pvscsi vmxnet3 hid_generic usbhid hid vmwgfx ttm drm_kms_helper syscopyarea sysfillrect mptspi mptscsih sysimgblt ahci psmouse fb_sys_fops pata_acpi mptbase libahci e1000 drm scsi_transport_spi [ 134.842161] CPU: 7 PID: 2629 Comm: python Tainted: GW OE 4.15.0-34-generic #37~16.04.1-Ubuntu [ 134.842162] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017 [ 134.842164] RIP: 0010:tcp_close+0x2c6/0x440 [ 134.842165] RSP: 0018:a6630422bde8 EFLAGS: 00010202 [ 134.842167] RAX: RBX: 99760dee3000 RCX: 000180400034 [ 134.842168] RDX: 5c4afd407207a6c4 RSI: e868495bd300 RDI: 997616f4f200 [ 134.842169] RBP: a6630422be08 R08: 16f4d401 R0
Re: [PATCH v2 1/3] staging: typec: fusb302: Rename fcs,extcon-name to linux,extcon-name
On Wed, Dec 05, 2018 at 10:25:36AM +0100, Greg Kroah-Hartman wrote: > On Mon, Dec 03, 2018 at 09:23:44PM +0200, Andy Shevchenko wrote: > > On Thu, Nov 15, 2018 at 04:53:03PM +0200, Heikki Krogerus wrote: > > > FWIW, the series: > > > Reviewed-by: Heikki Krogerus > > > > Greg, can you pick up this patch (first in the series, since dwc3 went > > through Felipe's tree)? > > Sorry for the delay, now queued up. NP, thanks! -- With Best Regards, Andy Shevchenko
Re: [PATCH] USB: serial: console: fix reported terminal settings
On Wed, Dec 05, 2018 at 10:50:49AM +0100, Johan Hovold wrote: > On Wed, Dec 05, 2018 at 11:36:52AM +0200, Jarkko Nikula wrote: > > On 12/4/18 6:31 PM, Johan Hovold wrote: > > > On Tue, Dec 04, 2018 at 05:15:18PM +0100, Greg Kroah-Hartman wrote: > > >> On Tue, Dec 04, 2018 at 05:00:36PM +0100, Johan Hovold wrote: > > >>> The USB-serial console implementation has never reported the actual > > >>> terminal settings used. Despite storing the corresponding cflags in its > > >>> struct console, this was never honoured on later tty open() where the > > >>> tty termios would be left initialised to the driver defaults. > > >>> > > >>> Unlike the serial console implementation, the USB-serial code calls > > >>> subdriver open() already at console setup. While calling set_termios() > > >>> before open() looks like it could work for some USB-serial drivers, > > >>> others definitely do not expect this, so modelling this after serial > > >>> core is going to be intrusive, if at all possible. > > > >>> This specifically fixes a regression that was triggered by a recent > > >>> change adding software flow control to the pl2303 driver: a getty trying > > >>> to disable flow control while leaving the baud rate unchanged would now > > >>> also set the baud rate to the driver default (prior to the flow-control > > >>> change this had been a noop). > > >>> > > >>> Fixes: 7041d9c3f01b ("USB: serial: pl2303: add support for tx xon/xoff > > >>> flow control") > > >>> Cc: stable # 4.18 > > >>> Reported-by: Jarkko Nikula > > >>> Cc: Florian Zumbiehl > > >>> Signed-off-by: Johan Hovold > > >>> --- > > >>> drivers/tty/tty_io.c | 11 +-- > > >>> drivers/usb/serial/console.c | 2 +- > > >>> include/linux/tty.h | 1 + > > >>> 3 files changed, 11 insertions(+), 3 deletions(-) > > >> > > >> Ah, messy :) > > >> > > >> Want me to take this through my tty tree? > > > > > > If you prefer. I was planning on including this in a USB-serial pull > > > request for -rc6 since it fixes a user-reported regression, but perhaps > > > taking this through your tty-linus branch (which already holds a console > > > fix) is easier/faster. > > > > > > We should wait for Jarkko to confirm that this fixes the problem he > > > reported first, though. > > > > > Great, this fixed the issue for both pl2303 based adapters I reported. > > > > Tested on top of 0072a0c14d5b ("Merge tag 'media/v4.20-4' of > > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media") > > > > Tested-by: Jarkko Nikula > > Great, thanks for testing. > > Greg, I noticed I left out the part about subdrivers not expecting > *write()* to be called before open() so I'll amend the commit message > when applying and include this one in a pull-request tomorrow, if that's > ok with you? Sure, that's fine. > Note that the changes to tty are trivial; I'm just renaming and > exporting an existing helper. Yeah, I have no objections to them. We can take them in my usb tree. thanks, greg k-h
[PATCH AUTOSEL 4.14 36/69] USB: omap_udc: fix omap_udc_start() on 15xx machines
From: Aaro Koskinen [ Upstream commit 6ca6695f576b8453fe68865e84d25946d63b10ad ] On OMAP 15xx machines there are no transceivers, and omap_udc_start() always fails as it forgot to adjust the default return value. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index d45dc14ef0a2..9060b3af27ff 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2045,7 +2045,7 @@ static inline int machine_without_vbus_sense(void) static int omap_udc_start(struct usb_gadget *g, struct usb_gadget_driver *driver) { - int status = -ENODEV; + int status; struct omap_ep *ep; unsigned long flags; @@ -2083,6 +2083,7 @@ static int omap_udc_start(struct usb_gadget *g, goto done; } } else { + status = 0; if (can_pullup(udc)) pullup_enable(udc); else -- 2.17.1
[PATCH AUTOSEL 4.14 35/69] USB: omap_udc: fix crashes on probe error and module removal
From: Aaro Koskinen [ Upstream commit 99f700366fcea1aa2fa3c49c99f371670c3c62f8 ] We currently crash if usb_add_gadget_udc_release() fails, since the udc->done is not initialized until in the remove function. Furthermore, on module removal the udc data is accessed although the release function is already triggered by usb_del_gadget_udc() early in the function. Fix by rewriting the release and remove functions, basically moving all the cleanup into the release function, and doing the completion only in the module removal case. The patch fixes omap_udc module probe with a failing gadged, and also allows the removal of omap_udc. Tested by running "modprobe omap_udc; modprobe -r omap_udc" in a loop. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 50 --- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index e515c85ef0c5..d45dc14ef0a2 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2612,9 +2612,22 @@ omap_ep_setup(char *name, u8 addr, u8 type, static void omap_udc_release(struct device *dev) { - complete(udc->done); + pullup_disable(udc); + if (!IS_ERR_OR_NULL(udc->transceiver)) { + usb_put_phy(udc->transceiver); + udc->transceiver = NULL; + } + omap_writew(0, UDC_SYSCON1); + remove_proc_file(); + if (udc->dc_clk) { + if (udc->clk_requested) + omap_udc_enable_clock(0); + clk_put(udc->hhc_clk); + clk_put(udc->dc_clk); + } + if (udc->done) + complete(udc->done); kfree(udc); - udc = NULL; } static int @@ -2919,12 +2932,8 @@ static int omap_udc_probe(struct platform_device *pdev) } create_proc_file(); - status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, - omap_udc_release); - if (!status) - return 0; - - remove_proc_file(); + return usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, + omap_udc_release); cleanup1: kfree(udc); @@ -2951,36 +2960,15 @@ static int omap_udc_remove(struct platform_device *pdev) { DECLARE_COMPLETION_ONSTACK(done); - if (!udc) - return -ENODEV; - - usb_del_gadget_udc(&udc->gadget); - if (udc->driver) - return -EBUSY; - udc->done = &done; - pullup_disable(udc); - if (!IS_ERR_OR_NULL(udc->transceiver)) { - usb_put_phy(udc->transceiver); - udc->transceiver = NULL; - } - omap_writew(0, UDC_SYSCON1); - - remove_proc_file(); + usb_del_gadget_udc(&udc->gadget); - if (udc->dc_clk) { - if (udc->clk_requested) - omap_udc_enable_clock(0); - clk_put(udc->hhc_clk); - clk_put(udc->dc_clk); - } + wait_for_completion(&done); release_mem_region(pdev->resource[0].start, pdev->resource[0].end - pdev->resource[0].start + 1); - wait_for_completion(&done); - return 0; } -- 2.17.1
[PATCH AUTOSEL 4.14 34/69] USB: omap_udc: use devm_request_irq()
From: Aaro Koskinen [ Upstream commit 286afdde1640d8ea8916a0f05e811441fbbf4b9d ] The current code fails to release the third irq on the error path (observed by reading the code), and we get also multiple WARNs with failing gadget drivers due to duplicate IRQ releases. Fix by using devm_request_irq(). Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 37 +-- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index f05ba6825bfe..e515c85ef0c5 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2886,8 +2886,8 @@ static int omap_udc_probe(struct platform_device *pdev) udc->clr_halt = UDC_RESET_EP; /* USB general purpose IRQ: ep0, state changes, dma, etc */ - status = request_irq(pdev->resource[1].start, omap_udc_irq, - 0, driver_name, udc); + status = devm_request_irq(&pdev->dev, pdev->resource[1].start, + omap_udc_irq, 0, driver_name, udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[1].start, status); @@ -2895,20 +2895,20 @@ static int omap_udc_probe(struct platform_device *pdev) } /* USB "non-iso" IRQ (PIO for all but ep0) */ - status = request_irq(pdev->resource[2].start, omap_udc_pio_irq, - 0, "omap_udc pio", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[2].start, + omap_udc_pio_irq, 0, "omap_udc pio", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[2].start, status); - goto cleanup2; + goto cleanup1; } #ifdef USE_ISO - status = request_irq(pdev->resource[3].start, omap_udc_iso_irq, - 0, "omap_udc iso", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[3].start, + omap_udc_iso_irq, 0, "omap_udc iso", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[3].start, status); - goto cleanup3; + goto cleanup1; } #endif if (cpu_is_omap16xx() || cpu_is_omap7xx()) { @@ -2921,22 +2921,11 @@ static int omap_udc_probe(struct platform_device *pdev) create_proc_file(); status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, omap_udc_release); - if (status) - goto cleanup4; - - return 0; + if (!status) + return 0; -cleanup4: remove_proc_file(); -#ifdef USE_ISO -cleanup3: - free_irq(pdev->resource[2].start, udc); -#endif - -cleanup2: - free_irq(pdev->resource[1].start, udc); - cleanup1: kfree(udc); udc = NULL; @@ -2980,12 +2969,6 @@ static int omap_udc_remove(struct platform_device *pdev) remove_proc_file(); -#ifdef USE_ISO - free_irq(pdev->resource[3].start, udc); -#endif - free_irq(pdev->resource[2].start, udc); - free_irq(pdev->resource[1].start, udc); - if (udc->dc_clk) { if (udc->clk_requested) omap_udc_enable_clock(0); -- 2.17.1
[PATCH AUTOSEL 4.14 38/69] USB: omap_udc: fix rejection of out transfers when DMA is used
From: Aaro Koskinen [ Upstream commit 069caf5950dfa75d0526cd89c439ff9d9d3136d8 ] Commit 387f869d2579 ("usb: gadget: u_ether: conditionally align transfer size") started aligning transfer size only if requested, breaking omap_udc DMA mode. Set quirk_ep_out_aligned_size to restore the old behaviour. Fixes: 387f869d2579 ("usb: gadget: u_ether: conditionally align transfer size") Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index c8facc8aa87e..ee0b87a0773c 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2661,6 +2661,7 @@ omap_udc_setup(struct platform_device *odev, struct usb_phy *xceiv) udc->gadget.speed = USB_SPEED_UNKNOWN; udc->gadget.max_speed = USB_SPEED_FULL; udc->gadget.name = driver_name; + udc->gadget.quirk_ep_out_aligned_size = 1; udc->transceiver = xceiv; /* ep0 is special; put it right after the SETUP buffer */ -- 2.17.1
[PATCH AUTOSEL 4.14 37/69] USB: omap_udc: fix USB gadget functionality on Palm Tungsten E
From: Aaro Koskinen [ Upstream commit 2c2322fbcab8102b8cadc09d66714700a2da42c2 ] On Palm TE nothing happens when you try to use gadget drivers and plug the USB cable. Fix by adding the board to the vbus sense quirk list. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 9060b3af27ff..c8facc8aa87e 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2037,6 +2037,7 @@ static inline int machine_without_vbus_sense(void) { return machine_is_omap_innovator() || machine_is_omap_osk() + || machine_is_omap_palmte() || machine_is_sx1() /* No known omap7xx boards with vbus sense */ || cpu_is_omap7xx(); -- 2.17.1
[PATCH AUTOSEL 4.14 45/69] usb: gadget: u_ether: fix unsafe list iteration
From: Marek Szyprowski [ Upstream commit c9287fa657b3328b4549c0ab39ea7f197a3d6a50 ] list_for_each_entry_safe() is not safe for deleting entries from the list if the spin lock, which protects it, is released and reacquired during the list iteration. Fix this issue by replacing this construction with a simple check if list is empty and removing the first entry in each iteration. This is almost equivalent to a revert of the commit mentioned in the Fixes: tag. This patch fixes following issue: --->8--- Unable to handle kernel NULL pointer dereference at virtual address 0104 pgd = (ptrval) [0104] *pgd= Internal error: Oops: 817 [#1] PREEMPT SMP ARM Modules linked in: CPU: 1 PID: 84 Comm: kworker/1:1 Not tainted 4.20.0-rc2-next-20181114-9-g8266b35ec404 #1061 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: events eth_work PC is at rx_fill+0x60/0xac LR is at _raw_spin_lock_irqsave+0x50/0x5c pc : []lr : []psr: 8093 sp : ee7fbee8 ip : 0100 fp : r10: 006000c0 r9 : c10b0ab0 r8 : ee7eb5c0 r7 : ee7eb614 r6 : ee7eb5ec r5 : 00dc r4 : ee12ac00 r3 : ee12ac24 r2 : 0200 r1 : 6013 r0 : ee7eb5ec Flags: Nzcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 6d5dc04a DAC: 0051 Process kworker/1:1 (pid: 84, stack limit = 0x(ptrval)) Stack: (0xee7fbee8 to 0xee7fc000) ... [] (rx_fill) from [] (process_one_work+0x200/0x738) [] (process_one_work) from [] (worker_thread+0x2c/0x4c8) [] (worker_thread) from [] (kthread+0x128/0x164) [] (kthread) from [] (ret_from_fork+0x14/0x20) Exception stack(0xee7fbfb0 to 0xee7fbff8) ... ---[ end trace 64480bc835eba7d6 ]--- Fixes: fea14e68ff5e ("usb: gadget: u_ether: use better list accessors") Signed-off-by: Marek Szyprowski Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/function/u_ether.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index bdbc3fdc7c4f..3a0e4f5d7b83 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -405,12 +405,12 @@ static int alloc_requests(struct eth_dev *dev, struct gether *link, unsigned n) static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags) { struct usb_request *req; - struct usb_request *tmp; unsigned long flags; /* fill unused rxq slots with some skb */ spin_lock_irqsave(&dev->req_lock, flags); - list_for_each_entry_safe(req, tmp, &dev->rx_reqs, list) { + while (!list_empty(&dev->rx_reqs)) { + req = list_first_entry(&dev->rx_reqs, struct usb_request, list); list_del_init(&req->list); spin_unlock_irqrestore(&dev->req_lock, flags); @@ -1125,7 +1125,6 @@ void gether_disconnect(struct gether *link) { struct eth_dev *dev = link->ioport; struct usb_request *req; - struct usb_request *tmp; WARN_ON(!dev); if (!dev) @@ -1142,7 +1141,8 @@ void gether_disconnect(struct gether *link) */ usb_ep_disable(link->in_ep); spin_lock(&dev->req_lock); - list_for_each_entry_safe(req, tmp, &dev->tx_reqs, list) { + while (!list_empty(&dev->tx_reqs)) { + req = list_first_entry(&dev->tx_reqs, struct usb_request, list); list_del(&req->list); spin_unlock(&dev->req_lock); @@ -1154,7 +1154,8 @@ void gether_disconnect(struct gether *link) usb_ep_disable(link->out_ep); spin_lock(&dev->req_lock); - list_for_each_entry_safe(req, tmp, &dev->rx_reqs, list) { + while (!list_empty(&dev->rx_reqs)) { + req = list_first_entry(&dev->rx_reqs, struct usb_request, list); list_del(&req->list); spin_unlock(&dev->req_lock); -- 2.17.1
[PATCH AUTOSEL 4.9 23/45] USB: omap_udc: fix crashes on probe error and module removal
From: Aaro Koskinen [ Upstream commit 99f700366fcea1aa2fa3c49c99f371670c3c62f8 ] We currently crash if usb_add_gadget_udc_release() fails, since the udc->done is not initialized until in the remove function. Furthermore, on module removal the udc data is accessed although the release function is already triggered by usb_del_gadget_udc() early in the function. Fix by rewriting the release and remove functions, basically moving all the cleanup into the release function, and doing the completion only in the module removal case. The patch fixes omap_udc module probe with a failing gadged, and also allows the removal of omap_udc. Tested by running "modprobe omap_udc; modprobe -r omap_udc" in a loop. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 50 --- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 2945408f0eec..2a23b21fe153 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2612,9 +2612,22 @@ omap_ep_setup(char *name, u8 addr, u8 type, static void omap_udc_release(struct device *dev) { - complete(udc->done); + pullup_disable(udc); + if (!IS_ERR_OR_NULL(udc->transceiver)) { + usb_put_phy(udc->transceiver); + udc->transceiver = NULL; + } + omap_writew(0, UDC_SYSCON1); + remove_proc_file(); + if (udc->dc_clk) { + if (udc->clk_requested) + omap_udc_enable_clock(0); + clk_put(udc->hhc_clk); + clk_put(udc->dc_clk); + } + if (udc->done) + complete(udc->done); kfree(udc); - udc = NULL; } static int @@ -2919,12 +2932,8 @@ static int omap_udc_probe(struct platform_device *pdev) } create_proc_file(); - status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, - omap_udc_release); - if (!status) - return 0; - - remove_proc_file(); + return usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, + omap_udc_release); cleanup1: kfree(udc); @@ -2951,36 +2960,15 @@ static int omap_udc_remove(struct platform_device *pdev) { DECLARE_COMPLETION_ONSTACK(done); - if (!udc) - return -ENODEV; - - usb_del_gadget_udc(&udc->gadget); - if (udc->driver) - return -EBUSY; - udc->done = &done; - pullup_disable(udc); - if (!IS_ERR_OR_NULL(udc->transceiver)) { - usb_put_phy(udc->transceiver); - udc->transceiver = NULL; - } - omap_writew(0, UDC_SYSCON1); - - remove_proc_file(); + usb_del_gadget_udc(&udc->gadget); - if (udc->dc_clk) { - if (udc->clk_requested) - omap_udc_enable_clock(0); - clk_put(udc->hhc_clk); - clk_put(udc->dc_clk); - } + wait_for_completion(&done); release_mem_region(pdev->resource[0].start, pdev->resource[0].end - pdev->resource[0].start + 1); - wait_for_completion(&done); - return 0; } -- 2.17.1
[PATCH AUTOSEL 4.9 22/45] USB: omap_udc: use devm_request_irq()
From: Aaro Koskinen [ Upstream commit 286afdde1640d8ea8916a0f05e811441fbbf4b9d ] The current code fails to release the third irq on the error path (observed by reading the code), and we get also multiple WARNs with failing gadget drivers due to duplicate IRQ releases. Fix by using devm_request_irq(). Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 37 +-- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index a8709f9e5648..2945408f0eec 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2886,8 +2886,8 @@ static int omap_udc_probe(struct platform_device *pdev) udc->clr_halt = UDC_RESET_EP; /* USB general purpose IRQ: ep0, state changes, dma, etc */ - status = request_irq(pdev->resource[1].start, omap_udc_irq, - 0, driver_name, udc); + status = devm_request_irq(&pdev->dev, pdev->resource[1].start, + omap_udc_irq, 0, driver_name, udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[1].start, status); @@ -2895,20 +2895,20 @@ static int omap_udc_probe(struct platform_device *pdev) } /* USB "non-iso" IRQ (PIO for all but ep0) */ - status = request_irq(pdev->resource[2].start, omap_udc_pio_irq, - 0, "omap_udc pio", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[2].start, + omap_udc_pio_irq, 0, "omap_udc pio", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[2].start, status); - goto cleanup2; + goto cleanup1; } #ifdef USE_ISO - status = request_irq(pdev->resource[3].start, omap_udc_iso_irq, - 0, "omap_udc iso", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[3].start, + omap_udc_iso_irq, 0, "omap_udc iso", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[3].start, status); - goto cleanup3; + goto cleanup1; } #endif if (cpu_is_omap16xx() || cpu_is_omap7xx()) { @@ -2921,22 +2921,11 @@ static int omap_udc_probe(struct platform_device *pdev) create_proc_file(); status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, omap_udc_release); - if (status) - goto cleanup4; - - return 0; + if (!status) + return 0; -cleanup4: remove_proc_file(); -#ifdef USE_ISO -cleanup3: - free_irq(pdev->resource[2].start, udc); -#endif - -cleanup2: - free_irq(pdev->resource[1].start, udc); - cleanup1: kfree(udc); udc = NULL; @@ -2980,12 +2969,6 @@ static int omap_udc_remove(struct platform_device *pdev) remove_proc_file(); -#ifdef USE_ISO - free_irq(pdev->resource[3].start, udc); -#endif - free_irq(pdev->resource[2].start, udc); - free_irq(pdev->resource[1].start, udc); - if (udc->dc_clk) { if (udc->clk_requested) omap_udc_enable_clock(0); -- 2.17.1
[PATCH AUTOSEL 4.9 25/45] USB: omap_udc: fix USB gadget functionality on Palm Tungsten E
From: Aaro Koskinen [ Upstream commit 2c2322fbcab8102b8cadc09d66714700a2da42c2 ] On Palm TE nothing happens when you try to use gadget drivers and plug the USB cable. Fix by adding the board to the vbus sense quirk list. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 8f044caa8ad4..9eed4947aad8 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2037,6 +2037,7 @@ static inline int machine_without_vbus_sense(void) { return machine_is_omap_innovator() || machine_is_omap_osk() + || machine_is_omap_palmte() || machine_is_sx1() /* No known omap7xx boards with vbus sense */ || cpu_is_omap7xx(); -- 2.17.1
[PATCH AUTOSEL 4.9 24/45] USB: omap_udc: fix omap_udc_start() on 15xx machines
From: Aaro Koskinen [ Upstream commit 6ca6695f576b8453fe68865e84d25946d63b10ad ] On OMAP 15xx machines there are no transceivers, and omap_udc_start() always fails as it forgot to adjust the default return value. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 2a23b21fe153..8f044caa8ad4 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2045,7 +2045,7 @@ static inline int machine_without_vbus_sense(void) static int omap_udc_start(struct usb_gadget *g, struct usb_gadget_driver *driver) { - int status = -ENODEV; + int status; struct omap_ep *ep; unsigned long flags; @@ -2083,6 +2083,7 @@ static int omap_udc_start(struct usb_gadget *g, goto done; } } else { + status = 0; if (can_pullup(udc)) pullup_enable(udc); else -- 2.17.1
Re: [PATCH v3 3/3] fpga: Add fpga manager driver for ARRI Altera FPP
Hi Alan, On Tue, 27 Nov 2018 18:54:38 +0100 Anatolij Gustschin ag...@denx.de wrote: >Add FPGA manager driver for loading ARRI Altera FPGAs via fast >passive parallel (FPP) interface using FTDI FT232H chip. Ping. Is this patch okay? I suppose that it will go via usb tree as part of the whole series (you send fpga driver patches to Greg anyway). Can you ack this patch or is there something that should be changed? I'd like to see it merged in v4.21, so if there are issues with this patch, I'll try to address it before it is to late. Thanks, Anatolij
[PATCH AUTOSEL 4.4 16/33] USB: omap_udc: fix omap_udc_start() on 15xx machines
From: Aaro Koskinen [ Upstream commit 6ca6695f576b8453fe68865e84d25946d63b10ad ] On OMAP 15xx machines there are no transceivers, and omap_udc_start() always fails as it forgot to adjust the default return value. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index da1030f69145..653963459d78 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2045,7 +2045,7 @@ static inline int machine_without_vbus_sense(void) static int omap_udc_start(struct usb_gadget *g, struct usb_gadget_driver *driver) { - int status = -ENODEV; + int status; struct omap_ep *ep; unsigned long flags; @@ -2083,6 +2083,7 @@ static int omap_udc_start(struct usb_gadget *g, goto done; } } else { + status = 0; if (can_pullup(udc)) pullup_enable(udc); else -- 2.17.1
[PATCH AUTOSEL 4.4 17/33] USB: omap_udc: fix USB gadget functionality on Palm Tungsten E
From: Aaro Koskinen [ Upstream commit 2c2322fbcab8102b8cadc09d66714700a2da42c2 ] On Palm TE nothing happens when you try to use gadget drivers and plug the USB cable. Fix by adding the board to the vbus sense quirk list. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 653963459d78..d1ed92acafa3 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2037,6 +2037,7 @@ static inline int machine_without_vbus_sense(void) { return machine_is_omap_innovator() || machine_is_omap_osk() + || machine_is_omap_palmte() || machine_is_sx1() /* No known omap7xx boards with vbus sense */ || cpu_is_omap7xx(); -- 2.17.1
[PATCH AUTOSEL 4.4 15/33] USB: omap_udc: fix crashes on probe error and module removal
From: Aaro Koskinen [ Upstream commit 99f700366fcea1aa2fa3c49c99f371670c3c62f8 ] We currently crash if usb_add_gadget_udc_release() fails, since the udc->done is not initialized until in the remove function. Furthermore, on module removal the udc data is accessed although the release function is already triggered by usb_del_gadget_udc() early in the function. Fix by rewriting the release and remove functions, basically moving all the cleanup into the release function, and doing the completion only in the module removal case. The patch fixes omap_udc module probe with a failing gadged, and also allows the removal of omap_udc. Tested by running "modprobe omap_udc; modprobe -r omap_udc" in a loop. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 50 --- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index b25eac2dcaf8..da1030f69145 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2612,9 +2612,22 @@ omap_ep_setup(char *name, u8 addr, u8 type, static void omap_udc_release(struct device *dev) { - complete(udc->done); + pullup_disable(udc); + if (!IS_ERR_OR_NULL(udc->transceiver)) { + usb_put_phy(udc->transceiver); + udc->transceiver = NULL; + } + omap_writew(0, UDC_SYSCON1); + remove_proc_file(); + if (udc->dc_clk) { + if (udc->clk_requested) + omap_udc_enable_clock(0); + clk_put(udc->hhc_clk); + clk_put(udc->dc_clk); + } + if (udc->done) + complete(udc->done); kfree(udc); - udc = NULL; } static int @@ -2919,12 +2932,8 @@ static int omap_udc_probe(struct platform_device *pdev) } create_proc_file(); - status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, - omap_udc_release); - if (!status) - return 0; - - remove_proc_file(); + return usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, + omap_udc_release); cleanup1: kfree(udc); @@ -2951,36 +2960,15 @@ static int omap_udc_remove(struct platform_device *pdev) { DECLARE_COMPLETION_ONSTACK(done); - if (!udc) - return -ENODEV; - - usb_del_gadget_udc(&udc->gadget); - if (udc->driver) - return -EBUSY; - udc->done = &done; - pullup_disable(udc); - if (!IS_ERR_OR_NULL(udc->transceiver)) { - usb_put_phy(udc->transceiver); - udc->transceiver = NULL; - } - omap_writew(0, UDC_SYSCON1); - - remove_proc_file(); + usb_del_gadget_udc(&udc->gadget); - if (udc->dc_clk) { - if (udc->clk_requested) - omap_udc_enable_clock(0); - clk_put(udc->hhc_clk); - clk_put(udc->dc_clk); - } + wait_for_completion(&done); release_mem_region(pdev->resource[0].start, pdev->resource[0].end - pdev->resource[0].start + 1); - wait_for_completion(&done); - return 0; } -- 2.17.1
[PATCH AUTOSEL 4.4 14/33] USB: omap_udc: use devm_request_irq()
From: Aaro Koskinen [ Upstream commit 286afdde1640d8ea8916a0f05e811441fbbf4b9d ] The current code fails to release the third irq on the error path (observed by reading the code), and we get also multiple WARNs with failing gadget drivers due to duplicate IRQ releases. Fix by using devm_request_irq(). Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 37 +-- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 9b7d39484ed3..b25eac2dcaf8 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2886,8 +2886,8 @@ static int omap_udc_probe(struct platform_device *pdev) udc->clr_halt = UDC_RESET_EP; /* USB general purpose IRQ: ep0, state changes, dma, etc */ - status = request_irq(pdev->resource[1].start, omap_udc_irq, - 0, driver_name, udc); + status = devm_request_irq(&pdev->dev, pdev->resource[1].start, + omap_udc_irq, 0, driver_name, udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[1].start, status); @@ -2895,20 +2895,20 @@ static int omap_udc_probe(struct platform_device *pdev) } /* USB "non-iso" IRQ (PIO for all but ep0) */ - status = request_irq(pdev->resource[2].start, omap_udc_pio_irq, - 0, "omap_udc pio", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[2].start, + omap_udc_pio_irq, 0, "omap_udc pio", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[2].start, status); - goto cleanup2; + goto cleanup1; } #ifdef USE_ISO - status = request_irq(pdev->resource[3].start, omap_udc_iso_irq, - 0, "omap_udc iso", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[3].start, + omap_udc_iso_irq, 0, "omap_udc iso", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[3].start, status); - goto cleanup3; + goto cleanup1; } #endif if (cpu_is_omap16xx() || cpu_is_omap7xx()) { @@ -2921,22 +2921,11 @@ static int omap_udc_probe(struct platform_device *pdev) create_proc_file(); status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, omap_udc_release); - if (status) - goto cleanup4; - - return 0; + if (!status) + return 0; -cleanup4: remove_proc_file(); -#ifdef USE_ISO -cleanup3: - free_irq(pdev->resource[2].start, udc); -#endif - -cleanup2: - free_irq(pdev->resource[1].start, udc); - cleanup1: kfree(udc); udc = NULL; @@ -2980,12 +2969,6 @@ static int omap_udc_remove(struct platform_device *pdev) remove_proc_file(); -#ifdef USE_ISO - free_irq(pdev->resource[3].start, udc); -#endif - free_irq(pdev->resource[2].start, udc); - free_irq(pdev->resource[1].start, udc); - if (udc->dc_clk) { if (udc->clk_requested) omap_udc_enable_clock(0); -- 2.17.1
[PATCH AUTOSEL 3.18 12/26] USB: omap_udc: fix crashes on probe error and module removal
From: Aaro Koskinen [ Upstream commit 99f700366fcea1aa2fa3c49c99f371670c3c62f8 ] We currently crash if usb_add_gadget_udc_release() fails, since the udc->done is not initialized until in the remove function. Furthermore, on module removal the udc data is accessed although the release function is already triggered by usb_del_gadget_udc() early in the function. Fix by rewriting the release and remove functions, basically moving all the cleanup into the release function, and doing the completion only in the module removal case. The patch fixes omap_udc module probe with a failing gadged, and also allows the removal of omap_udc. Tested by running "modprobe omap_udc; modprobe -r omap_udc" in a loop. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 50 --- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index b1f64608c373..a66031df2447 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2591,9 +2591,22 @@ omap_ep_setup(char *name, u8 addr, u8 type, static void omap_udc_release(struct device *dev) { - complete(udc->done); + pullup_disable(udc); + if (!IS_ERR_OR_NULL(udc->transceiver)) { + usb_put_phy(udc->transceiver); + udc->transceiver = NULL; + } + omap_writew(0, UDC_SYSCON1); + remove_proc_file(); + if (udc->dc_clk) { + if (udc->clk_requested) + omap_udc_enable_clock(0); + clk_put(udc->hhc_clk); + clk_put(udc->dc_clk); + } + if (udc->done) + complete(udc->done); kfree(udc); - udc = NULL; } static int @@ -2898,12 +2911,8 @@ static int omap_udc_probe(struct platform_device *pdev) } create_proc_file(); - status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, - omap_udc_release); - if (!status) - return 0; - - remove_proc_file(); + return usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, + omap_udc_release); cleanup1: kfree(udc); @@ -2930,36 +2939,15 @@ static int omap_udc_remove(struct platform_device *pdev) { DECLARE_COMPLETION_ONSTACK(done); - if (!udc) - return -ENODEV; - - usb_del_gadget_udc(&udc->gadget); - if (udc->driver) - return -EBUSY; - udc->done = &done; - pullup_disable(udc); - if (!IS_ERR_OR_NULL(udc->transceiver)) { - usb_put_phy(udc->transceiver); - udc->transceiver = NULL; - } - omap_writew(0, UDC_SYSCON1); - - remove_proc_file(); + usb_del_gadget_udc(&udc->gadget); - if (udc->dc_clk) { - if (udc->clk_requested) - omap_udc_enable_clock(0); - clk_put(udc->hhc_clk); - clk_put(udc->dc_clk); - } + wait_for_completion(&done); release_mem_region(pdev->resource[0].start, pdev->resource[0].end - pdev->resource[0].start + 1); - wait_for_completion(&done); - return 0; } -- 2.17.1
[PATCH AUTOSEL 3.18 13/26] USB: omap_udc: fix omap_udc_start() on 15xx machines
From: Aaro Koskinen [ Upstream commit 6ca6695f576b8453fe68865e84d25946d63b10ad ] On OMAP 15xx machines there are no transceivers, and omap_udc_start() always fails as it forgot to adjust the default return value. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index a66031df2447..184d88e341af 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2045,7 +2045,7 @@ static inline int machine_without_vbus_sense(void) static int omap_udc_start(struct usb_gadget *g, struct usb_gadget_driver *driver) { - int status = -ENODEV; + int status; struct omap_ep *ep; unsigned long flags; @@ -2083,6 +2083,7 @@ static int omap_udc_start(struct usb_gadget *g, goto done; } } else { + status = 0; if (can_pullup(udc)) pullup_enable(udc); else -- 2.17.1
[PATCH AUTOSEL 3.18 11/26] USB: omap_udc: use devm_request_irq()
From: Aaro Koskinen [ Upstream commit 286afdde1640d8ea8916a0f05e811441fbbf4b9d ] The current code fails to release the third irq on the error path (observed by reading the code), and we get also multiple WARNs with failing gadget drivers due to duplicate IRQ releases. Fix by using devm_request_irq(). Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 37 +-- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index dcdfea46003b..b1f64608c373 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2865,8 +2865,8 @@ static int omap_udc_probe(struct platform_device *pdev) udc->clr_halt = UDC_RESET_EP; /* USB general purpose IRQ: ep0, state changes, dma, etc */ - status = request_irq(pdev->resource[1].start, omap_udc_irq, - 0, driver_name, udc); + status = devm_request_irq(&pdev->dev, pdev->resource[1].start, + omap_udc_irq, 0, driver_name, udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[1].start, status); @@ -2874,20 +2874,20 @@ static int omap_udc_probe(struct platform_device *pdev) } /* USB "non-iso" IRQ (PIO for all but ep0) */ - status = request_irq(pdev->resource[2].start, omap_udc_pio_irq, - 0, "omap_udc pio", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[2].start, + omap_udc_pio_irq, 0, "omap_udc pio", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[2].start, status); - goto cleanup2; + goto cleanup1; } #ifdef USE_ISO - status = request_irq(pdev->resource[3].start, omap_udc_iso_irq, - 0, "omap_udc iso", udc); + status = devm_request_irq(&pdev->dev, pdev->resource[3].start, + omap_udc_iso_irq, 0, "omap_udc iso", udc); if (status != 0) { ERR("can't get irq %d, err %d\n", (int) pdev->resource[3].start, status); - goto cleanup3; + goto cleanup1; } #endif if (cpu_is_omap16xx() || cpu_is_omap7xx()) { @@ -2900,22 +2900,11 @@ static int omap_udc_probe(struct platform_device *pdev) create_proc_file(); status = usb_add_gadget_udc_release(&pdev->dev, &udc->gadget, omap_udc_release); - if (status) - goto cleanup4; - - return 0; + if (!status) + return 0; -cleanup4: remove_proc_file(); -#ifdef USE_ISO -cleanup3: - free_irq(pdev->resource[2].start, udc); -#endif - -cleanup2: - free_irq(pdev->resource[1].start, udc); - cleanup1: kfree(udc); udc = NULL; @@ -2959,12 +2948,6 @@ static int omap_udc_remove(struct platform_device *pdev) remove_proc_file(); -#ifdef USE_ISO - free_irq(pdev->resource[3].start, udc); -#endif - free_irq(pdev->resource[2].start, udc); - free_irq(pdev->resource[1].start, udc); - if (udc->dc_clk) { if (udc->clk_requested) omap_udc_enable_clock(0); -- 2.17.1
[PATCH AUTOSEL 3.18 14/26] USB: omap_udc: fix USB gadget functionality on Palm Tungsten E
From: Aaro Koskinen [ Upstream commit 2c2322fbcab8102b8cadc09d66714700a2da42c2 ] On Palm TE nothing happens when you try to use gadget drivers and plug the USB cable. Fix by adding the board to the vbus sense quirk list. Signed-off-by: Aaro Koskinen Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/udc/omap_udc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c index 184d88e341af..1a457dd8c7f7 100644 --- a/drivers/usb/gadget/udc/omap_udc.c +++ b/drivers/usb/gadget/udc/omap_udc.c @@ -2037,6 +2037,7 @@ static inline int machine_without_vbus_sense(void) { return machine_is_omap_innovator() || machine_is_omap_osk() + || machine_is_omap_palmte() || machine_is_sx1() /* No known omap7xx boards with vbus sense */ || cpu_is_omap7xx(); -- 2.17.1
Re: [PATCH] USB: serial: console: fix reported terminal settings
On Wed, Dec 05, 2018 at 11:36:52AM +0200, Jarkko Nikula wrote: > On 12/4/18 6:31 PM, Johan Hovold wrote: > > On Tue, Dec 04, 2018 at 05:15:18PM +0100, Greg Kroah-Hartman wrote: > >> On Tue, Dec 04, 2018 at 05:00:36PM +0100, Johan Hovold wrote: > >>> The USB-serial console implementation has never reported the actual > >>> terminal settings used. Despite storing the corresponding cflags in its > >>> struct console, this was never honoured on later tty open() where the > >>> tty termios would be left initialised to the driver defaults. > >>> > >>> Unlike the serial console implementation, the USB-serial code calls > >>> subdriver open() already at console setup. While calling set_termios() > >>> before open() looks like it could work for some USB-serial drivers, > >>> others definitely do not expect this, so modelling this after serial > >>> core is going to be intrusive, if at all possible. > >>> This specifically fixes a regression that was triggered by a recent > >>> change adding software flow control to the pl2303 driver: a getty trying > >>> to disable flow control while leaving the baud rate unchanged would now > >>> also set the baud rate to the driver default (prior to the flow-control > >>> change this had been a noop). > >>> > >>> Fixes: 7041d9c3f01b ("USB: serial: pl2303: add support for tx xon/xoff > >>> flow control") > >>> Cc: stable# 4.18 > >>> Reported-by: Jarkko Nikula > >>> Cc: Florian Zumbiehl > >>> Signed-off-by: Johan Hovold > >>> --- > >>> drivers/tty/tty_io.c | 11 +-- > >>> drivers/usb/serial/console.c | 2 +- > >>> include/linux/tty.h | 1 + > >>> 3 files changed, 11 insertions(+), 3 deletions(-) > >> > >> Ah, messy :) > >> > >> Want me to take this through my tty tree? > > > > If you prefer. I was planning on including this in a USB-serial pull > > request for -rc6 since it fixes a user-reported regression, but perhaps > > taking this through your tty-linus branch (which already holds a console > > fix) is easier/faster. > > > > We should wait for Jarkko to confirm that this fixes the problem he > > reported first, though. > > > Great, this fixed the issue for both pl2303 based adapters I reported. > > Tested on top of 0072a0c14d5b ("Merge tag 'media/v4.20-4' of > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media") > > Tested-by: Jarkko Nikula Great, thanks for testing. Greg, I noticed I left out the part about subdrivers not expecting *write()* to be called before open() so I'll amend the commit message when applying and include this one in a pull-request tomorrow, if that's ok with you? Note that the changes to tty are trivial; I'm just renaming and exporting an existing helper. Thanks, Johan
Re: USB driver resets
Hi, On Tue, Dec 04, 2018 at 04:02:56PM +, Richard van der Hoff wrote: > Mathias, Mika, thanks very much for your help. > > On 04/12/2018 15:09, Mika Westerberg wrote: > > On Tue, Dec 04, 2018 at 04:45:33PM +0200, Mathias Nyman wrote: > > > > On 19/11/2018 15:27, Richard van der Hoff wrote: > > > > > I have a USB-3.1 dock, which I use for video (via displayport alt > > > > > mode) and power delivery, as well as keyboard, mouse, etc. > > > > > > > > > > From time to time, all connectivity with the dock drops out for a few > > > > > seconds, before resetting itself. I'm trying to pin down if this is a > > > > > problem with the usb drivers, pci drivers, laptop hardware, or the > > > > > dock. It looks to me from the dmesg output like it could be a problem > > > > > at the PCI layer, but I am only speculating really. > > > > > > > > > > Attached are the dmesg output during the problem, lsusb -v, and lspci > > > > > -v. They are all recorded with a 4.15.0 kernel; I've seen exactly the > > > > > same symptoms with a number of kernels between 4.8 and 4.15. > > > > > > > > > > > Mika (cc) would be the right person to contact about these kind of > > > thunderbolt related issues. > > > I'm sure he would appreciate logs with a 4.19 - 4.20-rcx kernel. > > I've attached the kernel log output from a 4.19 kernel. I've actually had > some other problems with a 4.19 kernel [1] so have gone back to the 4.15 > kernel for now, but please do let me know if there is anything specific I > should try. > > I've also re-attached the lsusb -v and lspci -v output (from a 4.15 kernel) > for your reference. > > > Right, those would help and also some knowledge about which system and > > dock we are dealing with :) > > Sorry. The system is a Dell XPS13 9350, with an Intel DSL6340 Thunderbolt 3 > bridge. > > The dock is a Plugable UD-CA1 [2]. > > > Also if you have dual boot or similar, do you experience similar issue > > in Windows side? > > I do have dual-boot, though I very rarely boot into Windows. Since the issue > is somewhat intermittent (for some reason it seems to happen particularly > when I'm in a video conference...) I wouldn't have a huge amount of > confidence of being able to reproduce it under Windows. > > Thanks again. > > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=201853 > [2] https://plugable.com/products/ud-ca1/ This seems to be normal USB-C dock (not TBT). In the logs it looks like at some point the USB link just goes down which makes the host side to hot-remove xHCI and the intermediate PCIe ports. The UD-CA1 dock site you linked above mentions connection issues with some systems so I'm suspecting it is an issue with the dock itself not with the kernel. It would be good if you could try to reproduce on Windows side as well, basically doing the same things you do in Linux (assuming it is possible) and see if it triggers.
Re: [PATCH 0/3] Small improvements to the appledisplay driver
On Tue, Dec 04, 2018 at 11:43:34PM +0100, alex.theis...@me.com wrote: > With the exception of the first patch I am not entirely sure if my changes are > correct and justified. This is my first contribution and I am equally > satisfied > if someone could point out why my changes are not correct. > > The added display is my own and works well with the driver. All looks good to me, thanks for the patches! greg k-h
Re: [PATCH v1] usb: dwc3: trace: Refactor nested switch to make compiler happy
On Wed, Dec 05, 2018 at 11:18:45AM +0200, Andy Shevchenko wrote: > On Wed, Dec 05, 2018 at 11:10:46AM +0200, Felipe Balbi wrote: > > Andy Shevchenko writes: > > > > > The missed break statement in the outer switch makes the code fall through > > > always and thus always same value will be printed. > > > > > > Besides that, compiler warns about missed fall through marker: > > > > > > drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’: > > > drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall > > > through [-Wimplicit-fallthrough=] > > > switch (pcm) { > > > ^~ > > easier to add "break" here, no? That would be the minimal fix. > > No. Then you would need to add same default to the inner switch. Ah, you meant that pcm would be never outside of the given cases. Yes, that's fine then, consider my patch as a bugreport. -- With Best Regards, Andy Shevchenko
[PATCH AUTOSEL 4.19 088/123] usb: gadget: u_ether: fix unsafe list iteration
From: Marek Szyprowski [ Upstream commit c9287fa657b3328b4549c0ab39ea7f197a3d6a50 ] list_for_each_entry_safe() is not safe for deleting entries from the list if the spin lock, which protects it, is released and reacquired during the list iteration. Fix this issue by replacing this construction with a simple check if list is empty and removing the first entry in each iteration. This is almost equivalent to a revert of the commit mentioned in the Fixes: tag. This patch fixes following issue: --->8--- Unable to handle kernel NULL pointer dereference at virtual address 0104 pgd = (ptrval) [0104] *pgd= Internal error: Oops: 817 [#1] PREEMPT SMP ARM Modules linked in: CPU: 1 PID: 84 Comm: kworker/1:1 Not tainted 4.20.0-rc2-next-20181114-9-g8266b35ec404 #1061 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: events eth_work PC is at rx_fill+0x60/0xac LR is at _raw_spin_lock_irqsave+0x50/0x5c pc : []lr : []psr: 8093 sp : ee7fbee8 ip : 0100 fp : r10: 006000c0 r9 : c10b0ab0 r8 : ee7eb5c0 r7 : ee7eb614 r6 : ee7eb5ec r5 : 00dc r4 : ee12ac00 r3 : ee12ac24 r2 : 0200 r1 : 6013 r0 : ee7eb5ec Flags: Nzcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 6d5dc04a DAC: 0051 Process kworker/1:1 (pid: 84, stack limit = 0x(ptrval)) Stack: (0xee7fbee8 to 0xee7fc000) ... [] (rx_fill) from [] (process_one_work+0x200/0x738) [] (process_one_work) from [] (worker_thread+0x2c/0x4c8) [] (worker_thread) from [] (kthread+0x128/0x164) [] (kthread) from [] (ret_from_fork+0x14/0x20) Exception stack(0xee7fbfb0 to 0xee7fbff8) ... ---[ end trace 64480bc835eba7d6 ]--- Fixes: fea14e68ff5e ("usb: gadget: u_ether: use better list accessors") Signed-off-by: Marek Szyprowski Signed-off-by: Felipe Balbi Signed-off-by: Sasha Levin --- drivers/usb/gadget/function/u_ether.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index 1000d864929c..0f026d445e31 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -401,12 +401,12 @@ static int alloc_requests(struct eth_dev *dev, struct gether *link, unsigned n) static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags) { struct usb_request *req; - struct usb_request *tmp; unsigned long flags; /* fill unused rxq slots with some skb */ spin_lock_irqsave(&dev->req_lock, flags); - list_for_each_entry_safe(req, tmp, &dev->rx_reqs, list) { + while (!list_empty(&dev->rx_reqs)) { + req = list_first_entry(&dev->rx_reqs, struct usb_request, list); list_del_init(&req->list); spin_unlock_irqrestore(&dev->req_lock, flags); @@ -1125,7 +1125,6 @@ void gether_disconnect(struct gether *link) { struct eth_dev *dev = link->ioport; struct usb_request *req; - struct usb_request *tmp; WARN_ON(!dev); if (!dev) @@ -1142,7 +1141,8 @@ void gether_disconnect(struct gether *link) */ usb_ep_disable(link->in_ep); spin_lock(&dev->req_lock); - list_for_each_entry_safe(req, tmp, &dev->tx_reqs, list) { + while (!list_empty(&dev->tx_reqs)) { + req = list_first_entry(&dev->tx_reqs, struct usb_request, list); list_del(&req->list); spin_unlock(&dev->req_lock); @@ -1154,7 +1154,8 @@ void gether_disconnect(struct gether *link) usb_ep_disable(link->out_ep); spin_lock(&dev->req_lock); - list_for_each_entry_safe(req, tmp, &dev->rx_reqs, list) { + while (!list_empty(&dev->rx_reqs)) { + req = list_first_entry(&dev->rx_reqs, struct usb_request, list); list_del(&req->list); spin_unlock(&dev->req_lock); -- 2.17.1
Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Flare
On Wed, Nov 28, 2018 at 06:19:31PM -0500, Kyle Williams wrote: > Description: Some USB device / host controller combinations seem to have > problems with Link Power management. In particular it is described that > the combination of a Logitech Flare and other powered devices such as > the Atrus device causes 'not enough bandwidth for new device state'error > > This patch creates a quirk entry for the Logitech Flare device > indicating LPM should remain disabled for the device. > > Signed-off-by: Kyle Williams > Cc: stable > --- > drivers/usb/core/quirks.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > index 49c44d89a394..e43c72cc98ee 100644 > --- a/drivers/usb/core/quirks.c > +++ b/drivers/usb/core/quirks.c > @@ -224,6 +224,9 @@ static const struct usb_device_id usb_quirk_list[] = { > /* Logitech Logitech Screen Share */ > { USB_DEVICE(0x046d, 0x086c), .driver_info = USB_QUIRK_NO_LPM }, > > + /* Logitech Flare */ > + { USB_DEVICE(0x046d, 0x0876), .driver_info = USB_QUIRK_NO_LPM }, > + > /* Logitech Rally Camera */ > { USB_DEVICE(0x046d, 0x0881), .driver_info = USB_QUIRK_NO_LPM }, > { USB_DEVICE(0x046d, 0x0888), .driver_info = USB_QUIRK_NO_LPM }, > -- > 2.18.1 Tabs are all converted to spaces, making this patch impossible to apply :(
Re: [PATCH] USB: quirks: add NO_LPM quirk for Logitech Rally Camera
On Thu, Nov 29, 2018 at 07:59:09AM -0500, Kyle Williams wrote: > Description: Some USB device / host controller combinations seem to have > problems with Link Power management. In particular it is described that > the combination of a Logitech Rally and Atrus device causes > 'not enough bandwidth for new device state'error > > This patch creates quirk entries for the Logitech Rally Camera > indicating LPM should remain disabled for the device. > > Signed-off-by: Kyle Williams > Reviewed-on: https://chromium-review.googlesource.com/1297535 > Commit-Ready: Kyle Williams > Tested-by: Kyle Williams > Reviewed-by: Julius Werner > --- > drivers/usb/core/quirks.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > index 2deac3a55957..49c44d89a394 100644 > --- a/drivers/usb/core/quirks.c > +++ b/drivers/usb/core/quirks.c > @@ -224,6 +224,11 @@ static const struct usb_device_id usb_quirk_list[] = { > /* Logitech Logitech Screen Share */ > { USB_DEVICE(0x046d, 0x086c), .driver_info = USB_QUIRK_NO_LPM }, > > + /* Logitech Rally Camera */ > + { USB_DEVICE(0x046d, 0x0881), .driver_info = USB_QUIRK_NO_LPM }, > + { USB_DEVICE(0x046d, 0x0888), .driver_info = USB_QUIRK_NO_LPM }, > + { USB_DEVICE(0x046d, 0x0889), .driver_info = USB_QUIRK_NO_LPM }, > + > /* Logitech Quickcam Fusion */ > { USB_DEVICE(0x046d, 0x08c1), .driver_info = USB_QUIRK_RESET_RESUME }, > This patch had all tabs stripped out of it, and was sent in html :(
Re: [PATCH v3 1/5] usb: split code locating ACPI companion into port and device
On Wed, Nov 21, 2018 at 03:50:16PM -0800, Rajat Jain wrote: > From: Dmitry Torokhov > > In preparation for handling embedded USB devices let's split > usb_acpi_find_companion() into usb_acpi_find_companion_for_device() and > usb_acpi_find_companion_for_port(). > > Signed-off-by: Dmitry Torokhov > Signed-off-by: Rajat Jain Acked-by: Greg Kroah-Hartman
Re: [PATCH v3 2/5] usb: assign ACPI companions for embedded USB devices
On Wed, Nov 21, 2018 at 03:50:17PM -0800, Rajat Jain wrote: > From: Dmitry Torokhov > > USB devices permanently connected to USB ports may be described in ACPI > tables and share ACPI devices with ports they are connected to. See [1] > for details. > > This will allow us to describe sideband resources for devices, such as, > for example, hard reset line for BT USB controllers. > > [1] > https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects#acpi-namespace-hierarchy-and-adr-for-embedded-usb-devices > > Signed-off-by: Dmitry Torokhov > Signed-off-by: Rajat Jain (changed how we get the > usb_port) Acked-by: Greg Kroah-Hartman
Re: [PATCH v2 1/3] staging: typec: fusb302: Rename fcs,extcon-name to linux,extcon-name
On Mon, Dec 03, 2018 at 09:23:44PM +0200, Andy Shevchenko wrote: > On Thu, Nov 15, 2018 at 04:53:03PM +0200, Heikki Krogerus wrote: > > On Thu, Nov 15, 2018 at 03:16:19PM +0200, Andy Shevchenko wrote: > > > Since we are going to use the same in Designware USB 3 driver, > > > rename the property to be consistent across the drivers. > > > > > > No functional change intended. > > > > > > Signed-off-by: Andy Shevchenko > > > Cc: Hans de Goede > > > Cc: Guenter Roeck > > > Acked-by: Hans de Goede > > > Acked-by: Guenter Roeck > > > > FWIW, the series: > > Reviewed-by: Heikki Krogerus > > Greg, can you pick up this patch (first in the series, since dwc3 went > through Felipe's tree)? Sorry for the delay, now queued up. greg k-h
Re: [PATCH v1] usb: dwc3: trace: Refactor nested switch to make compiler happy
On Wed, Dec 05, 2018 at 11:10:46AM +0200, Felipe Balbi wrote: > Andy Shevchenko writes: > > > The missed break statement in the outer switch makes the code fall through > > always and thus always same value will be printed. > > > > Besides that, compiler warns about missed fall through marker: > > > > drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’: > > drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through > > [-Wimplicit-fallthrough=] > > switch (pcm) { > > ^~ > > > > Refactor nested switch statements to work correctly without > > compilation warnings. > > > > Fixes: fa8d965d736b ("usb: dwc3: trace: pretty print high-bandwidth > > transfers too") > > Cc: Felipe Balbi > > Signed-off-by: Andy Shevchenko > > --- > > drivers/usb/dwc3/trace.h | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h > > index f22714cce070..8e1625a6c19f 100644 > > --- a/drivers/usb/dwc3/trace.h > > +++ b/drivers/usb/dwc3/trace.h > > @@ -238,7 +238,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb, > > ), > > TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x > > (%c%c%c%c:%c%c:%s)", > > __get_str(name), __entry->trb, __entry->bph, __entry->bpl, > > - ({char *s; > > + ({ char *s = ""; > > int pcm = ((__entry->size >> 24) & 3) + 1; > > switch (__entry->type) { > > case USB_ENDPOINT_XFER_INT: > > @@ -254,8 +254,6 @@ DECLARE_EVENT_CLASS(dwc3_log_trb, > > s = "3x "; > > break; > > } > > easier to add "break" here, no? That would be the minimal fix. No. Then you would need to add same default to the inner switch. -- With Best Regards, Andy Shevchenko
Re: [PATCH v3 2/2] usb: dwc3: gadget: Report isoc transfer frame number
Hi, Thinh Nguyen writes: > Implement the new frame_number API to report the isochronous interval > frame number. This patch checks and reports the interval in which the > isoc transfer was transmitted or received via the Isoc-First TRB SOF > number field. > > Signed-off-by: Thinh Nguyen > --- > Change in v3: > - Implement the change with the redefined frame_number meaning > Change in v2: > - Capture frame number at request cleanup > > drivers/usb/dwc3/core.h | 1 + > drivers/usb/dwc3/gadget.c | 13 + > 2 files changed, 14 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index ed0359d1216d..2c9f7a93147a 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -777,6 +777,7 @@ enum dwc3_link_state { > #define DWC3_TRB_CTRL_ISP_IMIBIT(10) > #define DWC3_TRB_CTRL_IOCBIT(11) > #define DWC3_TRB_CTRL_SID_SOFN(n)(((n) & 0x) << 14) > +#define DWC3_TRB_CTRL_GET_SID_SOFN(n)(((n) & (0x << 14)) >> 14) > > #define DWC3_TRBCTL_TYPE(n) ((n) & (0x3f << 4)) > #define DWC3_TRBCTL_NORMAL DWC3_TRB_CTRL_TRBCTL(1) > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 679c12e14522..2de563124fc1 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2254,6 +2254,19 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct > dwc3_ep *dep, > if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO)) > trb->ctrl &= ~DWC3_TRB_CTRL_HWO; > > + /* > + * For isochronous transfers, the first TRB in a service interval must > + * have the Isoc-First type. Track and report its interval frame number. > + */ > + if (usb_endpoint_xfer_isoc(dep->endpoint.desc) && > + (trb->ctrl & DWC3_TRBCTL_ISOCHRONOUS_FIRST)) { > + unsigned int frame_number; > + > + frame_number = DWC3_TRB_CTRL_GET_SID_SOFN(trb->ctrl); > + frame_number &= ~(dep->interval - 1); > + req->request.frame_number = frame_number; > + } could you refresh my memory on how this is going to be used? -- balbi signature.asc Description: PGP signature
Re: [PATCH v2] usb: dwc2: host: use hrtimer for NAK retries
Hi, Doug Anderson writes: > >> Signed-off-by: Terin Stock >> >> --- >> >> drivers/usb/dwc2/hcd.h |2 +- >> >> drivers/usb/dwc2/hcd_queue.c | 19 --- >> >> 2 files changed, 13 insertions(+), 8 deletions(-) >> > >> > Reviewed-by: Douglas Anderson >> > Cc: sta...@vger.kernel.org >> > >> Acked-by: Minas Harutyunyan > > It looks like Terin missed CCing you on the original patch. I assume > you are still picking up dwc2 changes like this? If so, should he > resend the patch targeting you (and including Minas and my tags) or do > you want to pick it up from one of the mailing lists he CCed? I'm picking it up, thanks -- balbi signature.asc Description: PGP signature
Re: [PATCH v1] usb: dwc3: trace: Refactor nested switch to make compiler happy
Andy Shevchenko writes: > The missed break statement in the outer switch makes the code fall through > always and thus always same value will be printed. > > Besides that, compiler warns about missed fall through marker: > > drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’: > drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through > [-Wimplicit-fallthrough=] > switch (pcm) { > ^~ > > Refactor nested switch statements to work correctly without > compilation warnings. > > Fixes: fa8d965d736b ("usb: dwc3: trace: pretty print high-bandwidth transfers > too") > Cc: Felipe Balbi > Signed-off-by: Andy Shevchenko > --- > drivers/usb/dwc3/trace.h | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h > index f22714cce070..8e1625a6c19f 100644 > --- a/drivers/usb/dwc3/trace.h > +++ b/drivers/usb/dwc3/trace.h > @@ -238,7 +238,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb, > ), > TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x > (%c%c%c%c:%c%c:%s)", > __get_str(name), __entry->trb, __entry->bph, __entry->bpl, > - ({char *s; > + ({ char *s = ""; > int pcm = ((__entry->size >> 24) & 3) + 1; > switch (__entry->type) { > case USB_ENDPOINT_XFER_INT: > @@ -254,8 +254,6 @@ DECLARE_EVENT_CLASS(dwc3_log_trb, > s = "3x "; > break; > } easier to add "break" here, no? That would be the minimal fix. -- balbi signature.asc Description: PGP signature
Re: [PATCH v7 09/10] usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl fields
Hi, Anurag Kumar Vulisha writes: > The present code in dwc3_gadget_ep_reclaim_completed_trb() will check > for IOC/LST bit in the event->status and returns if IOC/LST bit is > set. This logic doesn't work if multiple TRBs are queued per > request and the IOC/LST bit is set on the last TRB of that request. > Consider an example where a queued request has multiple queued TRBs > and IOC/LST bit is set only for the last TRB. In this case, the Core > generates XferComplete/XferInProgress events only for the last TRB > (since IOC/LST are set only for the last TRB). As per the logic in > dwc3_gadget_ep_reclaim_completed_trb() event->status is checked for > IOC/LST bit and returns on the first TRB. This makes the remaining > TRBs left unhandled. > To aviod this, changed the code to check for IOC/LST bits in both > event->status & TRB->ctrl. This patch does the same. > > Signed-off-by: Anurag Kumar Vulisha > Reviewed-by: Thinh Nguyen > Tested-by: Tejas Joglekar > --- > Changes in v7: > 1. None > > Changes in v6: > 1. None > > Changes in v5: > 1. None > > Changes in v4: > 1. None > > Changes in v3: > 1. None > > Changes in v2: > 1. None > --- > drivers/usb/dwc3/gadget.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 9ddc9fd..216179e 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2286,7 +2286,12 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct > dwc3_ep *dep, > if (event->status & DEPEVT_STATUS_SHORT && !chain) > return 1; > > - if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST)) > + if ((event->status & DEPEVT_STATUS_IOC) && > + (trb->ctrl & DWC3_TRB_CTRL_IOC)) > + return 1; this shouldn't be necessary. According to databook, event->status contains the bits from the completed TRB. Which means that event->status & IOC will always be equal to trb->ctrl & IOC. Can you further describe the situation here? Perhaps share tracepoints exposing the problem? -- balbi signature.asc Description: PGP signature
Re: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
On Wed, Dec 5, 2018 at 4:55 PM Alan Douglas wrote: > > Hi Peter, > > On 05 December 2018 07:20, Pawel Laszczak wrote: > > Hi, > > > > >> >> > > >> >> Patch adds core.c and core.h file that implements initialization > > >> >> of platform driver and adds function responsible for selecting, > > >> >> switching and running appropriate Device/Host mode. > > >> >> > > >> >> Signed-off-by: Pawel Laszczak > > >> >> --- > > >> >> drivers/usb/cdns3/Makefile | 2 + > > >> >> drivers/usb/cdns3/core.c | 413 > > >> >> + > > >> >> drivers/usb/cdns3/core.h | 100 + > > >> >> 3 files changed, 515 insertions(+) > > >> >> create mode 100644 drivers/usb/cdns3/core.c > > >> >> create mode 100644 drivers/usb/cdns3/core.h > > >> >> > > >> >> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile > > >> >> index dcdd62003c6a..02d25b23c5d3 100644 > > >> >> --- a/drivers/usb/cdns3/Makefile > > >> >> +++ b/drivers/usb/cdns3/Makefile > > >> >> @@ -1,3 +1,5 @@ > > >> >> +obj-$(CONFIG_USB_CDNS3)+= cdns3.o > > >> >> obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci.o > > >> >> > > >> >> +cdns3-y:= core.o > > >> >> cdns3-pci-y:= cdns3-pci-wrap.o > > >> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > > >> >> new file mode 100644 > > >> >> index ..f9055d4da67f > > >> >> --- /dev/null > > >> >> +++ b/drivers/usb/cdns3/core.c > > >> >> @@ -0,0 +1,413 @@ > > >> >> +// SPDX-License-Identifier: GPL-2.0 > > >> >> +/* > > >> >> + * Cadence USBSS DRD Driver. > > >> >> + * > > >> >> + * Copyright (C) 2018 Cadence. > > >> >> + * > > >> > > > >> >Please add NXP copyright too. > > >> > > >> Ok, I don't know why I omitted this. > > >> I know that you are the main author of this file > > >> Sorry for that. > > >> > > >> One additional question. What year I should add in Copyright for NXP?. > > >> The original year 2017 or I should modified all to 2018. > > >> > > >Please use below copyright, thanks. > > > > > >Copyright 2017-2018 NXP > > > > I add this in all files modified or created by you. > > > > > > > > > > > >> >> + mutex_init(&cdns->mutex); > > >> >> + > > >> >> + cdns->phy = devm_phy_get(dev, "cdns3,usbphy"); > > >> >> + if (IS_ERR(cdns->phy)) { > > >> >> + dev_info(dev, "no generic phy found\n"); > > >> >> + cdns->phy = NULL; > > >> >> + /* > > >> >> +* fall through here! > > >> >> +* if no generic phy found, phy init > > >> >> +* should be done under boot! > > >> >> +*/ > > >> > > > >> >If the phy driver is defer-probed, it will be here, it is not an error. > > >> >I think you could have a generic phy driver or usb generic phy driver > > >> >(drivers/usb/phy/phy-generic.c) even you don't need any operations for > > >> >PHY. It will be easy for other platforms. > > >> > > >> Yes, Roger ask me to modify this fragment. In next version it will look > > >> like: > > >> cdns->phy = devm_phy_get(dev, "cdns3,usbphy"); > > >> if (IS_ERR(cdns->phy)) { > > >> ret = PTR_ERR(cdns->phy); > > >> if (ret == -ENOSYS || ret == -ENODEV) { > > >> cdns->phy = NULL; > > >> } else if (ret == -EPROBE_DEFER) { > > >> return ret; > > >> } else { > > >> dev_err(dev, "no phy found\n"); > > >> goto err0; > > >> } > > >> } > > >> > > >> phy_init(cdns->phy); > > >> > > >> We are going to use phy driver. I don't know if it correct. > > >> I don't have experience in this filed. > > >> We need phy initialization but I don't have testing platform now. > > >> In most usb drivers I see that there are used usb phy driverd instead > > >> phy dirverd. > > >> > > > > > >At my CDNS3 platform, there are some USB PHY initialization for register > > >setting > > >and clock enable. You could add generic PHY driver under: > > >drivers/phy/cadence/. > > > > > >Above PHY initialization code is OK for me. > > > > It will be added as separate driver. > > I think that Allan Douglas working on it. > > I ask him to add you to -cc in patch for phy. > Patch series for the cadence Sierra generic PHY driver can be found here: > https://lore.kernel.org/patchwork/cover/1011486/ > > It can also be found in Kishon's linux-phy git tree at > git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git > in the 'next' branch. > > It will be great if you are able to take a look at it. I will test it, and submit some changes if needed, thanks. Peter
Re: [PATCH v7 05/10] usb: dwc3: make controller clear transfer resources after complete
Hi, Anurag Kumar Vulisha writes: > @@ -2487,6 +2497,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, > } > > switch (event->endpoint_event) { > + case DWC3_DEPEVT_XFERCOMPLETE: > + if (!dep->stream_capable) > + break; > + dep->flags &= ~DWC3_EP_TRANSFER_STARTED; > + /* Fall Through */ instead, let's add a proper handler for this: dwc3_gadget_endpoint_transfer_complete(dep, event); That handler should be "self-sufficient". IOW, we shouldn't have this fall through here. This means that the other patch where you modify dwc3_gadget_transfer_in_progress() shouldn't be necessary, since that event shouldn't run for stream capable endpoints. While rewriting the patches, please rebase on my testing/next as I have applied a few of the patches in this series. -- balbi signature.asc Description: PGP signature
Re: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
> > > > On 04/12/18 10:50, Peter Chen wrote: > >>> + * Cadence USBSS DRD Driver. > >>> + * > >>> + * Copyright (C) 2018 Cadence. > >>> + * > >>> + * Author: Peter Chen > >>> + * Pawel Laszczak > >>> + */ > >>> + > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> + > >>> +#include "gadget.h" > >>> +#include "core.h" > >>> + > >>> +static inline struct cdns3_role_driver > >>> *cdns3_get_current_role_driver(struct cdns3 *cdns) > >>> +{ > >>> + WARN_ON(cdns->role >= CDNS3_ROLE_END || !cdns->roles[cdns->role]); > >>> + return cdns->roles[cdns->role]; > >>> +} > >>> + > >>> +static inline int cdns3_role_start(struct cdns3 *cdns, enum cdns3_roles > >>> role) > >>> +{ > >>> + int ret; > >>> + > >>> + if (role >= CDNS3_ROLE_END) > >> > >> WARN_ON()? > >> > >>> + return 0; > >>> + > >>> + if (!cdns->roles[role]) > >>> + return -ENXIO; > >>> + > >>> + mutex_lock(&cdns->mutex); > >>> + cdns->role = role; > >>> + ret = cdns->roles[role]->start(cdns); > >>> + mutex_unlock(&cdns->mutex); > >>> + return ret; > >>> +} > >>> + > >>> +static inline void cdns3_role_stop(struct cdns3 *cdns) > >>> +{ > >>> + enum cdns3_roles role = cdns->role; > >>> + > >>> + if (role == CDNS3_ROLE_END) > >> > >> WARN_ON(role >= CNDS3_ROLE_END) ? > >> > >>> + return; > >>> + > >>> + mutex_lock(&cdns->mutex); > >>> + cdns->roles[role]->stop(cdns); > >>> + cdns->role = CDNS3_ROLE_END; > >> > >> Why change the role here? You are just stopping the role not changing it. > >> I think cdns->role should remain unchanged, so we can call > >> cdns3_role_start() > >> if required without error. > >> > > > > The current version of this IP has some issues to detect vbus status > > correctly, > > we have to force vbus status accordingly, so we need a status to indicate > > vbus disconnection, and add some code to let controller know vbus > > removal, in that case, the controller's state machine can be correct. > > So, we increase one role 'CDNS3_ROLE_END' to for this purpose. > > > > CDNS3_ROLE_GADGET: gadget mode and VBUS on > > CDNS3_ROLE_HOST: host mode and VBUS on > > CDNS3_ROLE_END: VBUS off, eg either host or device cable on the port. > > > > So, we may start role from CDNS3_ROLE_END at probe when nothing is > > connected, > > and need to set role as CDNS3_ROLE_END at ->stop for further handling at > > role switch routine. > > OK. but still this (changing to ROLE_END) must be moved to the role switch > routine > and the explanation you just mentioned must be added there. > If we use host part as separate driver, something may need to change. The ROLE_END may need to add at role switch routine as your said. > > > >>> + mutex_unlock(&cdns->mutex); > >>> +} > >>> + > >>> +static enum cdns3_roles cdns3_get_role(struct cdns3 *cdns) > >>> +{ > >>> + if (cdns->roles[CDNS3_ROLE_HOST] && cdns->roles[CDNS3_ROLE_GADGET]) > >>> { > >>> + //TODO: implements selecting device/host mode > >>> + return CDNS3_ROLE_HOST; > >>> + } > >>> + return cdns->roles[CDNS3_ROLE_HOST] > >>> + ? CDNS3_ROLE_HOST > >>> + : CDNS3_ROLE_GADGET; > >> > >> Why not just > >> return cdns->role; > >> > >> I'm wondering if we really need this function. > > > > cdns->role gets from cdns3_get_role, and this API tells role at the runtime. > > If both roles are supported, the role is decided by external > > conditions, eg, vbus/id > > or external connector. If only single role is supported, only one role > > structure > > is allocated, cdns->roles[CDNS3_ROLE_HOST] or cdns->roles[CDNS3_ROLE_GADGET] > > > > How about adding this description in function documentation. > Sure, but may need to redesign it due to host part as a standalone driver. > >> > >> Then you can move cdns3_role_start() as well to core_init_role(). > >> > > > > Usually, we request irq after hardware initialization has finished, if not, > > there may unexpected interrupt. > > Doesn't kernel warn if interrupt happens and there is no handler? > To avoid that I was suggesting to request_irq first. > The interrupt will not happen (related bit at GIC is masked) until we call request_irq. And we don't want to handle interrupts until hardware initialization, don't we? Peter
RE: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.
Hi Peter, On 05 December 2018 07:20, Pawel Laszczak wrote: > Hi, > > >> >> > >> >> Patch adds core.c and core.h file that implements initialization > >> >> of platform driver and adds function responsible for selecting, > >> >> switching and running appropriate Device/Host mode. > >> >> > >> >> Signed-off-by: Pawel Laszczak > >> >> --- > >> >> drivers/usb/cdns3/Makefile | 2 + > >> >> drivers/usb/cdns3/core.c | 413 + > >> >> drivers/usb/cdns3/core.h | 100 + > >> >> 3 files changed, 515 insertions(+) > >> >> create mode 100644 drivers/usb/cdns3/core.c > >> >> create mode 100644 drivers/usb/cdns3/core.h > >> >> > >> >> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile > >> >> index dcdd62003c6a..02d25b23c5d3 100644 > >> >> --- a/drivers/usb/cdns3/Makefile > >> >> +++ b/drivers/usb/cdns3/Makefile > >> >> @@ -1,3 +1,5 @@ > >> >> +obj-$(CONFIG_USB_CDNS3)+= cdns3.o > >> >> obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci.o > >> >> > >> >> +cdns3-y:= core.o > >> >> cdns3-pci-y:= cdns3-pci-wrap.o > >> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > >> >> new file mode 100644 > >> >> index ..f9055d4da67f > >> >> --- /dev/null > >> >> +++ b/drivers/usb/cdns3/core.c > >> >> @@ -0,0 +1,413 @@ > >> >> +// SPDX-License-Identifier: GPL-2.0 > >> >> +/* > >> >> + * Cadence USBSS DRD Driver. > >> >> + * > >> >> + * Copyright (C) 2018 Cadence. > >> >> + * > >> > > >> >Please add NXP copyright too. > >> > >> Ok, I don't know why I omitted this. > >> I know that you are the main author of this file > >> Sorry for that. > >> > >> One additional question. What year I should add in Copyright for NXP?. > >> The original year 2017 or I should modified all to 2018. > >> > >Please use below copyright, thanks. > > > >Copyright 2017-2018 NXP > > I add this in all files modified or created by you. > > > > > > > >> >> + mutex_init(&cdns->mutex); > >> >> + > >> >> + cdns->phy = devm_phy_get(dev, "cdns3,usbphy"); > >> >> + if (IS_ERR(cdns->phy)) { > >> >> + dev_info(dev, "no generic phy found\n"); > >> >> + cdns->phy = NULL; > >> >> + /* > >> >> +* fall through here! > >> >> +* if no generic phy found, phy init > >> >> +* should be done under boot! > >> >> +*/ > >> > > >> >If the phy driver is defer-probed, it will be here, it is not an error. > >> >I think you could have a generic phy driver or usb generic phy driver > >> >(drivers/usb/phy/phy-generic.c) even you don't need any operations for > >> >PHY. It will be easy for other platforms. > >> > >> Yes, Roger ask me to modify this fragment. In next version it will look > >> like: > >> cdns->phy = devm_phy_get(dev, "cdns3,usbphy"); > >> if (IS_ERR(cdns->phy)) { > >> ret = PTR_ERR(cdns->phy); > >> if (ret == -ENOSYS || ret == -ENODEV) { > >> cdns->phy = NULL; > >> } else if (ret == -EPROBE_DEFER) { > >> return ret; > >> } else { > >> dev_err(dev, "no phy found\n"); > >> goto err0; > >> } > >> } > >> > >> phy_init(cdns->phy); > >> > >> We are going to use phy driver. I don't know if it correct. > >> I don't have experience in this filed. > >> We need phy initialization but I don't have testing platform now. > >> In most usb drivers I see that there are used usb phy driverd instead phy > >> dirverd. > >> > > > >At my CDNS3 platform, there are some USB PHY initialization for register > >setting > >and clock enable. You could add generic PHY driver under: > >drivers/phy/cadence/. > > > >Above PHY initialization code is OK for me. > > It will be added as separate driver. > I think that Allan Douglas working on it. > I ask him to add you to -cc in patch for phy. Patch series for the cadence Sierra generic PHY driver can be found here: https://lore.kernel.org/patchwork/cover/1011486/ It can also be found in Kishon's linux-phy git tree at git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git in the 'next' branch. It will be great if you are able to take a look at it. Regards, Alan
Re: [PATCH v1 10/12] hikey960: Support usb functionality of Hikey960
On Wed, Dec 5, 2018 at 3:57 AM Chen Yu wrote: > On 2018/12/5 1:47, Andy Shevchenko wrote: > > On Tue, Dec 4, 2018 at 3:40 AM Chen Yu wrote: > >> On 2018/12/3 17:23, Andy Shevchenko wrote: > >>> On Mon, Dec 3, 2018 at 5:47 AM Yu Chen wrote: > > > + .of_match_table = of_match_ptr(id_table_hisi_hikey_usb), > >>> > >>> Does it compiles for non-OF case? Why macro is in use? > > > >> OK. I will add the "CONFIG_OF". > > Why? Is this driver supposed to work without DT? > > > No. This driver should depend on OF. > Can I solve this by add "dpends on OF" in Kconfig? Yes and don't forget to drop unneeded macro(s) like of_match_ptr() above. -- With Best Regards, Andy Shevchenko
Re: [RFC PATCH v2 06/15] usb:cdns3: Adds Host support
> > --- a/drivers/usb/cdns3/Makefile > > +++ b/drivers/usb/cdns3/Makefile > > @@ -2,4 +2,5 @@ obj-$(CONFIG_USB_CDNS3) += cdns3.o > > obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci.o > > > > cdns3-y := core.o drd.o > > +cdns3-$(CONFIG_USB_CDNS3_HOST) += host.o > > cdns3-pci-y := cdns3-pci-wrap.o > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > > index dbee4325da7f..4cb820be9ff3 100644 > > --- a/drivers/usb/cdns3/core.c > > +++ b/drivers/usb/cdns3/core.c > > @@ -17,6 +17,7 @@ > > > > #include "gadget.h" > > #include "core.h" > > +#include "host-export.h" > > #include "drd.h" > > > > static inline struct cdns3_role_driver > > *cdns3_get_current_role_driver(struct cdns3 *cdns) > > @@ -98,7 +99,8 @@ static int cdns3_core_init_role(struct cdns3 *cdns) > > } > > > > if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) { > > - //TODO: implements host initialization > > + if (cdns3_host_init(cdns)) > > + dev_info(dev, "doesn't support host\n"); > > dev_err() > > And you need to error out with error code. There are two cases for it: - There is no cdsn3_host_init implementation due to kernel configuration, - The error happens at cdsn3_host_init We may need to handle these two different cases. > > + struct usb_hcd *hcd; > > + > > + if (dev) > > + hcd = dev_get_drvdata(dev); > > + else > > + return IRQ_NONE; > > + > > + if (hcd) > > + return usb_hcd_irq(cdns->irq, hcd); > > + else > > + return IRQ_NONE; > > Why can't you just reuse the xhci-platform driver and let it manage the IRQ? > Since it is a shared IRQ, different drivers can request the same IRQ and > return IRQ_NONE > if the IRQ wasn't from their device. > When I began to write this code, there are not so many quirks can be used at xhci-plat.c, it seems we can re-use this common code at cdns3 driver now, I will try to integrate it. Thanks for your suggestion. Peter > > +} > > + > > +static void cdns3_host_release(struct device *dev) > > +{ > > + struct cdns3_host *host = container_of(dev, struct cdns3_host, dev); > > + > > + kfree(host); > > +} > > + > > +static int cdns3_host_start(struct cdns3 *cdns) > > +{ > > + struct cdns3_host *host; > > + struct device *dev; > > + struct device *sysdev; > > + struct xhci_hcd *xhci; > > + int ret; > > + > > + host = kzalloc(sizeof(*host), GFP_KERNEL); > > + if (!host) > > + return -ENOMEM; > > + > > + dev = &host->dev; > > + dev->release = cdns3_host_release; > > + dev->parent = cdns->dev; > > + dev_set_name(dev, "xhci-cdns3"); > > + cdns->host_dev = dev; > > + ret = device_register(dev); > > + if (ret) > > + goto err1; > > + > > + sysdev = cdns->dev; > > + /* Try to set 64-bit DMA first */ > > + if (WARN_ON(!sysdev->dma_mask)) > > + /* Platform did not initialize dma_mask */ > > + ret = dma_coerce_mask_and_coherent(sysdev, > > +DMA_BIT_MASK(64)); > > + else > > + ret = dma_set_mask_and_coherent(sysdev, DMA_BIT_MASK(64)); > > + > > + /* If setting 64-bit DMA mask fails, fall back to 32-bit DMA mask */ > > + if (ret) { > > + ret = dma_set_mask_and_coherent(sysdev, DMA_BIT_MASK(32)); > > + if (ret) > > + return ret; > > + } > > + > > + pm_runtime_set_active(dev); > > + pm_runtime_no_callbacks(dev); > > + pm_runtime_enable(dev); > > + host->hcd = __usb_create_hcd(&xhci_cdns3_hc_driver, sysdev, dev, > > + dev_name(dev), NULL); > > + if (!host->hcd) { > > + ret = -ENOMEM; > > + goto err2; > > + } > > + > > + host->hcd->regs = cdns->xhci_regs; > > + host->hcd->rsrc_start = cdns->xhci_res->start; > > + host->hcd->rsrc_len = resource_size(cdns->xhci_res); > > + > > + device_wakeup_enable(host->hcd->self.controller); > > + xhci = hcd_to_xhci(host->hcd); > > + > > + xhci->main_hcd = host->hcd; > > + xhci->shared_hcd = __usb_create_hcd(&xhci_cdns3_hc_driver, sysdev, > > dev, > > + dev_name(dev), host->hcd); > > + if (!xhci->shared_hcd) { > > + ret = -ENOMEM; > > + goto err3; > > + } > > + > > + host->hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); > > + xhci->shared_hcd->tpl_support = host->hcd->tpl_support; > > + ret = usb_add_hcd(host->hcd, 0, IRQF_SHARED); > > + if (ret) > > + goto err4; > > + > > + ret = usb_add_hcd(xhci->shared_hcd, 0, IRQF_SHARED); > > + if (ret) > > + goto err5; > > + > > + device_set_wakeup_capable(dev, true); > > All this is being done by the xhci-plat.c > > You
Re: [PATCH] dma: cppi41: delete channel from pending list when stop channel
On 28-11-18, 13:15, Peter Ujfalusi wrote: > > > On 12/11/2018 17.40, Bin Liu wrote: > > Can you fix up the subject line to: > dmaengine: ti: cppi4: delete channel from pending list when stop channel > > > The driver defines three states for a cppi channel. > > - idle: .chan_busy == 0 && not in .pending list > > - pending: .chan_busy == 0 && in .pending list > > - busy: .chan_busy == 1 && not in .pending list > > > > There are cases in which the cppi channel could be in the pending state > > when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend() > > is called. > > > > cppi41_stop_chan() has a bug for these cases to set channels to idle state. > > It only checks the .chan_busy flag, but not the .pending list, then later > > when cppi41_runtime_resume() is called the channels in .pending list will > > be transitioned to busy state. > > > > Removing channels from the .pending list solves the problem. > > So, let me see if I understand this correctly: > - client issued a transfer _after_ the cppi4 driver is suspended > - cppi41_dma_issue_pending() will place it to pending list and will not > start the transfer right away as cdd->is_suspended is true. > - on resume the cppi4 will pick up the pending transfers from the > pending list > > This is so far a sane thing to do. > > If I guess right, then after the issue_pending the client driver will > call terminate_all, presumably from it's suspend callback? > > As per the purpose of terminate_all we should terminated all future > transfers on the channel, so clearing the pending list is the correct > thing to do. > > With the fixed subject: > Reviewed-by: Peter Ujfalusi Thanks Peter, Applied after fixing the title, thanks -- ~Vinod
Re: [PATCH v3 4/4] doc: usb: ci-hdrc-usb2: Add pinctrl properties for HSIC pin groups
On 05.12.18 08:45, Schrempf Frieder wrote: > Hi Fabio, > > On 04.12.18 21:01, Fabio Estevam wrote: >> Hi Frieder, >> >> On Tue, Dec 4, 2018 at 12:31 PM Schrempf Frieder >> wrote: >> >>> There are many other optional properties for this driver and a lot of >>> them are not in the given example. Maybe we should just keep the >>> pinctrls for HSIC-mode out of the example, too? >> >> I am just trying to make life easier for those who want to use HSIC >> support with chipidea. >> >> Can we just add a real dts snippet example of your board into the >> binding document? > > Sure, here is what I have in my dts: > > &usbh2 { > pinctrl-names = "idle", "active"; > pinctrl-0 = <&pinctrl_usbh2_idle>; > pinctrl-1 = <&pinctrl_usbh2_active>; > status = "okay"; > #address-cells = <1>; > #size-cells = <0>; > > usbnet: smsc@1 { > compatible = "usb424,9730"; > reg = <1>; > }; > }; Or merged with the settings from imx6qdl.dtsi this will look like below. Maybe this is better as it is the complete node without depending on imx6qdl.dtsi: usbh2: usb@2184400 { compatible = "fsl,imx6q-usb", "fsl,imx27-usb"; reg = <0x02184400 0x200>; interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6QDL_CLK_USBOH3>; fsl,usbphy = <&usbphynop1>; fsl,usbmisc = <&usbmisc 2>; phy_type = "hsic"; dr_mode = "host"; ahb-burst-config = <0x0>; tx-burst-size-dword = <0x10>; rx-burst-size-dword = <0x10>; pinctrl-names = "idle", "active"; pinctrl-0 = <&pinctrl_usbh2_idle>; pinctrl-1 = <&pinctrl_usbh2_active>; #address-cells = <1>; #size-cells = <0>; usbnet: smsc@1 { compatible = "usb424,9730"; reg = <1>; }; }; > > @Peter: Can you add this as a second example to the binding documentation? > > Thanks, > Frieder >