[PATCH] usb: ulpi: Add missing of_node_get() in ulpi_of_register()

[Stephen Boyd]

In ulpi_of_register() we call of_find_node_by_name() which
unconditionally calls of_node_put() on the 'from' argument. We
haven't called of_node_get() though, so we've put the node once
without getting it first. Add the of_node_get() call so that
things are properly balanced.

Fixes: ef6a7bcfb01c ("usb: ulpi: Support device discovery via DT")
Reported-by: Peter Robinson 
Signed-off-by: Stephen Boyd 
---
 drivers/usb/common/ulpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 4aa5195db8ea..59e05eff76f9 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -182,9 +182,9 @@ static int ulpi_of_register(struct ulpi *ulpi)
 
/* Find a ulpi bus underneath the parent or the grandparent */
parent = ulpi->dev.parent;
-   if (parent->of_node)
+   if (of_node_get(parent->of_node))
np = of_find_node_by_name(parent->of_node, "ulpi");
-   else if (parent->parent && parent->parent->of_node)
+   else if (parent->parent && of_node_get(parent->parent->of_node))
np = of_find_node_by_name(parent->parent->of_node, "ulpi");
if (!np)
return 0;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] drivers: usb: phy: Add qoriq usb 3.0 phy driver support

[Rob Herring]

On Tue, Nov 07, 2017 at 03:20:53PM +0800, Ran Wang wrote:
> Adds qoriq usb 3.0 phy driver to implement erratum related workaround
> for qoriq SoC.
> 
> Signed-off-by: Sriram Dash 
> Signed-off-by: Ran Wang 
> ---
> Change in v2:
>   - Replace funciont __raw_writel() by iowrite32be()
>   - Remove qoriq_usb3_phy_read() and qoriq_usb3_phy_write()
>   - Remove USB3PRM1CR_VAL define, use numbers directly
>   - Use C-style numeric constants on 32'h27672B2A in annotation
>   - Add prefix 'static' to struct qoriq_usb3_phy_erratum
> 
>  .../devicetree/bindings/phy/phy-qoriq-usb3.txt |  36 

Please split bindings to separate patch.

>  drivers/phy/Kconfig|   9 +
>  drivers/phy/Makefile   |   1 +
>  drivers/phy/phy-qoriq-usb3.c   | 191 
> +
>  4 files changed, 237 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
>  create mode 100644 drivers/phy/phy-qoriq-usb3.c
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt 
> b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
> new file mode 100644
> index ..d956f9c89fbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-qoriq-usb3.txt
> @@ -0,0 +1,36 @@
> +Driver for Freescale USB 3.0 PHY
> +
> +Required properties:
> +
> +- compatible :   fsl,qoriq-usb3-phy
> +- reg :  register mappings for Parameter Configuration Register
> + and Phy base offset.
> +- reg-names :"param_ctrl" and "phy_base"
> +- phy_type : For multi port host USB controllers, should be one of
> + "ulpi", or "serial". For dual role USB controllers,
> + should be one of "ulpi", "utmi", "utmi_wide", or "serial".
> +
> +Example:
> + usbphy0: usb-phy@084F {

Drop leading 0s and use lower case hex. Building dts with W=2 will tell 
you this.

> + compatible = "fsl,qoriq-usb3-phy";
> +reg = <0x0 0x01570070 0x0 0xC>, <0x0 0x084F 0x0 0x5000>;
> +reg-names = "param_ctrl", "phy_base";
> +#phy-cells = <0>;
> +phy_type = "utmi";
> + };
> +
> +usbphy1: usb-phy@0850 {
> +compatible = "fsl,qoriq-usb3-phy";
> +reg = <0x0 0x0157007C 0x0 0xC>, <0x0 0x0850 0x0 0x5000>;
> +reg-names = "param_ctrl", "phy_base";
> +#phy-cells = <0>;
> +phy_type = "utmi";
> +};
> +
> +usbphy2: usb-phy@0851 {
> +compatible = "fsl,qoriq-usb3-phy";
> +reg = <0x0 0x01570088 0x0 0xC>, <0x0 0x0851 0x0 0x5000>;
> +reg-names = "param_ctrl", "phy_base";
> +#phy-cells = <0>;
> +phy_type = "utmi";
> +};
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: f_fs: Drop check on Reserved1 field on OS_DESC_EXT_COMPAT

[John Keeping]

On Fri, 10 Nov 2017 12:40:39 +0200, Felipe Balbi wrote:

> John Keeping  writes:
> > This check has gone through several incompatible variations in commits
> > 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on reserved1 of
> > OS_DESC_EXT_COMPAT"), 354bc45bf329 ("usb: gadget: f_fs: Fix ExtCompat
> > descriptor validation") and 3ba534df815f ("Revert "usb: gadget: f_fs:
> > Fix ExtCompat descriptor validation"") after initially being introduced
> > in commit f0175ab51993 ("usb: gadget: f_fs: OS descriptors support").
> >
> > The various changes make it impossible for a single userspace
> > implementation to work with different kernel versions, so let's just
> > drop the condition to avoid breaking userspace.
> >
> > Fixes: 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on reserved1 of 
> > OS_DESC_EXT_COMPAT")
> > Cc: sta...@vger.kernel.org # v4.7+
> > Signed-off-by: John Keeping 
> > ---
> >  drivers/usb/gadget/function/f_fs.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_fs.c 
> > b/drivers/usb/gadget/function/f_fs.c
> > index 652397eda6d6..0d9962834345 100644
> > --- a/drivers/usb/gadget/function/f_fs.c
> > +++ b/drivers/usb/gadget/function/f_fs.c
> > @@ -2282,8 +2282,7 @@ static int __ffs_data_do_os_desc(enum 
> > ffs_os_desc_type type,
> > int i;
> >  
> > if (len < sizeof(*d) ||
> > -   d->bFirstInterfaceNumber >= ffs->interfaces_count ||
> > -   !d->Reserved1)
> > +   d->bFirstInterfaceNumber >= ffs->interfaces_count)
> > return -EINVAL;
> > for (i = 0; i < ARRAY_SIZE(d->Reserved2); ++i)
> > if (d->Reserved2[i])  
> 
> Sorry, but no. We want to be compliant with the specification. If there
> are older still-maintained stable trees which are not working, we need
> to backport a fix to them, but we're not allowing uncompliant
> implementations.

Aren't we allowing non-compliant implementations now?  The spec says the
value must be 1 but since v4.7 this code has allowed all non-zero
values.

At this point I don't think the kernel can disallow any values of
Reserved1 without breaking someone's userspace.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Alan Stern]

On Fri, 10 Nov 2017 abhij...@vger.kernel.org wrote:

> From: Abhijeet Kumar 
> 
> Runtime resume USB device in order to ensure that PM framework knows
> that the we might be using the device in a short time and doesn't
> autosuspend the device while we update it's interface. Without this
> change, if device autosuspends and the kernel polling for block
> devices is disabled through sysfs at runtime or through bootargs, then
> storage devices might never unmount since the disconnect IRQ wont be
> kicked at all.
> 
> Signed-off-by: Abhijeet Kumar 

I don't see any reason for doing this.  The places that call this 
subroutine will already have made sure that the device is active.  
There's no need for an additional runtime_get.

As for disconnect IRQs, they occur whether the device is suspended or 
not (they are generated by the host controller, not by the device).

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Greg Kroah-Hartman]

On Fri, Nov 10, 2017 at 09:32:07PM +0530, abhij...@vger.kernel.org wrote:
> From: Abhijeet Kumar 
> 
> Runtime resume USB device in order to ensure that PM framework knows
> that the we might be using the device in a short time and doesn't
> autosuspend the device while we update it's interface. Without this
> change, if device autosuspends and the kernel polling for block
> devices is disabled through sysfs at runtime or through bootargs, then
> storage devices might never unmount since the disconnect IRQ wont be
> kicked at all.
> 
> Signed-off-by: Abhijeet Kumar 
> ---
>  drivers/usb/core/message.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 371a07d874a3..54a9accf88cb 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1305,6 +1305,11 @@ int usb_set_interface(struct usb_device *dev, int 
> interface, int alternate)
>   if (iface->unregistering)
>   return -ENODEV;
>  
> + /*Letting runtime PM know that we wish to use the device in a
> +  * short time.
> +  */

Please place comments int the correct style.

> + pm_runtime_get(>dev);
> +

No dropping of the pm_runtime when we are finished?

This feels really wrong to me...

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

[PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Abhijeet]

From: Abhijeet Kumar 

Runtime resume USB device in order to ensure that PM framework knows
that the we might be using the device in a short time and doesn't
autosuspend the device while we update it's interface. Without this
change, if device autosuspends and the kernel polling for block
devices is disabled through sysfs at runtime or through bootargs, then
storage devices might never unmount since the disconnect IRQ wont be
kicked at all.

Signed-off-by: Abhijeet Kumar 
---
 drivers/usb/core/message.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 371a07d874a3..54a9accf88cb 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1305,6 +1305,11 @@ int usb_set_interface(struct usb_device *dev, int 
interface, int alternate)
if (iface->unregistering)
return -ENODEV;
 
+   /*Letting runtime PM know that we wish to use the device in a
+* short time.
+*/
+   pm_runtime_get(>dev);
+
alt = usb_altnum_to_altsetting(iface, alternate);
if (!alt) {
dev_warn(>dev, "selecting invalid altsetting %d\n",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Multiple g_zero gadgets

[Alan Stern]

On Fri, 10 Nov 2017, Hinko Kocevar wrote:

> By the way, if I try to do a control out transfer to the g_zero gadget
> I get 'Pipe error' (using libusb-1.0). Is this expected?

No, it is not expected (assuming your transfer has USB_TYPE_VENDOR and
bRequest = 0x5b and wValue = wIndex = 0).

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] uas: Add US_FL_NO_ATA_1X quirk for one more Seagate device

[Hans de Goede]

Just like all previous UAS capable Seagate disk enclosures, this
one needs a US_FL_NO_ATA_1X quirk.

Cc: sta...@vger.kernel.org # 3.16
Signed-off-by: Andrey Astafyev <1...@246060.ru>
Signed-off-by: Hans de Goede 
---
 drivers/usb/storage/unusual_uas.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/storage/unusual_uas.h 
b/drivers/usb/storage/unusual_uas.h
index cde115359793..2fe4fd336446 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -121,6 +121,13 @@ UNUSUAL_DEV(0x0bc2, 0xab2a, 0x, 0x,
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
US_FL_NO_ATA_1X),
 
+/* Reported-by: Andrey Astafyev  */
+UNUSUAL_DEV(0x0bc2, 0xab30, 0x, 0x,
+   "Seagate",
+   "BUP BK",
+   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+   US_FL_NO_ATA_1X),
+
 /* Reported-by: Benjamin Tissoires  */
 UNUSUAL_DEV(0x13fd, 0x3940, 0x, 0x,
"Initio Corporation",
-- 
2.14.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

Re: [PATCH] USB: fix buffer overflows with parsing CDC headers

[Greg KH]

On Wed, Nov 08, 2017 at 12:43:07PM +0100, Oliver Neukum wrote:
> In newer kernels this issue has been fixed at a central location with
> 
> commit 2e1c42391ff2556387b3cb6308b24f6f65619feb
> Author: Greg Kroah-Hartman 
> Date:   Thu Sep 21 16:58:48 2017 +0200
> 
> USB: core: harden cdc_parse_cdc_header
> 
> on anything older the parsing had not been centralised, so a separate
> fix for each driver is necessary.
> 
> Signed-off-by: Oliver Neukum 
> ---
>  drivers/net/usb/cdc_ether.c | 9 -
>  drivers/usb/class/cdc-acm.c | 8 +++-
>  drivers/usb/class/cdc-wdm.c | 2 ++
>  3 files changed, 17 insertions(+), 2 deletions(-)

What kernel tree is this made against?  It doesn't apply to 3.18-stable,
and commit 2e1c42391ff2556387b3cb6308b24f6f65619feb is in 4.4 and newer,
right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Felipe Balbi]

Hi,

abhij...@vger.kernel.org, ku...@vger.kernel.org writes:

these emails don't exist. Fix your email client.

> From: abhijeet kumar 

capitalize names

> Runtime resume USB device in order to ensure that PM framework
> knows that the we might be using the device in a short time and doesn't
> autosuspend the device while we updating it's interface.

this doesn't tell me about any problem. What, exactly, are you trying to
fix?

> Signed-off-by: abhijeet kumar 

capitalize names

> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 371a07d874a3..658603ed779e 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1305,6 +1305,9 @@ int usb_set_interface(struct usb_device *dev, int 
> interface, int alternate)
>   if (iface->unregistering)
>   return -ENODEV;
>  
> + /*Letting runtime PM now that we wish to use the device in a short time
> +  *pm_runtime_get(>dev);
> +  */

why is it so that adding commented out code help? Did you *really* test
this at all?

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] USB :core :Prevent USB devices to autosuspend while setting interface

[Abhijeet]

From: abhijeet kumar 

Runtime resume USB device in order to ensure that PM framework
knows that the we might be using the device in a short time and doesn't
autosuspend the device while we updating it's interface.

Signed-off-by: abhijeet kumar 
---
 drivers/usb/core/message.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 371a07d874a3..658603ed779e 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1305,6 +1305,9 @@ int usb_set_interface(struct usb_device *dev, int 
interface, int alternate)
if (iface->unregistering)
return -ENODEV;
 
+   /*Letting runtime PM now that we wish to use the device in a short time
+*pm_runtime_get(>dev);
+*/
+
alt = usb_altnum_to_altsetting(iface, alternate);
if (!alt) {
dev_warn(>dev, "selecting invalid altsetting %d\n",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: f_fs: Drop check on Reserved1 field on OS_DESC_EXT_COMPAT

[Felipe Balbi]

Hi,

John Keeping  writes:
> This check has gone through several incompatible variations in commits
> 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on reserved1 of
> OS_DESC_EXT_COMPAT"), 354bc45bf329 ("usb: gadget: f_fs: Fix ExtCompat
> descriptor validation") and 3ba534df815f ("Revert "usb: gadget: f_fs:
> Fix ExtCompat descriptor validation"") after initially being introduced
> in commit f0175ab51993 ("usb: gadget: f_fs: OS descriptors support").
>
> The various changes make it impossible for a single userspace
> implementation to work with different kernel versions, so let's just
> drop the condition to avoid breaking userspace.
>
> Fixes: 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on reserved1 of 
> OS_DESC_EXT_COMPAT")
> Cc: sta...@vger.kernel.org # v4.7+
> Signed-off-by: John Keeping 
> ---
>  drivers/usb/gadget/function/f_fs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c 
> b/drivers/usb/gadget/function/f_fs.c
> index 652397eda6d6..0d9962834345 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -2282,8 +2282,7 @@ static int __ffs_data_do_os_desc(enum ffs_os_desc_type 
> type,
>   int i;
>  
>   if (len < sizeof(*d) ||
> - d->bFirstInterfaceNumber >= ffs->interfaces_count ||
> - !d->Reserved1)
> + d->bFirstInterfaceNumber >= ffs->interfaces_count)
>   return -EINVAL;
>   for (i = 0; i < ARRAY_SIZE(d->Reserved2); ++i)
>   if (d->Reserved2[i])

Sorry, but no. We want to be compliant with the specification. If there
are older still-maintained stable trees which are not working, we need
to backport a fix to them, but we're not allowing uncompliant
implementations.

-- 
balbi


signature.asc
Description: PGP signature

Re: xhci_hcd WARN Event TRB for slot ep with no TDs queued?

[Juan Simón]

2017-11-02 18:18 GMT+01:00 Mathias Nyman :
> On 02.11.2017 16:31, Juan Simón wrote:
>>
>> Hi,
>> This is the trace output: https://pastebin.com/apt56yGe
>
>
> Thanks
> xhci logs look pretty good to me. A interrupt transfer TRB is queued
> asking for 32 bytes from the mouse, and mouse replies with 15 bytes.
> After this and we immediately queue the next TRB.
>
> Are you sure this generates the "WARN TRB for slot x ep with no TDs queued"?
> can you check dmesg if those really appear?
> In the logs we always have a TRB (TD) queued when we receive events.
>
> The mouse is however constantly responding with 15 bytes of data,
> as if it's constantly moving, sending data.
>
> Not sure how HID devices normally behave, but my guess is that they would
> respond with a NAK to the interrupt transfer if no data is pending.
>
> git bisect would show when it stopped working, could be a change
> somewhere else as well, hid maybe?
>
> -Mathias
>

Solved, FYI:
https://bbs.archlinux.org/viewtopic.php?pid=1739030#p1739030
https://bugzilla.redhat.com/show_bug.cgi?id=1497861
https://www.spinics.net/lists/kernel/msg2571180.html

It seems the problem is in PEAQ_WMI module.
I hope it will be resolved in the 4.14 kernel version.
Thank you too much for your help.
Regards.

NOTE: I forward it because the previous one I forgot to send in text format. :-(
--
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: Multiple g_zero gadgets

[Hinko Kocevar]

By the way, if I try to do a control out transfer to the g_zero gadget
I get 'Pipe error' (using libusb-1.0). Is this expected?

On Thu, Nov 9, 2017 at 4:24 PM, Alan Stern  wrote:
> On Thu, 9 Nov 2017, Felipe Balbi wrote:
>
>>
>> Hi,
>>
>> Hinko Kocevar  writes:
>> >> The way dummy was written, it can only instantiate one gadget. You
>> >> either need a real USB peripheral controller, or you need to patch dummy
>> >> to instantiate more than one gadget.
>> >>
>> >> --
>> >> balbi
>> >
>> > By dummy - are you referring to g_zero of dummy_hcd?
>>
>> Okay, we need a little more explanation. In order to have a gadget
>> driver like g_zero loaded, you need a USB peripheral controller (in your
>> case, that's handled by dummy).
>>
>> Now, when you want that g_zero to be useful, you need to plug it to a
>> USB host controller (in your case, that's also handled by dummy).
>>
>> What I'm saying, though, is that this dummy (dummy_hcd.c) can only
>> handle ONE gadget (a fake USB peripheral controller) at a time. It does
>> NOT instantiate more than one of those.
>
> In fact, this isn't correct.  See the "num" module parameter in
> dummy_hcd.c (added by commit c7a1db457bfc "usb: gadget: dummy_hcd: add
> setup / cleanup of multiple HW intances").
>
> Alan Stern
>
>> Because of that, there's no way
>> to get g_zero to probe for a second time.
>>
>> In fact, even if you were to patch dummy_hcd.c to instantiate N gadgets,
>> you wouldn't be able to use g_zero.ko, and would be required to rely on
>> our configfs interface for dynamically generating and binding different
>> gadget drivers to each of your gadget instances, but that's another
>> story altogether.
>
>
>



-- 
.. the more I see the less I believe.., AE AoR
--
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