Re: Microchip USB Hub Driver Harmonization
On Thu, May 18, 2017 at 7:33 AM, Richard Leitner wrote: >>> 2. What would be a good prefix for common headers/functions/macros/etc.? >>> I thought of "mcusbhub"... Would that be OK? Or are there any >>> conventions/better proposals on that? >> >> >> If you are going to develop one driver for entire family, then you could >> even choose just one name. Let's say the most generic. >> >> I don't quite understand the meaning behind "harmonizing drivers". > > > I'm currently evaluating if it is feasible to create one common driver for > all USB hubs. Due to the fact there are currently only 3 hub types > implemented mainline (the Microchip homepage lists 36 USB Hub products [1]) > it involves quite a lot data-sheet reading ;-) > > As a first step (and also the final one if implementing a common driver is > not feasible) I thought of creating a common header/source file which > implements all "re-useable" functions/macros/etc. Would that also be an > accepted solution? I understand. It is fine with me. You could make common part in few ways, e.g.: 1. prepare re-usable functions etc (as you said), 2. create one driver core (with init/probe/remove/exit etc) which delegates most of the body to specific handlers around. The benefit of the second approach is that one sees immediately that there is one driver for entire family. >>> 3. Currently only usb3503 supports "platform data". Is this still needed >>> or may it be removed? >> >> >> I think it is still used, e.g. by: >> arch/arm/boot/dts/exynos5250-spring.dts > > > Please correct me if I'm wrong, but isn't that DT only? > > The reason why I'm asking if it may be removed is because the only file > including "linux/platform_data/usb3503.h" is the usb3503.c driver itself and > it's also the only file using "struct usb3503_platform_data". Ah yes, my mistake. I understood that you are asking about "platform driver", not "platform data". From my perspective all boards use DT so there is no platform data. In other drivers I found that some folks from x86 world use SFI/platform_data but I do not know if it applies here. Anyway removing platform data is fine with me. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: xhci: add quirk flag for broken stop command on AMD platforms
on AMD platforms with SNPS 3.1 USB controller has an issue if the stop EP command is issued when the controller is not in running state. If issued, it is leading to a critical RTL bug because of which controller goes into irrecoverable state. This patch adds a appropriate checks to make sure that scenario does not happen. Signed-off-by: Shyam Sundar S K Signed-off-by: Nehal Shah --- drivers/usb/host/xhci-hub.c | 36 ++-- drivers/usb/host/xhci-pci.c | 8 drivers/usb/host/xhci.h | 1 + 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 5e3e9d4..fec849f 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -380,6 +380,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) { struct xhci_virt_device *virt_dev; struct xhci_command *cmd; + struct xhci_ep_ctx *ep_ctx; unsigned long flags; int ret; int i; @@ -407,11 +408,42 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend) return -ENOMEM; } - xhci_queue_stop_endpoint(xhci, command, slot_id, i, + + /* on AMD platforms with SNPS 3.1 USB controller has +* an issue if the stop EP command is issued when +* the controller is not in running state. +* If issued, its leading to a RTL bug because of +* which controller goes to a bad state. +* Adding a conditional check to prevent this. +*/ + + if (xhci->quirks & XHCI_BROKEN_STOP) { + ep_ctx = xhci_get_ep_ctx(xhci, + virt_dev->out_ctx, i); + + if (GET_EP_CTX_STATE(ep_ctx) == + EP_STATE_RUNNING) { + xhci_queue_stop_endpoint(xhci, command, + slot_id, i, suspend); + } + } else { + xhci_queue_stop_endpoint(xhci, command, + slot_id, i, suspend); + } + } + } + + if (xhci->quirks & XHCI_BROKEN_STOP) { + ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, 0); + + if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_RUNNING) { + xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend); } + } else { + xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend); } - xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend); + xhci_ring_cmd_db(xhci); spin_unlock_irqrestore(&xhci->lock, flags); diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 7b86508..403de8f 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -52,6 +52,8 @@ #define PCI_DEVICE_ID_INTEL_BROXTON_M_XHCI 0x0aa8 #define PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI 0x1aa8 #define PCI_DEVICE_ID_INTEL_APL_XHCI 0x5aa8 +#define PCI_DEVICE_ID_AMD_XHCI_30 0x15e0 +#define PCI_DEVICE_ID_AMD_XHCI_31 0x15e1 static const char hcd_name[] = "xhci_hcd"; @@ -205,6 +207,12 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) if (xhci->quirks & XHCI_RESET_ON_RESUME) xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, "QUIRK: Resetting on resume"); + + if (pdev->vendor == PCI_VENDOR_ID_AMD && + (pdev->device == PCI_DEVICE_ID_AMD_XHCI_30 || + pdev->device == PCI_DEVICE_ID_AMD_XHCI_31)) + xhci->quirks |= XHCI_BROKEN_STOP; + } #ifdef CONFIG_ACPI diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 73a28a9..c6fa3ca 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1819,6 +1819,7 @@ struct xhci_hcd { /* For controller with a broken Port Disable implementation */ #define XHCI_BROKEN_PORT_PED (1 << 25) #define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26) +#define XHCI_BROKEN_STOP (1 << 27) unsigned intnum_active_eps; unsigned intlimit_active_eps; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: add quirk flag for broken stop command on AMD platforms
Hi, On 18/05/17 11:53, Shyam Sundar S K wrote: > on AMD platforms with SNPS 3.1 USB controller has an issue > if the stop EP command is issued when the controller is not > in running state. If issued, it is leading to a critical RTL bug > because of which controller goes into irrecoverable state. > > This patch adds a appropriate checks to make sure that scenario > does not happen. > > Signed-off-by: Shyam Sundar S K > Signed-off-by: Nehal Shah > --- > drivers/usb/host/xhci-hub.c | 36 ++-- > drivers/usb/host/xhci-pci.c | 8 > drivers/usb/host/xhci.h | 1 + > 3 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c > index 5e3e9d4..fec849f 100644 > --- a/drivers/usb/host/xhci-hub.c > +++ b/drivers/usb/host/xhci-hub.c > @@ -380,6 +380,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int > slot_id, int suspend) > { > struct xhci_virt_device *virt_dev; > struct xhci_command *cmd; > + struct xhci_ep_ctx *ep_ctx; > unsigned long flags; > int ret; > int i; > @@ -407,11 +408,42 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int > slot_id, int suspend) > return -ENOMEM; > > } > - xhci_queue_stop_endpoint(xhci, command, slot_id, i, > + > + /* on AMD platforms with SNPS 3.1 USB controller has > + * an issue if the stop EP command is issued when > + * the controller is not in running state. > + * If issued, its leading to a RTL bug because of > + * which controller goes to a bad state. > + * Adding a conditional check to prevent this. > + */ > + > + if (xhci->quirks & XHCI_BROKEN_STOP) { > + ep_ctx = xhci_get_ep_ctx(xhci, > + virt_dev->out_ctx, i); > + > + if (GET_EP_CTX_STATE(ep_ctx) == > + EP_STATE_RUNNING) { In the comment you mention that we need to avoid the stop EP command when controller is not running but here you are checking if EP is not running. Is Controller not running and EP not running the same? If we're not queuing the stop endpoint command do we need still to allocate it and ring the doorbell? > + xhci_queue_stop_endpoint(xhci, command, > + slot_id, i, suspend); > + } > + } else { > + xhci_queue_stop_endpoint(xhci, command, > + slot_id, i, suspend); > + } > + } > + } > + > + if (xhci->quirks & XHCI_BROKEN_STOP) { > + ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, 0); > + > + if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_RUNNING) { > + xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, >suspend); > } > + } else { > + xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend); > } > - xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend); > + > xhci_ring_cmd_db(xhci); > spin_unlock_irqrestore(&xhci->lock, flags); What happens to the wait for completion code that is right after this? Since we didn't send the command there is no point waiting for it to complete right? > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index 7b86508..403de8f 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -52,6 +52,8 @@ > #define PCI_DEVICE_ID_INTEL_BROXTON_M_XHCI 0x0aa8 > #define PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI 0x1aa8 > #define PCI_DEVICE_ID_INTEL_APL_XHCI 0x5aa8 > +#define PCI_DEVICE_ID_AMD_XHCI_300x15e0 > +#define PCI_DEVICE_ID_AMD_XHCI_310x15e1 > > static const char hcd_name[] = "xhci_hcd"; > > @@ -205,6 +207,12 @@ static void xhci_pci_quirks(struct device *dev, struct > xhci_hcd *xhci) > if (xhci->quirks & XHCI_RESET_ON_RESUME) > xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, > "QUIRK: Resetting on resume"); > + > + if (pdev->vendor == PCI_VENDOR_ID_AMD && > + (pdev->device == PCI_DEVICE_ID_AMD_XHCI_30 || > + pdev->device == PCI_DEVICE_ID_AMD_XHCI_31)) > + xhci->quirks |= XHCI_BROKEN_STOP; > + > } > > #ifdef CONFIG_ACPI > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 73a28a9..c6fa3ca 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -1819,6 +1819,7 @@ struct xhci_hcd { > /* For controller
Re: [PATCH 3/4] usb: host: ohci-omap3: Print warning to get people to use ohci-platform
Hi, On Wed, May 17, 2017 at 03:59:21PM -0700, Tony Lindgren wrote: > We can't just remove ohci-omap3 as that could make some people's > systems unusable for keyboard and mouse. Let's print a warning for > now, and then remove the driver in a year or so. > > Cc: Hans de Goede > Cc: Rob Herring > Cc: Roger Quadros > Cc: Yoshihiro Shimoda > Signed-off-by: Tony Lindgren I guess you could drop the driver and modify the Kconfig entry, so that it selects USB_OHCI_HCD_PLATFORM? I just checked and there are already a few examples of this in the USB host Kconfig :) -- Sebastian > --- > drivers/usb/host/ohci-omap3.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/host/ohci-omap3.c b/drivers/usb/host/ohci-omap3.c > --- a/drivers/usb/host/ohci-omap3.c > +++ b/drivers/usb/host/ohci-omap3.c > @@ -130,6 +130,9 @@ static int ohci_hcd_omap3_probe(struct platform_device > *pdev) > dev_dbg(dev, "failed to add hcd with err %d\n", ret); > goto err_add_hcd; > } > + > + WARN(1, "ohci-omap3 is deprecated, please use ohci-platform instead\n"); > + > device_wakeup_enable(hcd->self.controller); > > return 0; > -- > 2.13.0 > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html signature.asc Description: PGP signature
Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers
Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck: > On 05/17/2017 12:34 AM, Oliver Neukum wrote: > > > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan > > Sridharan: > > > > Hi, > > > > > > > > "Two independent set of mechanisms are defined to allow a USB Type-C > > > DRP to functionally swap power and data roles. When USB PD is > > > supported, power and data role swapping is performed as a subsequent > > > step following the initial connection process. For non-PD implementations, > > > power/data role swapping can optionally be dealt with as part of the > > > initial > > > connection process." > > > > Well, as I read it, without PD once a connection is established, you > > are stuck with your role. So it seems to me that blocking a later > > attempt to change it makes sense. > > > > That seems to be a harsh and not very user friendly reading of the > specification. > > I would argue that the user doesn't care if the partner supports PD or not > when selecting a role, and I would prefer to provide an implementation which > is > as user friendly as possible. Data role, no question, you are right. Power role is a different question. A switch of power role with PD should not lead to a disconnect. Any other method might. So equating them does not look like a good idea. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] usb: host: ohci-platform: Add support for omap3 and later
On 18/05/17 01:59, Tony Lindgren wrote: > With the runtime PM implemented for ohci-platform driver, we can > now support omap3 and later OHCI by adding one device tree > property. > > Cc: Hans de Goede > Cc: Rob Herring > Cc: Roger Quadros > Cc: Yoshihiro Shimoda > Signed-off-by: Tony Lindgren Acked-by: Roger Quadros > --- > Documentation/devicetree/bindings/usb/usb-ohci.txt | 1 + > drivers/usb/host/ohci-platform.c | 5 + > 2 files changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt > b/Documentation/devicetree/bindings/usb/usb-ohci.txt > --- a/Documentation/devicetree/bindings/usb/usb-ohci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt > @@ -10,6 +10,7 @@ Optional properties: > - big-endian-desc : boolean, set this for hcds with big-endian descriptors > - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc > - no-big-frame-no : boolean, set if frame_no lives in bits [15:0] of HCCA > +- remote-wakeup-connected: remote wakeup is wired on the platform > - num-ports : u32, to override the detected port count > - clocks : a list of phandle + clock specifier pairs > - phys : phandle + phy specifier pair > diff --git a/drivers/usb/host/ohci-platform.c > b/drivers/usb/host/ohci-platform.c > --- a/drivers/usb/host/ohci-platform.c > +++ b/drivers/usb/host/ohci-platform.c > @@ -170,6 +170,10 @@ static int ohci_platform_probe(struct platform_device > *dev) > if (of_property_read_bool(dev->dev.of_node, "no-big-frame-no")) > ohci->flags |= OHCI_QUIRK_FRAME_NO; > > + if (of_property_read_bool(dev->dev.of_node, > + "remote-wakeup-connected")) > + ohci->hc_control = OHCI_CTRL_RWC; > + > of_property_read_u32(dev->dev.of_node, "num-ports", >&ohci->num_ports); > > @@ -361,6 +365,7 @@ static int ohci_platform_resume(struct device *dev) > static const struct of_device_id ohci_platform_ids[] = { > { .compatible = "generic-ohci", }, > { .compatible = "cavium,octeon-6335-ohci", }, > + { .compatible = "ti,ohci-omap3", }, > { } > }; > MODULE_DEVICE_TABLE(of, ohci_platform_ids); > -- cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] usb: host: ohci-platform: Add basic runtime PM support
On 18/05/17 01:59, Tony Lindgren wrote: > This is needed in preparation of adding support for omap3 and > later OHCI. The runtime PM will only do something on platforms > that implement it. > > Cc: devicet...@vger.kernel.org > Cc: Hans de Goede > Cc: Rob Herring > Cc: Roger Quadros > Cc: Yoshihiro Shimoda > Signed-off-by: Tony Lindgren Acked-by: Roger Quadros > --- > drivers/usb/host/ohci-platform.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/usb/host/ohci-platform.c > b/drivers/usb/host/ohci-platform.c > --- a/drivers/usb/host/ohci-platform.c > +++ b/drivers/usb/host/ohci-platform.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -51,6 +52,10 @@ static int ohci_platform_power_on(struct platform_device > *dev) > struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd); > int clk, ret, phy_num; > > + ret = pm_runtime_get_sync(&dev->dev); > + if (ret < 0) > + return ret; > + > for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) { > ret = clk_prepare_enable(priv->clks[clk]); > if (ret) > @@ -96,6 +101,8 @@ static void ohci_platform_power_off(struct platform_device > *dev) > for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--) > if (priv->clks[clk]) > clk_disable_unprepare(priv->clks[clk]); > + > + pm_runtime_put_sync(&dev->dev); > } > > static struct hc_driver __read_mostly ohci_platform_hc_driver; > @@ -242,6 +249,7 @@ static int ohci_platform_probe(struct platform_device > *dev) > } > #endif > > + pm_runtime_enable(&dev->dev); > if (pdata->power_on) { > err = pdata->power_on(dev); > if (err < 0) > @@ -271,6 +279,7 @@ static int ohci_platform_probe(struct platform_device > *dev) > if (pdata->power_off) > pdata->power_off(dev); > err_reset: > + pm_runtime_disable(&dev->dev); > while (--rst >= 0) > reset_control_assert(priv->resets[rst]); > err_put_clks: > @@ -305,6 +314,8 @@ static int ohci_platform_remove(struct platform_device > *dev) > > usb_put_hcd(hcd); > > + pm_runtime_disable(&dev->dev); > + > if (pdata == &ohci_platform_defaults) > dev->dev.platform_data = NULL; > > -- cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] ARM: dts: Add remote-wakeup-connected for omap OHCI
On 18/05/17 01:59, Tony Lindgren wrote: > Add remote-wakeup-connected for omap OHCI as that's needed by > ehci-platform driver. > > Cc: devicet...@vger.kernel.org > Cc: Hans de Goede > Cc: Rob Herring > Cc: Roger Quadros > Cc: Yoshihiro Shimoda > Signed-off-by: Tony Lindgren Acked-by: Roger Quadros > --- > arch/arm/boot/dts/omap3.dtsi | 1 + > arch/arm/boot/dts/omap4.dtsi | 1 + > arch/arm/boot/dts/omap5.dtsi | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi > --- a/arch/arm/boot/dts/omap3.dtsi > +++ b/arch/arm/boot/dts/omap3.dtsi > @@ -715,6 +715,7 @@ > reg = <0x48064400 0x400>; > interrupt-parent = <&intc>; > interrupts = <76>; > + remote-wakeup-connected; > }; > > usbhsehci: ehci@48064800 { > diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi > --- a/arch/arm/boot/dts/omap4.dtsi > +++ b/arch/arm/boot/dts/omap4.dtsi > @@ -863,6 +863,7 @@ > reg = <0x4a064800 0x400>; > interrupt-parent = <&gic>; > interrupts = ; > + remote-wakeup-connected; > }; > > usbhsehci: ehci@4a064c00 { > diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi > --- a/arch/arm/boot/dts/omap5.dtsi > +++ b/arch/arm/boot/dts/omap5.dtsi > @@ -939,6 +939,7 @@ > compatible = "ti,ohci-omap3"; > reg = <0x4a064800 0x400>; > interrupts = ; > + remote-wakeup-connected; > }; > > usbhsehci: ehci@4a064c00 { > -- cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v3 2/4] usb: usbip tool: Add ncontrollers in vhci_driver structure
From: Yuyang Du A new field ncontrollers is added to the vhci_driver structure. And this field is stored by scanning the vhci_hcd* dirs in the platform udev. Suggested-and-reviewed-by: Krzysztof Opasiak Signed-off-by: Yuyang Du --- tools/usb/usbip/libsrc/vhci_driver.c | 36 tools/usb/usbip/libsrc/vhci_driver.h | 1 + 2 files changed, 37 insertions(+) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index 151580a..f019686 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -7,6 +7,7 @@ #include #include #include +#include #include "sysfs_utils.h" #undef PROGNAME @@ -134,6 +135,33 @@ static int get_nports(void) return (int)strtoul(attr_nports, NULL, 10); } +static int vhci_hcd_filter(const struct dirent *dirent) +{ + return strcmp(dirent->d_name, "vhci_hcd") >= 0; +} + +static int get_ncontrollers(void) +{ + struct dirent **namelist; + struct udev_device *platform; + int n; + + platform = udev_device_get_parent(vhci_driver->hc_device); + if (platform == NULL) + return -1; + + n = scandir(udev_device_get_syspath(platform), &namelist, vhci_hcd_filter, NULL); + if (n < 0) + err("scandir failed"); + else { + for (int i = 0; i < n; i++) + free(namelist[i]); + free(namelist); + } + + return n; +} + /* * Read the given port's record. * @@ -231,6 +259,14 @@ int usbip_vhci_driver_open(void) goto err; } + vhci_driver->ncontrollers = get_ncontrollers(); + dbg("available controllers: %d", vhci_driver->ncontrollers); + + if (vhci_driver->ncontrollers <=0) { + err("no available usb controllers"); + goto err; + } + if (refresh_imported_device_list()) goto err; diff --git a/tools/usb/usbip/libsrc/vhci_driver.h b/tools/usb/usbip/libsrc/vhci_driver.h index fa2316c..33add14 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.h +++ b/tools/usb/usbip/libsrc/vhci_driver.h @@ -31,6 +31,7 @@ struct usbip_vhci_driver { /* /sys/devices/platform/vhci_hcd */ struct udev_device *hc_device; + int ncontrollers; int nports; struct usbip_imported_device idev[MAXNPORT]; }; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v3 3/4] usb: usbip tool: Fix refresh_imported_device_list()
From: Yuyang Du The commit 0775a9cbc694e8c7 ("usbip: vhci extension: modifications to vhci driver") introduced multiple controllers, but the status of the ports are only extracted from the first status file, fix it. Reviewed-by: Krzysztof Opasiak Signed-off-by: Yuyang Du --- tools/usb/usbip/libsrc/vhci_driver.c | 27 +-- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index f019686..ed2eba9 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -108,18 +108,33 @@ static int parse_status(const char *value) return 0; } +#define MAX_STATUS_NAME 16 + static int refresh_imported_device_list(void) { const char *attr_status; + char status[MAX_STATUS_NAME+1] = "status"; + int i, ret; - attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device, - "status"); - if (!attr_status) { - err("udev_device_get_sysattr_value failed"); - return -1; + for (i = 0; i < vhci_driver->ncontrollers; i++) { + if (i > 0) + snprintf(status, sizeof(status), "status.%d", i); + + attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device, + status); + if (!attr_status) { + err("udev_device_get_sysattr_value failed"); + return -1; + } + + dbg("controller %d", i); + + ret = parse_status(attr_status); + if (ret != 0) + return ret; } - return parse_status(attr_status); + return 0; } static int get_nports(void) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v3 0/4] usb: usbip: Fix ports and port status
From: Yuyang Du The commit 0775a9cbc694e8c7 ("usbip: vhci extension: modifications to vhci driver") introduced several bugs relating to the number of ports amd the port status. In addition, a small improvement is made to the vhci_hcd module. resend: - Remove already picked patches - Add a few *-bys v3: - Check both ends of the nports - Check the return of ncontrollers - Add a few Reviewed-bys and Acked-bys - Minor coding changes v2: - Remove ncontrollers RO attr as suggested by Krzysztof - As a result, scandir vhci_hcd* dir in platform to learn how many we have as suggested by Krzysztof - Minor coding changes Thanks, Yuyang -- Yuyang Du (4): usb: usbip tool: Check the return of get_nports() usb: usbip tool: Add ncontrollers in vhci_driver structure usb: usbip tool: Fix refresh_imported_device_list() usb: usbip tool: Fix parse_status() tools/usb/usbip/libsrc/vhci_driver.c | 111 +-- tools/usb/usbip/libsrc/vhci_driver.h | 1 + tools/usb/usbip/src/usbip_attach.c | 2 + 3 files changed, 84 insertions(+), 30 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v3 1/4] usb: usbip tool: Check the return of get_nports()
From: Yuyang Du If we get nonpositive number of ports, there is no sense to continue, then fail gracefully. In addition, the commit 0775a9cbc694e8c72 ("usbip: vhci extension: modifications to vhci driver") introduced configurable numbers of controllers and ports, but we have a static port number maximum, MAXNPORT. If exceeded, the idev array will be overflown. We fix it by validating the nports to make sure the port number max is not exceeded. Reviewed-by: Krzysztof Opasiak Signed-off-by: Yuyang Du Acked-by: Shuah Khan --- tools/usb/usbip/libsrc/vhci_driver.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index f659c14..151580a 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -220,9 +220,17 @@ int usbip_vhci_driver_open(void) } vhci_driver->nports = get_nports(); - dbg("available ports: %d", vhci_driver->nports); + if (vhci_driver->nports <=0) { + err("no available ports"); + goto err; + } + else if (vhci_driver->nports > MAXNPORT) { + err("port number exceeds %d", MAXNPORT); + goto err; + } + if (refresh_imported_device_list()) goto err; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH resend v3 4/4] usb: usbip tool: Fix parse_status()
From: Yuyang Du In parse_status(), all nports number of idev's are initiated to 0 by memset(), it is simply wrong, because parse_status() reads the status sys file one by one, therefore, it can only update the according vhci_driver->idev's for it to parse. Reviewed-by: Krzysztof Opasiak Signed-off-by: Yuyang Du --- tools/usb/usbip/libsrc/vhci_driver.c | 38 ++-- tools/usb/usbip/src/usbip_attach.c | 2 ++ 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index ed2eba9..af88447 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -36,18 +36,11 @@ imported_device_init(struct usbip_imported_device *idev, char *busid) return NULL; } - - static int parse_status(const char *value) { int ret = 0; char *c; - - for (int i = 0; i < vhci_driver->nports; i++) - memset(&vhci_driver->idev[i], 0, sizeof(vhci_driver->idev[i])); - - /* skip a header line */ c = strchr(value, '\n'); if (!c) @@ -58,6 +51,7 @@ static int parse_status(const char *value) int port, status, speed, devid; unsigned long socket; char lbusid[SYSFS_BUS_ID_SIZE]; + struct usbip_imported_device *idev; ret = sscanf(c, "%d %d %d %x %lx %31s\n", &port, &status, &speed, @@ -72,30 +66,28 @@ static int parse_status(const char *value) port, status, speed, devid); dbg("socket %lx lbusid %s", socket, lbusid); - /* if a device is connected, look at it */ - { - struct usbip_imported_device *idev = &vhci_driver->idev[port]; + idev = &vhci_driver->idev[port]; - idev->port = port; - idev->status= status; + memset(idev, 0, sizeof(*idev)); - idev->devid = devid; + idev->port = port; + idev->status= status; - idev->busnum= (devid >> 16); - idev->devnum= (devid & 0x); + idev->devid = devid; - if (idev->status != VDEV_ST_NULL - && idev->status != VDEV_ST_NOTASSIGNED) { - idev = imported_device_init(idev, lbusid); - if (!idev) { - dbg("imported_device_init failed"); - return -1; - } + idev->busnum= (devid >> 16); + idev->devnum= (devid & 0x); + + if (idev->status != VDEV_ST_NULL + && idev->status != VDEV_ST_NOTASSIGNED) { + idev = imported_device_init(idev, lbusid); + if (!idev) { + dbg("imported_device_init failed"); + return -1; } } - /* go to the next line */ c = strchr(c, '\n'); if (!c) diff --git a/tools/usb/usbip/src/usbip_attach.c b/tools/usb/usbip/src/usbip_attach.c index 70a6b50..62a297f 100644 --- a/tools/usb/usbip/src/usbip_attach.c +++ b/tools/usb/usbip/src/usbip_attach.c @@ -108,6 +108,8 @@ static int import_device(int sockfd, struct usbip_usb_device *udev) return -1; } + dbg("got free port %d", port); + rc = usbip_vhci_attach_device(port, sockfd, udev->busnum, udev->devnum, udev->speed); if (rc < 0) { -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC V1 1/1] net: cdc_ncm: Reduce memory use when kernel memory low
Am Mittwoch, den 17.05.2017, 14:18 -0400 schrieb David Miller: > From: Bjørn Mork > Date: Tue, 16 May 2017 20:24:10 +0200 > > > Jim Baxter writes: > > > >> The CDC-NCM driver can require large amounts of memory to create > >> skb's and this can be a problem when the memory becomes fragmented. > >> > >> This especially affects embedded systems that have constrained > >> resources but wish to maximise the throughput of CDC-NCM with 16KiB > >> NTB's. > >> > >> The issue is after running for a while the kernel memory can become > >> fragmented and it needs compacting. > >> If the NTB allocation is needed before the memory has been compacted > >> the atomic allocation can fail which can cause increased latency, > >> large re-transmissions or disconnections depending upon the data > >> being transmitted at the time. > >> This situation occurs for less than a second until the kernel has > >> compacted the memory but the failed devices can take a lot longer to > >> recover from the failed TX packets. > >> > >> To ease this temporary situation I modified the CDC-NCM TX path to > >> temporarily switch into a reduced memory mode which allocates an NTB > >> that will fit into a USB_CDC_NCM_NTB_MIN_OUT_SIZE (default 2048 Bytes) > >> sized memory block and only transmit NTB's with a single network frame > >> until the memory situation is resolved. > >> Once the memory is compacted the CDC-NCM data can resume transmitting > >> at the normal tx_max rate once again. > > > > I must say that I don't like the additional complexity added here. If > > there are memory issues and you can reduce the buffer size to > > USB_CDC_NCM_NTB_MIN_OUT_SIZE, then why don't you just set a lower tx_max > > buffer size in the first place? > > > > echo 2048 > /sys/class/net/wwan0/cdc_ncm/tx_max > > When there isn't memory pressure this will hurt performance of > course. > > It is a quite common paradigm to back down to 0 order memory requests > when higher order ones fail, so this isn't such a bad change from the > perspective. > > However, one negative about it is that when the system is under memory > stress it doesn't help at all to keep attemping high order allocations > when the system hasn't recovered yet. In fact, this can make it > worse. This makes me wonder why there is no notifier chain for this. Or am I just too stupid to find it? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/9] usbip: vhci-hcd: Add USB3 SuperSpeed support
From: Yuyang Du This patch adds a USB3 HCD to an existing USB2 HCD and provides the support of SuperSpeed, in case the device can only be enumerated with SuperSpeed. The bulk of the added code in usb3_bos_desc and hub_control to support SuperSpeed is borrowed from the commit 1cd8fd2887e162ad ("usb: gadget: dummy_hcd: add SuperSpeed support"). With this patch, each vhci will have VHCI_HC_PORTS HighSpeed ports and VHCI_HC_PORTS SuperSpeed ports. Suggested-by: Krzysztof Opasiak Signed-off-by: Yuyang Du Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci.h | 7 +- drivers/usb/usbip/vhci_hcd.c | 323 --- drivers/usb/usbip/vhci_sysfs.c | 109 tools/usb/usbip/libsrc/vhci_driver.c | 23 ++- tools/usb/usbip/libsrc/vhci_driver.h | 8 +- tools/usb/usbip/src/usbip_attach.c | 3 +- 6 files changed, 370 insertions(+), 103 deletions(-) diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h index 8a979fc..db28eb5 100644 --- a/drivers/usb/usbip/vhci.h +++ b/drivers/usb/usbip/vhci.h @@ -72,6 +72,11 @@ struct vhci_unlink { unsigned long unlink_seqnum; }; +enum hub_speed { + HUB_SPEED_HIGH = 0, + HUB_SPEED_SUPER, +}; + /* Number of supported ports. Value has an upperbound of USB_MAXCHILDREN */ #ifdef CONFIG_USBIP_VHCI_HC_PORTS #define VHCI_HC_PORTS CONFIG_USBIP_VHCI_HC_PORTS @@ -140,7 +145,7 @@ static inline __u32 port_to_rhport(__u32 port) static inline int port_to_pdev_nr(__u32 port) { - return port / VHCI_HC_PORTS; + return port / (VHCI_HC_PORTS * 2); } static inline struct vhci_hcd *hcd_to_vhci_hcd(struct usb_hcd *hcd) diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 8cfba1d..43cacbc 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -232,6 +232,40 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf) return changed ? retval : 0; } +/* usb 3.0 root hub device descriptor */ +static struct { + struct usb_bos_descriptor bos; + struct usb_ss_cap_descriptor ss_cap; +} __packed usb3_bos_desc = { + + .bos = { + .bLength= USB_DT_BOS_SIZE, + .bDescriptorType= USB_DT_BOS, + .wTotalLength = cpu_to_le16(sizeof(usb3_bos_desc)), + .bNumDeviceCaps = 1, + }, + .ss_cap = { + .bLength= USB_DT_USB_SS_CAP_SIZE, + .bDescriptorType= USB_DT_DEVICE_CAPABILITY, + .bDevCapabilityType = USB_SS_CAP_TYPE, + .wSpeedSupported= cpu_to_le16(USB_5GBPS_OPERATION), + .bFunctionalitySupport = ilog2(USB_5GBPS_OPERATION), + }, +}; + +static inline void +ss_hub_descriptor(struct usb_hub_descriptor *desc) +{ + memset(desc, 0, sizeof *desc); + desc->bDescriptorType = USB_DT_SS_HUB; + desc->bDescLength = 12; + desc->wHubCharacteristics = cpu_to_le16( + HUB_CHAR_INDV_PORT_LPSM | HUB_CHAR_COMMON_OCPM); + desc->bNbrPorts = VHCI_HC_PORTS; + desc->u.ss.bHubHdrDecLat = 0x04; /* Worst case: 0.4 micro sec*/ + desc->u.ss.DeviceRemovable = 0x; +} + static inline void hub_descriptor(struct usb_hub_descriptor *desc) { memset(desc, 0, sizeof(*desc)); @@ -260,13 +294,15 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, /* * NOTE: -* wIndex shows the port number and begins from 1. +* wIndex (bits 0-7) shows the port number and begins from 1? */ + wIndex = ((__u8)(wIndex & 0x00ff)); usbip_dbg_vhci_rh("typeReq %x wValue %x wIndex %x\n", typeReq, wValue, wIndex); + if (wIndex > VHCI_HC_PORTS) pr_err("invalid port number %d\n", wIndex); - rhport = ((__u8)(wIndex & 0x00ff)) - 1; + rhport = wIndex - 1; vhci_hcd = hcd_to_vhci_hcd(hcd); vhci = vhci_hcd->vhci; @@ -286,34 +322,26 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, case ClearPortFeature: switch (wValue) { case USB_PORT_FEAT_SUSPEND: + if (hcd->speed == HCD_USB3) { + pr_err(" ClearPortFeature: USB_PORT_FEAT_SUSPEND req not " + "supported for USB 3.0 roothub\n"); + goto error; + } + usbip_dbg_vhci_rh( + " ClearPortFeature: USB_PORT_FEAT_SUSPEND\n"); if (vhci_hcd->port_status[rhport] & USB_PORT_STAT_SUSPEND) { /* 20msec signaling */ vhci_hcd->resuming = 1; - vhci_hcd->re_timeout = - jiffies + msecs_to_jiffies(20); +
[PATCH v2 5/9] usbip: vhci-hcd: Set the vhci structure up to work
From: Yuyang Du This patch enables the new vhci structure. Its lock protects both the USB2 hub and the shared USB3 hub. Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci.h | 2 - drivers/usb/usbip/vhci_hcd.c | 206 - drivers/usb/usbip/vhci_rx.c| 16 ++-- drivers/usb/usbip/vhci_sysfs.c | 26 -- 4 files changed, 145 insertions(+), 105 deletions(-) diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h index 62ee537..8a979fc 100644 --- a/drivers/usb/usbip/vhci.h +++ b/drivers/usb/usbip/vhci.h @@ -100,8 +100,6 @@ struct vhci { struct vhci_hcd { struct vhci *vhci; - spinlock_t lock; - u32 port_status[VHCI_HC_PORTS]; unsigned resuming:1; diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 2bc77ee..8cfba1d 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -123,7 +123,8 @@ static void dump_port_status_diff(u32 prev_status, u32 new_status) void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed) { - struct vhci_hcd *vhci = vdev_to_vhci_hcd(vdev); + struct vhci_hcd *vhci_hcd = vdev_to_vhci_hcd(vdev); + struct vhci *vhci = vhci_hcd->vhci; int rhport = vdev->rhport; u32 status; unsigned long flags; @@ -132,7 +133,7 @@ void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed) spin_lock_irqsave(&vhci->lock, flags); - status = vhci->port_status[rhport]; + status = vhci_hcd->port_status[rhport]; status |= USB_PORT_STAT_CONNECTION | (1 << USB_PORT_FEAT_C_CONNECTION); @@ -147,16 +148,17 @@ void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed) break; } - vhci->port_status[rhport] = status; + vhci_hcd->port_status[rhport] = status; spin_unlock_irqrestore(&vhci->lock, flags); - usb_hcd_poll_rh_status(vhci_hcd_to_hcd(vhci)); + usb_hcd_poll_rh_status(vhci_hcd_to_hcd(vhci_hcd)); } static void rh_port_disconnect(struct vhci_device *vdev) { - struct vhci_hcd *vhci = vdev_to_vhci_hcd(vdev); + struct vhci_hcd *vhci_hcd = vdev_to_vhci_hcd(vdev); + struct vhci *vhci = vhci_hcd->vhci; int rhport = vdev->rhport; u32 status; unsigned long flags; @@ -165,15 +167,15 @@ static void rh_port_disconnect(struct vhci_device *vdev) spin_lock_irqsave(&vhci->lock, flags); - status = vhci->port_status[rhport]; + status = vhci_hcd->port_status[rhport]; status &= ~USB_PORT_STAT_CONNECTION; status |= (1 << USB_PORT_FEAT_C_CONNECTION); - vhci->port_status[rhport] = status; + vhci_hcd->port_status[rhport] = status; spin_unlock_irqrestore(&vhci->lock, flags); - usb_hcd_poll_rh_status(vhci_hcd_to_hcd(vhci)); + usb_hcd_poll_rh_status(vhci_hcd_to_hcd(vhci_hcd)); } #define PORT_C_MASK\ @@ -196,17 +198,15 @@ static void rh_port_disconnect(struct vhci_device *vdev) */ static int vhci_hub_status(struct usb_hcd *hcd, char *buf) { - struct vhci_hcd *vhci; - int retval; + struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd); + struct vhci *vhci = vhci_hcd->vhci; + int retval = DIV_ROUND_UP(VHCI_HC_PORTS + 1, 8); int rhport; int changed = 0; unsigned long flags; - retval = DIV_ROUND_UP(VHCI_HC_PORTS + 1, 8); memset(buf, 0, retval); - vhci = hcd_to_vhci_hcd(hcd); - spin_lock_irqsave(&vhci->lock, flags); if (!HCD_HW_ACCESSIBLE(hcd)) { usbip_dbg_vhci_rh("hw accessible flag not on?\n"); @@ -215,7 +215,7 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf) /* check pseudo status register for each port */ for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) { - if ((vhci->port_status[rhport] & PORT_C_MASK)) { + if ((vhci_hcd->port_status[rhport] & PORT_C_MASK)) { /* The status of a port has been changed, */ usbip_dbg_vhci_rh("port %d status changed\n", rhport); @@ -247,7 +247,8 @@ static inline void hub_descriptor(struct usb_hub_descriptor *desc) static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex, char *buf, u16 wLength) { - struct vhci_hcd *dum; + struct vhci_hcd *vhci_hcd; + struct vhci *vhci; int retval = 0; int rhport; unsigned long flags; @@ -267,13 +268,14 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, pr_err("invalid port number %d\n", wIndex); rhport = ((__u8)(wIndex & 0x00ff)) - 1; - dum = hcd_to_vhci_hcd(hcd); + vhci_hcd = hcd_to
[PATCH v2 4/9] usbip: vhci-hcd: Rework vhci_hcd_init
From: Yuyang Du A vhci struct is added as the platform-specific data to the vhci platform device, in order to get the vhci by its platform device. This is done in vhci_hcd_init(). Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci_hcd.c | 51 tools/usb/usbip/libsrc/vhci_driver.c | 2 +- tools/usb/usbip/libsrc/vhci_driver.h | 1 + 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 7a04497..2bc77ee 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -1173,24 +1173,6 @@ static struct platform_driver vhci_driver = { }, }; -static int add_platform_device(int id) -{ - struct platform_device *pdev; - int dev_nr; - - if (id == 0) - dev_nr = -1; - else - dev_nr = id; - - pdev = platform_device_register_simple(driver_name, dev_nr, NULL, 0); - if (IS_ERR(pdev)) - return PTR_ERR(pdev); - - vhcis[id].pdev = pdev; - return 0; -} - static void del_platform_devices(void) { struct platform_device *pdev; @@ -1219,23 +1201,46 @@ static int __init vhci_hcd_init(void) if (vhcis == NULL) return -ENOMEM; + for (i = 0; i < vhci_num_controllers; i++) { + vhcis[i].pdev = platform_device_alloc(driver_name, i); + if (!vhcis[i].pdev) { + i--; + while (i >= 0) + platform_device_put(vhcis[i--].pdev); + ret = -ENOMEM; + goto err_device_alloc; + } + } + for (i = 0; i < vhci_num_controllers; i++) { + void *vhci = &vhcis[i]; + ret = platform_device_add_data(vhcis[i].pdev, &vhci, sizeof(void *)); + if (ret) + goto err_driver_register; + } + ret = platform_driver_register(&vhci_driver); if (ret) goto err_driver_register; for (i = 0; i < vhci_num_controllers; i++) { - ret = add_platform_device(i); - if (ret) - goto err_platform_device_register; + ret = platform_device_add(vhcis[i].pdev); + if (ret < 0) { + i--; + while (i >= 0) + platform_device_del(vhcis[i--].pdev); + goto err_add_hcd; + } } pr_info(DRIVER_DESC " v" USBIP_VERSION "\n"); return ret; -err_platform_device_register: - del_platform_devices(); +err_add_hcd: platform_driver_unregister(&vhci_driver); err_driver_register: + for (i = 0; i < vhci_num_controllers; i++) + platform_device_put(vhcis[i].pdev); +err_device_alloc: kfree(vhcis); return ret; } diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index af88447..5b19a32 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -248,7 +248,7 @@ int usbip_vhci_driver_open(void) vhci_driver->hc_device = udev_device_new_from_subsystem_sysname(udev_context, USBIP_VHCI_BUS_TYPE, - USBIP_VHCI_DRV_NAME); + USBIP_VHCI_DEVICE_NAME); if (!vhci_driver->hc_device) { err("udev_device_new_from_subsystem_sysname failed"); goto err; diff --git a/tools/usb/usbip/libsrc/vhci_driver.h b/tools/usb/usbip/libsrc/vhci_driver.h index 33add14..dfe19c1 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.h +++ b/tools/usb/usbip/libsrc/vhci_driver.h @@ -11,6 +11,7 @@ #include "usbip_common.h" #define USBIP_VHCI_BUS_TYPE "platform" +#define USBIP_VHCI_DEVICE_NAME "vhci_hcd.0" #define MAXNPORT 128 struct usbip_imported_device { -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/9] usbip: Enable USB3 SuperSpeed
Hi Shuah, SuperSpeed (only) USB devices cannot be shared via usbip. This patch series attempts to fix it. The first 5 patches refactors the existing code to prepare for the SuperSpeed addition. With this series, our SuperSpeed device works fine. Many thanks to Greg and Krzysztof for their patience to answer my non-usb-professional questions, and special thanks to Krzysztof for the pointer to the SuperSpeed patch in dummy_hcd. This series is based on the series: "usb: usbip: Fix ports and port status v3" (https://www.spinics.net/lists/linux-usb/msg155834.html). v2: - Remove set_link_state() since it is not used Regards, Yuyang -- Yuyang Du (9): usbip: vhci-hcd: Rename function names to reflect their struct names usbip: vhci-hcd: Add vhci struct usbip: vhci-hcd: Move VHCI platform device into vhci struct usbip: vhci-hcd: Rework vhci_hcd_init usbip: vhci-hcd: Set the vhci structure up to work usbip: vhci-hcd: Add USB3 SuperSpeed support usbip: Add USB_SPEED_SUPER as valid arg usbip: vhci-hcd: Add USB3 port status bits usbip: vhci-hcd: Clean up the code by adding a new macro drivers/usb/usbip/vhci.h | 36 ++- drivers/usb/usbip/vhci_hcd.c | 605 +-- drivers/usb/usbip/vhci_rx.c | 16 +- drivers/usb/usbip/vhci_sysfs.c | 138 +--- tools/usb/usbip/libsrc/vhci_driver.c | 25 +- tools/usb/usbip/libsrc/vhci_driver.h | 9 +- tools/usb/usbip/src/usbip_attach.c | 3 +- 7 files changed, 601 insertions(+), 231 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/9] usbip: Add USB_SPEED_SUPER as valid arg
From: Yuyang Du With this patch, USB_SPEED_SUPER is a valid speed when attaching a USB3 SuperSpeed device. Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci_sysfs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c index cac2319..3ad68ff 100644 --- a/drivers/usb/usbip/vhci_sysfs.c +++ b/drivers/usb/usbip/vhci_sysfs.c @@ -277,6 +277,7 @@ static int valid_args(__u32 pdev_nr, __u32 rhport, enum usb_device_speed speed) case USB_SPEED_FULL: case USB_SPEED_HIGH: case USB_SPEED_WIRELESS: + case USB_SPEED_SUPER: break; default: pr_err("Failed attach request for unsupported USB speed: %s\n", -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/9] usbip: vhci-hcd: Move VHCI platform device into vhci struct
From: Yuyang Du Every VHCI is a platform device, so move the platform_device struct into the VHCI struct. Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci.h | 3 ++- drivers/usb/usbip/vhci_hcd.c | 17 - drivers/usb/usbip/vhci_sysfs.c | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h index 9959584..62ee537 100644 --- a/drivers/usb/usbip/vhci.h +++ b/drivers/usb/usbip/vhci.h @@ -90,6 +90,8 @@ struct vhci_unlink { struct vhci { spinlock_t lock; + struct platform_device *pdev; + struct vhci_hcd *vhci_hcd_hs; struct vhci_hcd *vhci_hcd_ss; }; @@ -116,7 +118,6 @@ struct vhci_hcd { }; extern int vhci_num_controllers; -extern struct platform_device **vhci_pdevs; extern struct vhci *vhcis; extern struct attribute_group vhci_attr_group; diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 624265a..7a04497 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -58,8 +58,7 @@ static const char driver_name[] = "vhci_hcd"; static const char driver_desc[] = "USB/IP Virtual Host Controller"; int vhci_num_controllers = VHCI_NR_HCS; - -struct platform_device **vhci_pdevs; +struct vhci *vhcis; static const char * const bit_desc[] = { "CONNECTION", /*0*/ @@ -1188,7 +1187,7 @@ static int add_platform_device(int id) if (IS_ERR(pdev)) return PTR_ERR(pdev); - *(vhci_pdevs + id) = pdev; + vhcis[id].pdev = pdev; return 0; } @@ -1198,10 +1197,10 @@ static void del_platform_devices(void) int i; for (i = 0; i < vhci_num_controllers; i++) { - pdev = *(vhci_pdevs + i); + pdev = vhcis[i].pdev; if (pdev != NULL) platform_device_unregister(pdev); - *(vhci_pdevs + i) = NULL; + vhcis[i].pdev = NULL; } sysfs_remove_link(&platform_bus.kobj, driver_name); } @@ -1216,8 +1215,8 @@ static int __init vhci_hcd_init(void) if (vhci_num_controllers < 1) vhci_num_controllers = 1; - vhci_pdevs = kcalloc(vhci_num_controllers, sizeof(void *), GFP_KERNEL); - if (vhci_pdevs == NULL) + vhcis = kcalloc(vhci_num_controllers, sizeof(struct vhci), GFP_KERNEL); + if (vhcis == NULL) return -ENOMEM; ret = platform_driver_register(&vhci_driver); @@ -1237,7 +1236,7 @@ static int __init vhci_hcd_init(void) del_platform_devices(); platform_driver_unregister(&vhci_driver); err_driver_register: - kfree(vhci_pdevs); + kfree(vhcis); return ret; } @@ -1245,7 +1244,7 @@ static void __exit vhci_hcd_exit(void) { del_platform_devices(); platform_driver_unregister(&vhci_driver); - kfree(vhci_pdevs); + kfree(vhcis); } module_init(vhci_hcd_init); diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c index d878faa..07f0d37 100644 --- a/drivers/usb/usbip/vhci_sysfs.c +++ b/drivers/usb/usbip/vhci_sysfs.c @@ -32,7 +32,7 @@ /* Sysfs entry to show port status */ static ssize_t status_show_vhci(int pdev_nr, char *out) { - struct platform_device *pdev = *(vhci_pdevs + pdev_nr); + struct platform_device *pdev = vhcis[pdev_nr].pdev; struct vhci_hcd *vhci; char *s = out; int i = 0; @@ -206,7 +206,7 @@ static ssize_t store_detach(struct device *dev, struct device_attribute *attr, if (!valid_port(pdev_nr, rhport)) return -EINVAL; - hcd = platform_get_drvdata(*(vhci_pdevs + pdev_nr)); + hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); if (hcd == NULL) { dev_err(dev, "port is not ready %u\n", port); return -EAGAIN; @@ -287,7 +287,7 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr, if (!valid_args(pdev_nr, rhport, speed)) return -EINVAL; - hcd = platform_get_drvdata(*(vhci_pdevs + pdev_nr)); + hcd = platform_get_drvdata(vhcis[pdev_nr].pdev); if (hcd == NULL) { dev_err(dev, "port %d is not ready\n", port); return -EAGAIN; -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/9] usbip: vhci-hcd: Add vhci struct
From: Yuyang Du In order to support SuperSpeed devices, a USB3 HCD is added to share the USB2 HCD. As a result, a VHCI is composed of two vhci_hcds associated with the two HCDs respectively. So we add another level of abstraction, vhci, and thus this vhci structure. Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci.h | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h index bff472f..9959584 100644 --- a/drivers/usb/usbip/vhci.h +++ b/drivers/usb/usbip/vhci.h @@ -87,8 +87,17 @@ struct vhci_unlink { #define MAX_STATUS_NAME 16 -/* for usb_bus.hcpriv */ +struct vhci { + spinlock_t lock; + + struct vhci_hcd *vhci_hcd_hs; + struct vhci_hcd *vhci_hcd_ss; +}; + +/* for usb_hcd.hcd_priv[0] */ struct vhci_hcd { + struct vhci *vhci; + spinlock_t lock; u32 port_status[VHCI_HC_PORTS]; @@ -108,6 +117,7 @@ struct vhci_hcd { extern int vhci_num_controllers; extern struct platform_device **vhci_pdevs; +extern struct vhci *vhcis; extern struct attribute_group vhci_attr_group; /* vhci_hcd.c */ -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 9/9] usbip: vhci-hcd: Clean up the code by adding a new macro
From: Yuyang Du Each vhci has 2*VHCI_HC_PORTS ports, in which VHCI_HC_PORTS ports are HighSpeed (or below), and VHCI_HC_PORTS are SuperSpeed. This new macro VHCI_PORTS reflects this configuration. Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci.h | 5 - drivers/usb/usbip/vhci_sysfs.c | 10 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h index db28eb5..5cfb59e 100644 --- a/drivers/usb/usbip/vhci.h +++ b/drivers/usb/usbip/vhci.h @@ -84,6 +84,9 @@ enum hub_speed { #define VHCI_HC_PORTS 8 #endif +/* Each VHCI has 2 hubs (USB2 and USB3), each has VHCI_HC_PORTS ports */ +#define VHCI_PORTS (VHCI_HC_PORTS*2) + #ifdef CONFIG_USBIP_VHCI_NR_HCS #define VHCI_NR_HCS CONFIG_USBIP_VHCI_NR_HCS #else @@ -145,7 +148,7 @@ static inline __u32 port_to_rhport(__u32 port) static inline int port_to_pdev_nr(__u32 port) { - return port / (VHCI_HC_PORTS * 2); + return port / VHCI_PORTS; } static inline struct vhci_hcd *hcd_to_vhci_hcd(struct usb_hcd *hcd) diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c index 3ad68ff..5778b64 100644 --- a/drivers/usb/usbip/vhci_sysfs.c +++ b/drivers/usb/usbip/vhci_sysfs.c @@ -92,7 +92,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out) spin_lock(&vdev->ud.lock); port_show_vhci(&out, HUB_SPEED_HIGH, - pdev_nr * VHCI_HC_PORTS * 2 + i, vdev); + pdev_nr * VHCI_PORTS + i, vdev); spin_unlock(&vdev->ud.lock); } @@ -101,7 +101,7 @@ static ssize_t status_show_vhci(int pdev_nr, char *out) spin_lock(&vdev->ud.lock); port_show_vhci(&out, HUB_SPEED_SUPER, - pdev_nr * VHCI_HC_PORTS * 2 + VHCI_HC_PORTS + i, vdev); + pdev_nr * VHCI_PORTS + VHCI_HC_PORTS + i, vdev); spin_unlock(&vdev->ud.lock); } @@ -117,7 +117,7 @@ static ssize_t status_show_not_ready(int pdev_nr, char *out) for (i = 0; i < VHCI_HC_PORTS; i++) { out += sprintf(out, "hs %04u %03u ", - (pdev_nr * VHCI_HC_PORTS * 2) + i, + (pdev_nr * VHCI_PORTS) + i, VDEV_ST_NOTASSIGNED); out += sprintf(out, "000 0-0"); out += sprintf(out, "\n"); @@ -125,7 +125,7 @@ static ssize_t status_show_not_ready(int pdev_nr, char *out) for (i = 0; i < VHCI_HC_PORTS; i++) { out += sprintf(out, "ss %04u %03u ", - (pdev_nr * VHCI_HC_PORTS * 2) + VHCI_HC_PORTS + i, + (pdev_nr * VHCI_PORTS) + VHCI_HC_PORTS + i, VDEV_ST_NOTASSIGNED); out += sprintf(out, "000 0-0"); out += sprintf(out, "\n"); @@ -176,7 +176,7 @@ static ssize_t nports_show(struct device *dev, struct device_attribute *attr, /* * Half the ports are for SPEED_HIGH and half for SPEED_SUPER, thus the * 2. */ - out += sprintf(out, "%d\n", VHCI_HC_PORTS * vhci_num_controllers * 2); + out += sprintf(out, "%d\n", VHCI_PORTS * vhci_num_controllers); return out - s; } static DEVICE_ATTR_RO(nports); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/9] usbip: vhci-hcd: Rename function names to reflect their struct names
From: Yuyang Du These helper function names are renamed to have their full struct names to avoid confusion: - hcd_to_vhci() -> hcd_to_vhci_hcd() - vhci_to_hcd() -> vhci_hcd_to_hcd() - vdev_to_vhci() -> vdev_to_vhci_hcd() Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci.h | 11 +-- drivers/usb/usbip/vhci_hcd.c | 34 +- drivers/usb/usbip/vhci_rx.c| 12 ++-- drivers/usb/usbip/vhci_sysfs.c | 6 +++--- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h index 88b71c4..bff472f 100644 --- a/drivers/usb/usbip/vhci.h +++ b/drivers/usb/usbip/vhci.h @@ -134,7 +134,7 @@ static inline int port_to_pdev_nr(__u32 port) return port / VHCI_HC_PORTS; } -static inline struct vhci_hcd *hcd_to_vhci(struct usb_hcd *hcd) +static inline struct vhci_hcd *hcd_to_vhci_hcd(struct usb_hcd *hcd) { return (struct vhci_hcd *) (hcd->hcd_priv); } @@ -149,15 +149,14 @@ static inline const char *hcd_name(struct usb_hcd *hcd) return (hcd)->self.bus_name; } -static inline struct usb_hcd *vhci_to_hcd(struct vhci_hcd *vhci) +static inline struct usb_hcd *vhci_hcd_to_hcd(struct vhci_hcd *vhci_hcd) { - return container_of((void *) vhci, struct usb_hcd, hcd_priv); + return container_of((void *) vhci_hcd, struct usb_hcd, hcd_priv); } -static inline struct vhci_hcd *vdev_to_vhci(struct vhci_device *vdev) +static inline struct vhci_hcd *vdev_to_vhci_hcd(struct vhci_device *vdev) { - return container_of( - (void *)(vdev - vdev->rhport), struct vhci_hcd, vdev); + return container_of((void *)(vdev - vdev->rhport), struct vhci_hcd, vdev); } #endif /* __USBIP_VHCI_H */ diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 5d8b2c2..624265a 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -124,7 +124,7 @@ static void dump_port_status_diff(u32 prev_status, u32 new_status) void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed) { - struct vhci_hcd *vhci = vdev_to_vhci(vdev); + struct vhci_hcd *vhci = vdev_to_vhci_hcd(vdev); int rhport = vdev->rhport; u32 status; unsigned long flags; @@ -152,12 +152,12 @@ void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed) spin_unlock_irqrestore(&vhci->lock, flags); - usb_hcd_poll_rh_status(vhci_to_hcd(vhci)); + usb_hcd_poll_rh_status(vhci_hcd_to_hcd(vhci)); } static void rh_port_disconnect(struct vhci_device *vdev) { - struct vhci_hcd *vhci = vdev_to_vhci(vdev); + struct vhci_hcd *vhci = vdev_to_vhci_hcd(vdev); int rhport = vdev->rhport; u32 status; unsigned long flags; @@ -174,7 +174,7 @@ static void rh_port_disconnect(struct vhci_device *vdev) vhci->port_status[rhport] = status; spin_unlock_irqrestore(&vhci->lock, flags); - usb_hcd_poll_rh_status(vhci_to_hcd(vhci)); + usb_hcd_poll_rh_status(vhci_hcd_to_hcd(vhci)); } #define PORT_C_MASK\ @@ -206,7 +206,7 @@ static int vhci_hub_status(struct usb_hcd *hcd, char *buf) retval = DIV_ROUND_UP(VHCI_HC_PORTS + 1, 8); memset(buf, 0, retval); - vhci = hcd_to_vhci(hcd); + vhci = hcd_to_vhci_hcd(hcd); spin_lock_irqsave(&vhci->lock, flags); if (!HCD_HW_ACCESSIBLE(hcd)) { @@ -268,7 +268,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, pr_err("invalid port number %d\n", wIndex); rhport = ((__u8)(wIndex & 0x00ff)) - 1; - dum = hcd_to_vhci(hcd); + dum = hcd_to_vhci_hcd(hcd); spin_lock_irqsave(&dum->lock, flags); @@ -440,7 +440,7 @@ static void vhci_tx_urb(struct urb *urb, struct vhci_device *vdev) pr_err("could not get virtual device"); return; } - vhci = vdev_to_vhci(vdev); + vhci = vdev_to_vhci_hcd(vdev); priv = kzalloc(sizeof(struct vhci_priv), GFP_ATOMIC); if (!priv) { @@ -468,7 +468,7 @@ static void vhci_tx_urb(struct urb *urb, struct vhci_device *vdev) static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) { - struct vhci_hcd *vhci = hcd_to_vhci(hcd); + struct vhci_hcd *vhci = hcd_to_vhci_hcd(hcd); struct device *dev = &urb->dev->dev; u8 portnum = urb->dev->portnum; int ret = 0; @@ -635,7 +635,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, */ static int vhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) { - struct vhci_hcd *vhci = hcd_to_vhci(hcd); + struct vhci_hcd *vhci = hcd_to_vhci_hcd(hcd); struct vhci_priv *priv; struct vhci_device *vdev; unsigned long flags; @@ -686,7 +686,7
[PATCH v2 8/9] usbip: vhci-hcd: Add USB3 port status bits
From: Yuyang Du As USB3 has (slightly) different bit meanings in the port status. Add a new status bit array for USB3. Signed-off-by: Yuyang Du --- drivers/usb/usbip/vhci_hcd.c | 56 +++- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 43cacbc..a71254b 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -66,7 +66,7 @@ static const char * const bit_desc[] = { "SUSPEND", /*2*/ "OVER_CURRENT", /*3*/ "RESET",/*4*/ - "R5", /*5*/ + "L1", /*5*/ "R6", /*6*/ "R7", /*7*/ "POWER",/*8*/ @@ -82,7 +82,7 @@ static const char * const bit_desc[] = { "C_SUSPEND",/*18*/ "C_OVER_CURRENT", /*19*/ "C_RESET", /*20*/ - "R21", /*21*/ + "C_L1", /*21*/ "R22", /*22*/ "R23", /*23*/ "R24", /*24*/ @@ -95,10 +95,49 @@ static const char * const bit_desc[] = { "R31", /*31*/ }; -static void dump_port_status_diff(u32 prev_status, u32 new_status) +static const char * const bit_desc_ss[] = { + "CONNECTION", /*0*/ + "ENABLE", /*1*/ + "SUSPEND", /*2*/ + "OVER_CURRENT", /*3*/ + "RESET",/*4*/ + "L1", /*5*/ + "R6", /*6*/ + "R7", /*7*/ + "R8", /*8*/ + "POWER",/*9*/ + "HIGHSPEED",/*10*/ + "PORT_TEST",/*11*/ + "INDICATOR",/*12*/ + "R13", /*13*/ + "R14", /*14*/ + "R15", /*15*/ + "C_CONNECTION", /*16*/ + "C_ENABLE", /*17*/ + "C_SUSPEND",/*18*/ + "C_OVER_CURRENT", /*19*/ + "C_RESET", /*20*/ + "C_BH_RESET", /*21*/ + "C_LINK_STATE", /*22*/ + "C_CONFIG_ERROR", /*23*/ + "R24", /*24*/ + "R25", /*25*/ + "R26", /*26*/ + "R27", /*27*/ + "R28", /*28*/ + "R29", /*29*/ + "R30", /*30*/ + "R31", /*31*/ +}; + +static void dump_port_status_diff(u32 prev_status, u32 new_status, bool usb3) { int i = 0; u32 bit = 1; + const char * const *desc = bit_desc; + + if (usb3) + desc = bit_desc_ss; pr_debug("status prev -> new: %08x -> %08x\n", prev_status, new_status); while (bit) { @@ -113,8 +152,12 @@ static void dump_port_status_diff(u32 prev_status, u32 new_status) else change = ' '; - if (prev || new) - pr_debug(" %c%s\n", change, bit_desc[i]); + if (prev || new) { + pr_debug(" %c%s\n", change, desc[i]); + + if (bit == 1) /* USB_PORT_STAT_CONNECTION */ + pr_debug(" %c%s\n", change, "USB_PORT_STAT_SPEED_5GBPS"); + } bit <<= 1; i++; } @@ -563,7 +606,8 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, /* Only dump valid port status */ if (rhport >= 0) { dump_port_status_diff(prev_port_status[rhport], - vhci_hcd->port_status[rhport]); + vhci_hcd->port_status[rhport], + hcd->speed == HCD_USB3); } } usbip_dbg_vhci_rh(" bye\n"); -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend v3 1/4] usb: usbip tool: Check the return of get_nports()
Hello! On 05/18/2017 12:50 PM, Yuyang Du wrote: From: Yuyang Du If we get nonpositive number of ports, there is no sense to Negative? continue, then fail gracefully. In addition, the commit 0775a9cbc694e8c72 ("usbip: vhci extension: modifications to vhci driver") introduced configurable numbers of controllers and ports, but we have a static port number maximum, MAXNPORT. If exceeded, the idev array will be overflown. We fix it by validating the nports to make sure the port number max is not exceeded. Reviewed-by: Krzysztof Opasiak Signed-off-by: Yuyang Du Acked-by: Shuah Khan --- tools/usb/usbip/libsrc/vhci_driver.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index f659c14..151580a 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -220,9 +220,17 @@ int usbip_vhci_driver_open(void) } vhci_driver->nports = get_nports(); - dbg("available ports: %d", vhci_driver->nports); + if (vhci_driver->nports <=0) { Please add a space after <= too. [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/9] dmaengine: omap-dma: port_window support correction for both direction
When the port_window support was verified it was done on setup where only the MEM_TO_DEV direction was enabled. This got un-noticed and thus only this direction worked. Now that I have managed to get a setup to verify both direction it turned out that the setup was incorrect: omap_desc members are settings for the slave port while the omap_sg members apply to the memory side of the sDMA setup. Fixes: 527a27591312 ("dmaengine: omap-dma: Fix the port_window support") Signed-off-by: Peter Ujfalusi Cc: Russell King Cc: dmaeng...@vger.kernel.org Cc: dan.j.willi...@intel.com Cc: vinod.k...@intel.com Tested-by: Tony Lindgren Acked-by: Vinod Koul --- drivers/dma/omap-dma.c | 39 +++ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c index daf479cce691..8c1665c8fe33 100644 --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -916,12 +916,6 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg( return NULL; } - /* When the port_window is used, one frame must cover the window */ - if (port_window) { - burst = port_window; - port_window_bytes = port_window * es_bytes[es]; - } - /* Now allocate and setup the descriptor. */ d = kzalloc(sizeof(*d) + sglen * sizeof(d->sg[0]), GFP_ATOMIC); if (!d) @@ -931,6 +925,21 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg( d->dev_addr = dev_addr; d->es = es; + /* When the port_window is used, one frame must cover the window */ + if (port_window) { + burst = port_window; + port_window_bytes = port_window * es_bytes[es]; + + d->ei = 1; + /* +* One frame covers the port_window and by configure +* the source frame index to be -1 * (port_window - 1) +* we instruct the sDMA that after a frame is processed +* it should move back to the start of the window. +*/ + d->fi = -(port_window_bytes - 1); + } + d->ccr = c->ccr | CCR_SYNC_FRAME; if (dir == DMA_DEV_TO_MEM) { d->csdp = CSDP_DST_BURST_64 | CSDP_DST_PACKED; @@ -955,14 +964,6 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg( d->ccr |= CCR_SRC_AMODE_POSTINC; if (port_window) { d->ccr |= CCR_DST_AMODE_DBLIDX; - d->ei = 1; - /* -* One frame covers the port_window and by configure -* the source frame index to be -1 * (port_window - 1) -* we instruct the sDMA that after a frame is processed -* it should move back to the start of the window. -*/ - d->fi = -(port_window_bytes - 1); if (port_window_bytes >= 64) d->csdp |= CSDP_DST_BURST_64; @@ -1018,16 +1019,6 @@ static struct dma_async_tx_descriptor *omap_dma_prep_slave_sg( osg->addr = sg_dma_address(sgent); osg->en = en; osg->fn = sg_dma_len(sgent) / frame_bytes; - if (port_window && dir == DMA_DEV_TO_MEM) { - osg->ei = 1; - /* -* One frame covers the port_window and by configure -* the source frame index to be -1 * (port_window - 1) -* we instruct the sDMA that after a frame is processed -* it should move back to the start of the window. -*/ - osg->fi = -(port_window_bytes - 1); - } if (d->using_ll) { osg->t2_desc = dma_pool_alloc(od->desc_pool, GFP_ATOMIC, -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 9/9] usb: musb: tusb6010_omap: Convert to DMAengine API
With the port_window support in DMAengine and the sDMA driver we can convert the driver to DMAengine. Signed-off-by: Peter Ujfalusi Tested-by: Tony Lindgren --- drivers/usb/musb/tusb6010_omap.c | 201 --- 1 file changed, 80 insertions(+), 121 deletions(-) diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c index 1c4592d753bf..e8060e49b0f4 100644 --- a/drivers/usb/musb/tusb6010_omap.c +++ b/drivers/usb/musb/tusb6010_omap.c @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include "musb_core.h" #include "tusb6010.h" @@ -24,17 +24,9 @@ #define MAX_DMAREQ 5 /* REVISIT: Really 6, but req5 not OK */ -#define OMAP24XX_DMA_EXT_DMAREQ0 2 -#define OMAP24XX_DMA_EXT_DMAREQ1 3 -#define OMAP242X_DMA_EXT_DMAREQ2 14 -#define OMAP242X_DMA_EXT_DMAREQ3 15 -#define OMAP242X_DMA_EXT_DMAREQ4 16 -#define OMAP242X_DMA_EXT_DMAREQ5 64 - struct tusb_dma_data { - int ch; s8 dmareq; - s8 sync_dev; + struct dma_chan *chan; }; struct tusb_omap_dma_ch { @@ -105,7 +97,7 @@ static inline void tusb_omap_free_shared_dmareq(struct tusb_omap_dma_ch *chdat) * See also musb_dma_completion in plat_uds.c and musb_g_[tx|rx]() in * musb_gadget.c. */ -static void tusb_omap_dma_cb(int lch, u16 ch_status, void *data) +static void tusb_omap_dma_cb(void *data) { struct dma_channel *channel = (struct dma_channel *)data; struct tusb_omap_dma_ch *chdat = to_chdat(channel); @@ -116,18 +108,11 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void *data) void __iomem*ep_conf = hw_ep->conf; void __iomem*mbase = musb->mregs; unsigned long remaining, flags, pio; - int ch; spin_lock_irqsave(&musb->lock, flags); - ch = chdat->dma_data->ch; - - if (ch_status != OMAP_DMA_BLOCK_IRQ) - printk(KERN_ERR "TUSB DMA error status: %i\n", ch_status); - - dev_dbg(musb->controller, "ep%i %s dma callback ch: %i status: %x\n", - chdat->epnum, chdat->tx ? "tx" : "rx", - ch, ch_status); + dev_dbg(musb->controller, "ep%i %s dma callback\n", + chdat->epnum, chdat->tx ? "tx" : "rx"); if (chdat->tx) remaining = musb_readl(ep_conf, TUSB_EP_TX_OFFSET); @@ -138,8 +123,8 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void *data) /* HW issue #10: XFR_SIZE may get corrupt on DMA (both async & sync) */ if (unlikely(remaining > chdat->transfer_len)) { - dev_dbg(musb->controller, "Corrupt %s dma ch%i XFR_SIZE: 0x%08lx\n", - chdat->tx ? "tx" : "rx", ch, remaining); + dev_dbg(musb->controller, "Corrupt %s XFR_SIZE: 0x%08lx\n", + chdat->tx ? "tx" : "rx", remaining); remaining = 0; } @@ -206,13 +191,16 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, struct musb_hw_ep *hw_ep = chdat->hw_ep; void __iomem*mbase = musb->mregs; void __iomem*ep_conf = hw_ep->conf; - dma_addr_t fifo = hw_ep->fifo_sync; - struct omap_dma_channel_params dma_params; + dma_addr_t fifo_addr = hw_ep->fifo_sync; u32 dma_remaining; - int src_burst, dst_burst; u16 csr; u32 psize; struct tusb_dma_data*dma_data; + struct dma_async_tx_descriptor *dma_desc; + struct dma_slave_config dma_cfg; + enum dma_transfer_direction dma_dir; + u32 port_window; + int ret; if (unlikely(dma_addr & 0x1) || (len < 32) || (len > packet_sz)) return false; @@ -238,10 +226,8 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, dma_remaining = TUSB_EP_CONFIG_XFR_SIZE(dma_remaining); if (dma_remaining) { - dev_dbg(musb->controller, "Busy %s dma ch%i, not using: %08x\n", - chdat->tx ? "tx" : "rx", - chdat->dma_data ? chdat->dma_data->ch : -1, - dma_remaining); + dev_dbg(musb->controller, "Busy %s dma, not using: %08x\n", + chdat->tx ? "tx" : "rx", dma_remaining); return false; } @@ -258,7 +244,7 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, dev_dbg(musb->controller, "could not get dma for ep%i\n", chdat->epnum); return false;
[PATCH v4 0/9] usb: musb: tusb6010_omap: Convert to DMAengine
Hi, Changes since v3: - typos in commit message of patch 3 and 5 fixed - long line fixed in patch 5 - Ack from Vinod is added to the first patch - The series depends on: http://marc.info/?l=linux-omap&m=149459699415599&w=2 Changes since v2: - patch 5 from the v1 has been sent separately (usb: musb: tusb6010_omap: Do not reset the other direction's packet size) - Added Tested-by from Tony. Changes since v1: - Fix the port_window support in omap-dma DMAengine driver - MUSB_G_NO_SKB_RESERVE quirk flag addition to msub core - packet size corruption fix for tusb6010 - Handle DMA completion for TX also in the DMA callback The v1 series was tested with g_cdc where only the DMA was only enabled for TX and because of that I have not noticed that the sDMA code was not correct for RX, it only worked for TX case. The ASYNC mode of tusb6010 is really unstable, we get corrupted TX/RX offset register quite easily, but the SYNC mode is stable. The series was tested on top of next-20170510 with g_ncm module since with this we can use the quirk to avoid skb_reserve and get properly aligned buffers for DMA. The n810 is using nfsroot. The device would not boot to prompt most of the time before patch 5 (packet size reset fix). With that patch in, the device would boot up fine most of the cases, but will fail pretty fast with my stress test [1]. After the first 9 patch the legacy DMA mode is going to be stable with g_ncm, it boots to prompt, and survives the stress test [1]. The last patch is going the DMAengine conversion and I have run the stress test against it over 3 hours (ping-test.sh wrap count is 139). [1] Running these in parallel: ping -f 192.168.0.2 ping -f -s 2048 192.168.0.2 ping-test.sh 192.168.0.2 1 and (nfsroot) time to time: scp root@192.168.0.1:/usr/portage/distfiles/thunderbird-52.1.0.source.tar.xz / $ ls -alh /usr/portage/distfiles/thunderbird-52.1.0.source.tar.xz 218M Apr 30 15:46 /usr/portage/distfiles/thunderbird-52.1.0.source.tar.xz In essence copy 218M from my host back to the host. ping-test.sh (modified version from Tony to show the wrap count): #!/bin/bash device=$1 size=$2 wraps=0 while [ 1 ]; do #echo "Pinging with size $size" if ! ping -w0 -c1 -s$size $device > /dev/null 2>&1; then break; fi size=$(expr $size + 1) if [ $size -gt 8192 ]; then wraps=$(expr $wraps + 1) echo "wrapping ($wraps) at $size" size=1 fi done echo "Test ran up to $size" Regards, Peter CC: dmaeng...@vger.kernel.org I only send the cover letter and the DMAengine patch for the dmaengine list, the rest can be checked - if there is interest - via lkml --- Peter Ujfalusi (9): dmaengine: omap-dma: port_window support correction for both direction usb: musb: Add quirk to avoid skb reserve in gadget mode usb: musb: tusb6010: Add MUSB_G_NO_SKB_RESERVE to quirks usb: musb: tusb6010_omap: Use one musb_ep_select call in tusb_omap_dma_program usb: musb: tusb6010_omap: Create new struct for DMA data/parameters usb: musb: tusb6010_omap: Allocate DMA channels upfront usb: musb: tusb6010: Handle DMA TX completion in DMA callback as well ARM: OMAP2+: DMA: Add slave map entries for 24xx external request lines usb: musb: tusb6010_omap: Convert to DMAengine API arch/arm/mach-omap2/dma.c| 24 +++ drivers/dma/omap-dma.c | 39 ++-- drivers/usb/musb/musb_core.c | 3 + drivers/usb/musb/musb_core.h | 1 + drivers/usb/musb/tusb6010.c | 21 +-- drivers/usb/musb/tusb6010_omap.c | 379 --- 6 files changed, 203 insertions(+), 264 deletions(-) -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 7/9] usb: musb: tusb6010: Handle DMA TX completion in DMA callback as well
Handle the DMA TX in a similar way as we do for the RX: in the DMA completion callback. Since we are no longer using DMA completion interrupt for the TX we can as wall keep these interrupts disabled, but keep the handler for debug purposes. Signed-off-by: Peter Ujfalusi Tested-by: Tony Lindgren --- drivers/usb/musb/tusb6010.c | 18 +++--- drivers/usb/musb/tusb6010_omap.c | 34 +- 2 files changed, 4 insertions(+), 48 deletions(-) diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c index 4253bfb22043..4eb640c54f2c 100644 --- a/drivers/usb/musb/tusb6010.c +++ b/drivers/usb/musb/tusb6010.c @@ -881,26 +881,14 @@ static irqreturn_t tusb_musb_interrupt(int irq, void *__hci) | TUSB_INT_SRC_ID_STATUS_CHNG)) idle_timeout = tusb_otg_ints(musb, int_src, tbase); - /* TX dma callback must be handled here, RX dma callback is -* handled in tusb_omap_dma_cb. + /* +* Just clear the DMA interrupt if it comes as the completion for both +* TX and RX is handled by the DMA callback in tusb6010_omap */ if ((int_src & TUSB_INT_SRC_TXRX_DMA_DONE)) { u32 dma_src = musb_readl(tbase, TUSB_DMA_INT_SRC); - u32 real_dma_src = musb_readl(tbase, TUSB_DMA_INT_MASK); dev_dbg(musb->controller, "DMA IRQ %08x\n", dma_src); - real_dma_src = ~real_dma_src & dma_src; - if (tusb_dma_omap(musb) && real_dma_src) { - int tx_source = (real_dma_src & 0x); - int i; - - for (i = 1; i <= 15; i++) { - if (tx_source & (1 << i)) { - dev_dbg(musb->controller, "completing ep%i %s\n", i, "tx"); - musb_dma_completion(musb, i, 1); - } - } - } musb_writel(tbase, TUSB_DMA_INT_CLEAR, dma_src); } diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c index f8671110168b..1c4592d753bf 100644 --- a/drivers/usb/musb/tusb6010_omap.c +++ b/drivers/usb/musb/tusb6010_omap.c @@ -173,13 +173,7 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void *data) channel->status = MUSB_DMA_STATUS_FREE; - /* Handle only RX callbacks here. TX callbacks must be handled based -* on the TUSB DMA status interrupt. -* REVISIT: Use both TUSB DMA status interrupt and OMAP DMA callback -* interrupt for RX and TX. -*/ - if (!chdat->tx) - musb_dma_completion(musb, chdat->epnum, chdat->tx); + musb_dma_completion(musb, chdat->epnum, chdat->tx); /* We must terminate short tx transfers manually by setting TXPKTRDY. * REVISIT: This same problem may occur with other MUSB dma as well. @@ -464,22 +458,12 @@ tusb_omap_dma_allocate(struct dma_controller *c, int ret, i; struct tusb_omap_dma*tusb_dma; struct musb *musb; - void __iomem*tbase; struct dma_channel *channel = NULL; struct tusb_omap_dma_ch *chdat = NULL; struct tusb_dma_data*dma_data = NULL; - u32 reg; tusb_dma = container_of(c, struct tusb_omap_dma, controller); musb = tusb_dma->controller.musb; - tbase = musb->ctrl_base; - - reg = musb_readl(tbase, TUSB_DMA_INT_MASK); - if (tx) - reg &= ~(1 << hw_ep->epnum); - else - reg &= ~(1 << (hw_ep->epnum + 15)); - musb_writel(tbase, TUSB_DMA_INT_MASK, reg); /* REVISIT: Why does dmareq5 not work? */ if (hw_ep->epnum == 0) { @@ -548,26 +532,10 @@ static void tusb_omap_dma_release(struct dma_channel *channel) { struct tusb_omap_dma_ch *chdat = to_chdat(channel); struct musb *musb = chdat->musb; - void __iomem*tbase = musb->ctrl_base; - u32 reg; dev_dbg(musb->controller, "ep%i ch%i\n", chdat->epnum, chdat->dma_data->ch); - reg = musb_readl(tbase, TUSB_DMA_INT_MASK); - if (chdat->tx) - reg |= (1 << chdat->epnum); - else - reg |= (1 << (chdat->epnum + 15)); - musb_writel(tbase, TUSB_DMA_INT_MASK, reg); - - reg = musb_readl(tbase, TUSB_DMA_INT_CLEAR); - if (chdat->tx) - reg |= (1 << chdat->epnum); - else - reg |= (1 << (chdat->epnum + 15)); - musb_writel(tbase, TUSB_DMA_INT_CLEAR, reg); - channel->status = MUSB_DMA_STATUS_UNKNOWN; omap_stop_dma(chdat->dma_data->ch); -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://v
[PATCH v4 8/9] ARM: OMAP2+: DMA: Add slave map entries for 24xx external request lines
The external request lines are used by tusb6010 on OMAP24xx platforms. Update the map so the driver can use dmaengine API to request the DMA channel. At the same time add temporary map containing only the external DMA request numbers for DT booted case on omap24xx since the tusb6010 stack is not yet supports DT boot. Signed-off-by: Peter Ujfalusi Acked-by: Tony Lindgren --- arch/arm/mach-omap2/dma.c | 24 1 file changed, 24 insertions(+) diff --git a/arch/arm/mach-omap2/dma.c b/arch/arm/mach-omap2/dma.c index e58c13a9bea5..0b77a0176018 100644 --- a/arch/arm/mach-omap2/dma.c +++ b/arch/arm/mach-omap2/dma.c @@ -249,6 +249,24 @@ static const struct dma_slave_map omap24xx_sdma_map[] = { { "omap_uart.2", "rx", SDMA_FILTER_PARAM(54) }, { "omap_hsmmc.0", "tx", SDMA_FILTER_PARAM(61) }, { "omap_hsmmc.0", "rx", SDMA_FILTER_PARAM(62) }, + + /* external DMA requests when tusb6010 is used */ + { "musb-tusb", "dmareq0", SDMA_FILTER_PARAM(2) }, + { "musb-tusb", "dmareq1", SDMA_FILTER_PARAM(3) }, + { "musb-tusb", "dmareq2", SDMA_FILTER_PARAM(14) }, /* OMAP2420 only */ + { "musb-tusb", "dmareq3", SDMA_FILTER_PARAM(15) }, /* OMAP2420 only */ + { "musb-tusb", "dmareq4", SDMA_FILTER_PARAM(16) }, /* OMAP2420 only */ + { "musb-tusb", "dmareq5", SDMA_FILTER_PARAM(64) }, /* OMAP2420 only */ +}; + +static const struct dma_slave_map omap24xx_sdma_dt_map[] = { + /* external DMA requests when tusb6010 is used */ + { "musb-hdrc.1.auto", "dmareq0", SDMA_FILTER_PARAM(2) }, + { "musb-hdrc.1.auto", "dmareq1", SDMA_FILTER_PARAM(3) }, + { "musb-hdrc.1.auto", "dmareq2", SDMA_FILTER_PARAM(14) }, /* OMAP2420 only */ + { "musb-hdrc.1.auto", "dmareq3", SDMA_FILTER_PARAM(15) }, /* OMAP2420 only */ + { "musb-hdrc.1.auto", "dmareq4", SDMA_FILTER_PARAM(16) }, /* OMAP2420 only */ + { "musb-hdrc.1.auto", "dmareq5", SDMA_FILTER_PARAM(64) }, /* OMAP2420 only */ }; static const struct dma_slave_map omap3xxx_sdma_map[] = { @@ -346,6 +364,12 @@ static int __init omap2_system_dma_init_dev(struct omap_hwmod *oh, void *unused) __func__); return -ENODEV; } + } else { + if (soc_is_omap24xx()) { + /* DMA slave map for drivers not yet converted to DT */ + p.slave_map = omap24xx_sdma_dt_map; + p.slavecnt = ARRAY_SIZE(omap24xx_sdma_dt_map); + } } pdev = omap_device_build(name, 0, oh, &p, sizeof(p)); -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 6/9] usb: musb: tusb6010_omap: Allocate DMA channels upfront
Instead of requesting the DMA channel in tusb_omap_dma_allocate() do it when the controller is created and in runtime work from the DMA channel pool. This change is needed for the DMAengine conversion of the driver since the tusb_omap_dma_allocate() is called in interrupt context which might lead to lock within the DMAengine API when requesting channel. Signed-off-by: Peter Ujfalusi Tested-by: Tony Lindgren --- drivers/usb/musb/tusb6010_omap.c | 185 +++ 1 file changed, 92 insertions(+), 93 deletions(-) diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c index c2ef8e3eb91e..f8671110168b 100644 --- a/drivers/usb/musb/tusb6010_omap.c +++ b/drivers/usb/musb/tusb6010_omap.c @@ -45,7 +45,7 @@ struct tusb_omap_dma_ch { u8 tx; struct musb_hw_ep *hw_ep; - struct tusb_dma_datadma_data; + struct tusb_dma_data*dma_data; struct tusb_omap_dma*tusb_dma; @@ -62,7 +62,7 @@ struct tusb_omap_dma { struct dma_controller controller; void __iomem*tbase; - struct tusb_dma_datadma_data; + struct tusb_dma_datadma_pool[MAX_DMAREQ]; unsignedmultichannel:1; }; @@ -120,10 +120,7 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void *data) spin_lock_irqsave(&musb->lock, flags); - if (tusb_dma->multichannel) - ch = chdat->dma_data.ch; - else - ch = tusb_dma->dma_data.ch; + ch = chdat->dma_data->ch; if (ch_status != OMAP_DMA_BLOCK_IRQ) printk(KERN_ERR "TUSB DMA error status: %i\n", ch_status); @@ -248,7 +245,8 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, dma_remaining = TUSB_EP_CONFIG_XFR_SIZE(dma_remaining); if (dma_remaining) { dev_dbg(musb->controller, "Busy %s dma ch%i, not using: %08x\n", - chdat->tx ? "tx" : "rx", chdat->dma_data.ch, + chdat->tx ? "tx" : "rx", + chdat->dma_data ? chdat->dma_data->ch : -1, dma_remaining); return false; } @@ -260,11 +258,8 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, else chdat->transfer_packet_sz = packet_sz; - if (tusb_dma->multichannel) { - dma_data = &chdat->dma_data; - } else { - dma_data = &tusb_dma->dma_data; - + dma_data = chdat->dma_data; + if (!tusb_dma->multichannel) { if (tusb_omap_use_shared_dmareq(chdat) != 0) { dev_dbg(musb->controller, "could not get dma for ep%i\n", chdat->epnum); return false; @@ -276,10 +271,10 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, WARN_ON(1); return false; } - - omap_set_dma_callback(dma_data->ch, tusb_omap_dma_cb, channel); } + omap_set_dma_callback(dma_data->ch, tusb_omap_dma_cb, channel); + chdat->packet_sz = packet_sz; chdat->len = len; channel->actual_len = 0; @@ -410,19 +405,9 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, static int tusb_omap_dma_abort(struct dma_channel *channel) { struct tusb_omap_dma_ch *chdat = to_chdat(channel); - struct tusb_omap_dma*tusb_dma = chdat->tusb_dma; - struct tusb_dma_data*dma_data = &tusb_dma->dma_data; - if (!tusb_dma->multichannel) { - if (dma_data->ch >= 0) { - omap_stop_dma(dma_data->ch); - omap_free_dma(dma_data->ch); - dma_data->ch = -1; - } - - dma_data->dmareq = -1; - dma_data->sync_dev = -1; - } + if (chdat->dma_data) + omap_stop_dma(chdat->dma_data->ch); channel->status = MUSB_DMA_STATUS_FREE; @@ -434,15 +419,6 @@ static inline int tusb_omap_dma_allocate_dmareq(struct tusb_omap_dma_ch *chdat) u32 reg = musb_readl(chdat->tbase, TUSB_DMA_EP_MAP); int i, dmareq_nr = -1; - const int sync_dev[6] = { - OMAP24XX_DMA_EXT_DMAREQ0, - OMAP24XX_DMA_EXT_DMAREQ1, - OMAP242X_DMA_EXT_DMAREQ2, - OMAP242X_DMA_EXT_DMAREQ3, - OMAP242X_DMA_EXT_DMAREQ4, - OMAP242X_DMA_EXT_DMAREQ5, - }; - for (i = 0; i < MAX_DMAREQ; i++) { int cur = (reg & (0xf << (i * 5))) >> (i * 5); if (cur == 0) { @@ -459,8 +435,7 @@ static inline int tusb_omap_dma_allocate_dmareq(struct tusb_omap_dma_ch *chdat) reg |= ((1 << 4) << (dmareq_nr * 5)); musb_writel(chdat->
[PATCH v4 5/9] usb: musb: tusb6010_omap: Create new struct for DMA data/parameters
For the DMA we have ch (channel), dmareq and sync_dev parameters both within the tusb_omap_dma_ch and tusb_omap_dma struct. By creating a common struct the code can be simplified when selecting between the shared or multichannel DMA parameters. Signed-off-by: Peter Ujfalusi Tested-by: Tony Lindgren --- drivers/usb/musb/tusb6010_omap.c | 164 --- 1 file changed, 85 insertions(+), 79 deletions(-) diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c index 025b52e0b34d..c2ef8e3eb91e 100644 --- a/drivers/usb/musb/tusb6010_omap.c +++ b/drivers/usb/musb/tusb6010_omap.c @@ -31,6 +31,12 @@ #define OMAP242X_DMA_EXT_DMAREQ4 16 #define OMAP242X_DMA_EXT_DMAREQ5 64 +struct tusb_dma_data { + int ch; + s8 dmareq; + s8 sync_dev; +}; + struct tusb_omap_dma_ch { struct musb *musb; void __iomem*tbase; @@ -39,9 +45,7 @@ struct tusb_omap_dma_ch { u8 tx; struct musb_hw_ep *hw_ep; - int ch; - s8 dmareq; - s8 sync_dev; + struct tusb_dma_datadma_data; struct tusb_omap_dma*tusb_dma; @@ -58,9 +62,7 @@ struct tusb_omap_dma { struct dma_controller controller; void __iomem*tbase; - int ch; - s8 dmareq; - s8 sync_dev; + struct tusb_dma_datadma_data; unsignedmultichannel:1; }; @@ -119,9 +121,9 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void *data) spin_lock_irqsave(&musb->lock, flags); if (tusb_dma->multichannel) - ch = chdat->ch; + ch = chdat->dma_data.ch; else - ch = tusb_dma->ch; + ch = tusb_dma->dma_data.ch; if (ch_status != OMAP_DMA_BLOCK_IRQ) printk(KERN_ERR "TUSB DMA error status: %i\n", ch_status); @@ -140,8 +142,7 @@ static void tusb_omap_dma_cb(int lch, u16 ch_status, void *data) /* HW issue #10: XFR_SIZE may get corrupt on DMA (both async & sync) */ if (unlikely(remaining > chdat->transfer_len)) { dev_dbg(musb->controller, "Corrupt %s dma ch%i XFR_SIZE: 0x%08lx\n", - chdat->tx ? "tx" : "rx", chdat->ch, - remaining); + chdat->tx ? "tx" : "rx", ch, remaining); remaining = 0; } @@ -220,9 +221,7 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, int src_burst, dst_burst; u16 csr; u32 psize; - int ch; - s8 dmareq; - s8 sync_dev; + struct tusb_dma_data*dma_data; if (unlikely(dma_addr & 0x1) || (len < 32) || (len > packet_sz)) return false; @@ -249,7 +248,7 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, dma_remaining = TUSB_EP_CONFIG_XFR_SIZE(dma_remaining); if (dma_remaining) { dev_dbg(musb->controller, "Busy %s dma ch%i, not using: %08x\n", - chdat->tx ? "tx" : "rx", chdat->ch, + chdat->tx ? "tx" : "rx", chdat->dma_data.ch, dma_remaining); return false; } @@ -262,15 +261,15 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, chdat->transfer_packet_sz = packet_sz; if (tusb_dma->multichannel) { - ch = chdat->ch; - dmareq = chdat->dmareq; - sync_dev = chdat->sync_dev; + dma_data = &chdat->dma_data; } else { + dma_data = &tusb_dma->dma_data; + if (tusb_omap_use_shared_dmareq(chdat) != 0) { dev_dbg(musb->controller, "could not get dma for ep%i\n", chdat->epnum); return false; } - if (tusb_dma->ch < 0) { + if (dma_data->ch < 0) { /* REVISIT: This should get blocked earlier, happens * with MSC ErrorRecoveryTest */ @@ -278,10 +277,7 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, return false; } - ch = tusb_dma->ch; - dmareq = tusb_dma->dmareq; - sync_dev = tusb_dma->sync_dev; - omap_set_dma_callback(ch, tusb_omap_dma_cb, channel); + omap_set_dma_callback(dma
[PATCH v4 3/9] usb: musb: tusb6010: Add MUSB_G_NO_SKB_RESERVE to quirks
When using the g_ncm for networking this flag will make sure that the buffer is aligned to 32bit so the DMA can be used to offload the data movement. Signed-off-by: Peter Ujfalusi Tested-by: Tony Lindgren --- drivers/usb/musb/tusb6010.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c index e85cc8e4e7a9..4253bfb22043 100644 --- a/drivers/usb/musb/tusb6010.c +++ b/drivers/usb/musb/tusb6010.c @@ -1181,7 +1181,8 @@ static int tusb_musb_exit(struct musb *musb) } static const struct musb_platform_ops tusb_ops = { - .quirks = MUSB_DMA_TUSB_OMAP | MUSB_IN_TUSB, + .quirks = MUSB_DMA_TUSB_OMAP | MUSB_IN_TUSB | + MUSB_G_NO_SKB_RESERVE, .init = tusb_musb_init, .exit = tusb_musb_exit, -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/9] usb: musb: tusb6010_omap: Use one musb_ep_select call in tusb_omap_dma_program
Having one musb_ep_select() instead the two calls in if/else is the same thing, but makes the code a bit simpler to follow. Signed-off-by: Peter Ujfalusi Tested-by: Tony Lindgren --- drivers/usb/musb/tusb6010_omap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/musb/tusb6010_omap.c b/drivers/usb/musb/tusb6010_omap.c index 7870b37e0ea5..025b52e0b34d 100644 --- a/drivers/usb/musb/tusb6010_omap.c +++ b/drivers/usb/musb/tusb6010_omap.c @@ -368,15 +368,14 @@ static int tusb_omap_dma_program(struct dma_channel *channel, u16 packet_sz, /* * Prepare MUSB for DMA transfer */ + musb_ep_select(mbase, chdat->epnum); if (chdat->tx) { - musb_ep_select(mbase, chdat->epnum); csr = musb_readw(hw_ep->regs, MUSB_TXCSR); csr |= (MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAENAB | MUSB_TXCSR_DMAMODE | MUSB_TXCSR_MODE); csr &= ~MUSB_TXCSR_P_UNDERRUN; musb_writew(hw_ep->regs, MUSB_TXCSR, csr); } else { - musb_ep_select(mbase, chdat->epnum); csr = musb_readw(hw_ep->regs, MUSB_RXCSR); csr |= MUSB_RXCSR_DMAENAB; csr &= ~(MUSB_RXCSR_AUTOCLEAR | MUSB_RXCSR_DMAMODE); -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
resolving issue by: USB_AUTOSUSPEND=0
Issue can be resolved by setting: USB_AUTOSUSPEND=0 See details here: https://bugzilla.kernel.org/show_bug.cgi?id=117811 (also, isn't refusing HTML emails kind of backward in 2017, since HTML is default in most of the clients? You are effectively limiting general accessibility of your mailing list. Pity) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/9] usb: musb: Add quirk to avoid skb reserve in gadget mode
For tusb6010 the DMA functionality only possible if the buffer is 32bit aligned (SYNC access to FIFO) since with ASYNC access the TX/RX offset registers will corrupt eventually. The MUSB_G_NO_SKB_RESERVE will set the quirk_avoids_skb_reserve flag in usb_gadget struct to provide correctly aligned buffer. Signed-off-by: Peter Ujfalusi Tested-by: Tony Lindgren --- drivers/usb/musb/musb_core.c | 3 +++ drivers/usb/musb/musb_core.h | 1 + 2 files changed, 4 insertions(+) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 870da18f5077..87cbd56cc761 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -2224,6 +2224,9 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) musb->io.ep_select = musb_flat_ep_select; } + if (musb->io.quirks & MUSB_G_NO_SKB_RESERVE) + musb->g.quirk_avoids_skb_reserve = 1; + /* At least tusb6010 has its own offsets */ if (musb->ops->ep_offset) musb->io.ep_offset = musb->ops->ep_offset; diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index 3e98d4268a64..9f22c5b8ce37 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -172,6 +172,7 @@ struct musb_io; */ struct musb_platform_ops { +#define MUSB_G_NO_SKB_RESERVE BIT(9) #define MUSB_DA8XX BIT(8) #define MUSB_PRESERVE_SESSION BIT(7) #define MUSB_DMA_UX500 BIT(6) -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] musb fixes for v4.12-rc2
On Wed, May 17, 2017 at 11:23:09AM -0500, Bin Liu wrote: > Hi Greg, > > Here are the couple musb fixes for v4.12-rc2. One fixes an regression caused > in > the runtime PM refactoring in musb since v4.9, and the other fixes a register > cross-talk between rx and tx in tusb6010 since the beginning when the driver > was > merged. > > Please let me know if any change is needed. Any reason you didn't mark these for the stable tree? I've done so now... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] musb fixes for v4.12-rc2
On Thu, May 18, 2017 at 03:21:06PM +0200, Greg KH wrote: > On Wed, May 17, 2017 at 11:23:09AM -0500, Bin Liu wrote: > > Hi Greg, > > > > Here are the couple musb fixes for v4.12-rc2. One fixes an regression > > caused in > > the runtime PM refactoring in musb since v4.9, and the other fixes a > > register > > cross-talk between rx and tx in tusb6010 since the beginning when the > > driver was > > merged. > > > > Please let me know if any change is needed. > > Any reason you didn't mark these for the stable tree? I've done so > now... Sorry, I should. I guess I confused myself about how to handle Cc:stable this time due to the patch I sent few days ago ("usb: musb: dsps: keep VBUS on for host-only mode") which will be for -rc3, it requires changes to be able to apply to v4.10 and older. Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: add quirk flag for broken stop command on AMD platforms
On Thu, 18 May 2017, Shyam Sundar S K wrote: > on AMD platforms with SNPS 3.1 USB controller has an issue > if the stop EP command is issued when the controller is not > in running state. If issued, it is leading to a critical RTL bug > because of which controller goes into irrecoverable state. > > This patch adds a appropriate checks to make sure that scenario > does not happen. > > Signed-off-by: Shyam Sundar S K > Signed-off-by: Nehal Shah > --- > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -1819,6 +1819,7 @@ struct xhci_hcd { > /* For controller with a broken Port Disable implementation */ > #define XHCI_BROKEN_PORT_PED (1 << 25) > #define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26) > +#define XHCI_BROKEN_STOP (1 << 27) Does there really need to be a quirk flag for this? I should think that you never want to issue a STOP EP command while the controller is not running, no matter what kind of controller it is. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] usb: host: ohci-omap3: Print warning to get people to use ohci-platform
* Sebastian Reichel [170518 02:18]: > On Wed, May 17, 2017 at 03:59:21PM -0700, Tony Lindgren wrote: > > We can't just remove ohci-omap3 as that could make some people's > > systems unusable for keyboard and mouse. Let's print a warning for > > now, and then remove the driver in a year or so. > > I guess you could drop the driver and modify the Kconfig entry, > so that it selects USB_OHCI_HCD_PLATFORM? I just checked and > there are already a few examples of this in the USB host Kconfig :) Right but there's still nothing telling people to load ohci-platform instead of ohci-omap3 if we just remove ohci-omap3 without any warnings. I don't think there's any need to rush to remove ohci-omap3. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: resolving issue by: USB_AUTOSUSPEND=0
On Thu, May 18, 2017 at 03:14:56PM +0200, Daniel Duris wrote: > Issue can be resolved by setting: > > USB_AUTOSUSPEND=0 What issue? > See details here: > > https://bugzilla.kernel.org/show_bug.cgi?id=117811 I don't understand, is a patch needed? > (also, isn't refusing HTML emails kind of backward in 2017, since HTML > is default in most of the clients? You are effectively limiting general > accessibility of your mailing list. Pity) Not at all, everyone can send non-html email. Not everyone can _read_ html email :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: resolving issue by: USB_AUTOSUSPEND=0
This issue that I have reported earlier - makes use of any external adapaters pain the ass as it often ends with USB "hanging" and it need sto be disconnected/reconnected to make it work again: repeating every few minutes and this causes network restarts (if ping is running in the background, this does not happen!): [49880.487900] usb 4-1.4: USB disconnect, device number 17 [49880.643818] usb 4-1.4: new SuperSpeed USB device number 18 using xhci_hcd [49880.664295] usb 4-1.4: New USB device found, idVendor=0bda, idProduct=8153 [49880.664296] usb 4-1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=6 [49880.664297] usb 4-1.4: Product: USB 10/100/1000 LAN [49880.664298] usb 4-1.4: Manufacturer: Realtek [49880.664299] usb 4-1.4: SerialNumber: C000 [49880.752076] usb 4-1.4: reset SuperSpeed USB device number 18 using xhci_hcd [49880.828613] r8152 4-1.4:1.0 eth0: v1.08.9 [49882.400139] r8152 4-1.4:1.0 enx00249b17d68f: renamed from eth0 [49882.425090] IPv6: ADDRCONF(NETDEV_UP): enx00249b17d68f: link is not ready [49882.458113] IPv6: ADDRCONF(NETDEV_UP): enx00249b17d68f: link is not ready [49883.377629] r8152 4-1.4:1.0 enx00249b17d68f: carrier on [49883.377674] IPv6: ADDRCONF(NETDEV_CHANGE): enx00249b17d68f: link becomes ready [49961.583838] [drm] Atomic update on pipe (A) took 162 us, max time under evasion is 100 us [50076.649389] usb 4-1.4: usb_reset_and_verify_device Failed to disable LTM . [50076.826678] usb 4-1.4: USB disconnect, device number 18 [50076.986626] usb 4-1.4: new SuperSpeed USB device number 19 using xhci_hcd [50077.006980] usb 4-1.4: New USB device found, idVendor=0bda, idProduct=8153 [50077.006982] usb 4-1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=6 [50077.006983] usb 4-1.4: Product: USB 10/100/1000 LAN [50077.006985] usb 4-1.4: Manufacturer: Realtek [50077.006986] usb 4-1.4: SerialNumber: C000 [50077.094765] usb 4-1.4: reset SuperSpeed USB device number 19 using xhci_hcd [50077.171166] r8152 4-1.4:1.0 eth0: v1.08.9 [50078.749974] r8152 4-1.4:1.0 enx00249b17d68f: renamed from eth0 [50078.783301] IPv6: ADDRCONF(NETDEV_UP): enx00249b17d68f: link is not ready [50078.812869] IPv6: ADDRCONF(NETDEV_UP): enx00249b17d68f: link is not ready [50079.709290] r8152 4-1.4:1.0 enx00249b17d68f: carrier on [50079.709341] IPv6: ADDRCONF(NETDEV_CHANGE): enx00249b17d68f: link becomes ready [50105.602646] [drm] Atomic update on pipe (A) took 133 us, max time under evasion is 100 us [50200.812935] usb 4-1.4: usb_reset_and_verify_device Failed to disable LTM . [50200.993767] usb 4-1.4: USB disconnect, device number 19 [50201.149848] usb 4-1.4: new SuperSpeed USB device number 20 using xhci_hcd [50201.170162] usb 4-1.4: New USB device found, idVendor=0bda, idProduct=8153 [50201.170165] usb 4-1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=6 [50201.170166] usb 4-1.4: Product: USB 10/100/1000 LAN [50201.170168] usb 4-1.4: Manufacturer: Realtek [50201.170169] usb 4-1.4: SerialNumber: C000 [50201.257954] usb 4-1.4: reset SuperSpeed USB device number 20 using xhci_hcd [50201.339251] r8152 4-1.4:1.0 eth0: v1.08.9 [50202.946884] r8152 4-1.4:1.0 enx00249b17d68f: renamed from eth0 [50202.974759] IPv6: ADDRCONF(NETDEV_UP): enx00249b17d68f: link is not ready [50203.004184] IPv6: ADDRCONF(NETDEV_UP): enx00249b17d68f: link is not ready [50203.867041] r8152 4-1.4:1.0 enx00249b17d68f: carrier on [50203.867091] IPv6: ADDRCONF(NETDEV_CHANGE): enx00249b17d68f: link becomes ready [50363.391768] usb 4-1.4: usb_reset_and_verify_device Failed to disable LTM . [50363.572743] usb 4-1.4: USB disconnect, device number 20 [50363.756687] usb 4-1.4: new SuperSpeed USB device number 21 using xhci_hcd [50363.777267] usb 4-1.4: New USB device found, idVendor=0bda, idProduct=8153 [50363.777269] usb 4-1.4: New USB device strings: Mfr=1, Product=2, SerialNumber=6 [50363.777270] usb 4-1.4: Product: USB 10/100/1000 LAN [50363.777272] usb 4-1.4: Manufacturer: Realtek [50363.777273] usb 4-1.4: SerialNumber: C000 [50363.864888] usb 4-1.4: reset SuperSpeed USB device number 21 using xhci_hcd [50363.941292] r8152 4-1.4:1.0 eth0: v1.08.9 [50365.512708] r8152 4-1.4:1.0 enx00249b17d68f: renamed from eth0 [50365.541731] IPv6: ADDRCONF(NETDEV_UP): enx00249b17d68f: link is not ready [50365.579258] IPv6: ADDRCONF(NETDEV_UP): enx00249b17d68f: link is not ready [50366.471677] r8152 4-1.4:1.0 enx00249b17d68f: carrier on [50366.471726] IPv6: ADDRCONF(NETDEV_CHANGE): enx00249b17d68f: link becomes ready On 18.05.2017 16:11, Greg KH wrote: > On Thu, May 18, 2017 at 03:14:56PM +0200, Daniel Duris wrote: >> Issue can be resolved by setting: >> >> USB_AUTOSUSPEND=0 > > What issue? > >> See details here: >> >> https://bugzilla.kernel.org/show_bug.cgi?id=117811 > > I don't understand, is a patch needed? > >> (also, isn't refusing HTML emails kind of backward in 2017, since HTML >> is default in most of the clients? You are effectively limiting general >>
[PATCH] usb: chipidea: debug: check before accessing ci_role
ci_role BUGs when the role is >= CI_ROLE_END. Signed-off-by: Michael Thalmeier --- drivers/usb/chipidea/debug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c index 6d23eed..1c31e8a 100644 --- a/drivers/usb/chipidea/debug.c +++ b/drivers/usb/chipidea/debug.c @@ -294,7 +294,8 @@ static int ci_role_show(struct seq_file *s, void *data) { struct ci_hdrc *ci = s->private; - seq_printf(s, "%s\n", ci_role(ci)->name); + if (ci->role != CI_ROLE_END) + seq_printf(s, "%s\n", ci_role(ci)->name); return 0; } -- 2.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net,stable] qmi_wwan: add another Lenovo EM74xx device ID
From: Bjørn Mork Date: Wed, 17 May 2017 16:31:41 +0200 > In their infinite wisdom, and never ending quest for end user frustration, > Lenovo has decided to use a new USB device ID for the wwan modules in > their 2017 laptops. The actual hardware is still the Sierra Wireless > EM7455 or EM7430, depending on region. > > Signed-off-by: Bjørn Mork Applied and queued up for -stable, thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
uas failing on multiple disk access on a jmicron JMS567 bridge
Hi, I have seen several reports around the internet regarding failing io on USB-SATA bridges. However, these reports seem to be partially old and/or fixes proposed are implemented in my kernel but don’t fix things. Therefore I thought I report here again. If this is know/duplicate please apologise. I am running on ubuntu 16.04 LTS with kernel $ ~> uname -a Linux gserv 4.8.0-52-generic #55~16.04.1-Ubuntu SMP Fri Apr 28 14:36:29 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux I was also using the 4.4.0-75-generic version of the kernel before, with same results. I am having a JMS567 bridge with (currently) three disks attached. $ ~> lsusb Bus 002 Device 002: ID 152d:0567 JMicron Technology Corp. / JMicron USA Technology Corp. JMS567 SATA 6Gb/s bridge Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub … $ ~> ls -l /dev/disk/by-id/ total 0 … lrwxrwxrwx 1 root root 9 May 18 00:20 usb-JMicron_Generic_DISK00_0123456789ABCDEF-0:0 -> ../../sdc lrwxrwxrwx 1 root root 10 May 18 00:20 usb-JMicron_Generic_DISK00_0123456789ABCDEF-0:0-part1 -> ../../sdc1 lrwxrwxrwx 1 root root 10 May 18 00:20 usb-JMicron_Generic_DISK00_0123456789ABCDEF-0:0-part9 -> ../../sdc9 lrwxrwxrwx 1 root root 9 May 18 00:20 usb-JMicron_Generic_DISK01_0123456789ABCDEF-0:1 -> ../../sdd lrwxrwxrwx 1 root root 10 May 18 00:20 usb-JMicron_Generic_DISK01_0123456789ABCDEF-0:1-part1 -> ../../sdd1 lrwxrwxrwx 1 root root 9 May 18 00:20 usb-JMicron_Generic_DISK02_0123456789ABCDEF-0:2 -> ../../sde lrwxrwxrwx 1 root root 10 May 18 00:20 usb-JMicron_Generic_DISK02_0123456789ABCDEF-0:2-part1 -> ../../sde1 lrwxrwxrwx 1 root root 10 May 18 00:20 usb-JMicron_Generic_DISK02_0123456789ABCDEF-0:2-part9 -> ../../sde9 … I am running a ZFS filesystem on those… (actually two pools…). The system seems to run fine as long as there is only reading going on or only writing to one disk (possibly from somewhere else)…. However once I start a copy operation (here data coming from one of the above devices and going to the two others (in a mirror configuration), I get frequent io errors from the kernel: May 17 22:53:13 gserv kernel: [ 474.505548] xhci_hcd :00:14.0: ERROR Transfer event for disabled endpoint or incorrect stream ring May 17 22:53:13 gserv kernel: [ 474.505670] xhci_hcd :00:14.0: @00026e54c460 0400 02088001 May 17 22:53:43 gserv kernel: [ 504.804185] sd 2:0:0:0: [sda] tag#5 uas_eh_abort_handler 0 uas-tag 6 inflight: CMD IN May 17 22:53:43 gserv kernel: [ 504.804192] sd 2:0:0:0: [sda] tag#5 CDB: Read(16) 88 00 00 00 00 00 74 e9 b3 48 00 00 01 00 00 00 May 17 22:53:43 gserv kernel: [ 504.804252] sd 2:0:0:0: [sda] tag#3 uas_eh_abort_handler 0 uas-tag 4 inflight: CMD IN May 17 22:53:43 gserv kernel: [ 504.804256] sd 2:0:0:0: [sda] tag#3 CDB: Read(16) 88 00 00 00 00 00 74 e9 b2 48 00 00 01 00 00 00 May 17 22:53:43 gserv kernel: [ 504.804299] sd 2:0:0:0: [sda] tag#1 uas_eh_abort_handler 0 uas-tag 2 inflight: CMD IN May 17 22:53:43 gserv kernel: [ 504.804303] sd 2:0:0:0: [sda] tag#1 CDB: Read(16) 88 00 00 00 00 00 74 e9 b1 48 00 00 01 00 00 00 May 17 22:53:43 gserv kernel: [ 504.804383] sd 2:0:0:2: [sdc] tag#7 uas_eh_abort_handler 0 uas-tag 1 inflight: CMD OUT May 17 22:53:43 gserv kernel: [ 504.804387] sd 2:0:0:2: [sdc] tag#7 CDB: Write(16) 8a 00 00 00 00 00 e8 23 79 10 00 00 01 00 00 00 May 17 22:53:43 gserv kernel: [ 504.804419] sd 2:0:0:2: [sdc] tag#6 uas_eh_abort_handler 0 uas-tag 7 inflight: CMD OUT May 17 22:53:43 gserv kernel: [ 504.804422] sd 2:0:0:2: [sdc] tag#6 CDB: Write(16) 8a 00 00 00 00 00 e8 23 78 10 00 00 01 00 00 00 May 17 22:53:43 gserv kernel: [ 504.804468] sd 2:0:0:2: [sdc] tag#0 uas_eh_abort_handler 0 uas-tag 3 inflight: CMD OUT May 17 22:53:43 gserv kernel: [ 504.804472] sd 2:0:0:2: [sdc] tag#0 CDB: Write(16) 8a 00 00 00 00 00 e8 23 77 10 00 00 01 00 00 00 May 17 22:53:43 gserv kernel: [ 504.804510] sd 2:0:0:2: [sdc] tag#2 uas_eh_abort_handler 0 uas-tag 5 inflight: CMD OUT May 17 22:53:43 gserv kernel: [ 504.804514] sd 2:0:0:2: [sdc] tag#2 CDB: Write(16) 8a 00 00 00 00 00 e8 23 76 10 00 00 01 00 00 00 May 17 22:53:43 gserv kernel: [ 504.804542] sd 2:0:0:2: [sdc] tag#4 uas_eh_abort_handler 0 uas-tag 8 inflight: CMD OUT May 17 22:53:43 gserv kernel: [ 504.804545] sd 2:0:0:2: [sdc] tag#4 CDB: Write(16) 8a 00 00 00 00 00 e8 23 75 10 00 00 01 00 00 00 May 17 22:53:43 gserv kernel: [ 504.804600] scsi host2: uas_eh_bus_reset_handler start May 17 22:53:44 gserv kernel: [ 504.924432] usb 2-2: reset SuperSpeed USB device number 2 using xhci_hcd May 17 22:53:44 gserv kernel: [ 504.946883] scsi host2: uas_eh_bus_reset_handler success These keep repeating unitl the filesystem driver (or the user) stops the transfer. I first was suspecting a hardware bug, but after playing around I found that thing seem to be OK once I use usb-storage instead of uas. If I add the following line to the modprobe blacklist $ ~> cat /etc/modprobe.d/blacklist_uas.conf option
Re: [PATCH] net1080: Mark nc_dump_ttl() as __maybe_unused
From: Matthias Kaehlcke Date: Wed, 17 May 2017 15:17:08 -0700 > The function is not used, but it looks useful for debugging. Adding the > attribute fixes the following clang warning: > > drivers/net/usb/net1080.c:271:20: error: unused function > 'nc_dump_ttl' [-Werror,-Wunused-function] > > Signed-off-by: Matthias Kaehlcke For this and the r8152 patch, I definitely prefer that the function is removed. If someone needs them, they can pull it out of the GIT history. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas failing on multiple disk access on a jmicron JMS567 bridge
On Thu, May 18, 2017 at 04:30:02PM +0200, Christoph Gohle wrote: > Hi, > > I have seen several reports around the internet regarding failing io > on USB-SATA bridges. However, these reports seem to be partially old > and/or fixes proposed are implemented in my kernel but don’t fix > things. Therefore I thought I report here again. If this is > know/duplicate please apologise. > > I am running on ubuntu 16.04 LTS with kernel > > $ ~> uname -a > Linux gserv 4.8.0-52-generic #55~16.04.1-Ubuntu SMP Fri Apr 28 14:36:29 UTC > 2017 x86_64 x86_64 x86_64 GNU/Linux Note, this is a very old kernel release, from last October, lots and lots of USB fixes have happened since then. Any chance you can try 4.11 and see if you are having the same issue? We can't do anything about old kernels like this, but your distro can, so you can ask them about this if you want. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] usb: host: ohci-platform: Add basic runtime PM support
On Wed, 17 May 2017, Tony Lindgren wrote: > This is needed in preparation of adding support for omap3 and > later OHCI. The runtime PM will only do something on platforms > that implement it. This patch isn't correct. See below. > Cc: devicet...@vger.kernel.org > Cc: Hans de Goede > Cc: Rob Herring > Cc: Roger Quadros > Cc: Yoshihiro Shimoda > Signed-off-by: Tony Lindgren > --- > drivers/usb/host/ohci-platform.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/usb/host/ohci-platform.c > b/drivers/usb/host/ohci-platform.c > --- a/drivers/usb/host/ohci-platform.c > +++ b/drivers/usb/host/ohci-platform.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -51,6 +52,10 @@ static int ohci_platform_power_on(struct platform_device > *dev) > struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd); > int clk, ret, phy_num; > > + ret = pm_runtime_get_sync(&dev->dev); > + if (ret < 0) > + return ret; > + > for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) { > ret = clk_prepare_enable(priv->clks[clk]); > if (ret) > @@ -96,6 +101,8 @@ static void ohci_platform_power_off(struct platform_device > *dev) > for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--) > if (priv->clks[clk]) > clk_disable_unprepare(priv->clks[clk]); > + > + pm_runtime_put_sync(&dev->dev); > } ohci_platform_power_on() is invoked (by default) by the runtime-resume callback routine ohci_platform_resume(), through the pdata->power_on method pointer. Likewise, ohci_platform_power_off() is invoked by the runtime-suspend callback routine. This means you shouldn't do pm_runtime_get/put calls from within these routines. Is there any particular reason you wanted to add these calls? In general, USB host controllers are expected to go into runtime suspend whenever there aren't any children keeping them awake. Hence they usually don't need to worry about initiating their own suspends and resumes. > static struct hc_driver __read_mostly ohci_platform_hc_driver; > @@ -242,6 +249,7 @@ static int ohci_platform_probe(struct platform_device > *dev) > } > #endif > > + pm_runtime_enable(&dev->dev); There should be a pm_runtime_set_active() just before this. > if (pdata->power_on) { > err = pdata->power_on(dev); > if (err < 0) > @@ -271,6 +279,7 @@ static int ohci_platform_probe(struct platform_device > *dev) > if (pdata->power_off) > pdata->power_off(dev); > err_reset: > + pm_runtime_disable(&dev->dev); > while (--rst >= 0) > reset_control_assert(priv->resets[rst]); > err_put_clks: > @@ -305,6 +314,8 @@ static int ohci_platform_remove(struct platform_device > *dev) > > usb_put_hcd(hcd); > > + pm_runtime_disable(&dev->dev); > + > if (pdata == &ohci_platform_defaults) > dev->dev.platform_data = NULL; These changes make sense. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 6/9] usb: dwc3: debug: remove static char buffer from dwc3_decode_event()
From: Felipe Balbi > Sent: 17 May 2017 14:06 > Instead, we can require caller to pass a buffer for the function to > use. This cleans things quite a bit. This is 2017 not 1982, you just can't pass an unsized char[] through to code that is (presumably) going to do an sprint() into it. You could define a structure containing a fixed size array, or a variable-sized structure where the first member is the size of the buffer area. David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] usb: host: ohci-platform: Add support for omap3 and later
On Wed, 17 May 2017, Tony Lindgren wrote: > With the runtime PM implemented for ohci-platform driver, we can > now support omap3 and later OHCI by adding one device tree > property. > > Cc: Hans de Goede > Cc: Rob Herring > Cc: Roger Quadros > Cc: Yoshihiro Shimoda > Signed-off-by: Tony Lindgren > --- > Documentation/devicetree/bindings/usb/usb-ohci.txt | 1 + > drivers/usb/host/ohci-platform.c | 5 + > 2 files changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt > b/Documentation/devicetree/bindings/usb/usb-ohci.txt > --- a/Documentation/devicetree/bindings/usb/usb-ohci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt > @@ -10,6 +10,7 @@ Optional properties: > - big-endian-desc : boolean, set this for hcds with big-endian descriptors > - big-endian : boolean, for hcds with big-endian-regs + big-endian-desc > - no-big-frame-no : boolean, set if frame_no lives in bits [15:0] of HCCA > +- remote-wakeup-connected: remote wakeup is wired on the platform > - num-ports : u32, to override the detected port count > - clocks : a list of phandle + clock specifier pairs > - phys : phandle + phy specifier pair > diff --git a/drivers/usb/host/ohci-platform.c > b/drivers/usb/host/ohci-platform.c > --- a/drivers/usb/host/ohci-platform.c > +++ b/drivers/usb/host/ohci-platform.c > @@ -170,6 +170,10 @@ static int ohci_platform_probe(struct platform_device > *dev) > if (of_property_read_bool(dev->dev.of_node, "no-big-frame-no")) > ohci->flags |= OHCI_QUIRK_FRAME_NO; > > + if (of_property_read_bool(dev->dev.of_node, > + "remote-wakeup-connected")) > + ohci->hc_control = OHCI_CTRL_RWC; > + > of_property_read_u32(dev->dev.of_node, "num-ports", >&ohci->num_ports); > > @@ -361,6 +365,7 @@ static int ohci_platform_resume(struct device *dev) > static const struct of_device_id ohci_platform_ids[] = { > { .compatible = "generic-ohci", }, > { .compatible = "cavium,octeon-6335-ohci", }, > + { .compatible = "ti,ohci-omap3", }, > { } > }; > MODULE_DEVICE_TABLE(of, ohci_platform_ids); Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] usb: host: ohci-platform: Add support for omap3 and later
* Alan Stern [170518 08:41]: > On Wed, 17 May 2017, Tony Lindgren wrote: > > > With the runtime PM implemented for ohci-platform driver, we can > > now support omap3 and later OHCI by adding one device tree > > property. ... > > diff --git a/drivers/usb/host/ohci-platform.c > > b/drivers/usb/host/ohci-platform.c > > --- a/drivers/usb/host/ohci-platform.c > > +++ b/drivers/usb/host/ohci-platform.c > > @@ -170,6 +170,10 @@ static int ohci_platform_probe(struct platform_device > > *dev) > > if (of_property_read_bool(dev->dev.of_node, "no-big-frame-no")) > > ohci->flags |= OHCI_QUIRK_FRAME_NO; > > > > + if (of_property_read_bool(dev->dev.of_node, > > + "remote-wakeup-connected")) > > + ohci->hc_control = OHCI_CTRL_RWC; > > + > > of_property_read_u32(dev->dev.of_node, "num-ports", > > &ohci->num_ports); > > > > @@ -361,6 +365,7 @@ static int ohci_platform_resume(struct device *dev) > > static const struct of_device_id ohci_platform_ids[] = { > > { .compatible = "generic-ohci", }, > > { .compatible = "cavium,octeon-6335-ohci", }, > > + { .compatible = "ti,ohci-omap3", }, > > { } > > }; > > MODULE_DEVICE_TABLE(of, ohci_platform_ids); > > Acked-by: Alan Stern FYI, this depends on the patch1/4 PM runtime issue getting sorted out first as otherwise the USB controller is powered off on omaps. So let's not apply this yet. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] usb: host: ohci-omap3: Print warning to get people to use ohci-platform
On Wed, 17 May 2017, Tony Lindgren wrote: > We can't just remove ohci-omap3 as that could make some people's > systems unusable for keyboard and mouse. Let's print a warning for > now, and then remove the driver in a year or so. > > Cc: Hans de Goede > Cc: Rob Herring > Cc: Roger Quadros > Cc: Yoshihiro Shimoda > Signed-off-by: Tony Lindgren > --- > drivers/usb/host/ohci-omap3.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/host/ohci-omap3.c b/drivers/usb/host/ohci-omap3.c > --- a/drivers/usb/host/ohci-omap3.c > +++ b/drivers/usb/host/ohci-omap3.c > @@ -130,6 +130,9 @@ static int ohci_hcd_omap3_probe(struct platform_device > *pdev) > dev_dbg(dev, "failed to add hcd with err %d\n", ret); > goto err_add_hcd; > } > + > + WARN(1, "ohci-omap3 is deprecated, please use ohci-platform instead\n"); > + > device_wakeup_enable(hcd->self.controller); > > return 0; Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] ARM: dts: Add remote-wakeup-connected for omap OHCI
On Wed, 17 May 2017, Tony Lindgren wrote: > Add remote-wakeup-connected for omap OHCI as that's needed by > ehci-platform driver. This should be "ohci", not "ehci". Otherwise, Acked-by: Alan Stern > Cc: devicet...@vger.kernel.org > Cc: Hans de Goede > Cc: Rob Herring > Cc: Roger Quadros > Cc: Yoshihiro Shimoda > Signed-off-by: Tony Lindgren > --- > arch/arm/boot/dts/omap3.dtsi | 1 + > arch/arm/boot/dts/omap4.dtsi | 1 + > arch/arm/boot/dts/omap5.dtsi | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi > --- a/arch/arm/boot/dts/omap3.dtsi > +++ b/arch/arm/boot/dts/omap3.dtsi > @@ -715,6 +715,7 @@ > reg = <0x48064400 0x400>; > interrupt-parent = <&intc>; > interrupts = <76>; > + remote-wakeup-connected; > }; > > usbhsehci: ehci@48064800 { > diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi > --- a/arch/arm/boot/dts/omap4.dtsi > +++ b/arch/arm/boot/dts/omap4.dtsi > @@ -863,6 +863,7 @@ > reg = <0x4a064800 0x400>; > interrupt-parent = <&gic>; > interrupts = ; > + remote-wakeup-connected; > }; > > usbhsehci: ehci@4a064c00 { > diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi > --- a/arch/arm/boot/dts/omap5.dtsi > +++ b/arch/arm/boot/dts/omap5.dtsi > @@ -939,6 +939,7 @@ > compatible = "ti,ohci-omap3"; > reg = <0x4a064800 0x400>; > interrupts = ; > + remote-wakeup-connected; > }; > > usbhsehci: ehci@4a064c00 { > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] usb: host: ohci-platform: Add basic runtime PM support
* Alan Stern [170518 08:28]: > On Wed, 17 May 2017, Tony Lindgren wrote: > > > This is needed in preparation of adding support for omap3 and > > later OHCI. The runtime PM will only do something on platforms > > that implement it. > > This patch isn't correct. See below. > > > Cc: devicet...@vger.kernel.org > > Cc: Hans de Goede > > Cc: Rob Herring > > Cc: Roger Quadros > > Cc: Yoshihiro Shimoda > > Signed-off-by: Tony Lindgren > > --- > > drivers/usb/host/ohci-platform.c | 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/usb/host/ohci-platform.c > > b/drivers/usb/host/ohci-platform.c > > --- a/drivers/usb/host/ohci-platform.c > > +++ b/drivers/usb/host/ohci-platform.c > > @@ -24,6 +24,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -51,6 +52,10 @@ static int ohci_platform_power_on(struct platform_device > > *dev) > > struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd); > > int clk, ret, phy_num; > > > > + ret = pm_runtime_get_sync(&dev->dev); > > + if (ret < 0) > > + return ret; > > + > > for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) { > > ret = clk_prepare_enable(priv->clks[clk]); > > if (ret) > > @@ -96,6 +101,8 @@ static void ohci_platform_power_off(struct > > platform_device *dev) > > for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--) > > if (priv->clks[clk]) > > clk_disable_unprepare(priv->clks[clk]); > > + > > + pm_runtime_put_sync(&dev->dev); > > } > > ohci_platform_power_on() is invoked (by default) by the runtime-resume > callback routine ohci_platform_resume(), through the pdata->power_on > method pointer. Likewise, ohci_platform_power_off() is invoked by the > runtime-suspend callback routine. > > This means you shouldn't do pm_runtime_get/put calls from within these > routines. Oh OK great, so the above should not be needed at all. > Is there any particular reason you wanted to add these calls? In > general, USB host controllers are expected to go into runtime suspend > whenever there aren't any children keeping them awake. Hence they > usually don't need to worry about initiating their own suspends and > resumes. No particular reason as it sounds things work without it :) I'll check. > > static struct hc_driver __read_mostly ohci_platform_hc_driver; > > @@ -242,6 +249,7 @@ static int ohci_platform_probe(struct platform_device > > *dev) > > } > > #endif > > > > + pm_runtime_enable(&dev->dev); > > There should be a pm_runtime_set_active() just before this. OK > > if (pdata->power_on) { > > err = pdata->power_on(dev); > > if (err < 0) > > @@ -271,6 +279,7 @@ static int ohci_platform_probe(struct platform_device > > *dev) > > if (pdata->power_off) > > pdata->power_off(dev); > > err_reset: > > + pm_runtime_disable(&dev->dev); > > while (--rst >= 0) > > reset_control_assert(priv->resets[rst]); > > err_put_clks: > > @@ -305,6 +314,8 @@ static int ohci_platform_remove(struct platform_device > > *dev) > > > > usb_put_hcd(hcd); > > > > + pm_runtime_disable(&dev->dev); > > + > > if (pdata == &ohci_platform_defaults) > > dev->dev.platform_data = NULL; > > These changes make sense. OK thanks for looking. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net1080: Mark nc_dump_ttl() as __maybe_unused
Hi David, El Thu, May 18, 2017 at 10:48:08AM -0400 David Miller ha dit: > From: Matthias Kaehlcke > Date: Wed, 17 May 2017 15:17:08 -0700 > > > The function is not used, but it looks useful for debugging. Adding the > > attribute fixes the following clang warning: > > > > drivers/net/usb/net1080.c:271:20: error: unused function > > 'nc_dump_ttl' [-Werror,-Wunused-function] > > > > Signed-off-by: Matthias Kaehlcke > > For this and the r8152 patch, I definitely prefer that the function is > removed. > > If someone needs them, they can pull it out of the GIT history. Thanks for you comments, I'll send out updated patches soon. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers
On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote: > Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck: > > On 05/17/2017 12:34 AM, Oliver Neukum wrote: > > > > > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan > > > Sridharan: > > > > > > Hi, > > > > > > > > > > > "Two independent set of mechanisms are defined to allow a USB Type-C > > > > DRP to functionally swap power and data roles. When USB PD is > > > > supported, power and data role swapping is performed as a subsequent > > > > step following the initial connection process. For non-PD > > > > implementations, > > > > power/data role swapping can optionally be dealt with as part of the > > > > initial > > > > connection process." > > > > > > Well, as I read it, without PD once a connection is established, you > > > are stuck with your role. So it seems to me that blocking a later > > > attempt to change it makes sense. > > > > > > > That seems to be a harsh and not very user friendly reading of the > > specification. > > > > I would argue that the user doesn't care if the partner supports PD or not > > when selecting a role, and I would prefer to provide an implementation > > which is > > as user friendly as possible. > > Data role, no question, you are right. > Power role is a different question. A switch of power role with PD should > not lead to a disconnect. Any other method might. So equating them does > not look like a good idea. > Not really sure I can follow. If a partner does not support PD, there is no real distinction between data role and power role, or am I missing something ? Are you saying that, if a partner does not support PD, user space should request a data role swap instead, and that this would be acceptable for you ? I don't really understand the difference - a data role swap doesn't cause a disconnect either if the partner supports PD, and it would still result in a disconnect/reconnect sequence if the partner does not support PD - but if it works for you, fine with me. Badhri, would that work for us ? Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
dwc2: g-tx-fifo-size configuration on bcm2835
Hi, since the dwc2 should be able to properly setup its fifos, i want to enable otg mode for Raspberry Pi Zero (BCM2835). According to the BCM2835 datasheet [1] it has 7 device EPs with a max FIFO depth for EP1..5 = 512 and EP6,7 = 768. This makes in sum 4096, which also seems to be okay regarding to datasheet. So i defined the following DTS part: &usb { dr_mode = "otg"; g-rx-fifo-size = <256>; g-np-tx-fifo-size = <32>; g-tx-fifo-size = <512 512 512 512 512 768 768>; }; Unfortunately i get the following errors from dwc2: [2.535879] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g-tx-fifo-size, setting to default average [2.561611] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[1]=539 [2.586045] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[2]=539 [2.611045] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[3]=539 [2.636209] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[4]=539 [2.661952] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[5]=539 [2.688044] dwc2 2098.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM [2.704753] g_serial gadget: Gadget Serial v2.4 [2.717962] g_serial gadget: g_serial ready [2.730830] dwc2 2098.usb: bound driver g_serial [2.745485] dwc2 2098.usb: DWC OTG Controller [2.759763] dwc2 2098.usb: new USB bus registered, assigned bus number 1 [2.776556] dwc2 2098.usb: irq 33, io mem 0x2098 Since these messages aren't really helpful, i added a info messages about fifo_count, sum of all fifo sizes and allowed total from hardware: [2.512808] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: fifo_count = 7, sum = 4096, total = 3776 So i decided to use the following setting, which works without any issues: g-tx-fifo-size = <512 512 512 512 512 512 512>; Here are my questions: Is the dwc2 still broken regarding to this error or is the datasheet wrong? Is it save to use the working g-tx-fifo-size for peripheral and otg mode? Or do i need to reserve some space in fifo (for host mode)? Is this the proper way to evaluate the right g-tx-fifo-size setting? Regards Stefan [1] - https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] usb: host: ohci-platform: Add basic runtime PM support
On Wed, 17 May 2017, Tony Lindgren wrote: > This is needed in preparation of adding support for omap3 and > later OHCI. The runtime PM will only do something on platforms > that implement it. > @@ -51,6 +52,10 @@ static int ohci_platform_power_on(struct platform_device > *dev) > struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd); > int clk, ret, phy_num; > > + ret = pm_runtime_get_sync(&dev->dev); > + if (ret < 0) > + return ret; > + I don't remember how probing for platform device drivers is set up. So although this is definitely the wrong place for pm_runtime_get_sync(), it may turn out that you need to do a pm_runtime_get_noresume() before calling pm_runtime_enable(), and a pm_runtime_put() at the end of ohci_platform_probe(). Otherwise runtime PM might kick in during the middle of the probe sequence -- not what you want. Similarly, you may need pm_runtime_get_sync() at the start of ohci_platform_remove() and pm_runtime_put_noidle() at the end. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] usb: host: ohci-omap3: Print warning to get people to use ohci-platform
Hi, On Thu, May 18, 2017 at 07:08:58AM -0700, Tony Lindgren wrote: > * Sebastian Reichel [170518 02:18]: > > On Wed, May 17, 2017 at 03:59:21PM -0700, Tony Lindgren wrote: > > > We can't just remove ohci-omap3 as that could make some people's > > > systems unusable for keyboard and mouse. Let's print a warning for > > > now, and then remove the driver in a year or so. > > > > I guess you could drop the driver and modify the Kconfig entry, > > so that it selects USB_OHCI_HCD_PLATFORM? I just checked and > > there are already a few examples of this in the USB host Kconfig :) > > Right but there's still nothing telling people to load ohci-platform > instead of ohci-omap3 if we just remove ohci-omap3 without any warnings. Shouldn't the driver be loaded automatically due to the DT compatible value? > I don't think there's any need to rush to remove ohci-omap3. Just looked like it can be dropped with trivial changes. -- Sebastian signature.asc Description: PGP signature
[PATCH] r8152: Remove unused function usb_ocp_read()
The function is not used, removing it fixes the following warning when building with clang: drivers/net/usb/r8152.c:825:5: error: unused function 'usb_ocp_read' [-Werror,-Wunused-function] Signed-off-by: Matthias Kaehlcke --- drivers/net/usb/r8152.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index ddc62cb69be8..e902df9595b9 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -841,12 +841,6 @@ int pla_ocp_write(struct r8152 *tp, u16 index, u16 byteen, u16 size, void *data) } static inline -int usb_ocp_read(struct r8152 *tp, u16 index, u16 size, void *data) -{ - return generic_ocp_read(tp, index, size, data, MCU_TYPE_USB); -} - -static inline int usb_ocp_write(struct r8152 *tp, u16 index, u16 byteen, u16 size, void *data) { return generic_ocp_write(tp, index, byteen, size, data, MCU_TYPE_USB); -- 2.13.0.303.g4ebf302169-goog -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net1080: Remove unused function nc_dump_ttl()
The function is not used, removing it fixes the following warning when building with clang: drivers/net/usb/net1080.c:271:20: error: unused function 'nc_dump_ttl' [-Werror,-Wunused-function] Also remove the definition of TTL_THIS, which is only used in nc_dump_ttl() Signed-off-by: Matthias Kaehlcke --- drivers/net/usb/net1080.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c index 4cbdb1307f3e..3202c19df83d 100644 --- a/drivers/net/usb/net1080.c +++ b/drivers/net/usb/net1080.c @@ -264,17 +264,9 @@ static inline void nc_dump_status(struct usbnet *dev, u16 status) * TTL register */ -#defineTTL_THIS(ttl) (0x00ff & ttl) #defineTTL_OTHER(ttl) (0x00ff & (ttl >> 8)) #define MK_TTL(this,other) ((u16)(((other)<<8)|(0x00ff&(this -static inline void nc_dump_ttl(struct usbnet *dev, u16 ttl) -{ - netif_dbg(dev, link, dev->net, "net1080 %s-%s ttl 0x%x this = %d, other = %d\n", - dev->udev->bus->bus_name, dev->udev->devpath, - ttl, TTL_THIS(ttl), TTL_OTHER(ttl)); -} - /*-*/ static int net1080_reset(struct usbnet *dev) @@ -308,7 +300,6 @@ static int net1080_reset(struct usbnet *dev) goto done; } ttl = vp; - // nc_dump_ttl(dev, ttl); nc_register_write(dev, REG_TTL, MK_TTL(NC_READ_TTL_MS, TTL_OTHER(ttl)) ); -- 2.13.0.303.g4ebf302169-goog -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] r8152: Remove unused function usb_ocp_read()
From: Matthias Kaehlcke Date: Thu, 18 May 2017 10:45:33 -0700 > The function is not used, removing it fixes the following warning when > building with clang: > > drivers/net/usb/r8152.c:825:5: error: unused function 'usb_ocp_read' > [-Werror,-Wunused-function] > > Signed-off-by: Matthias Kaehlcke Applied to net-next. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net1080: Remove unused function nc_dump_ttl()
From: Matthias Kaehlcke Date: Thu, 18 May 2017 10:57:19 -0700 > The function is not used, removing it fixes the following warning when > building with clang: > > drivers/net/usb/net1080.c:271:20: error: unused function > 'nc_dump_ttl' [-Werror,-Wunused-function] > > Also remove the definition of TTL_THIS, which is only used in > nc_dump_ttl() > > Signed-off-by: Matthias Kaehlcke Applied to net-next. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers
On Thu, May 18, 2017 at 9:51 AM, Guenter Roeck wrote: > On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote: >> Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck: >> > On 05/17/2017 12:34 AM, Oliver Neukum wrote: >> > > >> > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan >> > > Sridharan: >> > > >> > > Hi, >> > > >> > > > >> > > > "Two independent set of mechanisms are defined to allow a USB Type-C >> > > > DRP to functionally swap power and data roles. When USB PD is >> > > > supported, power and data role swapping is performed as a subsequent >> > > > step following the initial connection process. For non-PD >> > > > implementations, >> > > > power/data role swapping can optionally be dealt with as part of the >> > > > initial >> > > > connection process." >> > > >> > > Well, as I read it, without PD once a connection is established, you >> > > are stuck with your role. So it seems to me that blocking a later >> > > attempt to change it makes sense. >> > > >> > >> > That seems to be a harsh and not very user friendly reading of the >> > specification. >> > >> > I would argue that the user doesn't care if the partner supports PD or not >> > when selecting a role, and I would prefer to provide an implementation >> > which is >> > as user friendly as possible. >> >> Data role, no question, you are right. >> Power role is a different question. A switch of power role with PD should >> not lead to a disconnect. Any other method might. So equating them does >> not look like a good idea. >> > > Not really sure I can follow. If a partner does not support PD, there is no > real distinction between data role and power role, or am I missing something ? > > Are you saying that, if a partner does not support PD, user space should > request a data role swap instead, and that this would be acceptable for you ? > > I don't really understand the difference - a data role swap doesn't cause > a disconnect either if the partner supports PD, and it would still result > in a disconnect/reconnect sequence if the partner does not support PD - > but if it works for you, fine with me. > > Badhri, would that work for us ? Yes Geunter that should work as well. Requesting non-pd role swap either through current_power_role or current_data_role is virtually the same. > > Thanks, > Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers
Not Sure whether my previous response was sent properly. So re-sending. On Thu, May 18, 2017 at 2:08 PM, Badhri Jagan Sridharan wrote: > On Thu, May 18, 2017 at 9:51 AM, Guenter Roeck wrote: >> On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote: >>> Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck: >>> > On 05/17/2017 12:34 AM, Oliver Neukum wrote: >>> > > >>> > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan >>> > > Sridharan: >>> > > >>> > > Hi, >>> > > >>> > > > >>> > > > "Two independent set of mechanisms are defined to allow a USB Type-C >>> > > > DRP to functionally swap power and data roles. When USB PD is >>> > > > supported, power and data role swapping is performed as a subsequent >>> > > > step following the initial connection process. For non-PD >>> > > > implementations, >>> > > > power/data role swapping can optionally be dealt with as part of the >>> > > > initial >>> > > > connection process." >>> > > >>> > > Well, as I read it, without PD once a connection is established, you >>> > > are stuck with your role. So it seems to me that blocking a later >>> > > attempt to change it makes sense. >>> > > >>> > >>> > That seems to be a harsh and not very user friendly reading of the >>> > specification. >>> > >>> > I would argue that the user doesn't care if the partner supports PD or not >>> > when selecting a role, and I would prefer to provide an implementation >>> > which is >>> > as user friendly as possible. >>> >>> Data role, no question, you are right. >>> Power role is a different question. A switch of power role with PD should >>> not lead to a disconnect. Any other method might. So equating them does >>> not look like a good idea. >>> >> >> Not really sure I can follow. If a partner does not support PD, there is no >> real distinction between data role and power role, or am I missing something >> ? >> >> Are you saying that, if a partner does not support PD, user space should >> request a data role swap instead, and that this would be acceptable for you ? >> >> I don't really understand the difference - a data role swap doesn't cause >> a disconnect either if the partner supports PD, and it would still result >> in a disconnect/reconnect sequence if the partner does not support PD - >> but if it works for you, fine with me. >> >> Badhri, would that work for us ? Yes Geunter that should work as well. Requesting non-pd role swap either through current_power_role or current_data_role is virtually the same. > > Yes Geunter that should work as well. Requesting non-pd role swap either > through > current_power_role or current_data_role is virtually the same. > >> >> Thanks, >> Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] usb: gadget: f_mass_storage: Serialize wake and sleep execution
Hi Felipe, On 5/11/2017 5:27 PM, Thinh Nguyen wrote: > f_mass_storage has a memorry barrier issue with the sleep and wake > functions that can cause a deadlock. This results in intermittent hangs > during MSC file transfer. The host will reset the device after receiving > no response to resume the transfer. This issue is seen when dwc3 is > processing 2 transfer-in-progress events at the same time, invoking > completion handlers for CSW and CBW. Also this issue occurs depending on > the system timing and latency. > > To increase the chance to hit this issue, you can force dwc3 driver to > wait and process those 2 events at once by adding a small delay (~100us) > in dwc3_check_event_buf() whenever the request is for CSW and read the > event count again. Avoid debugging with printk and ftrace as extra > delays and memory barrier will mask this issue. > > Scenario which can lead to failure: > --- > 1) The main thread sleeps and waits for the next command in > get_next_command(). > 2) bulk_in_complete() wakes up main thread for CSW. > 3) bulk_out_complete() tries to wake up the running main thread for CBW. > 4) thread_wakeup_needed is not loaded with correct value in > sleep_thread(). > 5) Main thread goes to sleep again. > > The pattern is shown below. Note the 2 critical variables. > * common->thread_wakeup_needed > * bh->state > > CPU 0 (sleep_thread)CPU 1 (wakeup_thread) > == === > > bh->state = BH_STATE_FULL; > smp_wmb(); > thread_wakeup_needed = 0; thread_wakeup_needed = 1; > smp_rmb(); > if (bh->state != BH_STATE_FULL) > sleep again ... > > As pointed out by Alan Stern, this is an R-pattern issue. The issue can > be seen when there are two wakeups in quick succession. The > thread_wakeup_needed can be overwritten in sleep_thread, and the read of > the bh->state maybe reordered before the write to thread_wakeup_needed. > > This patch applies full memory barrier smp_mb() in both sleep_thread() > and wakeup_thread() to ensure the order which the thread_wakeup_needed > and bh->state are written and loaded. > > However, a better solution in the future would be to use wait_queue > method that takes care of managing memory barrier between waker and > waiter. > > Cc: sta...@vger.kernel.org > Signed-off-by: Thinh Nguyen > --- Will this patch go on the rc? Thanks, Thinh -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v1 1/1] usb:host:xhci support option to disable xHCI 1.0 USB2 HW LPM
On Sat, May 13, 2017 at 07:13:14AM +0700, Thang Q. Nguyen wrote: > XHCI specification 1.1 does not require xHCI 1.0 compliant controllers > to always enable hardware USB2 LPM. > However, the current xHCI driver always enable it by setting HLE=1 when > seeing HLC=1. This makes certain xHCI controllers that have broken USB2 > HW LPM fail to work as there is no way to disable this feature. > This patch adds support to control disabling USB2 Hardware LPM via > DT/ACPI attribute. > > Signed-off-by: Tung Nguyen > Signed-off-by: Thang Q. Nguyen > --- > Documentation/devicetree/bindings/usb/usb-xhci.txt |1 + > drivers/usb/host/xhci-plat.c |3 +++ > drivers/usb/host/xhci.c|7 ++- > drivers/usb/host/xhci.h|1 + > 4 files changed, 11 insertions(+), 1 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt > b/Documentation/devicetree/bindings/usb/usb-xhci.txt > index 2d80b60..b5da569 100644 > --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt > +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt > @@ -26,6 +26,7 @@ Required properties: > > Optional properties: >- clocks: reference to a clock > + - usb2-hle-disable: disable USB2 LPM for hardware does not support it >- usb3-lpm-capable: determines if platform is USB3 LPM capable Can we have some consistency in the naming of these 2 properties? At least use 'lpm' rather than 'hle'. I don't recall why the 2nd one is not default enabled, but that would make more sense. >- quirk-broken-port-ped: set if the controller has broken port disable > mechanism > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[uwb-i1480] question about value overwrite
Hello everybody, While looking into Coverity ID 1226913 I ran into the following piece of code at drivers/uwb/i1480/dfu/phy.c:99: 99static 100int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 srcaddr, size_t size) 101{ 102int result; 103struct i1480_cmd_mpi_read *cmd = i1480->cmd_buf; 104struct i1480_evt_mpi_read *reply = i1480->evt_buf; 105unsigned cnt; 106 107memset(i1480->cmd_buf, 0x69, 512); 108memset(i1480->evt_buf, 0x69, 512); 109 110BUG_ON(size > (i1480->buf_size - sizeof(*reply)) / 3); 111result = -ENOMEM; 112cmd->rccb.bCommandType = i1480_CET_VS1; 113cmd->rccb.wCommand = cpu_to_le16(i1480_CMD_MPI_READ); 114cmd->size = cpu_to_le16(3*size); 115for (cnt = 0; cnt < size; cnt++) { 116cmd->data[cnt].page = (srcaddr + cnt) >> 8; 117cmd->data[cnt].offset = (srcaddr + cnt) & 0xff; 118} 119reply->rceb.bEventType = i1480_CET_VS1; 120reply->rceb.wEvent = i1480_CMD_MPI_READ; 121result = i1480_cmd(i1480, "MPI-READ", sizeof(*cmd) + 2*size, 122sizeof(*reply) + 3*size); 123if (result < 0) 124goto out; 125if (reply->bResultCode != UWB_RC_RES_SUCCESS) { 126dev_err(i1480->dev, "MPI-READ: command execution failed: %d\n", 127reply->bResultCode); 128result = -EIO; 129} 130for (cnt = 0; cnt < size; cnt++) { 131if (reply->data[cnt].page != (srcaddr + cnt) >> 8) 132dev_err(i1480->dev, "MPI-READ: page inconsistency at " 133"index %u: expected 0x%02x, got 0x%02x\n", cnt, 134(srcaddr + cnt) >> 8, reply->data[cnt].page); 135if (reply->data[cnt].offset != ((srcaddr + cnt) & 0x00ff)) 136dev_err(i1480->dev, "MPI-READ: offset inconsistency at " 137"index %u: expected 0x%02x, got 0x%02x\n", cnt, 138(srcaddr + cnt) & 0x00ff, 139reply->data[cnt].offset); 140data[cnt] = reply->data[cnt].value; 141} 142result = 0; 143out: 144return result; 145} The issue is that the value store in variable _result_ at line 128 is overwritten by the one stored at line 142, before it can be used. My question is if the original intention was to return this value inmediately after the assignment at line 128, something like in the following patch: index 3b1a87d..1ac8526 100644 --- a/drivers/uwb/i1480/dfu/phy.c +++ b/drivers/uwb/i1480/dfu/phy.c @@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 srcaddr, size_t size) dev_err(i1480->dev, "MPI-READ: command execution failed: %d\n", reply->bResultCode); result = -EIO; + goto out; } for (cnt = 0; cnt < size; cnt++) { if (reply->data[cnt].page != (srcaddr + cnt) >> 8) What do you think? I'd really appreciate any comment on this. Thank you! -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend v3 1/4] usb: usbip tool: Check the return of get_nports()
On Thu, May 18, 2017 at 03:53:04PM +0300, Sergei Shtylyov wrote: > >From: Yuyang Du > > > >If we get nonpositive number of ports, there is no sense to > >Negative? Negative and zero. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: debug: check before accessing ci_role
On Thu, May 18, 2017 at 04:14:14PM +0200, Michael Thalmeier wrote: > ci_role BUGs when the role is >= CI_ROLE_END. > > Signed-off-by: Michael Thalmeier > --- > drivers/usb/chipidea/debug.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c > index 6d23eed..1c31e8a 100644 > --- a/drivers/usb/chipidea/debug.c > +++ b/drivers/usb/chipidea/debug.c > @@ -294,7 +294,8 @@ static int ci_role_show(struct seq_file *s, void *data) > { > struct ci_hdrc *ci = s->private; > > - seq_printf(s, "%s\n", ci_role(ci)->name); > + if (ci->role != CI_ROLE_END) > + seq_printf(s, "%s\n", ci_role(ci)->name); > > return 0; > } I will queue it, and cc stable -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: debug: check before accessing ci_role
On Thu, May 18, 2017 at 04:14:14PM +0200, Michael Thalmeier wrote: > ci_role BUGs when the role is >= CI_ROLE_END. > > Signed-off-by: Michael Thalmeier > --- > drivers/usb/chipidea/debug.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c > index 6d23eed..1c31e8a 100644 > --- a/drivers/usb/chipidea/debug.c > +++ b/drivers/usb/chipidea/debug.c > @@ -294,7 +294,8 @@ static int ci_role_show(struct seq_file *s, void *data) > { > struct ci_hdrc *ci = s->private; > > - seq_printf(s, "%s\n", ci_role(ci)->name); > + if (ci->role != CI_ROLE_END) > + seq_printf(s, "%s\n", ci_role(ci)->name); > > return 0; > } By the way, how can you trigger this issue? Do you mind sending another patch to fix the same issue for ci_role_show at core.c? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v1 1/1] usb:host:xhci support option to disable xHCI 1.0 USB2 HW LPM
On Fri, May 19, 2017 at 5:30 AM, Rob Herring wrote: > On Sat, May 13, 2017 at 07:13:14AM +0700, Thang Q. Nguyen wrote: >> XHCI specification 1.1 does not require xHCI 1.0 compliant controllers >> to always enable hardware USB2 LPM. >> However, the current xHCI driver always enable it by setting HLE=1 when >> seeing HLC=1. This makes certain xHCI controllers that have broken USB2 >> HW LPM fail to work as there is no way to disable this feature. >> This patch adds support to control disabling USB2 Hardware LPM via >> DT/ACPI attribute. >> >> Signed-off-by: Tung Nguyen >> Signed-off-by: Thang Q. Nguyen >> --- >> Documentation/devicetree/bindings/usb/usb-xhci.txt |1 + >> drivers/usb/host/xhci-plat.c |3 +++ >> drivers/usb/host/xhci.c|7 ++- >> drivers/usb/host/xhci.h|1 + >> 4 files changed, 11 insertions(+), 1 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt >> b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> index 2d80b60..b5da569 100644 >> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt >> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt >> @@ -26,6 +26,7 @@ Required properties: >> >> Optional properties: >>- clocks: reference to a clock >> + - usb2-hle-disable: disable USB2 LPM for hardware does not support it >>- usb3-lpm-capable: determines if platform is USB3 LPM capable > > Can we have some consistency in the naming of these 2 properties? At > least use 'lpm' rather than 'hle'. I don't recall why the 2nd one is not > default enabled, but that would make more sense. Thanks. I will update the attribute to "usb2-lpm-disable" in the next patch revision. > >>- quirk-broken-port-ped: set if the controller has broken port disable >> mechanism >> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Runtime PM enabled EHCI does not respond to device plugging.
Hi, This issue happens on Carrizo AMD laptops, only EHCI is affected, XHCI works fine on the same machine. I can see lots of USB wakeup and resume messages showed every two seconds. Plug USB devices to the EHCI port does not change anything. dmesg with ehci-hcd, ehci-pci and usbcore dynamic debug enabled is attached: [ 88.556535] ehci-pci :00:13.0: hcd_pci_runtime_resume: 0 [ 88.556563] usb usb2: usb auto-resume [ 88.556588] hub 2-0:1.0: hub_resume [ 88.556676] usb usb2-port1: status 0507 change [ 88.556794] hub 2-0:1.0: state 7 ports 2 chg evt [ 88.556862] usb 2-1: usb auto-resume [ 88.624577] usb 2-1: Waited 0ms for CONNECT [ 88.624585] usb 2-1: finish resume [ 88.624838] hub 2-1:1.0: hub_resume [ 88.624964] usb 2-1-port1: status 0507 change [ 88.625085] usb 2-1-port2: status 0107 change [ 88.625354] usb 2-1-port4: status 0507 change [ 88.625403] hub 2-1:1.0: state 7 ports 4 chg evt [ 88.625605] usb 2-1.1: usb auto-resume [ 88.692635] usb 2-1.1: Waited 0ms for CONNECT [ 88.692640] usb 2-1.1: finish resume [ 90.800874] usb 2-1.1: usb auto-suspend, wakeup 1 [ 90.820326] hub 2-1:1.0: hub_suspend [ 90.823161] usb 2-1: usb auto-suspend, wakeup 1 [ 90.840362] hub 2-0:1.0: hub_suspend [ 90.840399] usb usb2: bus auto-suspend, wakeup 1 [ 90.860346] ehci-pci :00:13.0: hcd_pci_runtime_suspend: 0 Thanks. dmesg Description: Binary data
Re: [uwb-i1480] question about value overwrite
On Thu, May 18, 2017 at 06:00:06PM -0500, Gustavo A. R. Silva wrote: > > Hello everybody, > > While looking into Coverity ID 1226913 I ran into the following piece of > code at drivers/uwb/i1480/dfu/phy.c:99: > > 99static > 100int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 srcaddr, size_t size) > 101{ > 102int result; > 103struct i1480_cmd_mpi_read *cmd = i1480->cmd_buf; > 104struct i1480_evt_mpi_read *reply = i1480->evt_buf; > 105unsigned cnt; > 106 > 107memset(i1480->cmd_buf, 0x69, 512); > 108memset(i1480->evt_buf, 0x69, 512); > 109 > 110BUG_ON(size > (i1480->buf_size - sizeof(*reply)) / 3); > 111result = -ENOMEM; > 112cmd->rccb.bCommandType = i1480_CET_VS1; > 113cmd->rccb.wCommand = cpu_to_le16(i1480_CMD_MPI_READ); > 114cmd->size = cpu_to_le16(3*size); > 115for (cnt = 0; cnt < size; cnt++) { > 116cmd->data[cnt].page = (srcaddr + cnt) >> 8; > 117cmd->data[cnt].offset = (srcaddr + cnt) & 0xff; > 118} > 119reply->rceb.bEventType = i1480_CET_VS1; > 120reply->rceb.wEvent = i1480_CMD_MPI_READ; > 121result = i1480_cmd(i1480, "MPI-READ", sizeof(*cmd) + 2*size, > 122sizeof(*reply) + 3*size); > 123if (result < 0) > 124goto out; > 125if (reply->bResultCode != UWB_RC_RES_SUCCESS) { > 126dev_err(i1480->dev, "MPI-READ: command execution failed: > %d\n", > 127reply->bResultCode); > 128result = -EIO; > 129} > 130for (cnt = 0; cnt < size; cnt++) { > 131if (reply->data[cnt].page != (srcaddr + cnt) >> 8) > 132dev_err(i1480->dev, "MPI-READ: page inconsistency > at " > 133"index %u: expected 0x%02x, got > 0x%02x\n", cnt, > 134(srcaddr + cnt) >> 8, > reply->data[cnt].page); > 135if (reply->data[cnt].offset != ((srcaddr + cnt) & 0x00ff)) > 136dev_err(i1480->dev, "MPI-READ: offset > inconsistency at " > 137"index %u: expected 0x%02x, got > 0x%02x\n", cnt, > 138(srcaddr + cnt) & 0x00ff, > 139reply->data[cnt].offset); > 140data[cnt] = reply->data[cnt].value; > 141} > 142result = 0; > 143out: > 144return result; > 145} > > The issue is that the value store in variable _result_ at line 128 is > overwritten by the one stored at line 142, before it can be used. > > My question is if the original intention was to return this value > inmediately after the assignment at line 128, something like in the > following patch: > > index 3b1a87d..1ac8526 100644 > --- a/drivers/uwb/i1480/dfu/phy.c > +++ b/drivers/uwb/i1480/dfu/phy.c > @@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 > srcaddr, size_t size) > dev_err(i1480->dev, "MPI-READ: command execution failed: > %d\n", > reply->bResultCode); > result = -EIO; > + goto out; > } > for (cnt = 0; cnt < size; cnt++) { > if (reply->data[cnt].page != (srcaddr + cnt) >> 8) > > What do you think? > > I'd really appreciate any comment on this. I think you are correct, I'll take a patch to fix this up if you want to write one :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REGRESSION] Failed network caused by: xhci: switch to pci_alloc_irq_vectors
On Thu, May 18, 2017 at 11:42:34PM -0400, Steven Rostedt wrote: > > One of my the configs I use to test ftrace with (configs that have > caused failures in the past), has lots of irq issues and fails to > initialize the network of my box. I bisected the problem down to a > single commit, and when I revert that commit, my box boots without any > network or irq issues. > > Note, my other configs work fine on this box. I haven't investigated > which config is also the culprit. But since it use to work with this > config, I want to report it. So what commit is causing the problem? It looks like the ehci driver is having problems, but first, your interrupts are whack: > irq 16: nobody cared (try booting with the "irqpoll" option) > CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.12.0-rc1-test-dirty #24 > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 > Call Trace: > > devtmpfs: mounted > dump_stack+0x9a/0xd6 > __report_bad_irq+0x35/0xc0 > note_interrupt+0x234/0x270 > handle_irq_event_percpu+0x45/0x60 > handle_irq_event+0x39/0x60 > handle_fasteoi_irq+0x8f/0x160 > handle_irq+0x6f/0x110 > do_IRQ+0x46/0xd0 > common_interrupt+0x93/0x93 > RIP: 0010:native_safe_halt+0x6/0x10 > RSP: :b54240cd7e90 EFLAGS: 0286 ORIG_RAX: ff7e > RAX: RBX: 8ea214498040 RCX: > RDX: RSI: RDI: > RBP: b54240cd7e90 R08: 0001 R09: 41129b0c > R10: b54240cd7d68 R11: 0001 R12: 0002 > R13: 8ea214498040 R14: R15: 8ea214498040 > > default_idle+0x38/0x160 > arch_cpu_idle+0xf/0x20 > default_idle_call+0x28/0x50 > do_idle+0x182/0x220 > cpu_startup_entry+0x1d/0x20 > start_secondary+0x132/0x160 > secondary_startup_64+0x9f/0x9f > handlers: > [] xhci_msi_irq > Disabling IRQ #16 Have you tried taking the kernel's advice? :) > ehci-pci :00:1a.0: new USB bus registered, assigned bus number 3 > ehci-pci :00:1a.0: debug port 2 > ehci-pci :00:1a.0: cache line size of 64 is not supported > genirq: Flags mismatch irq 16. 0080 (ehci_hcd:usb3) vs. > (xhci_hcd) What does that mean? > CPU: 0 PID: 307 Comm: modprobe Tainted: GE > 4.12.0-rc1-test-dirty #24 > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 > Call Trace: > dump_stack+0x9a/0xd6 > __setup_irq+0x5d4/0x630 > request_threaded_irq+0x10d/0x190 > usb_add_hcd+0x658/0x970 > ? for_each_companion+0x3e/0xb0 > usb_hcd_pci_probe+0x3e4/0x490 > ehci_pci_probe+0x36/0x40 [ehci_pci] > local_pci_probe+0x45/0xa0 > ? pci_match_device+0xca/0x110 > pci_device_probe+0xdb/0x130 > driver_probe_device+0x2ed/0x480 > __driver_attach+0xd5/0x100 > ? driver_probe_device+0x480/0x480 > bus_for_each_dev+0x62/0xa0 > driver_attach+0x1e/0x20 > bus_add_driver+0x1c6/0x290 > driver_register+0x60/0xe0 > __pci_register_driver+0x60/0x70 > ? 0xc0346000 > ehci_pci_init+0x6a/0x1000 [ehci_pci] > do_one_initcall+0x43/0x190 > ? kmem_cache_alloc_trace+0x1be/0x200 > do_init_module+0x7d/0x210 > load_module+0x1891/0x1eb0 > ? vmap_page_range_noflush+0x29b/0x370 > ? show_coresize+0x30/0x30 > SYSC_init_module+0x143/0x180 > ? load_module+0x5/0x1eb0 > ? SYSC_init_module+0x143/0x180 > SyS_init_module+0xe/0x10 > entry_SYSCALL_64_fastpath+0x23/0xc2 > RIP: 0033:0x3b918e0ffa > RSP: 002b:7ffd11d575c8 EFLAGS: 0246 ORIG_RAX: 00af > RAX: ffda RBX: 0061f950 RCX: 003b918e0ffa > RDX: 0061f7d0 RSI: 36b0 RDI: 0062c9e0 > RBP: R08: 00630090 R09: 7f019c07c700 > R10: 7ffd11d574f0 R11: 0246 R12: 00626200 > R13: 0061f930 R14: R15: 0061f420 > ehci-pci :00:1a.0: request interrupt 16 failed So ehci can't use the same irq line as xhci? No sharing allowed? But other configs on this same hardware work, can you do a diff of a working vs. not working? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH resend v3 1/4] usb: usbip tool: Check the return of get_nports()
On Thu, May 18, 2017 at 03:53:04PM +0300, Sergei Shtylyov wrote: > >+if (vhci_driver->nports <=0) { > >Please add a space after <= too. Indeed. Thanks. Hi Greg, Do you want me to send another version to fix this? Thanks, Yuyang -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html