Re: [PATCHv2] usb: musb: Fix unbalanced platform_disable
Hi Tony, On Monday 19 Sep 2016 15:41:50 Tony Lindgren wrote: > * Laurent Pinchart [160919 13:35]: > > On Sunday 18 Sep 2016 08:19:02 Tony Lindgren wrote: > >>> [5.711303] [] (_raw_spin_unlock_irqrestore) from > >>> [] > >>> (musb_gadget_queue+0x128/0x4ac) > >>> [5.711303] [] (musb_gadget_queue) from [] > >>> (usb_ep_queue+0x38/0x1d4) > >>> [5.729766] [] (usb_ep_queue) from [] > >>> (rx_submit+0xc8/0x19c) > >>> [5.737548] [] (rx_submit) from [] > >>> (rx_fill+0x7c/0xa0) [5.737548] [] (rx_fill) from > >>> [] (eth_start+0x28/0x48) [5.751983] [] > >>> (eth_start) from [] (eth_open+0x6c/0x7c) [5.751983] > >>> [] (eth_open) from [] > >>> (__dev_open+0x9c/0x104) > >> > >> This could be something else though. Care to email me your .config, > >> maybe this is related to legacy g_ether being built in? > > > > Sure, please find it attached. The legacy g_ether is indeed built-in, > > that's what I use to boot over nfsroot. > > OK, I think g_ether may have issues in general.. > > I mostly test with configfs based gadgets and shell script as then I can > test load/configure/connect/disconnect/unconfigure/unload easily :) g_ether is very convenient when using nfsroot, as it allows booting the system without an initramfs. > >> Anyways, please also give the following patch a try. > > > > I've tested the patch and if fixes the original problem. Warm reboots are > > still broken though. > > No luck here with your .config a try with my pandaboard es against v4.8-rc7 > plus the patch I posted on Sunday. It reboots with no errors for me with > NFSroot. Do you do something other than just reboot? No, I perform the following steps: - Connect the panda board to the USB through USB (which powers the board on) - Let the board boot over NFS - Log in as root, run 'reboot' The second boot produces the warning. [5.189025] [ cut here ] [5.193450] WARNING: CPU: 0 PID: 1 at /home/laurent/src/kernel/omap4/linux-2.6/drivers/bus/omap_l3_noc.c:147 l3_interrupt_handler+0x220/0x348 [5.198059] 4400.ocp:L3 Custom Error: MASTER MPU TARGET L4CFG (Read): Data Access in User mode during Functional access [5.218933] Modules linked in: [5.218933] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc2-00816- g0caf606bb84a #20 [5.222167] Hardware name: Generic OMAP4 (Flattened Device Tree) [5.233612] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [5.233612] [] (show_stack) from [] (dump_stack+0xa8/0xe0) [5.233612] [] (dump_stack) from [] (__warn+0xd8/0x104) [5.253662] [] (__warn) from [] (warn_slowpath_fmt+0x38/0x48) [5.253662] [] (warn_slowpath_fmt) from [] (l3_interrupt_handler+0x220/0x348) [5.277191] [] (l3_interrupt_handler) from [] (__handle_irq_event_percpu+0x98/0x3ec) [5.277191] [] (__handle_irq_event_percpu) from [] (handle_irq_event_percpu+0x1c/0x58) [5.293426] [] (handle_irq_event_percpu) from [] (handle_irq_event+0x38/0x5c) [5.306610] [] (handle_irq_event) from [] (handle_fasteoi_irq+0xcc/0x1a4) [5.306610] [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x18/0x28) [5.315582] [] (generic_handle_irq) from [] (__handle_domain_irq+0x64/0xdc) [5.315582] [] (__handle_domain_irq) from [] (gic_handle_irq+0x48/0x9c) [5.315582] [] (gic_handle_irq) from [] (__irq_svc+0x70/0x98) [5.350402] Exception stack(0xee8b7d18 to 0xee8b7d60) [5.350402] 7d00: 0001 ee8b5328 [5.364288] 7d20: ee8b4d80 6153 eee54010 eee54010 6153 0002 c1603ae4 [5.364288] 7d40: c0d029cc 016b c0f19314 ee8b7d68 c019268c c086f5c0 2153 [5.381469] [] (__irq_svc) from [] (_raw_spin_unlock_irqrestore+0x34/0x44) [5.381469] [] (_raw_spin_unlock_irqrestore) from [] (musb_gadget_queue+0x128/0x4ac) [5.390533] [] (musb_gadget_queue) from [] (usb_ep_queue+0x38/0x1d4) [5.408996] [] (usb_ep_queue) from [] (rx_submit+0xc8/0x19c) [5.408996] [] (rx_submit) from [] (rx_fill+0x7c/0xa0) [5.408996] [] (rx_fill) from [] (eth_start+0x28/0x48) [5.431213] [] (eth_start) from [] (eth_open+0x6c/0x7c) [5.431213] [] (eth_open) from [] (__dev_open+0x9c/0x104) [5.446044] [] (__dev_open) from [] (__dev_change_flags+0x88/0x150) [5.446044] [] (__dev_change_flags) from [] (dev_change_flags+0x18/0x48) [5.454467] [] (dev_change_flags) from [] (ip_auto_config+0x194/0x1148) [5.454467] [] (ip_auto_config) from [] (do_one_initcall+0x3c/0x174) [5.480621] [] (do_one_initcall) from [] (kernel_init_freeable+0x204/0x2e0) [5.480621] [] (kernel_init_freeable) from [] (kernel_init+0x8/0x118) [5.480621] [] (kernel_init) from [] (ret_from_fork+0x14/0x24) [5.506347] ---[ end trace 9a597c69856245a5 ]--- -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://
Re: [PATCH] usb: misc: legousbtower: Fix NULL pointer deference
On Mon, Sep 19, 2016 at 07:09:51PM +0100, James wrote: > This patch fixes a NULL pointer dereference caused by a race codition in the > probe function of the legousbtower driver. It re-structures the probe > function to only register the interface after successfully reading the > board's firmware ID. > > The probe function does not deregister the usb interface after an error > receiving the devices firmware ID. The device file registered > (/dev/usb/legousbtower%d) may be read/written globally before the probe > function returns. When tower_delete is called in the probe function > (after an r/w has been initiated), core dev structures are deleted while > the file operation functions are still running. If the 0 address is mappable > on the machine, this vulnerability can be used to create a Local Priviege > Escalation exploit via a write-what-where condition by remapping > dev->interrupt_out_buffer in tower_write. A forged USB device and local > program execution would be required for LPE. The USB > device would have to delay the control message in tower_probe and accept > the control urb in tower_open whilst guest code initiated a write to the > device file as tower_delete is called from the error in tower_probe. > > This bug has existed since 2003. Patch tested by emulated device. > > Signed-off-by: James Patrick-Evans > --- > drivers/usb/misc/legousbtower.c | 35 +-- > 1 file changed, 17 insertions(+), 18 deletions(-) When you sent this the last time (back on Sept 2), I sent you a "simpler" patch, the same one below, and asked you if that worked instead. I never heard back, could you test this patch and see if it resolves the issue for you? thanks, greg k-h --- How about this simpler patch, that moves the registering for the minor number as the last thing that happens in the probe function (which is what it should be). Can you test this to verify it prevents the above problem? And this driver really needs an overhaul one of these days... thanks, greg k-h diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c index 7771be3ac178..4dd531ac5a7f 100644 --- a/drivers/usb/misc/legousbtower.c +++ b/drivers/usb/misc/legousbtower.c @@ -898,24 +898,6 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device dev->interrupt_in_interval = interrupt_in_interval ? interrupt_in_interval : dev->interrupt_in_endpoint->bInterval; dev->interrupt_out_interval = interrupt_out_interval ? interrupt_out_interval : dev->interrupt_out_endpoint->bInterval; - /* we can register the device now, as it is ready */ - usb_set_intfdata (interface, dev); - - retval = usb_register_dev (interface, &tower_class); - - if (retval) { - /* something prevented us from registering this driver */ - dev_err(idev, "Not able to get a minor for this device.\n"); - usb_set_intfdata (interface, NULL); - goto error; - } - dev->minor = interface->minor; - - /* let the user know what node this device is now attached to */ - dev_info(&interface->dev, "LEGO USB Tower #%d now attached to major " -"%d minor %d\n", (dev->minor - LEGO_USB_TOWER_MINOR_BASE), -USB_MAJOR, dev->minor); - /* get the firmware version and log it */ result = usb_control_msg (udev, usb_rcvctrlpipe(udev, 0), @@ -936,6 +918,23 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device get_version_reply.minor, le16_to_cpu(get_version_reply.build_no)); + /* we can register the device now, as it is ready */ + usb_set_intfdata (interface, dev); + + retval = usb_register_dev (interface, &tower_class); + + if (retval) { + /* something prevented us from registering this driver */ + dev_err(idev, "Not able to get a minor for this device.\n"); + usb_set_intfdata (interface, NULL); + goto error; + } + dev->minor = interface->minor; + + /* let the user know what node this device is now attached to */ + dev_info(&interface->dev, "LEGO USB Tower #%d now attached to major " +"%d minor %d\n", (dev->minor - LEGO_USB_TOWER_MINOR_BASE), +USB_MAJOR, dev->minor); exit: return retval; -- 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: [PATCHv2] usb: musb: Fix unbalanced platform_disable
On Mon, 19 Sep 2016 09:02:50 -0700 Tony Lindgren wrote: > * Andreas Kemnade [160918 23:00]: > > On Sun, 18 Sep 2016 08:19:02 -0700 > > Tony Lindgren wrote: > > > > > * Laurent Pinchart [160918 > > > 05:13]: > > > > > > > > FYI, while this patch allows me to boot my Panda board with NFS > > > > over usbnet, it only works with cold boots. A warm reboot > > > > results in the following warning, and no ethernet traffic going > > > > through. The USB device is detected by the host though. > > > > > > Yeah I noticed too that we still have issues. For example doing > > > rmmod of omap2430 with gadget configured and connected will > > > produce a hardirq-safe hardirq-unsafe lock order error. That also > > > happens with reboot with gadget configured and connected. > > > > > hmm, well, there is a musb_platform_disable() in musb_remove() > > which is simply superfluous... > > Some days ago we had a locking problem with musb_start() and moved > > it out of the locked area. Maybe we could do also something similar > > here. > > Well I don't think we can do that safely at this point because we > have unpaired calls to musb_start() and musb_stop(). So trying to > make musb_platform_enable/disable() paired right now will just lead > into mystery breakage in various use cases. > I am primarily talking about doing things like the patch usb: musb: Fix locking errors for host only mode 2c5575401e34de3d2f did for musb_start(), for musb_stop() also. Regards, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] phy: meson: add USB2 PHY support for Meson8b and GXBB
On Monday 19 September 2016 10:12 PM, Kevin Hilman wrote: > Kishon Vijay Abraham I writes: > >> Hi Kevin, >> >> On Wednesday 14 September 2016 09:36 PM, Kevin Hilman wrote: >>> Kishon, >>> >>> Martin Blumenstingl writes: >>> This is a new driver for the USB PHY found in Meson8b and GXBB SoCs. Signed-off-by: Martin Blumenstingl Signed-off-by: Jerome Brunet Tested-by: Kevin Hilman >>> >>> Will you be picking this up for v4.9? >> >> It's already late for 4.9. Generally send pull request to Greg around -rc6. >> This can go only in 4.10. > > That's fine. > > Does that mean you have it queued someplace? I don't see it in any of > your branches. I haven't queued so far. I'll create a testing branch to queue pending patches. Thanks Kishon -- 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/2] phy-sun4i-usb: Add sun4i_usb_phy_force_session_end() function
Hi, On Sunday 18 September 2016 10:20 PM, Hans de Goede wrote: > The sunxi musb has a bug where sometimes it will generate a babble > error on device disconnect instead of a disconnect irq. When this > happens the musb-controller switches from host mode to device mode > (it clears MUSB_DEVCTL_SESSION and sets MUSB_DEVCTL_BDEVICE) and > gets stuck in this state. > > Clearing this requires reporting Vbus low for 200 or more ms, but > on some devices Vbus is simply always high (host-only mode, no Vbus > control). The phy-sun4i-usb code already has code to force a session > end for devices without Vbus control. > > This commit adds a sun4i_usb_phy_force_session_end() function exporting > this functionality to the sunxi-musb glue, so that it can force a session > end to fixup the stuck state after a babble error. > > Signed-off-by: Hans de Goede > --- > drivers/phy/phy-sun4i-usb.c | 11 +++ > include/linux/phy/phy-sun4i-usb.h | 7 +++ > 2 files changed, 18 insertions(+) > > diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c > index 43c0d98..06f4e11a 100644 > --- a/drivers/phy/phy-sun4i-usb.c > +++ b/drivers/phy/phy-sun4i-usb.c > @@ -470,6 +470,17 @@ void sun4i_usb_phy_set_squelch_detect(struct phy *_phy, > bool enabled) > } > EXPORT_SYMBOL_GPL(sun4i_usb_phy_set_squelch_detect); > > +void sun4i_usb_phy_force_session_end(struct phy *_phy) > +{ > + struct sun4i_usb_phy *phy = phy_get_drvdata(_phy); > + struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy); > + > + data->id_det = -1; > + data->force_session_end = true; > + queue_delayed_work(system_wq, &data->detect, 0); > +} > +EXPORT_SYMBOL_GPL(sun4i_usb_phy_force_session_end); Er.. one more export symbol :-( > + > static const struct phy_ops sun4i_usb_phy_ops = { > .init = sun4i_usb_phy_init, > .exit = sun4i_usb_phy_exit, > diff --git a/include/linux/phy/phy-sun4i-usb.h > b/include/linux/phy/phy-sun4i-usb.h > index 50aed92..3bb773f 100644 > --- a/include/linux/phy/phy-sun4i-usb.h > +++ b/include/linux/phy/phy-sun4i-usb.h > @@ -23,4 +23,11 @@ > */ > void sun4i_usb_phy_set_squelch_detect(struct phy *phy, bool enabled); > > +/** > + * sun4i_usb_force_session_end() - Force the current session to end > + * by reporting VBus low for 200+ ms > + * @phy: reference to a sun4i usb phy > + */ > +void sun4i_usb_phy_force_session_end(struct phy *phy); Should we include a static inline function if sun4i phy is not defined? Thanks Kishon -- 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 v7 5/8] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node
From: Peter Chen At device tree, we have no device node for chipidea core, the glue layer's node is the parent node for host and udc device. But in related driver, the parent device is chipidea core. So, in order to let the common driver get parent's node, we let the core's device node equals glue layer device node. Signed-off-by: Peter Chen Tested-by: Maciej S. Szmigiero Tested-by Joshua Clayton --- drivers/usb/chipidea/core.c | 27 ++- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 69426e6..6839e19 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -927,6 +927,16 @@ static int ci_hdrc_probe(struct platform_device *pdev) return -ENODEV; } + /* +* At device tree, we have no device node for chipidea core, +* the glue layer's node is the parent node for host and udc +* device. But in related driver, the parent device is chipidea +* core. So, in order to let the common driver get parent's node, +* we let the core's device node equals glue layer's node. +*/ + if (dev->parent && dev->parent->of_node) + dev->of_node = dev->parent->of_node; + if (ci->platdata->phy) { ci->phy = ci->platdata->phy; } else if (ci->platdata->usb_phy) { @@ -937,11 +947,15 @@ static int ci_hdrc_probe(struct platform_device *pdev) /* if both generic PHY and USB PHY layers aren't enabled */ if (PTR_ERR(ci->phy) == -ENOSYS && - PTR_ERR(ci->usb_phy) == -ENXIO) - return -ENXIO; + PTR_ERR(ci->usb_phy) == -ENXIO) { + ret = -ENXIO; + goto clear_of_node; + } - if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy)) - return -EPROBE_DEFER; + if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy)) { + ret = -EPROBE_DEFER; + goto clear_of_node; + } if (IS_ERR(ci->phy)) ci->phy = NULL; @@ -952,7 +966,7 @@ static int ci_hdrc_probe(struct platform_device *pdev) ret = ci_usb_phy_init(ci); if (ret) { dev_err(dev, "unable to init phy: %d\n", ret); - return ret; + goto clear_of_node; } ci->hw_bank.phys = res->start; @@ -1058,6 +1072,8 @@ stop: ci_role_destroy(ci); deinit_phy: ci_usb_phy_exit(ci); +clear_of_node: + dev->of_node = NULL; return ret; } @@ -1076,6 +1092,7 @@ static int ci_hdrc_remove(struct platform_device *pdev) ci_extcon_unregister(ci); ci_role_destroy(ci); ci_hdrc_enter_lpm(ci, true); + ci->dev->of_node = NULL; ci_usb_phy_exit(ci); return 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
[PATCH v7 8/8] ARM: dts: imx6q-evi: Fix onboard hub reset line
From: Joshua Clayton Previously the onboard hub was made to work by treating its reset gpio as a regulator enable. Get rid of that kludge now that pwseq has added reset gpio support Move pin muxing the hub reset pin into the usbh1 group Signed-off-by: Joshua Clayton Signed-off-by: Peter Chen --- arch/arm/boot/dts/imx6q-evi.dts | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts index 4fa5601..49c6f61 100644 --- a/arch/arm/boot/dts/imx6q-evi.dts +++ b/arch/arm/boot/dts/imx6q-evi.dts @@ -54,18 +54,6 @@ reg = <0x1000 0x4000>; }; - reg_usbh1_vbus: regulator-usbhubreset { - compatible = "regulator-fixed"; - regulator-name = "usbh1_vbus"; - regulator-min-microvolt = <500>; - regulator-max-microvolt = <500>; - enable-active-high; - startup-delay-us = <2>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_usbh1_hubreset>; - gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>; - }; - reg_usb_otg_vbus: regulator-usbotgvbus { compatible = "regulator-fixed"; regulator-name = "usb_otg_vbus"; @@ -204,12 +192,18 @@ }; &usbh1 { - vbus-supply = <®_usbh1_vbus>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_usbh1>; dr_mode = "host"; disable-over-current; status = "okay"; + + usb2415host: hub@1 { + compatible = "usb424,2513"; + reg = <1>; + reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>; + reset-duration-us = <3000>; + }; }; &usbotg { @@ -467,11 +461,6 @@ MX6QDL_PAD_GPIO_3__USB_H1_OC 0x1b0b0 /* usbh1_b OC */ MX6QDL_PAD_GPIO_0__GPIO1_IO00 0x1b0b0 - >; - }; - - pinctrl_usbh1_hubreset: usbh1hubresetgrp { - fsl,pins = < MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x1b0b0 >; }; -- 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 v7 6/8] ARM: dts: imx6qdl: Enable usb node children with
From: Joshua Clayton Give usb nodes #address and #size attributes, so that a child node representing a permanently connected device such as an onboard hub may be addressed with a attribute Signed-off-by: Joshua Clayton Signed-off-by: Peter Chen --- arch/arm/boot/dts/imx6qdl.dtsi | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index b13b0b2..8fc66e1 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -935,6 +935,8 @@ usbh1: usb@02184200 { compatible = "fsl,imx6q-usb", "fsl,imx27-usb"; + #address-cells = <1>; + #size-cells = <0>; reg = <0x02184200 0x200>; interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6QDL_CLK_USBOH3>; @@ -949,6 +951,8 @@ usbh2: usb@02184400 { compatible = "fsl,imx6q-usb", "fsl,imx27-usb"; + #address-cells = <1>; + #size-cells = <0>; reg = <0x02184400 0x200>; interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6QDL_CLK_USBOH3>; @@ -962,6 +966,8 @@ usbh3: usb@02184600 { compatible = "fsl,imx6q-usb", "fsl,imx27-usb"; + #address-cells = <1>; + #size-cells = <0>; reg = <0x02184600 0x200>; interrupts = <0 42 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clks IMX6QDL_CLK_USBOH3>; -- 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 v7 1/8] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library
Add binding doc for generic power sequence library. Signed-off-by: Peter Chen Acked-by: Philipp Zabel Acked-by: Rob Herring --- .../bindings/power/pwrseq/pwrseq-generic.txt | 48 ++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt new file mode 100644 index 000..ebf0d47 --- /dev/null +++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt @@ -0,0 +1,48 @@ +The generic power sequence library + +Some hard-wired devices (eg USB/MMC) need to do power sequence before +the device can be enumerated on the bus, the typical power sequence +like: enable USB PHY clock, toggle reset pin, etc. But current +Linux device driver lacks of such code to do it, it may cause some +hard-wired devices works abnormal or can't be recognized by +controller at all. The power sequence will be done before this device +can be found at the bus. + +The power sequence properties is under the device node. + +Optional properties: +- clocks: the input clocks for device. +- reset-gpios: Should specify the GPIO for reset. +- reset-duration-us: the duration in microsecond for assert reset signal. + +Below is the example of USB power sequence properties on USB device +nodes which have two level USB hubs. + +&usbotg1 { + vbus-supply = <®_usb_otg1_vbus>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_usb_otg1_id>; + status = "okay"; + + #address-cells = <1>; + #size-cells = <0>; + genesys: hub@1 { + compatible = "usb5e3,608"; + reg = <1>; + + clocks = <&clks IMX6SX_CLK_CKO>; + reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */ + reset-duration-us = <10>; + + #address-cells = <1>; + #size-cells = <0>; + asix: ethernet@1 { + compatible = "usbb95,1708"; + reg = <1>; + + clocks = <&clks IMX6SX_CLK_IPG>; + reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */ + reset-duration-us = <15>; + }; + }; +}; -- 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 v7 2/8] power: add power sequence library
We have an well-known problem that the device needs to do some power sequence before it can be recognized by related host, the typical example like hard-wired mmc devices and usb devices. This power sequence is hard to be described at device tree and handled by related host driver, so we have created a common power sequence library to cover this requirement. The core code has supplied some common helpers for host driver, and individual power sequence libraries handle kinds of power sequence for devices. pwrseq_generic is intended for general purpose of power sequence, which handles gpios and clocks currently, and can cover regulator and pinctrl in future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off if only one power sequence is needed, else call of_pwrseq_on_list /of_pwrseq_off_list instead (eg, USB hub driver). Signed-off-by: Peter Chen Tested-by Joshua Clayton Reviewed-by: Matthias Kaehlcke Tested-by: Matthias Kaehlcke --- MAINTAINERS | 9 ++ drivers/power/Kconfig | 1 + drivers/power/Makefile | 1 + drivers/power/pwrseq/Kconfig| 45 ++ drivers/power/pwrseq/Makefile | 3 + drivers/power/pwrseq/core.c | 190 drivers/power/pwrseq/pwrseq_compatible_sample.c | 178 ++ drivers/power/pwrseq/pwrseq_generic.c | 177 ++ include/linux/power/pwrseq.h| 73 + 9 files changed, 677 insertions(+) create mode 100644 drivers/power/pwrseq/Kconfig create mode 100644 drivers/power/pwrseq/Makefile create mode 100644 drivers/power/pwrseq/core.c create mode 100644 drivers/power/pwrseq/pwrseq_compatible_sample.c create mode 100644 drivers/power/pwrseq/pwrseq_generic.c create mode 100644 include/linux/power/pwrseq.h diff --git a/MAINTAINERS b/MAINTAINERS index b3e9395..b353769 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9343,6 +9343,15 @@ F: include/linux/pm_* F: include/linux/powercap.h F: drivers/powercap/ +POWER SEQUENCE LIBRARY +M: Peter Chen +T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git +L: linux...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/power/pwrseq/ +F: drivers/power/pwrseq/ +F: include/linux/power/pwrseq.h/ + POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS M: Sebastian Reichel M: Dmitry Eremin-Solenikov diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index acd4a15..f6aa4fd 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -515,3 +515,4 @@ endif # POWER_SUPPLY source "drivers/power/reset/Kconfig" source "drivers/power/avs/Kconfig" +source "drivers/power/pwrseq/Kconfig" diff --git a/drivers/power/Makefile b/drivers/power/Makefile index e46b75d..4ed2e12 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65217)+= tps65217_charger.o obj-$(CONFIG_POWER_RESET) += reset/ obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o +obj-$(CONFIG_POWER_SEQUENCE) += pwrseq/ diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig new file mode 100644 index 000..dff5e35 --- /dev/null +++ b/drivers/power/pwrseq/Kconfig @@ -0,0 +1,45 @@ +# +# Power Sequence library +# + +config POWER_SEQUENCE + bool + +menu "Power Sequence Support" + +config PWRSEQ_GENERIC + bool "Generic power sequence control" + depends on OF + select POWER_SEQUENCE + help + It is used for drivers which needs to do power sequence + (eg, turn on clock, toggle reset gpio) before the related + devices can be found by hardware. This generic one can be + used for common power sequence control. + +config PWRSEQ_GENERIC_INSTANCE_NUMBER + int "Number of Generic Power Sequence Instance" + depends on PWRSEQ_GENERIC + range 1 10 + default 2 + help + Usually, there are not so many devices needs power sequence, we set two + as default value. + +config PWRSEQ_SAMPLE + bool "sample power sequence control using compatible string" + depends on OF + select POWER_SEQUENCE + help + It is a sample library which implements power sequence for device id, + it is an example purpose. + +config PWRSEQ_SAMPLE_INSTANCE_NUMBER + int "Number of Sample Power Sequence Instance" + depends on PWRSEQ_SAMPLE + range 1 5 + default 1 + help + Usually, this file is special for certain device, so the default for this number + is 1. +endmenu diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile new file mode 100644 index 000..62f3cbf --- /dev/null +++ b/drivers/power/pwrseq/Makefile @@ -0,0 +1,3 @@ +obj-$(CONFIG_PO
[PATCH v7 4/8] usb: core: add power sequence handling for USB devices
Some hard-wired USB devices need to do power sequence to let the device work normally, the typical power sequence like: enable USB PHY clock, toggle reset pin, etc. But current Linux USB driver lacks of such code to do it, it may cause some hard-wired USB devices works abnormal or can't be recognized by controller at all. In this patch, it calls power sequence library APIs to finish the power sequence events. It will do power on sequence at hub's probe for all devices under this hub (includes root hub). At hub_disconnect, it will do power off sequence which is at powered on list. Signed-off-by: Peter Chen Tested-by Joshua Clayton --- drivers/usb/core/hub.c | 41 ++--- drivers/usb/core/hub.h | 1 + 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index b48dc76..f3de1de 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -1695,6 +1696,7 @@ static void hub_disconnect(struct usb_interface *intf) hub->error = 0; hub_quiesce(hub, HUB_DISCONNECT); + of_pwrseq_off_list(&hub->pwrseq_on_list); mutex_lock(&usb_port_peer_mutex); /* Avoid races with recursively_mark_NOTATTACHED() */ @@ -1722,12 +1724,41 @@ static void hub_disconnect(struct usb_interface *intf) kref_put(&hub->kref, hub_release); } +#ifdef CONFIG_OF +static int hub_of_pwrseq_on(struct usb_hub *hub) +{ + struct device *parent; + struct usb_device *hdev = hub->hdev; + struct device_node *np; + int ret; + + if (hdev->parent) + parent = &hdev->dev; + else + parent = bus_to_hcd(hdev->bus)->self.controller; + + for_each_child_of_node(parent->of_node, np) { + ret = of_pwrseq_on_list(np, &hub->pwrseq_on_list); + if (ret) + return ret; + } + + return 0; +} +#else +static int hub_of_pwrseq_on(struct usb_hub *hub) +{ + return 0; +} +#endif + static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_host_interface *desc; struct usb_endpoint_descriptor *endpoint; struct usb_device *hdev; struct usb_hub *hub; + int ret = -ENODEV; desc = intf->cur_altsetting; hdev = interface_to_usbdev(intf); @@ -1832,6 +1863,7 @@ descriptor_error: INIT_DELAYED_WORK(&hub->leds, led_work); INIT_DELAYED_WORK(&hub->init_work, NULL); INIT_WORK(&hub->events, hub_event); + INIT_LIST_HEAD(&hub->pwrseq_on_list); usb_get_intf(intf); usb_get_dev(hdev); @@ -1845,11 +1877,14 @@ descriptor_error: if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND) hub->quirk_check_port_auto_suspend = 1; - if (hub_configure(hub, endpoint) >= 0) - return 0; + if (hub_configure(hub, endpoint) >= 0) { + ret = hub_of_pwrseq_on(hub); + if (!ret) + return 0; + } hub_disconnect(intf); - return -ENODEV; + return ret; } static int diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index 34c1a7e..cd86f91 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -78,6 +78,7 @@ struct usb_hub { struct delayed_work init_work; struct work_struct events; struct usb_port **ports; + struct list_headpwrseq_on_list; /* powered pwrseq node list */ }; /** -- 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 v7 7/8] ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property
The current dts describes USB HUB's property at USB controller's entry, it is improper. The USB HUB should be the child node under USB controller, and power sequence properties are under it. Besides, using gpio pinctrl setting for USB2415's reset pin. Signed-off-by: Peter Chen Signed-off-by: Joshua Clayton --- arch/arm/boot/dts/imx6qdl-udoo.dtsi | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/arch/arm/boot/dts/imx6qdl-udoo.dtsi b/arch/arm/boot/dts/imx6qdl-udoo.dtsi index 3bee2f9..87fe31f 100644 --- a/arch/arm/boot/dts/imx6qdl-udoo.dtsi +++ b/arch/arm/boot/dts/imx6qdl-udoo.dtsi @@ -9,6 +9,8 @@ * */ +#include + / { aliases { backlight = &backlight; @@ -58,17 +60,6 @@ #address-cells = <1>; #size-cells = <0>; - reg_usb_h1_vbus: regulator@0 { - compatible = "regulator-fixed"; - reg = <0>; - regulator-name = "usb_h1_vbus"; - regulator-min-microvolt = <500>; - regulator-max-microvolt = <500>; - enable-active-high; - startup-delay-us = <2>; /* USB2415 requires a POR of 1 us minimum */ - gpio = <&gpio7 12 0>; - }; - reg_panel: regulator@1 { compatible = "regulator-fixed"; reg = <1>; @@ -188,7 +179,7 @@ pinctrl_usbh: usbhgrp { fsl,pins = < - MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x8000 + MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x1b0b0 MX6QDL_PAD_NANDF_CS2__CCM_CLKO2 0x130b0 >; }; @@ -259,9 +250,16 @@ &usbh1 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_usbh>; - vbus-supply = <®_usb_h1_vbus>; - clocks = <&clks IMX6QDL_CLK_CKO>; status = "okay"; + + usb2415: hub@1 { + compatible = "usb424,2514"; + reg = <1>; + + clocks = <&clks IMX6QDL_CLK_CKO>; + reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>; + reset-duration-us = <3000>; + }; }; &usdhc3 { -- 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 v7 3/8] binding-doc: usb: usb-device: add optional properties for power sequence
Add optional properties for power sequence. Signed-off-by: Peter Chen Acked-by: Rob Herring --- Documentation/devicetree/bindings/usb/usb-device.txt | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt index 1c35e7b..3661dd2 100644 --- a/Documentation/devicetree/bindings/usb/usb-device.txt +++ b/Documentation/devicetree/bindings/usb/usb-device.txt @@ -13,6 +13,10 @@ Required properties: - reg: the port number which this device is connecting to, the range is 1-31. +Optional properties: +power sequence properties, see +Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt for detail + Example: &usb1 { @@ -21,8 +25,12 @@ Example: #address-cells = <1>; #size-cells = <0>; - hub: genesys@1 { + genesys: hub@1 { compatible = "usb5e3,608"; reg = <1>; + + clocks = <&clks IMX6SX_CLK_CKO>; + reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */ + reset-duration-us = <10>; }; } -- 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 v7 0/8] power: add power sequence library
Hi all, This is a follow-up for my last power sequence framework patch set [1]. According to Rob Herring and Ulf Hansson's comments[2]. The kinds of power sequence instances will be added at postcore_initcall, the match criteria is compatible string first, if the compatible string is not matched between dts and library, it will try to use generic power sequence. The host driver just needs to call of_pwrseq_on/of_pwrseq_off if only one power sequence instance is needed, for more power sequences are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver). In future, if there are special power sequence requirements, the special power sequence library can be created. This patch set is tested on i.mx6 sabresx evk using a dts change, I use two hot-plug devices to simulate this use case, the related binding change is updated at patch [1/6], The udoo board changes were tested using my last power sequence patch set.[3] Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also need to power on itself before it can be found by ULPI bus. [1] http://www.spinics.net/lists/linux-usb/msg142755.html [2] http://www.spinics.net/lists/linux-usb/msg143106.html [3] http://www.spinics.net/lists/linux-usb/msg142815.html Changes for v7: - Create kinds of power sequence instance at postcore_initcall, and match the instance with node using compatible string, the beneit of this is the host driver doesn't need to consider which pwrseq instance needs to be used, and pwrseq core will match it, however, it eats some memories if less power sequence instances are used. [Patch 2/8] - Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 2/8] - Fix the comments Vaibhav Hiremath adds for error path for clock and do not use device_node for parameters at pwrseq_on. [Patch 2/8] - Simplify the caller to use power sequence, follows Alan's commnets [Patch 4/8] - Tested three pwrseq instances together using both specific compatible string and generic libraries. Changes for v6: - Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6]) - Change chipidea core of_node assignment for coming user. (patch [5/6]) - Applies Joshua Clayton's three dts changes for two boards, the USB device's reg has only #address-cells, but without #size-cells. Changes for v5: - Delete pwrseq_register/pwrseq_unregister, which is useless currently - Fix the linker error when the pwrseq user is compiled as module Changes for v4: - Create the patch on next-20160722 - Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6] - Using more friendly wait method for reset gpio [Patch 2/6] - Support multiple input clocks [Patch 2/6] - Add Rob Herring's ack for DT changes - Add Joshua Clayton's Tested-by Changes for v3: - Delete "power-sequence" property at binding-doc, and change related code at both library and user code. - Change binding-doc example node name with Rob's comments - of_get_named_gpio_flags only gets the gpio, but without setting gpio flags, add additional code request gpio with proper gpio flags - Add Philipp Zabel's Ack and MAINTAINER's entry Changes for v2: - Delete "pwrseq" prefix and clock-names for properties at dt binding - Should use structure not but its pointer for kzalloc - Since chipidea core has no of_node, let core's of_node equals glue layer's at core's probe Joshua Clayton (2): ARM: dts: imx6qdl: Enable usb node children with ARM: dts: imx6q-evi: Fix onboard hub reset line Peter Chen (6): binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library power: add power sequence library binding-doc: usb: usb-device: add optional properties for power sequence usb: core: add power sequence handling for USB devices usb: chipidea: let chipidea core device of_node equal's glue layer device of_node ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property .../bindings/power/pwrseq/pwrseq-generic.txt | 48 ++ .../devicetree/bindings/usb/usb-device.txt | 10 +- MAINTAINERS| 9 + arch/arm/boot/dts/imx6q-evi.dts| 25 +-- arch/arm/boot/dts/imx6qdl-udoo.dtsi| 26 ++- arch/arm/boot/dts/imx6qdl.dtsi | 6 + drivers/power/Kconfig | 1 + drivers/power/Makefile | 1 + drivers/power/pwrseq/Kconfig | 45 + drivers/power/pwrseq/Makefile | 3 + drivers/power/pwrseq/core.c| 190 + drivers/power/pwrseq/pwrseq_compatible_sample.c| 178 +++ drivers/power/pwrseq/pwrseq_generic.c | 177 +++ drivers/usb/chipidea/core.c| 27 ++- drivers/usb/core/hub.c | 41 - drivers/usb/core/hub.h | 1 + include/l
Re: UAS and f_tcm -- is anyone using it?
On 8/29/2016 11:26 AM, John Youn wrote: > On 8/29/2016 12:33 AM, Felipe Balbi wrote: >> >> Hi, >> >> John Youn writes: >>> On 8/26/2016 12:48 AM, Felipe Balbi wrote: Hi, John Youn writes: > I was wondering if anyone is using the f_tcm function? Specifically > for UAS in superspeed with streams? Any idea if it is being using in > production for this anywhere? > > I've been trying to get the tcm gadget running without success. It > seems I need to configure the target system via configfs and somehow > interface it to this gadget. But I have not found any documentation or > examples on how to get it working. Anyone have ideas or pointers? Sebastian has posted his scripts here several times, it's in the archives :-) >>> >>> I found some scripts before but none of them resulted in a working >>> system. Though I did manage to get something semi-working >>> eventually. It would still be nice to have some documentation about >>> this part especially as I have no knowledge of the target side and no >>> idea what those scripts are doing. >> >> What do you mean by "semi-working"? How far did you get? Can you capture >> tracepoints so we figure out what's wrong? >> >> # mkdir -p /t >> # mount -t tracefs none /t >> # echo 8192 > /t/buffer_size_kb >> # echo 1 > /t/events/dwc3/enable >> >> (trigger issue) >> >> # cp /t/trace ~/trace.txt >> >> Send me trace.txt ;-) >> There's also a TCM python tool somewhere which helps with this. I haven't used f_tcm in a long while, but Sebastian and I used it long back to test streams with dwc3. >>> >>> Have you tried it recently? Or do you know of anyone who has? >> >> Sebastian is the only one I know who has used this in the past. >> > Just from the code it seems there will be some fundamental issues with > it, such as the value of maxpacket size and some alt-interface > stuff. At least when used with DWC3. such as? >>> >>> I'll see if I can write up the exact issues later. I have to go back >>> to my notes to remind myself. Ok, coming back to this uas gadget stuff. I've finally had a chance to go back to my notes. Here are some of the concrete issues that I found, that I was able to workaround in some way. * EP's for UAS alt-interface cannot be configured correctly because the config_ep_for_speed() in composite.c does not take into account alt-interfaces. This results in incorrectly configured EP in the controller (no streaming enabled, wrong direction, etc). * START_XFER command needs to enable streams * XFER_NOT_READY event needs to be enabled for streams * For OUT EPs, the TCM/target framework sets receive buffers size as 512 bytes. This cannot work in SUPERSPEED as you must always provide at least MPS-sized buffers. This causes all writes to fail. I'm not sure how to properly fix this as it should be fixed at the function driver level, and this size comes from the target framework. I put a workaround at the controller-level. After addressing these issues, UAS read/write works to some extent. But it is still unstable in ways that point to issues in the target framework, things to do with locking/race conditions there. Even the BOT interface does not work reliably. When I get a chance, I'll try to get everything running again rebased on latest, then submit patches and bug reports. >>> >>> But just in trying to get it to work, these issues make me suspect no >>> one is using this driver in superspeed, or at least regularly testing >>> it, let alone using it in production. >> >> that's probably true, but it's not enough argument to have a duplicate >> implementation of it :-) Rather, we should be figuring out what's broken >> and fixing it. I have a ton of other stuff to be done, but I'll add this >> to my queue, no issues. >> Let me know if you make any progress on this. Regards, John -- 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: USB hot-plug not working (ASUS TP301UA-C4028T)
Dear Oliver, Sorry about my last message, the bug is actually still going on. I found something interesting though: when AC is plugged, USB is working totally OK, but when on battery, then here comes trouble... Could this be power management? Below is my TLP config (mostly default) if it's any help: http://paste.opensuse.org/8168b819 I tried to disable USB-suspend: doesn't change anything. I cannot unload xhci_hcd module: # modprobe -r xhci_hcd modprobe: FATAL: Module xhci_hcd is in use. Something specific I should use? Also, after entering your command below to enhance debug, dmesg is still not showing anything when USB is plugged. Still, below is the output of dmesg when "udevadm trigger" is entered with a USB mouse plugged: http://paste.opensuse.org/325663de Hope this helps and sorry for the confusion with my last e-mail. Cheers, Pierre. Le mardi 20 septembre 2016, 09:43:57 NZST Pierre de Villemereuil a écrit : > Dear Oliver, > > I'm not really sure what happened, but now USB appears to be working > perfectly well... In the meantime, since my last checks, I simply updated > Tumbleweed and entered the command below as root (I rebooted twice since > then: USB still working!). > > I didn't find anything relevant in the Tumbleweed updates, only this one is > related to USB, but the only commit doesn't seem to be fixing anything at > runtime: > > usb_modeswitch > Subpackages: usb_modeswitch-data > > - Avoid a race in make install, which lead to packaging a truncated > usb_modeswitch_dispatcher script. Fixes boo#998641 > > But I dont think the command below is still active after a reboot (is it?) > and it seems just to activate some more debugging, so I really don't > understand how this happened! > > Sorry, this must be a quite frustrating report... > > Cheers, > Pierre. > > Le lundi 19 septembre 2016, 11:22:52 NZST Oliver Neukum a écrit : > > On Sun, 2016-09-18 at 19:48 +1200, Pierre de Villemereuil wrote: > > > Dear kernel devs, > > > > > > I'm using openSUSE Tumbleweed (kernel 4.7.2-2-default) on an ASUS > > > Vivobook > > > Flip TP301UA-C4028T, see here for specs: > > > https://www.asus.com/Notebooks/ASUS-VivoBook-Flip-TP301UA/specifications > > > / > > > > > > Since my install in July, USB hot-plug (a.k.a. plug'n'play) is not > > > working. To make any USB device working (I tested many USB keys, a > > > slideshow remote control and a mouse), it has to be plugged on boot up > > > or > > > wake up from suspend. Using the following command also make the system > > > aware that USB is connected: udevadm trigger > > > > > > More information can be found on the following thread on the openSUSE > > > forums: > > > https://forums.opensuse.org/showthread.php/519926-No-USB-device-detected > > > > Basically dmesg should show a hotplug. What happens if you unload and > > reload the xhci_hcd modules? > > > > And please do > > > > echo "module xhci_hcd +mpf" > /sys/kernel/debug/dynamic_debug/control > > > > (as root) and check dmesg again. > > > > 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: [PATCHv2] usb: musb: Fix unbalanced platform_disable
* Laurent Pinchart [160919 13:35]: > On Sunday 18 Sep 2016 08:19:02 Tony Lindgren wrote: > > > [5.711303] [] (_raw_spin_unlock_irqrestore) from > > > [] > > > (musb_gadget_queue+0x128/0x4ac) > > > [5.711303] [] (musb_gadget_queue) from [] > > > (usb_ep_queue+0x38/0x1d4) > > > [5.729766] [] (usb_ep_queue) from [] > > > (rx_submit+0xc8/0x19c) > > > [5.737548] [] (rx_submit) from [] > > > (rx_fill+0x7c/0xa0) [5.737548] [] (rx_fill) from > > > [] (eth_start+0x28/0x48) [5.751983] [] > > > (eth_start) from [] (eth_open+0x6c/0x7c) [5.751983] > > > [] (eth_open) from [] > > > (__dev_open+0x9c/0x104) > > > > This could be something else though. Care to email me your .config, maybe > > this is related to legacy g_ether being built in? > > Sure, please find it attached. The legacy g_ether is indeed built-in, that's > what I use to boot over nfsroot. OK, I think g_ether may have issues in general.. I mostly test with configfs based gadgets and shell script as then I can test load/configure/connect/disconnect/unconfigure/unload easily :) > > Anyways, please also give the following patch a try. > > I've tested the patch and if fixes the original problem. Warm reboots are > still broken though. No luck here with your .config a try with my pandaboard es against v4.8-rc7 plus the patch I posted on Sunday. It reboots with no errors for me with NFSroot. Do you do something other than just reboot? 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: USB hot-plug not working (ASUS TP301UA-C4028T)
Dear Oliver, I'm not really sure what happened, but now USB appears to be working perfectly well... In the meantime, since my last checks, I simply updated Tumbleweed and entered the command below as root (I rebooted twice since then: USB still working!). I didn't find anything relevant in the Tumbleweed updates, only this one is related to USB, but the only commit doesn't seem to be fixing anything at runtime: usb_modeswitch Subpackages: usb_modeswitch-data - Avoid a race in make install, which lead to packaging a truncated usb_modeswitch_dispatcher script. Fixes boo#998641 But I dont think the command below is still active after a reboot (is it?) and it seems just to activate some more debugging, so I really don't understand how this happened! Sorry, this must be a quite frustrating report... Cheers, Pierre. Le lundi 19 septembre 2016, 11:22:52 NZST Oliver Neukum a écrit : > On Sun, 2016-09-18 at 19:48 +1200, Pierre de Villemereuil wrote: > > Dear kernel devs, > > > > I'm using openSUSE Tumbleweed (kernel 4.7.2-2-default) on an ASUS Vivobook > > Flip TP301UA-C4028T, see here for specs: > > https://www.asus.com/Notebooks/ASUS-VivoBook-Flip-TP301UA/specifications/ > > > > Since my install in July, USB hot-plug (a.k.a. plug'n'play) is not > > working. To make any USB device working (I tested many USB keys, a > > slideshow remote control and a mouse), it has to be plugged on boot up or > > wake up from suspend. Using the following command also make the system > > aware that USB is connected: udevadm trigger > > > > More information can be found on the following thread on the openSUSE > > forums: > > https://forums.opensuse.org/showthread.php/519926-No-USB-device-detected > Basically dmesg should show a hotplug. What happens if you unload and > reload the xhci_hcd modules? > > And please do > > echo "module xhci_hcd +mpf" > /sys/kernel/debug/dynamic_debug/control > > (as root) and check dmesg again. > > 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: [RESEND PATCH v7 2/4] phy: rockchip-usb: use rockchip_usb_phy_reset to reset phy during wakeup
On Sat, Sep 10, 2016 at 02:59:38AM +0800, Randy Li wrote: > It is a hardware bug in RK3288, the only way to solve it is to > reset the phy. > > Signed-off-by: Randy Li > --- > .../devicetree/bindings/phy/rockchip-usb-phy.txt | 3 +++ Acked-by: Rob Herring > drivers/phy/phy-rockchip-usb.c | 20 > > 2 files changed, 23 insertions(+) -- 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 v4 22/22] phy: Add support for Qualcomm's USB HS phy
On Fri, Sep 16, 2016 at 7:05 PM, Stephen Boyd wrote: > Quoting Rob Herring (2016-09-16 08:19:51) >> On Wed, Sep 07, 2016 at 02:35:19PM -0700, Stephen Boyd wrote: >> > The high-speed phy on qcom SoCs is controlled via the ULPI >> > viewport. >> > [...] >> > +- qcom,init-seq: >> > +Usage: optional >> > +Value type: >> > +Definition: Should contain a sequence of ULPI register and address >> > pairs to >> > +program into the ULPI_EXT_VENDOR_SPECIFIC area. This is >> > related >> > +to Device Mode Eye Diagram test. >> >> We generally nak this type of property. For 1 register I don't care so >> much. For 100, that would be another story. >> >> Is this value per unit, per board, per SoC? Can you limit it to certain >> registers? > > I'm told that this can be per board, depending on how it's wired from > the phy pins to the usb port. Typically it's the same though for the > boards I have, mostly because those boards are similar designs with > respect to how USB is wired. The set of registers is not that many, 4 or > 5 at most. My understanding is these are tuning registers. Right now the > register part in the binding is the full register offset, and not an > offset from ULPI_EXT_VENDOR_SPECIFIC (0x80). I could change this to be > an offset from that area if you like so that this can't be abused to > write into standard ULPI registers (there really isn't any way to > enforce this in software though). Okay, that sounds fine to me. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: musb: Fix unbalanced platform_disable
Hi Tony, On Sunday 18 Sep 2016 08:19:02 Tony Lindgren wrote: > * Laurent Pinchart [160918 05:13]: > > FYI, while this patch allows me to boot my Panda board with NFS over > > usbnet, it only works with cold boots. A warm reboot results in the > > following warning, and no ethernet traffic going through. The USB device > > is detected by the host though. > > Yeah I noticed too that we still have issues. For example doing rmmod of > omap2430 with gadget configured and connected will produce a hardirq-safe > hardirq-unsafe lock order error. That also happens with reboot with gadget > configured and connected. > > > I'm not sure whether this is a regression introduced by commit > > a83e17d0f73b ("usb: musb: Improve PM runtime and phy handling for 2430 > > glue layer") or an entirely different issue; > > ... > > > [5.711303] [] (_raw_spin_unlock_irqrestore) from > > [] > > (musb_gadget_queue+0x128/0x4ac) > > [5.711303] [] (musb_gadget_queue) from [] > > (usb_ep_queue+0x38/0x1d4) > > [5.729766] [] (usb_ep_queue) from [] > > (rx_submit+0xc8/0x19c) > > [5.737548] [] (rx_submit) from [] > > (rx_fill+0x7c/0xa0) [5.737548] [] (rx_fill) from > > [] (eth_start+0x28/0x48) [5.751983] [] > > (eth_start) from [] (eth_open+0x6c/0x7c) [5.751983] > > [] (eth_open) from [] > > (__dev_open+0x9c/0x104) > > This could be something else though. Care to email me your .config, maybe > this is related to legacy g_ether being built in? Sure, please find it attached. The legacy g_ether is indeed built-in, that's what I use to boot over nfsroot. > Anyways, please also give the following patch a try. I've tested the patch and if fixes the original problem. Warm reboots are still broken though. > The reason why we > currently have no chance of getting musb_platform_enable/disable balanced > is because we need to try to set the musb devctl session bit in various > places and possibly retry too. So musb_start should really be called > musb_try_start_session() or something. > > So I think the only sane thing to do at this point is to revert the changes > trying to enable/disable USB PHY from omap2430_musb_enable/disable. The > other fixes are OK too as they get us a bit closer to making the platform > glue calls balanced. > > Regards, > > Tony > > 8< -- > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c > --- a/drivers/usb/musb/omap2430.c > +++ b/drivers/usb/musb/omap2430.c > @@ -337,6 +337,7 @@ static int omap2430_musb_init(struct musb *musb) > } > musb->isr = omap2430_musb_interrupt; > phy_init(musb->phy); > + phy_power_on(musb->phy); > > l = musb_readl(musb->mregs, OTG_INTERFSEL); > > @@ -373,9 +374,6 @@ static void omap2430_musb_enable(struct musb *musb) > struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev); > struct omap_musb_board_data *data = pdata->board_data; > > - if (!WARN_ON(!musb->phy)) > - phy_power_on(musb->phy); > - > omap2430_set_power(musb, true, glue->cable_connected); > > switch (glue->status) { > @@ -413,9 +411,6 @@ static void omap2430_musb_disable(struct musb *musb) > struct device *dev = musb->controller; > struct omap2430_glue *glue = dev_get_drvdata(dev->parent); > > - if (!WARN_ON(!musb->phy)) > - phy_power_off(musb->phy); > - > if (glue->status != MUSB_UNKNOWN) > omap_control_usb_set_mode(glue->control_otghs, > USB_MODE_DISCONNECT); > @@ -429,6 +424,7 @@ static int omap2430_musb_exit(struct musb *musb) > struct omap2430_glue *glue = dev_get_drvdata(dev->parent); > > omap2430_low_level_exit(musb); > + phy_power_off(musb->phy); > phy_exit(musb->phy); > musb->phy = NULL; > cancel_work_sync(&glue->omap_musb_mailbox_work); -- Regards, Laurent Pinchart config.gz Description: application/gzip
[PATCH] usb: misc: legousbtower: Fix NULL pointer deference
This patch fixes a NULL pointer dereference caused by a race codition in the probe function of the legousbtower driver. It re-structures the probe function to only register the interface after successfully reading the board's firmware ID. The probe function does not deregister the usb interface after an error receiving the devices firmware ID. The device file registered (/dev/usb/legousbtower%d) may be read/written globally before the probe function returns. When tower_delete is called in the probe function (after an r/w has been initiated), core dev structures are deleted while the file operation functions are still running. If the 0 address is mappable on the machine, this vulnerability can be used to create a Local Priviege Escalation exploit via a write-what-where condition by remapping dev->interrupt_out_buffer in tower_write. A forged USB device and local program execution would be required for LPE. The USB device would have to delay the control message in tower_probe and accept the control urb in tower_open whilst guest code initiated a write to the device file as tower_delete is called from the error in tower_probe. This bug has existed since 2003. Patch tested by emulated device. Signed-off-by: James Patrick-Evans --- drivers/usb/misc/legousbtower.c | 35 +-- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c index 7771be3..4dd531a 100644 --- a/drivers/usb/misc/legousbtower.c +++ b/drivers/usb/misc/legousbtower.c @@ -898,24 +898,6 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device dev->interrupt_in_interval = interrupt_in_interval ? interrupt_in_interval : dev->interrupt_in_endpoint->bInterval; dev->interrupt_out_interval = interrupt_out_interval ? interrupt_out_interval : dev->interrupt_out_endpoint->bInterval; - /* we can register the device now, as it is ready */ - usb_set_intfdata (interface, dev); - - retval = usb_register_dev (interface, &tower_class); - - if (retval) { - /* something prevented us from registering this driver */ - dev_err(idev, "Not able to get a minor for this device.\n"); - usb_set_intfdata (interface, NULL); - goto error; - } - dev->minor = interface->minor; - - /* let the user know what node this device is now attached to */ - dev_info(&interface->dev, "LEGO USB Tower #%d now attached to major " -"%d minor %d\n", (dev->minor - LEGO_USB_TOWER_MINOR_BASE), -USB_MAJOR, dev->minor); - /* get the firmware version and log it */ result = usb_control_msg (udev, usb_rcvctrlpipe(udev, 0), @@ -936,6 +918,23 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device get_version_reply.minor, le16_to_cpu(get_version_reply.build_no)); + /* we can register the device now, as it is ready */ + usb_set_intfdata (interface, dev); + + retval = usb_register_dev (interface, &tower_class); + + if (retval) { + /* something prevented us from registering this driver */ + dev_err(idev, "Not able to get a minor for this device.\n"); + usb_set_intfdata (interface, NULL); + goto error; + } + dev->minor = interface->minor; + + /* let the user know what node this device is now attached to */ + dev_info(&interface->dev, "LEGO USB Tower #%d now attached to major " +"%d minor %d\n", (dev->minor - LEGO_USB_TOWER_MINOR_BASE), +USB_MAJOR, dev->minor); exit: return retval; -- signature.asc Description: Digital signature
Re: [PATCH] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing led
On Mon, 19 Sep 2016, Ulf Hansson wrote: > On 18 September 2016 at 04:30, Alan Stern wrote: > > On Sat, 17 Sep 2016, Ulf Hansson wrote: > > > >> Each access of the parent device (usb device) needs to be done in runtime > >> resumed state. Currently this isn't case while changing the leds, so let's > >> add pm_runtime_get_sync() and pm_runtime_put() around these calls. > >> > >> Signed-off-by: Ulf Hansson > >> --- > >> > >> While discussing an issue[1] related to runtime PM, I found out that this > >> minor change at least improves the behavior that has been observed. > >> > >> [1] > >> http://www.spinics.net/lists/linux-usb/msg144634.html > >> > >> --- > >> drivers/mmc/host/rtsx_usb_sdmmc.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c > >> b/drivers/mmc/host/rtsx_usb_sdmmc.c > >> index 6c71fc9..a59c7fa 100644 > >> --- a/drivers/mmc/host/rtsx_usb_sdmmc.c > >> +++ b/drivers/mmc/host/rtsx_usb_sdmmc.c > >> @@ -1314,6 +1314,7 @@ static void rtsx_usb_update_led(struct work_struct > >> *work) > >> container_of(work, struct rtsx_usb_sdmmc, led_work); > >> struct rtsx_ucr *ucr = host->ucr; > >> > >> + pm_runtime_get_sync(sdmmc_dev(host)); > >> mutex_lock(&ucr->dev_mutex); > >> > >> if (host->led.brightness == LED_OFF) > >> @@ -1322,6 +1323,7 @@ static void rtsx_usb_update_led(struct work_struct > >> *work) > >> rtsx_usb_turn_on_led(ucr); > >> > >> mutex_unlock(&ucr->dev_mutex); > >> + pm_runtime_put(sdmmc_dev(host)); > >> } > >> #endif > > > > The missing aspect here is that this won't stop the parent USB device > > from going into autosuspend every 2 seconds and then resuming shortly > > afterward. There are two ways of preventing this: > > > > Call usb_mark_last_busy() at appropriate places. > > > > Enable autosuspend for the sdmmc device. > > > > The second approach would also prevent the sdmmc device from going into > > autosuspend as soon as the LED update is finished. Maybe that's okay, > > but if going into suspend is a lightweight procedure then you may want > > to prevent it. > > > > We can for sure enable autosuspend for the sdmmc device, although as > soon as an SD card will be detected the rtsx driver will increase the > runtime PM usage count. The count is decreased when the card is > removed (or failed to be initialized), thus runtime suspend is > prevented as long as there is a functional card inserted. Which means that autosuspend matters only when a card isn't present, and the host is polled every second or so to see whether a card has been inserted. Under those circumstances you probably don't want to use autosuspend. That is, resuming before each poll and suspending afterward may use less energy than staying at full power all the time. > I am wondering what you think would be a good autosuspend timeout in > this case? It seems to me that the only thing the rtsx driver really > care about is to tell the parent device that it needs to be runtime > resumed during a certain timeframe, more or less it would like to > inherit the parents settings. > > Other mmc hosts, not being usb-mmc devices, are using an autosuspend > timeout of ~50-100ms but that doesn't seem like good value here, > right? Well, if you decide to let the device go into runtime suspend between polls then there's no reason to use autosuspend at all. Once a poll has ended, you know there won't be any more activity until the next poll. On the other hand, if you decide to keep the device at full power all the time during polling, then any autosuspend timeout larger than 1000 ms would do what you want. Mostly I'm concerned about how this will interact with the USB runtime PM. The thing is, suspending the sdmmc device doesn't save any energy, whereas suspending the USB device does. Let's see how well everything works with the patch to the rtsx memstick driver. 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: xHCI problem? [was Re: Erratic USB device behavior and device loss]
On Mon, 19 Sep 2016, Ulf Hansson wrote: > On 18 September 2016 at 03:42, Alan Stern wrote: > > Well, this is pretty clear: > > > > Sep 17 15:55:52 learner kernel: CPU: 1 PID: 535 Comm: rtsx_usb_ms_1 > > Tainted: G U 4.8.0-rc6ulf1alan1+ #19 > > Sep 17 15:55:52 learner kernel: Hardware name: LENOVO 20344/INVALID, BIOS > > 96CN31WW(V1.17) 07/21/2015 > > Sep 17 15:55:52 learner kernel: 81314be5 > > 8802476746c0 0240 > > Sep 17 15:55:52 learner kernel: a016f719 523bec00 > > 88025f255780 88024feff600 > > Sep 17 15:55:52 learner kernel: 00018080 > > 88025f258080 815a0e60 > > Sep 17 15:55:52 learner kernel: Call Trace: > > Sep 17 15:55:52 learner kernel: [] ? dump_stack+0x7d/0xb8 > > Sep 17 15:55:52 learner kernel: [] ? > > usb_hcd_submit_urb+0x3c9/0xad0 [usbcore] > > Sep 17 15:55:52 learner kernel: [] ? > > _raw_spin_lock_irqsave+0x20/0x47 > > Sep 17 15:55:52 learner kernel: [] ? > > lock_timer_base.isra.24+0x7b/0xa0 > > Sep 17 15:55:52 learner kernel: [] ? > > try_to_del_timer_sync+0x49/0x60 > > Sep 17 15:55:52 learner kernel: [] ? > > usb_start_wait_urb+0x5d/0x140 [usbcore] > > Sep 17 15:55:52 learner kernel: [] ? > > rtsx_usb_send_cmd+0x5e/0x80 [rtsx_usb] > > Sep 17 15:55:52 learner kernel: [] ? > > rtsx_usb_read_register+0x67/0xb0 [rtsx_usb] > > Sep 17 15:55:52 learner kernel: [] ? > > rtsx_usb_detect_ms_card+0x61/0xe0 [rtsx_usb_ms] > > Sep 17 15:55:52 learner kernel: [] ? > > rtsx_usb_ms_set_param+0x770/0x770 [rtsx_usb_ms] > > Sep 17 15:55:52 learner kernel: [] ? kthread+0xbd/0xe0 > > Sep 17 15:55:52 learner kernel: [] ? > > __switch_to+0x2b1/0x6a0 > > Sep 17 15:55:52 learner kernel: [] ? > > ret_from_fork+0x1f/0x40 > > Sep 17 15:55:52 learner kernel: [] ? > > kthread_create_on_node+0x180/0x180 > > > > This is the rtsx_usb_detect_ms_card() routine in > > drivers/memstick/host/rtsx_usb_ms.c, which runs as a kthread. It > > doesn't do any runtime PM. So it looks like the bug is present in both > > the MMC and MemoryStick interfaces. > > I think the problem is even worse in the MemoryStick case, as the > memstick core doesn't help with runtime PM. I am pretty sure there are > other cases when the MemoryStick driver accesses the usb device > without first runtime resuming it. Maybe we should get a MemoryStick maintainer involved in this thread. I CC'ed Alex Dubov. Alex, the problem here is that drivers/memstick/host/rtsx_usb_ms.c tries to communicate with the host USB device while it is runtime suspended. > Of course we could start simple an fix the bug observed above and see > if that solves the reported problem. Alan, do you want to post to > patch or you want me? This ought to help. Ritesh, please apply this patch on top of the two earlier ones and let's see what happens. Alan Stern Index: usb-4.x/drivers/memstick/host/rtsx_usb_ms.c === --- usb-4.x.orig/drivers/memstick/host/rtsx_usb_ms.c +++ usb-4.x/drivers/memstick/host/rtsx_usb_ms.c @@ -681,6 +681,7 @@ static int rtsx_usb_detect_ms_card(void int err; for (;;) { + pm_runtime_get_sync(ms_dev(host)); mutex_lock(&ucr->dev_mutex); /* Check pending MS card changes */ @@ -703,6 +704,7 @@ static int rtsx_usb_detect_ms_card(void } poll_again: + pm_runtime_put(ms_dev(host)); if (host->eject) break; -- 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: Memory barrier needed with wake_up_process()?
On Mon, 19 Sep 2016, Felipe Balbi wrote: > >> file-storage-3578 [002] 21167.727072: fsg_main_thread: next: bh > >> 880111e69a80 state 1 > >> file-storage-3578 [002] 21167.729458: fsg_main_thread: next: bh > >> 880111e6aac0 state 2 > >> irq/17-dwc3-3579 [003] d..1 21167.729666: bulk_out_complete: compl: > >> bh 880111e6aac0 state 1 > >> file-storage-3578 [002] 21167.729670: fsg_main_thread: next: bh > >> 880111e6aac0 state 1 > > > > And this is where everything stopped? > > yeah, that's everything. > > > This also looks normal. So the question is what happened when > > get_next_command() returned after this? > > > > Felipe, maybe the patch below (in place of your current patch) will > > help. Since the events that it logs are all supposed to be unusual, > > you can use printk if you want, but I wrote it with trace_printk. > > I've applied your patch and it wasn't giving me any output, which hinted > that g_mass_storage wasn't returning any failures. So I enabled dwc3's > traces to get more data out of it. Here's the final snippet (with > comments, again). Let me know if you want the entire thing (it's > ~14MiB). > > > irq/17-dwc3-3527 [003] d..134.215214: dwc3_event: event (00110301): > > Link Change [U1] > > irq/17-dwc3-3527 [003] d..134.215214: dwc3_event: event (4086): > > ep1in: Transfer In-Progress > > irq/17-dwc3-3527 [003] d..134.215214: dwc3_complete_trb: ep1in: 2/255 > > trb 880084813e70 buf 80808000 size 0 ctrl 0c18 > > (hlcS:SC:normal) > > irq/17-dwc3-3527 [003] d..134.215214: dwc3_gadget_giveback: ep1in: req > > 88016d046840 length 16384/16384 zsI ==> 0 > > irq/17-dwc3-3527 [003] d...34.215215: usb_gadget_giveback_request: > > ep1in: length 16384/16384 sgs 0/0 stream 0 zsI status 0 --> 0 > > irq/17-dwc3-3527 [003] d..134.215218: dwc3_gadget_ep_cmd: ep1in: cmd > > 'Update Transfer' [196615] params --> status: > > Successful > > irq/17-dwc3-3527 [003] d..134.215281: dwc3_event: event (00100301): > > Link Change [U0] > > irq/17-dwc3-3527 [003] d..134.215281: dwc3_event: event (00110301): > > Link Change [U1] > > irq/17-dwc3-3527 [003] d..134.215282: dwc3_event: event (4086): > > ep1in: Transfer In-Progress > > irq/17-dwc3-3527 [003] d..134.215282: dwc3_complete_trb: ep1in: 1/255 > > trb 880084813e80 buf 8080c000 size 0 ctrl 0c18 > > (hlcS:SC:normal) > > irq/17-dwc3-3527 [003] d..134.215282: dwc3_gadget_giveback: ep1in: req > > 88016d046f00 length 13/13 zsI ==> 0 > > irq/17-dwc3-3527 [003] d...34.215283: usb_gadget_giveback_request: > > ep1in: length 13/13 sgs 0/0 stream 0 zsI status 0 --> 0 > > irq/17-dwc3-3527 [003] d..134.215284: dwc3_event: event (00100301): > > Link Change [U0] > > irq/17-dwc3-3527 [003] d..134.215284: dwc3_event: event (00110301): > > Link Change [U1] > > irq/17-dwc3-3527 [003] d..134.215284: dwc3_event: event (6084): > > ep1out: Transfer In-Progress > > irq/17-dwc3-3527 [003] d..134.215284: dwc3_complete_trb: ep1out: 1/255 > > trb 880084804320 buf 8080c800 size 993 ctrl 0c18 > > (hlcS:SC:normal) > > irq/17-dwc3-3527 [003] d..134.215285: dwc3_gadget_giveback: ep1out: > > req 880154205300 length 31/1024 zsI ==> 0 > > irq/17-dwc3-3527 [003] d...34.215285: usb_gadget_giveback_request: > > ep1out: length 31/1024 sgs 0/0 stream 0 zsI status 0 --> 0 > > completed and gave back CBW. > > > irq/17-dwc3-3527 [003] d..134.215349: dwc3_event: event (00100301): > > Link Change [U0] > > irq/17-dwc3-3527 [003] d..134.215349: dwc3_event: event (00110301): > > Link Change [U1] > > irq/17-dwc3-3527 [003] d..134.225616: dwc3_event: event (00120301): > > Link Change [U2] > > irq/17-dwc3-3527 [003] d...34.225617: usb_gadget_vbus_draw: speed 5/5 > > state 7 0mA [sg:out_aligned:self-powered:activated:connected] --> -95 > > irq/17-dwc3-3527 [003] d..164.832221: dwc3_event: event (00100301): > > Link Change [U0] > > 30 seconds of nothing. > > > irq/17-dwc3-3527 [003] d..164.832240: dwc3_event: event (c040): > > ep0out: Transfer Complete > > irq/17-dwc3-3527 [003] d..164.832243: dwc3_ep0: ep0out: Transfer > > Complete: state 'Setup Phase' > > irq/17-dwc3-3527 [003] d..164.832243: dwc3_ep0: Setup Phase > > irq/17-dwc3-3527 [003] d..164.832244: dwc3_ctrl_req: bRequestType 00 > > bRequest 01 wValue 0030 wIndex wLength 0 > > irq/17-dwc3-3527 [003] d..164.832244: dwc3_ep0: USB_REQ_CLEAR_FEATURE > > irq/17-dwc3-3527 [003] d..164.832253: dwc3_event: event (20c2): > > ep0in: Transfer Not Ready (Not Active) > > irq/17-dwc3-3527 [003] d..164.832254: dwc3_ep0: ep0in: Transfer Not > > Ready (Not Active): state 'Setup Phase' > > irq/17-dwc3-3527 [003] d..164.832254: dwc3_ep0: Control Status > > irq/17-dwc3-3527 [003] d..164.832255: dwc3_prepare_trb:
Re: [PATCH v2 3/6] phy: meson: add USB2 PHY support for Meson8b and GXBB
Kishon Vijay Abraham I writes: > Hi Kevin, > > On Wednesday 14 September 2016 09:36 PM, Kevin Hilman wrote: >> Kishon, >> >> Martin Blumenstingl writes: >> >>> This is a new driver for the USB PHY found in Meson8b and GXBB SoCs. >>> >>> Signed-off-by: Martin Blumenstingl >>> Signed-off-by: Jerome Brunet >>> Tested-by: Kevin Hilman >> >> Will you be picking this up for v4.9? > > It's already late for 4.9. Generally send pull request to Greg around -rc6. > This can go only in 4.10. That's fine. Does that mean you have it queued someplace? I don't see it in any of your branches. Kevin -- 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: [PATCHv2] usb: musb: Fix unbalanced platform_disable
* Andreas Kemnade [160918 23:00]: > On Sun, 18 Sep 2016 08:19:02 -0700 > Tony Lindgren wrote: > > > * Laurent Pinchart [160918 05:13]: > > > > > > FYI, while this patch allows me to boot my Panda board with NFS > > > over usbnet, it only works with cold boots. A warm reboot results > > > in the following warning, and no ethernet traffic going through. > > > The USB device is detected by the host though. > > > > Yeah I noticed too that we still have issues. For example doing rmmod > > of omap2430 with gadget configured and connected will produce a > > hardirq-safe hardirq-unsafe lock order error. That also happens with > > reboot with gadget configured and connected. > > > hmm, well, there is a musb_platform_disable() in musb_remove() which is > simply superfluous... > Some days ago we had a locking problem with musb_start() and moved > it out of the locked area. Maybe we could do also something similar > here. Well I don't think we can do that safely at this point because we have unpaired calls to musb_start() and musb_stop(). So trying to make musb_platform_enable/disable() paired right now will just lead into mystery breakage in various use cases. > > So I think the only sane thing to do at this point is to revert the > > changes trying to enable/disable USB PHY from > > omap2430_musb_enable/disable. The other fixes are OK too as they get > > us a bit closer to making the platform glue calls balanced. > > > or to balance it there (in a better way as done by my first patch). We can't do that until we have musb_start() and musb_stop() paired :) 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
[PATCH v3 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget
When we change the USB function with configfs dynamically, we possibly met this situation: one core is doing the control transfer, another core is trying to unregister the USB gadget from userspace, we must wait for completing this control tranfer, or it will hang the controller to set the DEVCTRLHLT flag. Signed-off-by: Baolin Wang --- drivers/usb/dwc3/core.h |2 ++ drivers/usb/dwc3/ep0.c|2 ++ drivers/usb/dwc3/gadget.c | 23 +++ 3 files changed, 27 insertions(+) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index b2317e7..01a6fbd 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -745,6 +745,7 @@ struct dwc3_scratchpad_array { * @ep0_usb_req: dummy req used while handling STD USB requests * @ep0_bounce_addr: dma address of ep0_bounce * @scratch_addr: dma address of scratchbuf + * @ep0_in_setup: One control tranfer is completed and enter setup phase * @lock: for synchronizing * @dev: pointer to our struct device * @xhci: pointer to our xHCI child @@ -843,6 +844,7 @@ struct dwc3 { dma_addr_t ep0_bounce_addr; dma_addr_t scratch_addr; struct dwc3_request ep0_usb_req; + struct completion ep0_in_setup; /* device lock */ spinlock_t lock; diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index fe79d77..06c167a 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -311,6 +311,8 @@ void dwc3_ep0_out_start(struct dwc3 *dwc) ret = dwc3_ep0_start_trans(dwc, 0, dwc->ctrl_req_addr, 8, DWC3_TRBCTL_CONTROL_SETUP, false); WARN_ON(ret < 0); + + complete(&dwc->ep0_in_setup); } static struct dwc3_ep *dwc3_wIndex_to_dep(struct dwc3 *dwc, __le16 wIndex_le) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index ca2ae5b..3a30d51 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1437,6 +1437,15 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend) if (pm_runtime_suspended(dwc->dev)) return 0; + /* +* Per databook, when we want to stop the gadget, if a control transfer +* is still in process, complete it and get the core into setup phase. +*/ + if (!is_on && dwc->ep0state != EP0_SETUP_PHASE) { + reinit_completion(&dwc->ep0_in_setup); + return -EBUSY; + } + reg = dwc3_readl(dwc->regs, DWC3_DCTL); if (is_on) { if (dwc->revision <= DWC3_REVISION_187A) { @@ -1487,10 +1496,22 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) is_on = !!is_on; +try_again: spin_lock_irqsave(&dwc->lock, flags); ret = dwc3_gadget_run_stop(dwc, is_on, false); spin_unlock_irqrestore(&dwc->lock, flags); + if (ret == -EBUSY) { + ret = wait_for_completion_timeout(&dwc->ep0_in_setup, + msecs_to_jiffies(500)); + if (ret == 0) { + dev_err(dwc->dev, "timeout to stop gadget.\n"); + return -ETIMEDOUT; + } else { + goto try_again; + } + } + return ret; } @@ -2914,6 +2935,8 @@ int dwc3_gadget_init(struct dwc3 *dwc) goto err4; } + init_completion(&dwc->ep0_in_setup); + dwc->gadget.ops = &dwc3_gadget_ops; dwc->gadget.speed = USB_SPEED_UNKNOWN; dwc->gadget.sg_supported= true; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically
When system has stpped the gadget, we should avoid queuing any requests which will cause tranfer failed. Thus adding some disconnect checking to avoid this situation. Signed-off-by: Baolin Wang --- Changes since v2: - Move disconnect checking into dwc3_send_gadget_ep_cmd(). - Rename completion name and issue complete() at one place. - Move completion initialization into dwc3_gadget_init(). Changes since v1: - Split into 2 separate ptaches. - Choose complete mechanism instead of polling. --- drivers/usb/dwc3/gadget.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 1783406..ca2ae5b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd, int susphy = false; int ret = -EINVAL; + if (!dwc->pullups_connected) + return -ESHUTDOWN; + /* * Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if * we're issuing an endpoint command, we must check if -- 1.7.9.5 -- 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/3] usb: chipidea: imx: Change switch order
Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index switch and then test for features if they exist for this index. This patch also remove useless test of reg and val. Those two values cannot be NULL. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 38 -- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..9549821 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { - case 0: + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { + case 0: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; - break; - case 1: + writel(val, reg); + } + break; + case 1: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; - break; - case 2: + writel(val, reg); + } + break; + case 2: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; - break; - case 3: + writel(val, reg); + } + break; + case 3: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; - break; - } - if (reg && val) writel(val, reg); - spin_unlock_irqrestore(&usbmisc->lock, flags); + } + break; } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 0; } -- 2.1.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 v4 2/3] usb: chipidea: imx: configure imx for ULPI phy
In order to use ULPI phy with usb host 2 and 3, we need to configure controller register to enable ULPI features. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 5 + drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 37 + 3 files changed, 43 insertions(+) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 0991794..96c0e33 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "ci.h" @@ -146,6 +147,10 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev) if (of_find_property(np, "external-vbus-divider", NULL)) data->evdo = 1; + + if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) + data->ulpi = 1; + return data; } diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h index 409aa5ca8..d666c9f 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -19,6 +19,7 @@ struct imx_usbmisc_data { unsigned int disable_oc:1; /* over current detect disabled */ unsigned int oc_polarity:1; /* over current polarity if oc enabled */ unsigned int evdo:1; /* set external vbus divider option */ + unsigned int ulpi:1; /* connected to an ULPI phy */ }; int imx_usbmisc_init(struct imx_usbmisc_data *); diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 9549821..11f51bd 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -46,11 +46,20 @@ #define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08 #define MX53_USB_OTG_PHY_CTRL_1_OFFSET 0x0c +#define MX53_USB_CTRL_1_OFFSET 0x10 +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2) +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2) +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6) +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6) #define MX53_USB_UH2_CTRL_OFFSET 0x14 #define MX53_USB_UH3_CTRL_OFFSET 0x18 #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) +#define MX53_USB_CTRL_1_UH2_ULPI_ENBIT(26) +#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(27) +#define MX53_USB_UHx_CTRL_WAKE_UP_EN BIT(7) +#define MX53_USB_UHx_CTRL_ULPI_INT_EN BIT(8) #define MX53_USB_PHYCTRL1_PLLDIV_MASK 0x3 #define MX53_USB_PLL_DIV_24_MHZ0x01 @@ -217,6 +226,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) } break; case 2: + if (data->ulpi) { + /* set USBH2 into ULPI-mode. */ + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET; + val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN; + /* select ULPI clock */ + val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK; + val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI; + writel(val, reg); + /* Set interrupt wake up enable */ + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN + | MX53_USB_UHx_CTRL_ULPI_INT_EN; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -224,6 +247,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) } break; case 3: + if (data->ulpi) { + /* set USBH3 into ULPI-mode. */ + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET; + val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN; + /* select ULPI clock */ + val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK; + val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI; + writel(val, reg); + /* Set interrupt wake up enable */ + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN + | MX53_USB_UHx_CTRL_ULPI_INT_EN; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.
[PATCH v4 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
This binding allow to disable the internal 60Mhz clock for USB host2 and host3. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 13 + 3 files changed, 16 insertions(+) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 96c0e33..89a9d98 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -147,6 +147,8 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev) if (of_find_property(np, "external-vbus-divider", NULL)) data->evdo = 1; + if (of_find_property(np, "disable-int60ck", NULL)) + data->disable_int60ck = 1; if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) data->ulpi = 1; diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h index d666c9f..43bafae 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -20,6 +20,7 @@ struct imx_usbmisc_data { unsigned int oc_polarity:1; /* over current polarity if oc enabled */ unsigned int evdo:1; /* set external vbus divider option */ unsigned int ulpi:1; /* connected to an ULPI phy */ + unsigned int disable_int60ck:1; /* disable 60 MHZ clock */ }; int imx_usbmisc_init(struct imx_usbmisc_data *); diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 11f51bd..a781f87 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -53,6 +53,9 @@ #define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6) #define MX53_USB_UH2_CTRL_OFFSET 0x14 #define MX53_USB_UH3_CTRL_OFFSET 0x18 +#define MX53_USB_CLKONOFF_CTRL_OFFSET 0x24 +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21) +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22) #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) @@ -240,6 +243,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -261,6 +269,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; -- 2.1.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 v4 0/3] usb: chipidea: imx: Add USB configuration for imx53
Changes in V2: - Patches sent to early with bad contents Changes in V3: - Change subject - Split "configure imx for ULPI phy" for disable-oc code Changes in V4: - Fix "Change switch order" commit message - Indent switch/case (set case on the same column as switch) - Remove useless test in "Change switch order" Fabien Lahoudere (3): usb: chipidea: imx: Change switch order usb: chipidea: imx: configure imx for ULPI phy usb: chipidea: imx: Add binding to disable USB 60Mhz clock drivers/usb/chipidea/ci_hdrc_imx.c | 7 +++ drivers/usb/chipidea/ci_hdrc_imx.h | 2 + drivers/usb/chipidea/usbmisc_imx.c | 88 -- 3 files changed, 83 insertions(+), 14 deletions(-) -- 2.1.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 2/2] usb: chipidea: otg: fix reading otgsc register
When switching from gadget to host the driver waits for VBUS to disappear before doing the actual switch. This waiting is done by letting hw_wait_reg() poll the OTGSC register. This is buggy. hw_wait_reg() directly reads the register, but for reading the OTGSC register hw_read_otgsc() must be used since this function eventually patches the VBUS status read from extcon into the raw register value. To fix this inline the hw_wait_reg() code into its only user and use the correct function to read the OTGSC register. Since hw_wait_reg() is unused now remove it. Signed-off-by: Sascha Hauer --- drivers/usb/chipidea/ci.h | 3 --- drivers/usb/chipidea/core.c | 32 drivers/usb/chipidea/otg.c | 16 3 files changed, 12 insertions(+), 39 deletions(-) diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index cd41455..05bc4d6 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -428,9 +428,6 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode); u8 hw_port_test_get(struct ci_hdrc *ci); -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask, - u32 value, unsigned int timeout_ms); - void ci_platform_configure(struct ci_hdrc *ci); int dbg_create_files(struct ci_hdrc *ci); diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 69426e6..01390e0 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci) return 0; } -/** - * hw_wait_reg: wait the register value - * - * Sometimes, it needs to wait register value before going on. - * Eg, when switch to device mode, the vbus value should be lower - * than OTGSC_BSV before connects to host. - * - * @ci: the controller - * @reg: register index - * @mask: mast bit - * @value: the bit value to wait - * @timeout_ms: timeout in millisecond - * - * This function returns an error code if timeout - */ -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask, - u32 value, unsigned int timeout_ms) -{ - unsigned long elapse = jiffies + msecs_to_jiffies(timeout_ms); - - while (hw_read(ci, reg, mask) != value) { - if (time_after(jiffies, elapse)) { - dev_err(ci->dev, "timeout waiting for %08x in %d\n", - mask, reg); - return -ETIMEDOUT; - } - msleep(20); - } - - return 0; -} - static irqreturn_t ci_irq(int irq, void *data) { struct ci_hdrc *ci = data; diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index 91989be..d1c4cdf 100644 --- a/drivers/usb/chipidea/otg.c +++ b/drivers/usb/chipidea/otg.c @@ -104,7 +104,6 @@ void ci_handle_vbus_change(struct ci_hdrc *ci) usb_gadget_vbus_disconnect(&ci->gadget); } -#define CI_VBUS_STABLE_TIMEOUT_MS 5000 static void ci_handle_id_switch(struct ci_hdrc *ci) { enum ci_role role = ci_otg_role(ci); @@ -117,10 +116,19 @@ static void ci_handle_id_switch(struct ci_hdrc *ci) ci_role_stop(ci); - if (role == CI_ROLE_GADGET) + if (role == CI_ROLE_GADGET) { + unsigned long elapse = jiffies + msecs_to_jiffies(5000); + /* wait vbus lower than OTGSC_BSV */ - hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0, - CI_VBUS_STABLE_TIMEOUT_MS); + while (hw_read_otgsc(ci, OTGSC_BSV)) { + if (time_after(jiffies, elapse)) { + dev_err(ci->dev, + "timeout waiting for VBUS disappear\n"); + break; + } + msleep(20); + } + } ci_role_start(ci, role); } -- 2.8.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: chipidea: otg: save indention level
Return early from ci_handle_id_switch() to save an indention level. Signed-off-by: Sascha Hauer --- drivers/usb/chipidea/otg.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index 03b6743..91989be 100644 --- a/drivers/usb/chipidea/otg.c +++ b/drivers/usb/chipidea/otg.c @@ -109,19 +109,20 @@ static void ci_handle_id_switch(struct ci_hdrc *ci) { enum ci_role role = ci_otg_role(ci); - if (role != ci->role) { - dev_dbg(ci->dev, "switching from %s to %s\n", - ci_role(ci)->name, ci->roles[role]->name); + if (role == ci->role) + return; - ci_role_stop(ci); + dev_dbg(ci->dev, "switching from %s to %s\n", + ci_role(ci)->name, ci->roles[role]->name); - if (role == CI_ROLE_GADGET) - /* wait vbus lower than OTGSC_BSV */ - hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0, - CI_VBUS_STABLE_TIMEOUT_MS); + ci_role_stop(ci); - ci_role_start(ci, role); - } + if (role == CI_ROLE_GADGET) + /* wait vbus lower than OTGSC_BSV */ + hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0, + CI_VBUS_STABLE_TIMEOUT_MS); + + ci_role_start(ci, role); } /** * ci_otg_work - perform otg (vbus/id) event handle -- 2.8.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
USB: chipidea: Fix VBUS valid polling when extcon is used
This series fixes a bug when the VBUS detection is done by using extcon. When this is and the driver leaves host mode it will always respond with: ci_hdrc.0: timeout waiting for 0800 in 11 This goes back to reading the OTGSC register with the raw register read function. See 2/2 for more info. Sascha Sascha Hauer (2): usb: chipidea: otg: save indention level usb: chipidea: otg: fix reading otgsc register drivers/usb/chipidea/ci.h | 3 --- drivers/usb/chipidea/core.c | 32 drivers/usb/chipidea/otg.c | 29 +++-- 3 files changed, 19 insertions(+), 45 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATHCv10 1/2] usb: USB Type-C connector class
The purpose of USB Type-C connector class is to provide unified interface for the user space to get the status and basic information about USB Type-C connectors on a system, control over data role swapping, and when the port supports USB Power Delivery, also control over power role swapping and Alternate Modes. Reviewed-by: Guenter Roeck Tested-by: Guenter Roeck Signed-off-by: Heikki Krogerus --- Documentation/ABI/testing/sysfs-class-typec | 218 ++ Documentation/usb/typec.txt | 103 +++ MAINTAINERS |9 + drivers/usb/Kconfig |2 + drivers/usb/Makefile|2 + drivers/usb/typec/Kconfig |7 + drivers/usb/typec/Makefile |1 + drivers/usb/typec/typec.c | 1075 +++ include/linux/usb/typec.h | 252 +++ 9 files changed, 1669 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-typec create mode 100644 Documentation/usb/typec.txt create mode 100644 drivers/usb/typec/Kconfig create mode 100644 drivers/usb/typec/Makefile create mode 100644 drivers/usb/typec/typec.c create mode 100644 include/linux/usb/typec.h diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec new file mode 100644 index 000..dcca6bd --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-typec @@ -0,0 +1,218 @@ +USB Type-C port devices (eg. /sys/class/typec/usbc0/) + +What: /sys/class/typec//current_data_role +Date: June 2016 +Contact: Heikki Krogerus +Description: + The current USB data role the port is operating in. This + attribute can be used for requesting data role swapping on the + port. Swapping is supported as synchronous operation, so + write(2) to the attribute will not return until the operation + has finished. The attribute is notified about role changes so + that poll(2) on the attribute wakes up. + + Valid values: + - host + - device + +What: /sys/class/typec//current_power_role +Date: June 2016 +Contact: Heikki Krogerus +Description: + The current power role of the port. This attribute can be used + to request power role swap on the port when the port supports + USB Power Delivery. Swapping is supported as synchronous + operation, so write(2) to the attribute will not return until + the operation has finished. The attribute is notified about role + changes so that poll(2) on the attribute wakes up. + + Valid values: + - source + - sink + +What: /sys/class/typec//vconn_source +Date: June 2016 +Contact: Heikki Krogerus +Description: + Shows is the port VCONN Source. This attribute can be used to + request VCONN swap to change the VCONN Source during connection + when both the port and the partner support USB Power Delivery. + Swapping is supported as synchronous operation, so write(2) to + the attribute will not return until the operation has finished. + The attribute is notified about VCONN source changes so that + poll(2) on the attribute wakes up. + + Valid values are: + - 0 when the port is not the VCONN Source + - 1 when the port is the VCONN Source + +What: /sys/class/typec//power_operation_mode +Date: June 2016 +Contact: Heikki Krogerus +Description: + Shows the current power operational mode the port is in. + + Valid values: + - USB - Normal power levels defined in USB specifications + - BC1.2 - Power levels defined in Battery Charging Specification + v1.2 + - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C + specification. + - USB Type-C 3.0A - Higher 3A current defined in USB Type-C + specification. +- USB Power Delivery - The voltages and currents defined in USB + Power Delivery specification + +What: /sys/class/typec//preferred_role +Date: June 2016 +Contact: Heikki Krogerus +Description: + The user space can notify the driver about the preferred role. + It should be handled as enabling of Try.SRC or Try.SNK, as + defined in USB Type-C specification, in the port drivers. By + default there is no preferred role. + + Valid values: + - source + - sink + - none - to remove
[PATHCv10 0/2] USB Type-C Connector class
The USB Type-C class is meant to provide unified interface to the userspace to present the USB Type-C ports in a system. Changes since v9: - Minor typec_wcove.c cleanup as proposed by Guenter Roeck. No function affect. Changes since v8: - checking sysfs_streq() result correctly in sysfs_strmatch - fixed accessory check in supported_accessory_mode - using "none" as the only string that can clear the preferred role Changes since v7: - Removed "type" attribute from partners - Added supports_usb_power_delivery attribute for partner and cable Changes since v6: - current_vconn_role attr renamed to vconn_source (no API changes) - Small documentation improvements proposed by Vincent Palatin Changes since v5: - Only updating the roles based on driver notifications - Added MODULE_ALIAS for the WhiskeyCove module - Including the patch that creates the actual platform device for the WhiskeyCove Type-C PHY in this series. Changes since v4: - Remove the port lock completely Changes since v3: - Documentation cleanup as proposed by Roger Quadros - Setting partner altmodes member to NULL on removal and fixing a warning, as proposed by Guenter Roeck - Added the following attributes for partners and cables: * supports_usb_power_delivery * id_header_vdo - "id_header_vdo" is visible only when the partner or cable supports USB Power Delivery communication. - Partner attribute "accessory" is hidden when the partner type is not "Accessory". Changes since v2: - Notification on role and alternate mode changes - cleanups Changes since v1: - Completely rewrote alternate mode support - Patners, cables and cable plugs presented as devices. Heikki Krogerus (2): usb: USB Type-C connector class usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY Documentation/ABI/testing/sysfs-class-typec | 218 ++ Documentation/usb/typec.txt | 103 +++ MAINTAINERS |9 + drivers/usb/Kconfig |2 + drivers/usb/Makefile|2 + drivers/usb/typec/Kconfig | 21 + drivers/usb/typec/Makefile |2 + drivers/usb/typec/typec.c | 1075 +++ drivers/usb/typec/typec_wcove.c | 372 + include/linux/usb/typec.h | 252 +++ 10 files changed, 2056 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-typec create mode 100644 Documentation/usb/typec.txt create mode 100644 drivers/usb/typec/Kconfig create mode 100644 drivers/usb/typec/Makefile create mode 100644 drivers/usb/typec/typec.c create mode 100644 drivers/usb/typec/typec_wcove.c create mode 100644 include/linux/usb/typec.h -- 2.9.3 -- 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
[PATHCv10 2/2] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
This adds driver for the USB Type-C PHY on Intel WhiskeyCove PMIC which is available on some of the Intel Broxton SoC based platforms. Reviewed-by: Guenter Roeck Signed-off-by: Heikki Krogerus --- drivers/usb/typec/Kconfig | 14 ++ drivers/usb/typec/Makefile | 1 + drivers/usb/typec/typec_wcove.c | 372 3 files changed, 387 insertions(+) create mode 100644 drivers/usb/typec/typec_wcove.c diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig index b229fb9..7a345a4 100644 --- a/drivers/usb/typec/Kconfig +++ b/drivers/usb/typec/Kconfig @@ -4,4 +4,18 @@ menu "USB PD and Type-C drivers" config TYPEC tristate +config TYPEC_WCOVE + tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver" + depends on ACPI + depends on INTEL_SOC_PMIC + depends on INTEL_PMC_IPC + select TYPEC + help + This driver adds support for USB Type-C detection on Intel Broxton + platforms that have Intel Whiskey Cove PMIC. The driver can detect the + role and cable orientation. + + To compile this driver as module, choose M here: the module will be + called typec_wcove + endmenu diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile index 1012a8b..b9cb862 100644 --- a/drivers/usb/typec/Makefile +++ b/drivers/usb/typec/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_TYPEC)+= typec.o +obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c new file mode 100644 index 000..953d59b --- /dev/null +++ b/drivers/usb/typec/typec_wcove.c @@ -0,0 +1,372 @@ +/** + * typec_wcove.c - WhiskeyCove PMIC USB Type-C PHY driver + * + * Copyright (C) 2016 Intel Corporation + * Author: Heikki Krogerus + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include + +/* Register offsets */ +#define WCOVE_CHGRIRQ0 0x4e09 +#define WCOVE_PHYCTRL 0x5e07 + +#define USBC_CONTROL1 0x7001 +#define USBC_CONTROL2 0x7002 +#define USBC_CONTROL3 0x7003 +#define USBC_CC1_CTRL 0x7004 +#define USBC_CC2_CTRL 0x7005 +#define USBC_STATUS1 0x7007 +#define USBC_STATUS2 0x7008 +#define USBC_STATUS3 0x7009 +#define USBC_IRQ1 0x7015 +#define USBC_IRQ2 0x7016 +#define USBC_IRQMASK1 0x7017 +#define USBC_IRQMASK2 0x7018 + +/* Register bits */ + +#define USBC_CONTROL1_MODE_DRP(r) (((r) & ~0x7) | 4) + +#define USBC_CONTROL2_UNATT_SNKBIT(0) +#define USBC_CONTROL2_UNATT_SRCBIT(1) +#define USBC_CONTROL2_DIS_ST BIT(2) + +#define USBC_CONTROL3_PD_DIS BIT(1) + +#define USBC_CC_CTRL_VCONN_EN BIT(1) + +#define USBC_STATUS1_DET_ONGOING BIT(6) +#define USBC_STATUS1_RSLT(r) ((r) & 0xf) +#define USBC_RSLT_NOTHING 0 +#define USBC_RSLT_SRC_DEFAULT 1 +#define USBC_RSLT_SRC_1_5A 2 +#define USBC_RSLT_SRC_3_0A 3 +#define USBC_RSLT_SNK 4 +#define USBC_RSLT_DEBUG_ACC5 +#define USBC_RSLT_AUDIO_ACC6 +#define USBC_RSLT_UNDEF15 +#define USBC_STATUS1_ORIENT(r) (((r) >> 4) & 0x3) +#define USBC_ORIENT_NORMAL 1 +#define USBC_ORIENT_REVERSE2 + +#define USBC_STATUS2_VBUS_REQ BIT(5) + +#define USBC_IRQ1_ADCDONE1 BIT(2) +#define USBC_IRQ1_OVERTEMP BIT(1) +#define USBC_IRQ1_SHORTBIT(0) + +#define USBC_IRQ2_CC_CHANGEBIT(7) +#define USBC_IRQ2_RX_PDBIT(6) +#define USBC_IRQ2_RX_HRBIT(5) +#define USBC_IRQ2_RX_CRBIT(4) +#define USBC_IRQ2_TX_SUCCESS BIT(3) +#define USBC_IRQ2_TX_FAIL BIT(2) + +#define USBC_IRQMASK1_ALL (USBC_IRQ1_ADCDONE1 | USBC_IRQ1_OVERTEMP | \ +USBC_IRQ1_SHORT) + +#define USBC_IRQMASK2_ALL (USBC_IRQ2_CC_CHANGE | USBC_IRQ2_RX_PD | \ +USBC_IRQ2_RX_HR | USBC_IRQ2_RX_CR | \ +USBC_IRQ2_TX_SUCCESS | USBC_IRQ2_TX_FAIL) + +struct wcove_typec { + struct mutex lock; /* device lock */ + struct device *dev; + struct regmap *regmap; + struct typec_port *port; + struct typec_capability cap; + struct typec_connection con; + struct typec_partner partner; +}; + +enum wcove_typec_func { + WCOVE_FUNC_DRIVE_VBUS = 1, + WCOVE_FUNC_ORIENTATION, + WCOVE_FUNC_ROLE, + WCOVE_FUNC_DRIVE_VCONN, +}; + +enum wcove_typec_orientation { + WCOVE_ORIENTATION_NORMAL, + WCOVE_ORIENTATION_REVERSE, +}; + +enu
Re: Memory barrier needed with wake_up_process()?
Hi Alan, Alan Stern writes: > On Fri, 9 Sep 2016, Felipe Balbi wrote: > >> Finally :-) Here's the diff I used: >> >> diff --git a/drivers/usb/gadget/function/f_mass_storage.c >> b/drivers/usb/gadget/function/f_mass_storage.c >> index 8f3659b65f53..0716024f6b65 100644 >> --- a/drivers/usb/gadget/function/f_mass_storage.c >> +++ b/drivers/usb/gadget/function/f_mass_storage.c >> @@ -481,6 +481,8 @@ static void bulk_out_complete(struct usb_ep *ep, struct >> usb_request *req) >> spin_lock(&common->lock); >> bh->outreq_busy = 0; >> bh->state = BUF_STATE_FULL; >> + if (bh->bulk_out_intended_length == US_BULK_CB_WRAP_LEN) >> + trace_printk("compl: bh %p state %d\n", bh, bh->state); >> wakeup_thread(common); >> spin_unlock(&common->lock); >> } >> @@ -2208,6 +2210,7 @@ static int get_next_command(struct fsg_common *common) >> rc = sleep_thread(common, true); >> if (rc) >> return rc; >> + trace_printk("next: bh %p state %d\n", bh, bh->state); >> } >> smp_rmb(); >> rc = fsg_is_set(common) ? received_cbw(common->fsg, bh) : -EIO; >> >> >> And here's trace output: >> >> # tracer: nop >> # >> # entries-in-buffer/entries-written: 1002/1002 #P:4 >> # >> # _-=> irqs-off >> # / _=> need-resched >> #| / _---=> hardirq/softirq >> #|| / _--=> preempt-depth >> #||| / delay >> # TASK-PID CPU# TIMESTAMP FUNCTION >> # | | | | | >> file-storage-3578 [000] 21166.789127: fsg_main_thread: next: bh >> 880111e69a00 state 2 >> file-storage-3578 [000] 21166.789312: fsg_main_thread: next: bh >> 880111e69a00 state 2 >> irq/17-dwc3-3579 [003] d..1 21166.789395: bulk_out_complete: compl: bh >> 880111e69a00 state 1 >> file-storage-3578 [000] 21166.789445: fsg_main_thread: next: bh >> 880111e69a00 state 1 > > Okay, that's normal. 2 = BUF_STATE_BUSY, 1 = BUF_STATE_FULL. So we get woken > up a couple of times while the transfer is in progress (probably because some > earlier buffers have finished transferring), then the CBW transfer completes > and the buffer is read. > > ... > >> file-storage-3578 [002] 21167.726827: fsg_main_thread: next: bh >> 880111e69a80 state 2 >> irq/17-dwc3-3579 [003] d..1 21167.727066: bulk_out_complete: compl: bh >> 880111e69a80 state 1 >> file-storage-3578 [002] 21167.727072: fsg_main_thread: next: bh >> 880111e69a80 state 1 >> file-storage-3578 [002] 21167.729458: fsg_main_thread: next: bh >> 880111e6aac0 state 2 >> irq/17-dwc3-3579 [003] d..1 21167.729666: bulk_out_complete: compl: bh >> 880111e6aac0 state 1 >> file-storage-3578 [002] 21167.729670: fsg_main_thread: next: bh >> 880111e6aac0 state 1 > > And this is where everything stopped? yeah, that's everything. > This also looks normal. So the question is what happened when > get_next_command() returned after this? > > Felipe, maybe the patch below (in place of your current patch) will > help. Since the events that it logs are all supposed to be unusual, > you can use printk if you want, but I wrote it with trace_printk. I've applied your patch and it wasn't giving me any output, which hinted that g_mass_storage wasn't returning any failures. So I enabled dwc3's traces to get more data out of it. Here's the final snippet (with comments, again). Let me know if you want the entire thing (it's ~14MiB). > irq/17-dwc3-3527 [003] d..134.215214: dwc3_event: event (00110301): Link > Change [U1] > irq/17-dwc3-3527 [003] d..134.215214: dwc3_event: event (4086): > ep1in: Transfer In-Progress > irq/17-dwc3-3527 [003] d..134.215214: dwc3_complete_trb: ep1in: 2/255 > trb 880084813e70 buf 80808000 size 0 ctrl 0c18 > (hlcS:SC:normal) > irq/17-dwc3-3527 [003] d..134.215214: dwc3_gadget_giveback: ep1in: req > 88016d046840 length 16384/16384 zsI ==> 0 > irq/17-dwc3-3527 [003] d...34.215215: usb_gadget_giveback_request: > ep1in: length 16384/16384 sgs 0/0 stream 0 zsI status 0 --> 0 > irq/17-dwc3-3527 [003] d..134.215218: dwc3_gadget_ep_cmd: ep1in: cmd > 'Update Transfer' [196615] params --> status: > Successful > irq/17-dwc3-3527 [003] d..134.215281: dwc3_event: event (00100301): Link > Change [U0] > irq/17-dwc3-3527 [003] d..134.215281: dwc3_event: event (00110301): Link > Change [U1] > irq/17-dwc3-3527 [003] d..134.215282: dwc3_event: event (4086): > ep1in: Transfer In-Progress > irq/17-dwc3-3527 [003] d..134.215282: dwc3_complete_trb: ep1in: 1/255 > trb 880084813e80 buf 8080c000 size 0 ctrl 0c18 > (hlcS:SC:normal) > irq/17-dwc3-3527
Re: [PATCH v2 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget
Hi Felipe, On 19 September 2016 at 17:58, Felipe Balbi wrote: > > Hi, > > Baolin Wang writes: diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 1a33308..c9026ce 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1441,6 +1441,15 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend) if (pm_runtime_suspended(dwc->dev)) return 0; + /* + * Per databook, when we want to stop the gadget, if a control transfer + * is still in process, complete it and get the core into setup phase. + */ + if (!is_on && dwc->ep0state != EP0_SETUP_PHASE) { + reinit_completion(&dwc->ep0_completed); >>> >>> this seems unnecessary to me. Also, why return here so the caller has to >> >> We should re-init the completion due to it will complete control >> transfer many times before we try to stop gadget. > > not sure I get this comment, care to furter explain what you mean? Sorry for confusing comment. What I mean is it will issue 'complete(&dwc->ep0_completed)' when one control transfer is completed, and it will increase the completion->done when every 'complete()' function is issued. If there are seveal control transfers are completed (completion->done is not 0 bow) and dwc->ep0state is not in EP0_SETUP_PHASE satus when we want to stop gadget, then we should wait for completion. If we don't call 'reinit_completion(&dwc->ep0_completed)' (it will reset completion->done as 0 ), it will not wait for 500ms in wait_for_completion_timeout() funtion. Hope I make it clear. > >>> wait? You could just have called wait_for_completion() here straight >>> away: >>> >>> if (!is_on && dwc->ep0state != EP0_SETUP_PHASE) { >>> /* should this be interruptible? */ >>> ret = wait_for_completion_timeout(&dwc->ep0_in_setup, >>> msecs_to_jiffies(500)); >>> if (ret == 0) { >>> dwc3_trace(trace_dwc3_gadget, "RUN/STOP timeout"); >>> return -ETIMEDOUT; >>> } >>> } >>> >>> There's also no need for that "try_again" trickery. We either can halt >>> the controller within 500ms or we cannot. >> >> But this is in atomic context and we can not issue >> wait_for_completion_timeout() in atomic context, then we should just >> return here. > > heh, good point. Missed that :-) OK:) -- Baolin.wang Best Regards -- 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 05/15] usb: chipidea: imx: Change switch order
On Mon, 2016-09-19 at 12:18 +0200, Fabien Lahoudere wrote: > Each USB controller have different behaviour, so in order to avoid to have > several "swicth(data->index)" and lock/unlock, we prefer to get the index > and then test for features if they exist for this index. [] > diff --git a/drivers/usb/chipidea/usbmisc_imx.c > b/drivers/usb/chipidea/usbmisc_imx.c [] > @@ -199,31 +199,45 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data > *data) > val |= MX53_USB_PLL_DIV_24_MHZ; > writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); > > - if (data->disable_oc) { > - spin_lock_irqsave(&usbmisc->lock, flags); > - switch (data->index) { > + spin_lock_irqsave(&usbmisc->lock, flags); > + > + switch (data->index) { > case 0: > - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; > - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; > + if (data->disable_oc) { > + reg = usbmisc->base + > MX53_USB_OTG_PHY_CTRL_0_OFFSET; > + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; > + if (reg && val) > + writel(val, reg); > + } > break; It'd be nicer to unindent the switches case blocks one level too. switch (case->index){ case 0: etc... -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] usb: chipidea: imx: Change switch order
Hello. On 9/19/2016 1:22 PM, Fabien Lahoudere wrote: Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index Switch. and then test for features if they exist for this index. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 44 +- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..ed324d1 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,45 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { case 0: Should start at the same column as *switch*. - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (reg && val) You mean 'reg' can be NULL? If so, readl() would cause kernel oops. + writel(val, reg); + } break; case 1: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (reg && val) + writel(val, reg); Same here. + } break; case 2: - reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); Same here. + } break; case 3: - reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } And here. break; - } - if (reg && val) - writel(val, reg); Ah, the original code was wrong... - spin_unlock_irqrestore(&usbmisc->lock, flags); } [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/15] usb: chipidea: imx: Change switch order
Please forget this submission. On 19/09/16 12:18, Fabien Lahoudere wrote: Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index and then test for features if they exist for this index. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 44 +- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..ed324d1 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,45 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { case 0: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (reg && val) + writel(val, reg); + } break; case 1: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (reg && val) + writel(val, reg); + } break; case 2: - reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; case 3: - reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; - } - if (reg && val) - writel(val, reg); - spin_unlock_irqrestore(&usbmisc->lock, flags); } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 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: [RFC PATCH] xhci: do not halt the secondary HCD
Hello. On 9/19/2016 9:35 AM, Joel Stanley wrote: We can't halt the secondary HCD, because it's also the primary HCD, which will cause problems if we have devices attached to the primary HCD, like a keyboard. We've been carrying this in our Linux-as-a-bootloader environment for a little while now. The machines all have the same TI TUSB73x0 part, and when we kexec the devices don't come back until a system power cycle. I'd like some advice on an acceptable way to upstream the fix, so that the xhci device survives kexec. Signed-off-by: Joel Stanley --- drivers/usb/host/xhci.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index adc169d2fd76..ec92a843325b 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -682,6 +682,21 @@ void xhci_stop(struct usb_hcd *hcd) mutex_lock(&xhci->mutex); + /* +* We can't halt the secondary HCD, because it's also the primary +* HCD, which will cause problems if we have devices attached to the +* primary HCD, like a keyboard. +*/ + if (!usb_hcd_is_primary_hcd(hcd)) { + /* The shared_hcd is going to be deallocated shortly (the USB +* core only calls this function when allocation fails in +* usb_add_hcd(), or usb_remove_hcd() is called). So we need +* to unset xHCI's pointer. */ Please format this comment the same way as the comment above it. + xhci->shared_hcd = NULL; + mutex_unlock(&xhci->mutex); + return; + } + if (!(xhci->xhc_state & XHCI_STATE_HALTED)) { spin_lock_irq(&xhci->lock); [...] 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 v3 1/3] usb: chipidea: imx: Change switch order
Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index and then test for features if they exist for this index. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 44 +- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..ed324d1 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,45 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { case 0: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (reg && val) + writel(val, reg); + } break; case 1: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (reg && val) + writel(val, reg); + } break; case 2: - reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; case 3: - reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; - } - if (reg && val) - writel(val, reg); - spin_unlock_irqrestore(&usbmisc->lock, flags); } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 0; } -- 2.1.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 v3 2/3] usb: chipidea: imx: configure imx for ULPI phy
In order to use ULPI phy with usb host 2 and 3, we need to configure controller register to enable ULPI features. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 5 + drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 37 + 3 files changed, 43 insertions(+) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 0991794..96c0e33 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "ci.h" @@ -146,6 +147,10 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev) if (of_find_property(np, "external-vbus-divider", NULL)) data->evdo = 1; + + if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) + data->ulpi = 1; + return data; } diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h index 409aa5ca8..d666c9f 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -19,6 +19,7 @@ struct imx_usbmisc_data { unsigned int disable_oc:1; /* over current detect disabled */ unsigned int oc_polarity:1; /* over current polarity if oc enabled */ unsigned int evdo:1; /* set external vbus divider option */ + unsigned int ulpi:1; /* connected to an ULPI phy */ }; int imx_usbmisc_init(struct imx_usbmisc_data *); diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index ed324d1..5472900 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -46,11 +46,20 @@ #define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08 #define MX53_USB_OTG_PHY_CTRL_1_OFFSET 0x0c +#define MX53_USB_CTRL_1_OFFSET 0x10 +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2) +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2) +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6) +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6) #define MX53_USB_UH2_CTRL_OFFSET 0x14 #define MX53_USB_UH3_CTRL_OFFSET 0x18 #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) +#define MX53_USB_CTRL_1_UH2_ULPI_ENBIT(26) +#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(27) +#define MX53_USB_UHx_CTRL_WAKE_UP_EN BIT(7) +#define MX53_USB_UHx_CTRL_ULPI_INT_EN BIT(8) #define MX53_USB_PHYCTRL1_PLLDIV_MASK 0x3 #define MX53_USB_PLL_DIV_24_MHZ0x01 @@ -219,6 +228,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) } break; case 2: + if (data->ulpi) { + /* set USBH2 into ULPI-mode. */ + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET; + val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN; + /* select ULPI clock */ + val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK; + val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI; + writel(val, reg); + /* Set interrupt wake up enable */ + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN + | MX53_USB_UHx_CTRL_ULPI_INT_EN; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -227,6 +250,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) } break; case 3: + if (data->ulpi) { + /* set USBH3 into ULPI-mode. */ + reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET; + val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN; + /* select ULPI clock */ + val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK; + val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI; + writel(val, reg); + /* Set interrupt wake up enable */ + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN + | MX53_USB_UHx_CTRL_ULPI_INT_EN; + writel(val, reg); + }
[PATCH v3 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
This binding allow to disable the internal 60Mhz clock for USB host2 and host3. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 17 + 3 files changed, 20 insertions(+) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 96c0e33..89a9d98 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -147,6 +147,8 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev) if (of_find_property(np, "external-vbus-divider", NULL)) data->evdo = 1; + if (of_find_property(np, "disable-int60ck", NULL)) + data->disable_int60ck = 1; if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) data->ulpi = 1; diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h index d666c9f..43bafae 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -20,6 +20,7 @@ struct imx_usbmisc_data { unsigned int oc_polarity:1; /* over current polarity if oc enabled */ unsigned int evdo:1; /* set external vbus divider option */ unsigned int ulpi:1; /* connected to an ULPI phy */ + unsigned int disable_int60ck:1; /* disable 60 MHZ clock */ }; int imx_usbmisc_init(struct imx_usbmisc_data *); diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 5472900..bffc667 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -53,6 +53,9 @@ #define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6) #define MX53_USB_UH2_CTRL_OFFSET 0x14 #define MX53_USB_UH3_CTRL_OFFSET 0x18 +#define MX53_USB_CLKONOFF_CTRL_OFFSET 0x24 +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21) +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22) #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) @@ -242,6 +245,13 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) + | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -264,6 +274,13 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) + | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; -- 2.1.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 05/15] usb: chipidea: imx: Change switch order
Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index and then test for features if they exist for this index. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 44 +- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..ed324d1 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,45 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { case 0: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (reg && val) + writel(val, reg); + } break; case 1: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (reg && val) + writel(val, reg); + } break; case 2: - reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; case 3: - reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; - } - if (reg && val) - writel(val, reg); - spin_unlock_irqrestore(&usbmisc->lock, flags); } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 0; } -- 2.1.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: xHCI problem? [was Re: Erratic USB device behavior and device loss]
On 18 September 2016 at 03:42, Alan Stern wrote: > On Sat, 17 Sep 2016, Ritesh Raj Sarraf wrote: > >> -BEGIN PGP SIGNED MESSAGE- >> Hash: SHA512 >> >> Hello Alan, >> >> >> On Fri, 2016-09-16 at 17:40 -0400, Alan Stern wrote: >> > We're still getting runtime suspends, but now at 2-second intervals. >> > This is partly because the driver isn't calling >> > pm_runtime_mark_last_busy(), but there may be more to it. The 2-second >> > period is the default autosuspend timeout for USB devices. However, I >> > don't see the activity that rtsx_usb_get_card_status() should produce >> > when rtsx_usb_suspend() runs; I don't know why not. >> > >> > We're also getting occasional I/O attempts while the device is >> > suspended. They must be on some other pathway, not the one fixed by >> > the patch above. Let's see if we can find out just where they come >> > from. >> > >> > Ritesh, please try applying this patch on top of the previous one. It >> > will produce output in the kernel log whenever these bad I/O attempts >> > occur. Also, enable dynamic debugging for the rtsx_usb driver: >> > >> >> Please find links to the usbmon trace and the kernel trace. >> >> https://people.debian.org/~rrs/tmp/4.8.0-rc6ulf1alan1+.kern.log >> https://people.debian.org/~rrs/tmp/usb-4.8.0-rc6ulf1alan1+.log.gz > > Well, this is pretty clear: > > Sep 17 15:55:52 learner kernel: CPU: 1 PID: 535 Comm: rtsx_usb_ms_1 Tainted: > G U 4.8.0-rc6ulf1alan1+ #19 > Sep 17 15:55:52 learner kernel: Hardware name: LENOVO 20344/INVALID, BIOS > 96CN31WW(V1.17) 07/21/2015 > Sep 17 15:55:52 learner kernel: 81314be5 > 8802476746c0 0240 > Sep 17 15:55:52 learner kernel: a016f719 523bec00 > 88025f255780 88024feff600 > Sep 17 15:55:52 learner kernel: 00018080 > 88025f258080 815a0e60 > Sep 17 15:55:52 learner kernel: Call Trace: > Sep 17 15:55:52 learner kernel: [] ? dump_stack+0x7d/0xb8 > Sep 17 15:55:52 learner kernel: [] ? > usb_hcd_submit_urb+0x3c9/0xad0 [usbcore] > Sep 17 15:55:52 learner kernel: [] ? > _raw_spin_lock_irqsave+0x20/0x47 > Sep 17 15:55:52 learner kernel: [] ? > lock_timer_base.isra.24+0x7b/0xa0 > Sep 17 15:55:52 learner kernel: [] ? > try_to_del_timer_sync+0x49/0x60 > Sep 17 15:55:52 learner kernel: [] ? > usb_start_wait_urb+0x5d/0x140 [usbcore] > Sep 17 15:55:52 learner kernel: [] ? > rtsx_usb_send_cmd+0x5e/0x80 [rtsx_usb] > Sep 17 15:55:52 learner kernel: [] ? > rtsx_usb_read_register+0x67/0xb0 [rtsx_usb] > Sep 17 15:55:52 learner kernel: [] ? > rtsx_usb_detect_ms_card+0x61/0xe0 [rtsx_usb_ms] > Sep 17 15:55:52 learner kernel: [] ? > rtsx_usb_ms_set_param+0x770/0x770 [rtsx_usb_ms] > Sep 17 15:55:52 learner kernel: [] ? kthread+0xbd/0xe0 > Sep 17 15:55:52 learner kernel: [] ? > __switch_to+0x2b1/0x6a0 > Sep 17 15:55:52 learner kernel: [] ? > ret_from_fork+0x1f/0x40 > Sep 17 15:55:52 learner kernel: [] ? > kthread_create_on_node+0x180/0x180 > > This is the rtsx_usb_detect_ms_card() routine in > drivers/memstick/host/rtsx_usb_ms.c, which runs as a kthread. It > doesn't do any runtime PM. So it looks like the bug is present in both > the MMC and MemoryStick interfaces. I think the problem is even worse in the MemoryStick case, as the memstick core doesn't help with runtime PM. I am pretty sure there are other cases when the MemoryStick driver accesses the usb device without first runtime resuming it. Of course we could start simple an fix the bug observed above and see if that solves the reported problem. Alan, do you want to post to patch or you want me? Kind regards Uffe -- 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: dwc3: Wait for control tranfer completed when stopping gadget
Hi, Baolin Wang writes: >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index 1a33308..c9026ce 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -1441,6 +1441,15 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, >>> int is_on, int suspend) >>> if (pm_runtime_suspended(dwc->dev)) >>> return 0; >>> >>> + /* >>> + * Per databook, when we want to stop the gadget, if a control >>> transfer >>> + * is still in process, complete it and get the core into setup phase. >>> + */ >>> + if (!is_on && dwc->ep0state != EP0_SETUP_PHASE) { >>> + reinit_completion(&dwc->ep0_completed); >> >> this seems unnecessary to me. Also, why return here so the caller has to > > We should re-init the completion due to it will complete control > transfer many times before we try to stop gadget. not sure I get this comment, care to furter explain what you mean? >> wait? You could just have called wait_for_completion() here straight >> away: >> >> if (!is_on && dwc->ep0state != EP0_SETUP_PHASE) { >> /* should this be interruptible? */ >> ret = wait_for_completion_timeout(&dwc->ep0_in_setup, >> msecs_to_jiffies(500)); >> if (ret == 0) { >> dwc3_trace(trace_dwc3_gadget, "RUN/STOP timeout"); >> return -ETIMEDOUT; >> } >> } >> >> There's also no need for that "try_again" trickery. We either can halt >> the controller within 500ms or we cannot. > > But this is in atomic context and we can not issue > wait_for_completion_timeout() in atomic context, then we should just > return here. heh, good point. Missed that :-) -- balbi signature.asc Description: PGP signature
Re: USB hot-plug not working (ASUS TP301UA-C4028T)
On Sun, 2016-09-18 at 19:48 +1200, Pierre de Villemereuil wrote: > Dear kernel devs, > > I'm using openSUSE Tumbleweed (kernel 4.7.2-2-default) on an ASUS Vivobook > Flip TP301UA-C4028T, see here for specs: > https://www.asus.com/Notebooks/ASUS-VivoBook-Flip-TP301UA/specifications/ > > Since my install in July, USB hot-plug (a.k.a. plug'n'play) is not working. > To > make any USB device working (I tested many USB keys, a slideshow remote > control and a mouse), it has to be plugged on boot up or wake up from > suspend. > Using the following command also make the system aware that USB is connected: > udevadm trigger > > More information can be found on the following thread on the openSUSE forums: > https://forums.opensuse.org/showthread.php/519926-No-USB-device-detected Basically dmesg should show a hotplug. What happens if you unload and reload the xhci_hcd modules? And please do echo "module xhci_hcd +mpf" > /sys/kernel/debug/dynamic_debug/control (as root) and check dmesg again. 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] mmc: rtsx_usb_sdmmc: Handle runtime PM while changing led
On 18 September 2016 at 04:30, Alan Stern wrote: > On Sat, 17 Sep 2016, Ulf Hansson wrote: > >> Each access of the parent device (usb device) needs to be done in runtime >> resumed state. Currently this isn't case while changing the leds, so let's >> add pm_runtime_get_sync() and pm_runtime_put() around these calls. >> >> Signed-off-by: Ulf Hansson >> --- >> >> While discussing an issue[1] related to runtime PM, I found out that this >> minor change at least improves the behavior that has been observed. >> >> [1] >> http://www.spinics.net/lists/linux-usb/msg144634.html >> >> --- >> drivers/mmc/host/rtsx_usb_sdmmc.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/mmc/host/rtsx_usb_sdmmc.c >> b/drivers/mmc/host/rtsx_usb_sdmmc.c >> index 6c71fc9..a59c7fa 100644 >> --- a/drivers/mmc/host/rtsx_usb_sdmmc.c >> +++ b/drivers/mmc/host/rtsx_usb_sdmmc.c >> @@ -1314,6 +1314,7 @@ static void rtsx_usb_update_led(struct work_struct >> *work) >> container_of(work, struct rtsx_usb_sdmmc, led_work); >> struct rtsx_ucr *ucr = host->ucr; >> >> + pm_runtime_get_sync(sdmmc_dev(host)); >> mutex_lock(&ucr->dev_mutex); >> >> if (host->led.brightness == LED_OFF) >> @@ -1322,6 +1323,7 @@ static void rtsx_usb_update_led(struct work_struct >> *work) >> rtsx_usb_turn_on_led(ucr); >> >> mutex_unlock(&ucr->dev_mutex); >> + pm_runtime_put(sdmmc_dev(host)); >> } >> #endif > > The missing aspect here is that this won't stop the parent USB device > from going into autosuspend every 2 seconds and then resuming shortly > afterward. There are two ways of preventing this: > > Call usb_mark_last_busy() at appropriate places. > > Enable autosuspend for the sdmmc device. > > The second approach would also prevent the sdmmc device from going into > autosuspend as soon as the LED update is finished. Maybe that's okay, > but if going into suspend is a lightweight procedure then you may want > to prevent it. > We can for sure enable autosuspend for the sdmmc device, although as soon as an SD card will be detected the rtsx driver will increase the runtime PM usage count. The count is decreased when the card is removed (or failed to be initialized), thus runtime suspend is prevented as long as there is a functional card inserted. I am wondering what you think would be a good autosuspend timeout in this case? It seems to me that the only thing the rtsx driver really care about is to tell the parent device that it needs to be runtime resumed during a certain timeframe, more or less it would like to inherit the parents settings. Other mmc hosts, not being usb-mmc devices, are using an autosuspend timeout of ~50-100ms but that doesn't seem like good value here, right? Kind regards Uffe -- 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: Gadget regression with u_ether in Linux next
On Mon, Sep 19, 2016 at 11:19:07AM +0300, Felipe Balbi wrote: > > Hi Greg, > > Tony Lindgren writes: > > Hi, > > > > Looks like commit c9ffc78745f8 ("usb: gadget: NCM: Protect dev->port_usb > > using dev->lock") causes hangs for me with Linux next. > > > > Reverting c9ffc78745f8 makes the issues go away, some more info below. > > Can you revert this commit from your tree? > > Here's the full commit for reference: > > commit c9ffc78745f89e300fe704348dd8e6800acf4d18 > Author: Harish Jenny K N > Date: Fri Sep 9 11:30:42 2016 +0200 > > usb: gadget: NCM: Protect dev->port_usb using dev->lock > > This commit incorporates findings from > https://lkml.org/lkml/2016/4/25/594 > > The function has been modified to make sure we hold > the dev lock when accessing the net device pointer. > > Acked-by: Jim Baxter > Signed-off-by: Harish Jenny K N > Signed-off-by: Felipe Balbi Now reverted, 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: Gadget regression with u_ether in Linux next
Hi Greg, Tony Lindgren writes: > Hi, > > Looks like commit c9ffc78745f8 ("usb: gadget: NCM: Protect dev->port_usb > using dev->lock") causes hangs for me with Linux next. > > Reverting c9ffc78745f8 makes the issues go away, some more info below. Can you revert this commit from your tree? Here's the full commit for reference: commit c9ffc78745f89e300fe704348dd8e6800acf4d18 Author: Harish Jenny K N Date: Fri Sep 9 11:30:42 2016 +0200 usb: gadget: NCM: Protect dev->port_usb using dev->lock This commit incorporates findings from https://lkml.org/lkml/2016/4/25/594 The function has been modified to make sure we hold the dev lock when accessing the net device pointer. Acked-by: Jim Baxter Signed-off-by: Harish Jenny K N Signed-off-by: Felipe Balbi diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index 9c8c9ed1dc9e..8cb08033b7c1 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -553,14 +553,16 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb, spin_lock_irqsave(&dev->lock, flags); if (dev->port_usb) skb = dev->wrap(dev->port_usb, skb); - spin_unlock_irqrestore(&dev->lock, flags); if (!skb) { /* Multi frame CDC protocols may store the frame for * later which is not a dropped frame. */ if (dev->port_usb && - dev->port_usb->supports_multi_frame) + dev->port_usb->supports_multi_frame) { + spin_unlock_irqrestore(&dev->lock, flags); goto multiframe; + } + spin_unlock_irqrestore(&dev->lock, flags); goto drop; } } -- balbi signature.asc Description: PGP signature
Re: [RFC PATCH] xhci: do not halt the secondary HCD
Hi Mathias, On Mon, Sep 19, 2016 at 4:33 PM, Greg KH wrote: > On Mon, Sep 19, 2016 at 04:05:45PM +0930, Joel Stanley wrote: >> We can't halt the secondary HCD, because it's also the primary HCD, >> which will cause problems if we have devices attached to the primary >> HCD, like a keyboard. >> >> We've been carrying this in our Linux-as-a-bootloader environment for a >> little >> while now. The machines all have the same TI TUSB73x0 part, and when we kexec >> the devices don't come back until a system power cycle. >> >> I'd like some advice on an acceptable way to upstream the fix, so that the >> xhci >> device survives kexec. > > Any reason you didn't cc: Mathias? Fat fingers - I missed him when grabbing names from get_maintainers. Thanks for adding him in. On Mon, Sep 19, 2016 at 5:11 PM, Mathias Nyman wrote: > What kernel version is this? This patch is against 4.4.21. I've tested newer releases but haven't seen any improvement. > As Greg said there are fixes in this area in the 4.8 latest rc kernel. > > If that doesn't work then we need to figure out what the real issue is. No dice on 4.8-rc7 (without any patches). Here's 4.8-rc7 loading: [3.699524] xhci_hcd 0021:09:00.0: xHCI Host Controller [3.699556] xhci_hcd 0021:09:00.0: new USB bus registered, assigned bus number 1 [3.699640] xhci_hcd 0021:09:00.0: Using 64-bit DMA iommu bypass [3.699697] xhci_hcd 0021:09:00.0: hcc params 0x0270f06d hci version 0x96 quirks 0x [3.700286] hub 1-0:1.0: USB hub found [3.700299] hub 1-0:1.0: 4 ports detected [3.700493] xhci_hcd 0021:09:00.0: xHCI Host Controller [3.700522] xhci_hcd 0021:09:00.0: new USB bus registered, assigned bus number 2 [3.700552] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM. [3.700733] hub 2-0:1.0: USB hub found [3.700748] hub 2-0:1.0: 4 ports detected Then we kexec into the second kernel. Here's what the second kernel prints when trying to bring the controller up: [1.588272] xhci_hcd 0021:09:00.0: xHCI Host Controller [1.588282] xhci_hcd 0021:09:00.0: new USB bus registered, assigned bus number 1 [1.619279] xhci_hcd 0021:09:00.0: Host not halted after 16000 microseconds. [1.619281] xhci_hcd 0021:09:00.0: can't setup: -110 [1.619447] xhci_hcd 0021:09:00.0: USB bus 1 deregistered [1.619457] xhci_hcd 0021:09:00.0: init 0021:09:00.0 fail, -110 [1.619571] xhci_hcd: probe of 0021:09:00.0 failed with error -110 Note that the second kernel is a distro one (Ubuntu 4.4.0-36-generic). > xhci hardware is really just one controller. The split into primary and > secondary HCD > is a software only. We always load the primary HCD first (USB2) and > secondary second (USB3). > We unload them in reverse order, and need to stop the xhci (halt the hcd) as > a first step. > > load primary > load secondary (starts the xhci controller > ... > unload secondary (halts the controller) > unload primary (free memory) Thanks for the explanation. I wasn't the author of the first hack we put in our tree, but I have rewritten it as we rebase on the stable tree regularly. So the hack as I sent it doesn't do any halt the secondary, and lets the primary unload path halt the controller. Any theory as to why this helps? Cheers, Joel -- 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 v6 0/8] power: add power sequence library
On Monday 19 September 2016 01:16 PM, Peter Chen wrote: On Mon, Sep 19, 2016 at 01:09:10PM +0530, Vaibhav Hiremath wrote: On Friday 09 September 2016 02:17 PM, Ulf Hansson wrote: [...] We had an agreement that keep mmc's pwrseq framework unchanging. Unless Ulf and rob both agree to change. Why 2 separate approach for same problem ? And I see this as possible duplication of code/functionality :) How the new kernel compatibles old dts? If we do not need to consider this problem, the mmc can try to use power sequence library too in future. I think we should attempt to get both MMC and USB power seq come on one agreement, so that it can be reused. That would be nice. Although, to do that you would have to allow some DT bindings to be deprecated in the new generic power seq bindings, as otherwise you would break existing DTBs. I guess that is what Rob was objecting to!? yeah, thats right. So lets adopt similar implementation for USB as well instead of library, but keeping MMC untouched as of now. What I am trying to propose here is, Lets have power-sequence framework (similar to V1 of this series), with, pwrseq: Core framework for power sequence. pwrseq_generic/simple: for all generic control, like reset and clock pwrseq_emmc: probably duplication of existing code - the idea here is, all future code should be using this new binding, so that we can deprecate the drivers/mmc/core/pwrseq pwrseq_arche: The usecase which I am dealing with today, which is more complex in nature. Then the respective drivers can add their drivers (if needed) based on complexity. comments ?? The key point here is DT maintainer (Rob) doesn't agree with adding new node for power sequence at dts. Hmmm. We haven't heard from Rob lately especially after introduction of complex usecases. I hope he would revisit on above proposal. Thanks, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] Usb gadget ncm code cleanup
This patchset consists of some code cleanup in usb gadget ncm code. Note: Testing has only been done on an ARM i.MX6 based platform. --- Change from v1 to v2 Subject line changed on Patch 2. Torsten Polle (3): usb: gadget: NCM: link socket buffers to the device for tx packets usb: gadget: u_ether: link socket buffers to the device for received packets usb: gadget: NCM: differentiate consumed packets from dropped packets drivers/usb/gadget/function/f_ncm.c | 11 +++ drivers/usb/gadget/function/u_ether.c |5 +++-- 2 files changed, 10 insertions(+), 6 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] usb: gadget: NCM: differentiate consumed packets from dropped packets
From: Torsten Polle dev_kfree_skb_any() is used to free packets that are dropped by the network stack. Therefore the function should not be used for packets that have been successfully processed by the network stack. Instead dev_consume_skb_any() has to be used for such consumed packets. This separation helps to identify dropped packets. Signed-off-by: Torsten Polle Signed-off-by: Harish Jenny K N --- drivers/usb/gadget/function/f_ncm.c |8 drivers/usb/gadget/function/u_ether.c |3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c index b6771ad..e8008fa 100644 --- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -998,7 +998,7 @@ static struct sk_buff *package_for_tx(struct f_ncm *ncm) /* Merge the skbs */ swap(skb2, ncm->skb_tx_data); if (ncm->skb_tx_data) { - dev_kfree_skb_any(ncm->skb_tx_data); + dev_consume_skb_any(ncm->skb_tx_data); ncm->skb_tx_data = NULL; } @@ -1009,7 +1009,7 @@ static struct sk_buff *package_for_tx(struct f_ncm *ncm) /* Copy NTB across. */ ntb_iter = (void *) skb_put(skb2, ncm->skb_tx_ndp->len); memcpy(ntb_iter, ncm->skb_tx_ndp->data, ncm->skb_tx_ndp->len); - dev_kfree_skb_any(ncm->skb_tx_ndp); + dev_consume_skb_any(ncm->skb_tx_ndp); ncm->skb_tx_ndp = NULL; /* Insert zero'd datagram. */ @@ -1136,7 +1136,7 @@ static struct sk_buff *ncm_wrap_ntb(struct gether *port, memset(ntb_data, 0, dgram_pad); ntb_data = (void *) skb_put(ncm->skb_tx_data, skb->len); memcpy(ntb_data, skb->data, skb->len); - dev_kfree_skb_any(skb); + dev_consume_skb_any(skb); skb = NULL; } else if (ncm->skb_tx_data && ncm->timer_force_tx) { @@ -1332,7 +1332,7 @@ static int ncm_unwrap_ntb(struct gether *port, } while (ndp_len > 2 * (opts->dgram_item_len * 2)); } while (ndp_index); - dev_kfree_skb_any(skb); + dev_consume_skb_any(skb); VDBG(port->func.config->cdev, "Parsed NTB with %d frames\n", dgram_counter); diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index d8593c7..8a6be06 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -455,16 +455,17 @@ static void tx_complete(struct usb_ep *ep, struct usb_request *req) /* FALLTHROUGH */ case -ECONNRESET: /* unlink */ case -ESHUTDOWN:/* disconnect etc */ + dev_kfree_skb_any(skb); break; case 0: dev->net->stats.tx_bytes += skb->len; + dev_consume_skb_any(skb); } dev->net->stats.tx_packets++; spin_lock(&dev->req_lock); list_add(&req->list, &dev->tx_reqs); spin_unlock(&dev->req_lock); - dev_kfree_skb_any(skb); atomic_dec(&dev->tx_qlen); if (netif_carrier_ok(dev->net)) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] usb: gadget: u_ether: link socket buffers to the device for received packets
From: Torsten Polle Socket buffers should be linked to the (network) device that allocated the buffers. __netdev_alloc_skb performs this task. Signed-off-by: Torsten Polle Signed-off-by: Jim Baxter Signed-off-by: Harish Jenny K N --- Change from v1 to v2 Subject line changed drivers/usb/gadget/function/u_ether.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index 8cb0803..d8593c7 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -224,7 +224,7 @@ static void defer_kevent(struct eth_dev *dev, int flag) if (dev->port_usb->is_fixed) size = max_t(size_t, size, dev->port_usb->fixed_out_len); - skb = alloc_skb(size + NET_IP_ALIGN, gfp_flags); + skb = __netdev_alloc_skb(dev->net, size + NET_IP_ALIGN, gfp_flags); if (skb == NULL) { DBG(dev, "no rx skb\n"); goto enomem; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] usb: gadget: NCM: link socket buffers to the device for tx packets
From: Torsten Polle Socket buffers should be linked to the (network) device that allocated the buffers. Signed-off-by: Torsten Polle Signed-off-by: Harish Jenny K N --- drivers/usb/gadget/function/f_ncm.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c index 6396037..b6771ad 100644 --- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -1078,6 +1078,7 @@ static struct sk_buff *ncm_wrap_ntb(struct gether *port, if (!ncm->skb_tx_data) goto err; + ncm->skb_tx_data->dev = ncm->netdev; ntb_data = (void *) skb_put(ncm->skb_tx_data, ncb_len); memset(ntb_data, 0, ncb_len); /* dwSignature */ @@ -1096,6 +1097,8 @@ static struct sk_buff *ncm_wrap_ntb(struct gether *port, GFP_ATOMIC); if (!ncm->skb_tx_ndp) goto err; + + ncm->skb_tx_ndp->dev = ncm->netdev; ntb_ndp = (void *) skb_put(ncm->skb_tx_ndp, opts->ndp_size); memset(ntb_ndp, 0, ncb_len); -- 1.7.9.5 -- 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 v6 0/8] power: add power sequence library
On Mon, Sep 19, 2016 at 01:09:10PM +0530, Vaibhav Hiremath wrote: > > > On Friday 09 September 2016 02:17 PM, Ulf Hansson wrote: > >[...] > > > >We had an agreement that keep mmc's pwrseq framework unchanging. > >Unless Ulf and rob both agree to change. > Why 2 separate approach for same problem ? > And I see this as possible duplication of code/functionality :) > >>>How the new kernel compatibles old dts? If we do not need to > >>>consider this problem, the mmc can try to use power sequence library > >>>too in future. > >> > >>I think we should attempt to get both MMC and USB power seq > >>come on one agreement, so that it can be reused. > >That would be nice. Although, to do that you would have to allow some > >DT bindings to be deprecated in the new generic power seq bindings, as > >otherwise you would break existing DTBs. > > > >I guess that is what Rob was objecting to!? > > yeah, thats right. > > So lets adopt similar implementation for USB as well instead of > library, but keeping MMC untouched as of now. > > What I am trying to propose here is, > > Lets have power-sequence framework (similar to V1 of this series), > with, > > pwrseq: Core framework for power sequence. > pwrseq_generic/simple: for all generic control, like reset and clock > pwrseq_emmc: probably duplication of existing code - the idea > here is, all future code should be using this new > binding, so that we can deprecate the > drivers/mmc/core/pwrseq > pwrseq_arche: The usecase which I am dealing with today, which is more > complex in nature. > > Then the respective drivers can add their drivers (if needed) based on > complexity. > > comments ?? The key point here is DT maintainer (Rob) doesn't agree with adding new node for power sequence at dts. -- 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: [RFC PATCH] xhci: do not halt the secondary HCD
On 19.09.2016 09:35, Joel Stanley wrote: We can't halt the secondary HCD, because it's also the primary HCD, which will cause problems if we have devices attached to the primary HCD, like a keyboard. We've been carrying this in our Linux-as-a-bootloader environment for a little while now. The machines all have the same TI TUSB73x0 part, and when we kexec the devices don't come back until a system power cycle. I'd like some advice on an acceptable way to upstream the fix, so that the xhci device survives kexec. Signed-off-by: Joel Stanley --- What kernel version is this? As Greg said there are fixes in this area in the 4.8 latest rc kernel. If that doesn't work then we need to figure out what the real issue is. xhci hardware is really just one controller. The split into primary and secondary HCD is a software only. We always load the primary HCD first (USB2) and secondary second (USB3). We unload them in reverse order, and need to stop the xhci (halt the hcd) as a first step. load primary load secondary (starts the xhci controller ... unload secondary (halts the controller) unload primary (free memory) -Mathias -- 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 v6 0/8] power: add power sequence library
On Friday 09 September 2016 02:17 PM, Ulf Hansson wrote: [...] We had an agreement that keep mmc's pwrseq framework unchanging. Unless Ulf and rob both agree to change. Why 2 separate approach for same problem ? And I see this as possible duplication of code/functionality :) How the new kernel compatibles old dts? If we do not need to consider this problem, the mmc can try to use power sequence library too in future. I think we should attempt to get both MMC and USB power seq come on one agreement, so that it can be reused. That would be nice. Although, to do that you would have to allow some DT bindings to be deprecated in the new generic power seq bindings, as otherwise you would break existing DTBs. I guess that is what Rob was objecting to!? yeah, thats right. So lets adopt similar implementation for USB as well instead of library, but keeping MMC untouched as of now. What I am trying to propose here is, Lets have power-sequence framework (similar to V1 of this series), with, pwrseq: Core framework for power sequence. pwrseq_generic/simple: for all generic control, like reset and clock pwrseq_emmc: probably duplication of existing code - the idea here is, all future code should be using this new binding, so that we can deprecate the drivers/mmc/core/pwrseq pwrseq_arche: The usecase which I am dealing with today, which is more complex in nature. Then the respective drivers can add their drivers (if needed) based on complexity. comments ?? MMC Power Seq : It is based on platform_device/platform_driver approach, We recently converted MMC to this. It's has a clear benefit as you can rely on the behaviour from the driver core and PM core. So, it simply avoids duplication of code. Agreed. USB Power seq : We are trying to propose library approach, with compatible string match. We should try to have one approach. - Lets also add suspend/resume callback to struct pwrseq Why suspend/resume can't do at related driver's suspend/resume API? Nope... The pwrseq library would have taken ownership of resources, so related driver cannot suspend the device. Again it is architecture specific, but we should have provision to handle this. The system I am dealing with today, does need suspend/resume callback. To be precise, suspend is close to off state for some devices or they could enter standby or different low power state, but to do that, we need same resource as used for ON/OFF functionality. Ok, I will have API for suspend/resume. You can implement it at your own library or generic one. As stated, using a platform device + driver would simplify this, as you wouldn't need an API but only a driver. I guess. Exactly. Thanks, Vaibhav -- 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/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
On Monday, September 19, 2016 10:29:27 AM CEST Kishon Vijay Abraham I wrote: > On Monday 19 September 2016 01:26 AM, Martin Blumenstingl wrote: > > On Fri, Sep 16, 2016 at 10:19 AM, Kishon Vijay Abraham I > > wrote: > >> This way the driver will be probed only once (the reset can be done during > >> probe). The phy driver should scan the dt node and for every sub-node it > >> invokes phy_create? > > I'll recap what we have discussed so far (so you don't have to re-read > > the whole thread): > > The reference driver treats both USB PHYs as separate devices (the > > datasheet has no information about the PHYs though). The only > > "special" thing is the shared reset line -> together with Philipp > > Zabel (the reset framework maintainer) we decided to make > > reset_control_reset work for shared reset lines. > > > > That means we can keep the two PHYs as separate devices inside the > > .dts, while keeping everything else separate (just like the reference > > driver) > > Is this fine for you and Arnd? > > yeah.. I'm fine with that. > Agreed, if we don't need to have a device modeled around both instance, that's ideal. Arnd -- 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 PATCH] xhci: do not halt the secondary HCD
On Mon, Sep 19, 2016 at 04:05:45PM +0930, Joel Stanley wrote: > We can't halt the secondary HCD, because it's also the primary HCD, > which will cause problems if we have devices attached to the primary > HCD, like a keyboard. > > We've been carrying this in our Linux-as-a-bootloader environment for a little > while now. The machines all have the same TI TUSB73x0 part, and when we kexec > the devices don't come back until a system power cycle. > > I'd like some advice on an acceptable way to upstream the fix, so that the > xhci > device survives kexec. Any reason you didn't cc: Mathias? And have you tried 4.8-rc kernels? I thought we just fixed an issue around secondary HCDs... 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