Re: [RFC usb-next v3 2/2] usb: core: use phy_exit during suspend if wake up is not supported

[Chunfeng Yun]

hi,
On Mon, 2018-03-26 at 22:38 +0200, Martin Blumenstingl wrote:
> If the USB controller can wake up the system (which is the case for
> example with the Mediatek USB3 IP) then we must not call phy_exit during
> suspend to ensure that the USB controller doesn't have to re-enumerate
> the devices during resume.
> However, if the USB controller cannot wake up the system (which is the
> case for example on various TI platforms using a dwc3 controller) then
> we must call phy_exit during suspend. Otherwise the PHY driver keeps the
> clocks enabled, which prevents the system from reaching the lowest power
> levels in the suspend state.
> 
> Solve this by introducing two new functions in the PHY wrapper which are
> dedicated to the suspend and resume handling.
> If the controller can wake up the system the new usb_phy_roothub_suspend
> function will simply call usb_phy_roothub_power_off. However, if wake up
> is not supported by the controller it will also call
> usb_phy_roothub_exit.
> The also new usb_phy_roothub_resume function takes care of calling
> usb_phy_roothub_init (if the controller can't wake up the system) in
> addition to usb_phy_roothub_power_on.
> 
> Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the HCD")
> Fixes: 178a0bce05cbc1 ("usb: core: hcd: integrate the PHY wrapper into the 
> HCD core")
> Reported-by: Roger Quadros 
> Suggested-by: Roger Quadros 
> Suggested-by: Chunfeng Yun 
> Signed-off-by: Martin Blumenstingl 
> ---
>  drivers/usb/core/hcd.c |  8 +---
>  drivers/usb/core/phy.c | 35 +++
>  drivers/usb/core/phy.h |  5 +
>  3 files changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 15b0418e3b6a..78bae4ecd68b 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2262,7 +2262,8 @@ int hcd_bus_suspend(struct usb_device *rhdev, 
> pm_message_t msg)
>   hcd->state = HC_STATE_SUSPENDED;
>  
>   if (!PMSG_IS_AUTO(msg))
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev,
> + hcd->phy_roothub);
>  
>   /* Did we race with a root-hub wakeup event? */
>   if (rhdev->do_remote_wakeup) {
> @@ -2302,7 +2303,8 @@ int hcd_bus_resume(struct usb_device *rhdev, 
> pm_message_t msg)
>   }
>  
>   if (!PMSG_IS_AUTO(msg)) {
> - status = usb_phy_roothub_power_on(hcd->phy_roothub);
> + status = usb_phy_roothub_resume(hcd->self.sysdev,
> + hcd->phy_roothub);
>   if (status)
>   return status;
>   }
> @@ -2344,7 +2346,7 @@ int hcd_bus_resume(struct usb_device *rhdev, 
> pm_message_t msg)
>   }
>   } else {
>   hcd->state = old_state;
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
>   dev_dbg(>dev, "bus %s fail, err %d\n",
>   "resume", status);
>   if (status != -ESHUTDOWN)
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index 44f008cda7a8..a39d9bb26a4f 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -157,3 +157,38 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub 
> *phy_roothub)
>   phy_power_off(roothub_entry->phy);
>  }
>  EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
> +
> +int usb_phy_roothub_suspend(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub)
> +{
> + usb_phy_roothub_power_off(phy_roothub);
> +
> + /* keep the PHYs initialized so the device can wake up the system */
> + if (device_may_wakeup(controller_dev))
> + return 0;
> +
> + return usb_phy_roothub_exit(phy_roothub);
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
> +
> +int usb_phy_roothub_resume(struct device *controller_dev,
> +struct usb_phy_roothub *phy_roothub)
> +{
> + int err;
> +
> + /* if the device can't wake up the system _exit was called */
> + if (!device_may_wakeup(controller_dev)) {
> + err = usb_phy_roothub_init(phy_roothub);
> + if (err)
> + return err;
> + }
> +
> + err = usb_phy_roothub_power_on(phy_roothub);
> +
> + /* undo _init if _power_on failed */
> + if (err && !device_may_wakeup(controller_dev))
> + usb_phy_roothub_exit(phy_roothub);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_resume);
> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
> index eb31253201ad..60901d44 100644
> --- a/drivers/usb/core/phy.h
> +++ b/drivers/usb/core/phy.h
> @@ -7,3 +7,8 @@ 

Re: [RFC usb-next v3 1/2] usb: core: split usb_phy_roothub_{init,alloc}

[Chunfeng Yun]

hi,
On Mon, 2018-03-26 at 22:38 +0200, Martin Blumenstingl wrote:
> Before this patch usb_phy_roothub_init served two purposes (from a
> caller's point of view - like hcd.c):
> - parsing the PHYs and allocating the list entries
> - calling phy_init on each list entry
> 
> While this worked so far it has one disadvantage: if we need to call
> phy_init for each PHY instance then the existing code cannot be re-used.
> Solve this by splitting off usb_phy_roothub_alloc which only parses the
> PHYs and allocates the list entries.
> usb_phy_roothub_init then gets a struct usb_phy_roothub and only calls
> phy_init on each PHY instance (along with the corresponding cleanup if
> that failed somewhere).
> 
> This is a preparation step for adding proper suspend support for some
> hardware that requires phy_exit to be called during suspend and phy_init
> to be called during resume.
> 
> Signed-off-by: Martin Blumenstingl 
> ---
>  drivers/usb/core/hcd.c | 10 +++---
>  drivers/usb/core/phy.c | 53 
> +-
>  drivers/usb/core/phy.h |  4 +++-
>  3 files changed, 37 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae6367..15b0418e3b6a 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2758,12 +2758,16 @@ int usb_add_hcd(struct usb_hcd *hcd,
>   }
>  
>   if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
> - hcd->phy_roothub = usb_phy_roothub_init(hcd->self.sysdev);
> + hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
>   if (IS_ERR(hcd->phy_roothub)) {
>   retval = PTR_ERR(hcd->phy_roothub);
> - goto err_phy_roothub_init;
> + goto err_phy_roothub_alloc;
>   }
>  
> + retval = usb_phy_roothub_init(hcd->phy_roothub);
> + if (retval)
> + goto err_phy_roothub_alloc;
> +
>   retval = usb_phy_roothub_power_on(hcd->phy_roothub);
>   if (retval)
>   goto err_usb_phy_roothub_power_on;
> @@ -2936,7 +2940,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>   usb_phy_roothub_power_off(hcd->phy_roothub);
>  err_usb_phy_roothub_power_on:
>   usb_phy_roothub_exit(hcd->phy_roothub);
> -err_phy_roothub_init:
> +err_phy_roothub_alloc:
>   if (hcd->remove_phy && hcd->usb_phy) {
>   usb_phy_shutdown(hcd->usb_phy);
>   usb_put_phy(hcd->usb_phy);
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index f19aaa3c899c..44f008cda7a8 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -19,19 +19,6 @@ struct usb_phy_roothub {
>   struct list_headlist;
>  };
>  
> -static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
> -{
> - struct usb_phy_roothub *roothub_entry;
> -
> - roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
> - if (!roothub_entry)
> - return ERR_PTR(-ENOMEM);
> -
> - INIT_LIST_HEAD(_entry->list);
> -
> - return roothub_entry;
> -}
> -
>  static int usb_phy_roothub_add_phy(struct device *dev, int index,
>  struct list_head *list)
>  {
> @@ -45,9 +32,11 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
> index,
>   return PTR_ERR(phy);
>   }
>  
> - roothub_entry = usb_phy_roothub_alloc(dev);
> - if (IS_ERR(roothub_entry))
> - return PTR_ERR(roothub_entry);
> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
> + if (!roothub_entry)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(_entry->list);
>  
>   roothub_entry->phy = phy;
>  
> @@ -56,11 +45,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
> index,
>   return 0;
>  }
>  
> -struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev)
> +struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
>  {
>   struct usb_phy_roothub *phy_roothub;
> - struct usb_phy_roothub *roothub_entry;
> - struct list_head *head;
>   int i, num_phys, err;
>  
>   num_phys = of_count_phandle_with_args(dev->of_node, "phys",
> @@ -68,16 +55,31 @@ struct usb_phy_roothub *usb_phy_roothub_init(struct 
> device *dev)
>   if (num_phys <= 0)
>   return NULL;
>  
> - phy_roothub = usb_phy_roothub_alloc(dev);
> - if (IS_ERR(phy_roothub))
> - return phy_roothub;
> + phy_roothub = devm_kzalloc(dev, sizeof(*phy_roothub), GFP_KERNEL);
> + if (!phy_roothub)
> + return ERR_PTR(-ENOMEM);
> +
> + INIT_LIST_HEAD(_roothub->list);
>  
>   for (i = 0; i < num_phys; i++) {
>   err = usb_phy_roothub_add_phy(dev, i, _roothub->list);
>   if (err)
> - goto err_out;
> + return 

Re: [PATCH usb-next v1] usb: core: phy: fix return value of usb_phy_roothub_exit()

[Chunfeng Yun]

Hi,
On Mon, 2018-03-26 at 22:12 +0200, Martin Blumenstingl wrote:
> Hi Chunfeng,
> 
> On Mon, Mar 26, 2018 at 5:43 AM, Chunfeng Yun  
> wrote:
> > On Sat, 2018-03-24 at 14:56 +0100, Martin Blumenstingl wrote:
> >> usb_phy_roothub_exit() should return the error code from the phy_exit()
> >> call if exiting the PHY failed.
> >> However, since a wrong variable is used usb_phy_roothub_exit() currently
> >> always returns 0, even if one of the phy_exit calls returned an error.
> >> Fix this by assigning the error code from phy_exit() to the "ret"
> >> variable to propagate the error correctly.
> >>
> >> Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the 
> >> HCD")
> >> Signed-off-by: Martin Blumenstingl 
> >> ---
> >>  drivers/usb/core/phy.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> >> index 09b7c43c0ea4..f19aaa3c899c 100644
> >> --- a/drivers/usb/core/phy.c
> >> +++ b/drivers/usb/core/phy.c
> >> @@ -111,7 +111,7 @@ int usb_phy_roothub_exit(struct usb_phy_roothub 
> >> *phy_roothub)
> >>   list_for_each_entry(roothub_entry, head, list) {
> >>   err = phy_exit(roothub_entry->phy);
> >>   if (err)
> >> - ret = ret;
> >> + ret = err;
> > Need break the loop?
> in the original implementation I decided not to break the loop here so
> phy_exit is called for all PHYs -> only the problematic ones will
> remain initialized
> (in the _power_on implementation we can try to fix the state by adding
> a break and then calling _power_off for all PHYs before the "broken"
> one where _power_on failed)
> 
got it
> also if phy_exit fails then something is probably very wrong
> 
> do you have any specific use-case in mind where the missing break
> could be a problem?
No
> 
> >>   }
> >>
> >>   return ret;
> >
> >
> 
> 
> Regards
> Martin


--
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 usb-next v2 1/2] usb: core: split usb_phy_roothub_{init,alloc}

[Chunfeng Yun]

On Mon, 2018-03-26 at 22:31 +0200, Martin Blumenstingl wrote:
> Hi Chunfeng,
> 
> On Mon, Mar 26, 2018 at 5:37 AM, Chunfeng Yun  
> wrote:
> > On Sat, 2018-03-24 at 15:21 +0100, Martin Blumenstingl wrote:
> >> Before this patch usb_phy_roothub_init served two purposes (from a
> >> caller's point of view - like hcd.c):
> >> - parsing the PHYs and allocating the list entries
> >> - calling phy_init on each list entry
> >>
> >> While this worked so far it has one disadvantage: if we need to call
> >> phy_init for each PHY instance then the existing code cannot be re-used.
> >> Solve this by splitting off usb_phy_roothub_alloc which only parses the
> >> PHYs and allocates the list entries.
> >> usb_phy_roothub_init then gets a struct usb_phy_roothub and only calls
> >> phy_init on each PHY instance (along with the corresponding cleanup if
> >> that failed somewhere).
> >>
> >> This is a preparation step for adding proper suspend support for some
> >> hardware that requires phy_exit to be called during suspend and phy_init
> >> to be called during resume.
> >>
> >> Signed-off-by: Martin Blumenstingl 
> >> ---
> >>  drivers/usb/core/hcd.c | 10 +++---
> >>  drivers/usb/core/phy.c | 51 
> >> +-
> >>  drivers/usb/core/phy.h |  4 +++-
> >>  3 files changed, 35 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> >> index 777036ae6367..15b0418e3b6a 100644
> >> --- a/drivers/usb/core/hcd.c
> >> +++ b/drivers/usb/core/hcd.c
> >> @@ -2758,12 +2758,16 @@ int usb_add_hcd(struct usb_hcd *hcd,
> >>   }
> >>
> >>   if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
> >> - hcd->phy_roothub = usb_phy_roothub_init(hcd->self.sysdev);
> >> + hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
> >>   if (IS_ERR(hcd->phy_roothub)) {
> >>   retval = PTR_ERR(hcd->phy_roothub);
> >> - goto err_phy_roothub_init;
> >> + goto err_phy_roothub_alloc;
> >>   }
> >>
> >> + retval = usb_phy_roothub_init(hcd->phy_roothub);
> >> + if (retval)
> >> + goto err_phy_roothub_alloc;
> >> +
> >>   retval = usb_phy_roothub_power_on(hcd->phy_roothub);
> >>   if (retval)
> >>   goto err_usb_phy_roothub_power_on;
> >> @@ -2936,7 +2940,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
> >>   usb_phy_roothub_power_off(hcd->phy_roothub);
> >>  err_usb_phy_roothub_power_on:
> >>   usb_phy_roothub_exit(hcd->phy_roothub);
> >> -err_phy_roothub_init:
> >> +err_phy_roothub_alloc:
> >>   if (hcd->remove_phy && hcd->usb_phy) {
> >>   usb_phy_shutdown(hcd->usb_phy);
> >>   usb_put_phy(hcd->usb_phy);
> >> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> >> index f19aaa3c899c..d1861c5a74de 100644
> >> --- a/drivers/usb/core/phy.c
> >> +++ b/drivers/usb/core/phy.c
> >> @@ -19,19 +19,6 @@ struct usb_phy_roothub {
> >>   struct list_headlist;
> >>  };
> >>
> >> -static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
> >> -{
> >> - struct usb_phy_roothub *roothub_entry;
> >> -
> >> - roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), 
> >> GFP_KERNEL);
> >> - if (!roothub_entry)
> >> - return ERR_PTR(-ENOMEM);
> >> -
> >> - INIT_LIST_HEAD(_entry->list);
> >> -
> >> - return roothub_entry;
> >> -}
> >> -
> >>  static int usb_phy_roothub_add_phy(struct device *dev, int index,
> >>  struct list_head *list)
> >>  {
> >> @@ -45,9 +32,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, 
> >> int index,
> >>   return PTR_ERR(phy);
> >>   }
> >
> >>
> >> - roothub_entry = usb_phy_roothub_alloc(dev);
> >> - if (IS_ERR(roothub_entry))
> >> - return PTR_ERR(roothub_entry);
> >> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), 
> >> GFP_KERNEL);
> >> + if (!roothub_entry)
> >> + return -ENOMEM;
> >>
> >>   roothub_entry->phy = phy;
> >>
> >> @@ -56,11 +43,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, 
> >> int index,
> >>   return 0;
> >>  }
> >>
> >> -struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev)
> >> +struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
> >>  {
> >>   struct usb_phy_roothub *phy_roothub;
> >> - struct usb_phy_roothub *roothub_entry;
> >> - struct list_head *head;
> >>   int i, num_phys, err;
> >>
> >>   num_phys = of_count_phandle_with_args(dev->of_node, "phys",
> >> @@ -68,16 +53,31 @@ struct usb_phy_roothub *usb_phy_roothub_init(struct 
> >> device *dev)
> >>   if (num_phys <= 0)
> >>   return NULL;
> >>
> >> - phy_roothub = usb_phy_roothub_alloc(dev);
> >> -   

Re: [PATCHv4] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4

[Tony Lindgren]

* Dan Williams  [180326 15:18]:
> On Sun, 2018-03-25 at 13:52 -0700, Tony Lindgren wrote:
> > Anyways, looks like qmi_wwan needs to be loaded before
> > qcserial module, otherwise we get nine ttyUSB instances
> > and ModemManager can't find any modems.
> 
> Use qcaux.c or option, unless the 6600 actually *does* have the same
> layout as Gobi 1K/2K/etc devices.

OK yeah I don't think it's Gobi.

> If you're going to use qcaux or optoin, then you need to use some
> variant of USB_DEVICE_AND_INTERFACE_INFO to lock the serial driver to
> the specific USB interfaces that expose the TTYs and to ignore the QMI
> interfaces and netdevs.

OK thanks I'll take a look and post a patch after some testing.

Regards,

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

Re: [PATCH v2 3/5] dt-bindings: usb: fusb302: remove max-sink-* properties

[Rob Herring]

On Fri, Mar 23, 2018 at 10:58:45PM +0800, Li Jun wrote:
> Remove max-sink-* properties since they are deprecated.
> 
> Signed-off-by: Li Jun 
> ---
>  Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 6 --
>  1 file changed, 6 deletions(-)

Reviewed-by: Rob Herring 

--
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 2/2] dt-bindings: usb: rt1711h device tree binding document

[Rob Herring]

On Tue, Mar 20, 2018 at 05:15:04PM +0800, ShuFan Lee wrote:
> From: ShuFan Lee 
> 
> Add device tree binding document for Richtek RT1711H Type-C chip driver
> 
> Signed-off-by: ShuFan Lee 
> ---
>  .../devicetree/bindings/usb/richtek,rt1711h.txt| 30 
> ++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/richtek,rt1711h.txt
> 
>  changelogs between v2 & v3
>  - add dt-bindings for rt1711h typec driver
> 
> diff --git a/Documentation/devicetree/bindings/usb/richtek,rt1711h.txt 
> b/Documentation/devicetree/bindings/usb/richtek,rt1711h.txt
> new file mode 100644
> index ..7da4dac78ea7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/richtek,rt1711h.txt
> @@ -0,0 +1,30 @@
> +Richtek RT1711H TypeC PD Controller.
> +
> +Required properties:
> + - compatible : Must be "richtek,rt1711h".
> + - reg : Must be 0x4e, it's slave address of RT1711H.
> +
> +Recommended properties :
> + - interrupt-parent : the phandle for the interrupt controller that
> +   provides interrupts for this device.
> + - interrupts :  where a is the interrupt number and b represents an
> +   encoding of the sense and level information for the interrupt.
> +
> +Optional properties :
> + - rt,intr-gpios : IRQ GPIO pin that's connected to RT1711H interrupt.
> +   if interrupt-parent & interrupts are not defined, use this property 
> instead.

Drop this. You should simply always have interrupts property.

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

[RFC usb-next v3 2/2] usb: core: use phy_exit during suspend if wake up is not supported

[Martin Blumenstingl]

If the USB controller can wake up the system (which is the case for
example with the Mediatek USB3 IP) then we must not call phy_exit during
suspend to ensure that the USB controller doesn't have to re-enumerate
the devices during resume.
However, if the USB controller cannot wake up the system (which is the
case for example on various TI platforms using a dwc3 controller) then
we must call phy_exit during suspend. Otherwise the PHY driver keeps the
clocks enabled, which prevents the system from reaching the lowest power
levels in the suspend state.

Solve this by introducing two new functions in the PHY wrapper which are
dedicated to the suspend and resume handling.
If the controller can wake up the system the new usb_phy_roothub_suspend
function will simply call usb_phy_roothub_power_off. However, if wake up
is not supported by the controller it will also call
usb_phy_roothub_exit.
The also new usb_phy_roothub_resume function takes care of calling
usb_phy_roothub_init (if the controller can't wake up the system) in
addition to usb_phy_roothub_power_on.

Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the HCD")
Fixes: 178a0bce05cbc1 ("usb: core: hcd: integrate the PHY wrapper into the HCD 
core")
Reported-by: Roger Quadros 
Suggested-by: Roger Quadros 
Suggested-by: Chunfeng Yun 
Signed-off-by: Martin Blumenstingl 
---
 drivers/usb/core/hcd.c |  8 +---
 drivers/usb/core/phy.c | 35 +++
 drivers/usb/core/phy.h |  5 +
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 15b0418e3b6a..78bae4ecd68b 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2262,7 +2262,8 @@ int hcd_bus_suspend(struct usb_device *rhdev, 
pm_message_t msg)
hcd->state = HC_STATE_SUSPENDED;
 
if (!PMSG_IS_AUTO(msg))
-   usb_phy_roothub_power_off(hcd->phy_roothub);
+   usb_phy_roothub_suspend(hcd->self.sysdev,
+   hcd->phy_roothub);
 
/* Did we race with a root-hub wakeup event? */
if (rhdev->do_remote_wakeup) {
@@ -2302,7 +2303,8 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t 
msg)
}
 
if (!PMSG_IS_AUTO(msg)) {
-   status = usb_phy_roothub_power_on(hcd->phy_roothub);
+   status = usb_phy_roothub_resume(hcd->self.sysdev,
+   hcd->phy_roothub);
if (status)
return status;
}
@@ -2344,7 +2346,7 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t 
msg)
}
} else {
hcd->state = old_state;
-   usb_phy_roothub_power_off(hcd->phy_roothub);
+   usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
dev_dbg(>dev, "bus %s fail, err %d\n",
"resume", status);
if (status != -ESHUTDOWN)
diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
index 44f008cda7a8..a39d9bb26a4f 100644
--- a/drivers/usb/core/phy.c
+++ b/drivers/usb/core/phy.c
@@ -157,3 +157,38 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub 
*phy_roothub)
phy_power_off(roothub_entry->phy);
 }
 EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
+
+int usb_phy_roothub_suspend(struct device *controller_dev,
+   struct usb_phy_roothub *phy_roothub)
+{
+   usb_phy_roothub_power_off(phy_roothub);
+
+   /* keep the PHYs initialized so the device can wake up the system */
+   if (device_may_wakeup(controller_dev))
+   return 0;
+
+   return usb_phy_roothub_exit(phy_roothub);
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
+
+int usb_phy_roothub_resume(struct device *controller_dev,
+  struct usb_phy_roothub *phy_roothub)
+{
+   int err;
+
+   /* if the device can't wake up the system _exit was called */
+   if (!device_may_wakeup(controller_dev)) {
+   err = usb_phy_roothub_init(phy_roothub);
+   if (err)
+   return err;
+   }
+
+   err = usb_phy_roothub_power_on(phy_roothub);
+
+   /* undo _init if _power_on failed */
+   if (err && !device_may_wakeup(controller_dev))
+   usb_phy_roothub_exit(phy_roothub);
+
+   return err;
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_resume);
diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
index eb31253201ad..60901d44 100644
--- a/drivers/usb/core/phy.h
+++ b/drivers/usb/core/phy.h
@@ -7,3 +7,8 @@ int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
 
 int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
 void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);
+
+int 

[RFC usb-next v3 0/2] fix HCD PHY suspend handling

[Martin Blumenstingl]

This is a follow-up to my previous series "initialize (multiple) PHYs
for a HCD": [0].

Roger Quadros reported [1] that it "is breaking low power cases on TI
SoCs when USB is in host mode". He further explains that "Not doing the
phy_exit() here [when entering suspend] leaves the clocks enabled on
our SoC and we're no longer able to reach low power states on system
suspend."
Chunfeng Yun from Mediatek noted [2] that we cannot unconditionally call
phy_exit while entering system suspend, because this would "disconnect
plugged devices on MTK platforms, due to re-initialize u2 phys when
resume"

In the discussion (which followed Roger's bug report: [1]) Roger,
Chunfeng and me came to the conclusion that we can fix suspend on the
TI SoCs without breaking it on the Mediatek SoCs by extending the
suspend and resume code in usb/core/phy.c by checking whether the USB
controller can wake up the system (which is the case for the Mediatek
MTU3 controller, but now for the dwc3 controller used on the TI SoCs):
- if the controller can wake up the system (Mediatek MTU3 use-case) we
  only call usb_phy_roothub_power_off (which calls phy_power_off) when
  entering system suspend
- if the controller however cannot wake up the system (dwc3 on TI SoCs)
  we additionally call usb_phy_roothub_exit (which calls phy_exit) when
  entering system suspend
- (we undo the previous steps during system resume)

The goal of this series is to fix the issue reported by Roger without
breaking suspend/resume on the Mediatek SoCs.
Since I neither have a TI nor a Mediatek device I am sending this as
RFC. I have tested it on an Amlogic Meson GXM board (Khadas VIM2) which
does NOT support suspend/resume yet.

this should be applied on top of [3] "usb: core: phy: fix return value
of usb_phy_roothub_exit()" (even though there's no strict dependency,
this is the order I wrote the patches in).


changes since RFC v2 at [5]:
- add missing INIT_LIST_HEAD call in usb_phy_roothub_add_phy (affects
  patch #1 - spotted by Roger Quadros, thank you!)
- fixed swapped conditions using device_may_wakeup() in
  usb_phy_roothub_resume because we need to call usb_phy_roothub_init
  if the controller cannot wake up the device (affects patch #2, spotted
  by Chunfeng Yun, thank you!)
- simplified the error condition to "undo" usb_phy_roothub_init if
  usb_phy_roothub_power_on failed in usb_phy_roothub_resume (suggested
  by Chunfeng Yun)
- updated the commit message (using Roger's wording) because (quote from
  Roger "it doesn't prevent the system from entering suspend but just
  prevents the system from reaching lowest power levels in the suspend
  state."

Changes since RFC v1 (blob attachments) at [4]:
- use device_may_wakeup instead of device_can_wakeup as suggested by
  Roger Quadros
- use the controller device from hcd->self.controller as suggested by
  Chunfeng Yun
- compile time fixes thanks to Roger Quadros
- if usb_phy_roothub_power_on in usb_phy_roothub_resume failes then
  we now call usb_phy_roothub_exit to keep the PHYs in the correct
  state if usb_phy_roothub_resume partially failed


[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006599.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006737.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006758.html
[3] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006819.html
[4] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006794.html
[5] http://lists.infradead.org/pipermail/linux-amlogic/2018-March/006820.html

Martin Blumenstingl (2):
  usb: core: split usb_phy_roothub_{init,alloc}
  usb: core: use phy_exit during suspend if wake up is not supported

 drivers/usb/core/hcd.c | 18 +++
 drivers/usb/core/phy.c | 88 +++---
 drivers/usb/core/phy.h |  9 +-
 3 files changed, 82 insertions(+), 33 deletions(-)

-- 
2.16.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

[RFC usb-next v3 1/2] usb: core: split usb_phy_roothub_{init,alloc}

[Martin Blumenstingl]

Before this patch usb_phy_roothub_init served two purposes (from a
caller's point of view - like hcd.c):
- parsing the PHYs and allocating the list entries
- calling phy_init on each list entry

While this worked so far it has one disadvantage: if we need to call
phy_init for each PHY instance then the existing code cannot be re-used.
Solve this by splitting off usb_phy_roothub_alloc which only parses the
PHYs and allocates the list entries.
usb_phy_roothub_init then gets a struct usb_phy_roothub and only calls
phy_init on each PHY instance (along with the corresponding cleanup if
that failed somewhere).

This is a preparation step for adding proper suspend support for some
hardware that requires phy_exit to be called during suspend and phy_init
to be called during resume.

Signed-off-by: Martin Blumenstingl 
---
 drivers/usb/core/hcd.c | 10 +++---
 drivers/usb/core/phy.c | 53 +-
 drivers/usb/core/phy.h |  4 +++-
 3 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 777036ae6367..15b0418e3b6a 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2758,12 +2758,16 @@ int usb_add_hcd(struct usb_hcd *hcd,
}
 
if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
-   hcd->phy_roothub = usb_phy_roothub_init(hcd->self.sysdev);
+   hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
if (IS_ERR(hcd->phy_roothub)) {
retval = PTR_ERR(hcd->phy_roothub);
-   goto err_phy_roothub_init;
+   goto err_phy_roothub_alloc;
}
 
+   retval = usb_phy_roothub_init(hcd->phy_roothub);
+   if (retval)
+   goto err_phy_roothub_alloc;
+
retval = usb_phy_roothub_power_on(hcd->phy_roothub);
if (retval)
goto err_usb_phy_roothub_power_on;
@@ -2936,7 +2940,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
usb_phy_roothub_power_off(hcd->phy_roothub);
 err_usb_phy_roothub_power_on:
usb_phy_roothub_exit(hcd->phy_roothub);
-err_phy_roothub_init:
+err_phy_roothub_alloc:
if (hcd->remove_phy && hcd->usb_phy) {
usb_phy_shutdown(hcd->usb_phy);
usb_put_phy(hcd->usb_phy);
diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
index f19aaa3c899c..44f008cda7a8 100644
--- a/drivers/usb/core/phy.c
+++ b/drivers/usb/core/phy.c
@@ -19,19 +19,6 @@ struct usb_phy_roothub {
struct list_headlist;
 };
 
-static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
-{
-   struct usb_phy_roothub *roothub_entry;
-
-   roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
-   if (!roothub_entry)
-   return ERR_PTR(-ENOMEM);
-
-   INIT_LIST_HEAD(_entry->list);
-
-   return roothub_entry;
-}
-
 static int usb_phy_roothub_add_phy(struct device *dev, int index,
   struct list_head *list)
 {
@@ -45,9 +32,11 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
index,
return PTR_ERR(phy);
}
 
-   roothub_entry = usb_phy_roothub_alloc(dev);
-   if (IS_ERR(roothub_entry))
-   return PTR_ERR(roothub_entry);
+   roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
+   if (!roothub_entry)
+   return -ENOMEM;
+
+   INIT_LIST_HEAD(_entry->list);
 
roothub_entry->phy = phy;
 
@@ -56,11 +45,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
index,
return 0;
 }
 
-struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev)
+struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
 {
struct usb_phy_roothub *phy_roothub;
-   struct usb_phy_roothub *roothub_entry;
-   struct list_head *head;
int i, num_phys, err;
 
num_phys = of_count_phandle_with_args(dev->of_node, "phys",
@@ -68,16 +55,31 @@ struct usb_phy_roothub *usb_phy_roothub_init(struct device 
*dev)
if (num_phys <= 0)
return NULL;
 
-   phy_roothub = usb_phy_roothub_alloc(dev);
-   if (IS_ERR(phy_roothub))
-   return phy_roothub;
+   phy_roothub = devm_kzalloc(dev, sizeof(*phy_roothub), GFP_KERNEL);
+   if (!phy_roothub)
+   return ERR_PTR(-ENOMEM);
+
+   INIT_LIST_HEAD(_roothub->list);
 
for (i = 0; i < num_phys; i++) {
err = usb_phy_roothub_add_phy(dev, i, _roothub->list);
if (err)
-   goto err_out;
+   return ERR_PTR(err);
}
 
+   return phy_roothub;
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_alloc);
+
+int usb_phy_roothub_init(struct usb_phy_roothub *phy_roothub)
+{
+   struct usb_phy_roothub *roothub_entry;
+ 

Re: [RFC usb-next v2 1/2] usb: core: split usb_phy_roothub_{init,alloc}

[Martin Blumenstingl]

Hi Chunfeng,

On Mon, Mar 26, 2018 at 5:37 AM, Chunfeng Yun  wrote:
> On Sat, 2018-03-24 at 15:21 +0100, Martin Blumenstingl wrote:
>> Before this patch usb_phy_roothub_init served two purposes (from a
>> caller's point of view - like hcd.c):
>> - parsing the PHYs and allocating the list entries
>> - calling phy_init on each list entry
>>
>> While this worked so far it has one disadvantage: if we need to call
>> phy_init for each PHY instance then the existing code cannot be re-used.
>> Solve this by splitting off usb_phy_roothub_alloc which only parses the
>> PHYs and allocates the list entries.
>> usb_phy_roothub_init then gets a struct usb_phy_roothub and only calls
>> phy_init on each PHY instance (along with the corresponding cleanup if
>> that failed somewhere).
>>
>> This is a preparation step for adding proper suspend support for some
>> hardware that requires phy_exit to be called during suspend and phy_init
>> to be called during resume.
>>
>> Signed-off-by: Martin Blumenstingl 
>> ---
>>  drivers/usb/core/hcd.c | 10 +++---
>>  drivers/usb/core/phy.c | 51 
>> +-
>>  drivers/usb/core/phy.h |  4 +++-
>>  3 files changed, 35 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 777036ae6367..15b0418e3b6a 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -2758,12 +2758,16 @@ int usb_add_hcd(struct usb_hcd *hcd,
>>   }
>>
>>   if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
>> - hcd->phy_roothub = usb_phy_roothub_init(hcd->self.sysdev);
>> + hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
>>   if (IS_ERR(hcd->phy_roothub)) {
>>   retval = PTR_ERR(hcd->phy_roothub);
>> - goto err_phy_roothub_init;
>> + goto err_phy_roothub_alloc;
>>   }
>>
>> + retval = usb_phy_roothub_init(hcd->phy_roothub);
>> + if (retval)
>> + goto err_phy_roothub_alloc;
>> +
>>   retval = usb_phy_roothub_power_on(hcd->phy_roothub);
>>   if (retval)
>>   goto err_usb_phy_roothub_power_on;
>> @@ -2936,7 +2940,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>>   usb_phy_roothub_power_off(hcd->phy_roothub);
>>  err_usb_phy_roothub_power_on:
>>   usb_phy_roothub_exit(hcd->phy_roothub);
>> -err_phy_roothub_init:
>> +err_phy_roothub_alloc:
>>   if (hcd->remove_phy && hcd->usb_phy) {
>>   usb_phy_shutdown(hcd->usb_phy);
>>   usb_put_phy(hcd->usb_phy);
>> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
>> index f19aaa3c899c..d1861c5a74de 100644
>> --- a/drivers/usb/core/phy.c
>> +++ b/drivers/usb/core/phy.c
>> @@ -19,19 +19,6 @@ struct usb_phy_roothub {
>>   struct list_headlist;
>>  };
>>
>> -static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
>> -{
>> - struct usb_phy_roothub *roothub_entry;
>> -
>> - roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
>> - if (!roothub_entry)
>> - return ERR_PTR(-ENOMEM);
>> -
>> - INIT_LIST_HEAD(_entry->list);
>> -
>> - return roothub_entry;
>> -}
>> -
>>  static int usb_phy_roothub_add_phy(struct device *dev, int index,
>>  struct list_head *list)
>>  {
>> @@ -45,9 +32,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
>> index,
>>   return PTR_ERR(phy);
>>   }
>
>>
>> - roothub_entry = usb_phy_roothub_alloc(dev);
>> - if (IS_ERR(roothub_entry))
>> - return PTR_ERR(roothub_entry);
>> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
>> + if (!roothub_entry)
>> + return -ENOMEM;
>>
>>   roothub_entry->phy = phy;
>>
>> @@ -56,11 +43,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, 
>> int index,
>>   return 0;
>>  }
>>
>> -struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev)
>> +struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
>>  {
>>   struct usb_phy_roothub *phy_roothub;
>> - struct usb_phy_roothub *roothub_entry;
>> - struct list_head *head;
>>   int i, num_phys, err;
>>
>>   num_phys = of_count_phandle_with_args(dev->of_node, "phys",
>> @@ -68,16 +53,31 @@ struct usb_phy_roothub *usb_phy_roothub_init(struct 
>> device *dev)
>>   if (num_phys <= 0)
>>   return NULL;
>>
>> - phy_roothub = usb_phy_roothub_alloc(dev);
>> - if (IS_ERR(phy_roothub))
>> - return phy_roothub;
>> + phy_roothub = devm_kzalloc(dev, sizeof(*phy_roothub), GFP_KERNEL);
>> + if (!phy_roothub)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + INIT_LIST_HEAD(_roothub->list);
>>
>>   for (i = 0; i < num_phys; i++) {
>>   

Re: [PATCH usb-next v1] usb: core: phy: fix return value of usb_phy_roothub_exit()

[Martin Blumenstingl]

Hi Chunfeng,

On Mon, Mar 26, 2018 at 5:43 AM, Chunfeng Yun  wrote:
> On Sat, 2018-03-24 at 14:56 +0100, Martin Blumenstingl wrote:
>> usb_phy_roothub_exit() should return the error code from the phy_exit()
>> call if exiting the PHY failed.
>> However, since a wrong variable is used usb_phy_roothub_exit() currently
>> always returns 0, even if one of the phy_exit calls returned an error.
>> Fix this by assigning the error code from phy_exit() to the "ret"
>> variable to propagate the error correctly.
>>
>> Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the 
>> HCD")
>> Signed-off-by: Martin Blumenstingl 
>> ---
>>  drivers/usb/core/phy.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
>> index 09b7c43c0ea4..f19aaa3c899c 100644
>> --- a/drivers/usb/core/phy.c
>> +++ b/drivers/usb/core/phy.c
>> @@ -111,7 +111,7 @@ int usb_phy_roothub_exit(struct usb_phy_roothub 
>> *phy_roothub)
>>   list_for_each_entry(roothub_entry, head, list) {
>>   err = phy_exit(roothub_entry->phy);
>>   if (err)
>> - ret = ret;
>> + ret = err;
> Need break the loop?
in the original implementation I decided not to break the loop here so
phy_exit is called for all PHYs -> only the problematic ones will
remain initialized
(in the _power_on implementation we can try to fix the state by adding
a break and then calling _power_off for all PHYs before the "broken"
one where _power_on failed)

also if phy_exit fails then something is probably very wrong

do you have any specific use-case in mind where the missing break
could be a problem?

>>   }
>>
>>   return ret;
>
>


Regards
Martin
--
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] usb: gadget: udc: core: update usb_ep_queue() documentation

[Yavuz, Tuba]

Oops; I was looking at kernel version v4.14-rc2.

Tuba Yavuz, Ph.D.
Assistant Professor
Electrical and Computer Engineering Department
University of Florida
Gainesville, FL 32611
Webpage: http://www.tuba.ece.ufl.edu/
Email: t...@ece.ufl.edu
Phone: (352) 846 0202


From: Alan Stern 
Sent: Monday, March 26, 2018 2:38 PM
To: Yavuz, Tuba
Cc: Felipe Balbi; Greg Kroah-Hartman; USB list
Subject: Re: [PATCH 1/2] usb: gadget: udc: core: update usb_ep_queue() 
documentation

On Mon, 26 Mar 2018, Yavuz, Tuba wrote:

> I agree with Alan that the spinlock must be dropped before calling
> usb_ep_queue. An example can be found in the ep0_queue function of
> the f_mass_storage driver.

Thanks for the note of support, but you must be looking at a different
version of the f_mass_storage code than I am -- in my copy, ep0_queue
does not acquire or release any locks.

I forgot to mention in my earlier email...  Felipe said that the
problem case involved calling the completion handler for a failed
submission.  That definitely is not the right approach; if a submission
fails then usb_ep_queue() should return an error code and the
completion routine should not be called.

In fact, there is a general principal here which should be documented
but doesn't seem to be (as far as I can tell).  Namely, a request's
completion routine will be called if and only if usb_ep_queue() returns
0.  That would make a good addition to the kerneldoc for
usb_ep_queue().

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 1/2] usb: gadget: udc: core: update usb_ep_queue() documentation

[Alan Stern]

On Mon, 26 Mar 2018, Yavuz, Tuba wrote:

> I agree with Alan that the spinlock must be dropped before calling
> usb_ep_queue. An example can be found in the ep0_queue function of
> the f_mass_storage driver.

Thanks for the note of support, but you must be looking at a different 
version of the f_mass_storage code than I am -- in my copy, ep0_queue 
does not acquire or release any locks.

I forgot to mention in my earlier email...  Felipe said that the 
problem case involved calling the completion handler for a failed 
submission.  That definitely is not the right approach; if a submission 
fails then usb_ep_queue() should return an error code and the 
completion routine should not be called.

In fact, there is a general principal here which should be documented
but doesn't seem to be (as far as I can tell).  Namely, a request's
completion routine will be called if and only if usb_ep_queue() returns
0.  That would make a good addition to the kerneldoc for 
usb_ep_queue().

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: host: Remove the deprecated ATH79 USB host config options

[Alan Stern]

On Sat, 24 Mar 2018, Alban Bedel wrote:

> The options USB_EHCI_ATH79 and USB_OHCI_ATH79 only enable the
> generic EHCI and OHCI platform drivers, and have been marked as
> deprecated since 2012.
> 
> These can be safely removed if we make sure that USB_EHCI_ROOT_HUB_TT
> still get enabled for the EHCI driver. This is now done be selecting
> this option when the EHCI platform driver is enabled on the ATH79
> platform.
> 
> Signed-off-by: Alban Bedel 
> ---
>  arch/mips/Kconfig|  1 +
>  drivers/usb/host/Kconfig | 25 -
>  2 files changed, 1 insertion(+), 25 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 8128c3b..61e9a24 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -200,6 +200,7 @@ config ATH79
>   select SYS_SUPPORTS_MIPS16
>   select SYS_SUPPORTS_ZBOOT_UART_PROM
>   select USE_OF
> + select USB_EHCI_ROOT_HUB_TT if USB_EHCI_HCD_PLATFORM
>   help
> Support for the Atheros AR71XX/AR724X/AR913X SoCs.
>  
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 4fcfb30..55b45dc 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -293,19 +293,6 @@ config USB_CNS3XXX_EHCI
> It is needed for high-speed (480Mbit/sec) USB 2.0 device
> support.
>  
> -config USB_EHCI_ATH79
> - bool "EHCI support for AR7XXX/AR9XXX SoCs (DEPRECATED)"
> - depends on (SOC_AR71XX || SOC_AR724X || SOC_AR913X || SOC_AR933X)
> - select USB_EHCI_ROOT_HUB_TT
> - select USB_EHCI_HCD_PLATFORM
> - default y
> - ---help---
> -   This option is deprecated now and the driver was removed, use
> -   USB_EHCI_HCD_PLATFORM instead.
> -
> -   Enables support for the built-in EHCI controller present
> -   on the Atheros AR7XXX/AR9XXX SoCs.
> -
>  config USB_EHCI_HCD_PLATFORM
>   tristate "Generic EHCI driver for a platform device"
>   default n
> @@ -489,18 +476,6 @@ config USB_OHCI_HCD_DAVINCI
> controller. This driver cannot currently be a loadable
> module because it lacks a proper PHY abstraction.
>  
> -config USB_OHCI_ATH79
> - bool "USB OHCI support for the Atheros AR71XX/AR7240 SoCs (DEPRECATED)"
> - depends on (SOC_AR71XX || SOC_AR724X)
> - select USB_OHCI_HCD_PLATFORM
> - default y
> - help
> -   This option is deprecated now and the driver was removed, use
> -   USB_OHCI_HCD_PLATFORM instead.
> -
> -   Enables support for the built-in OHCI controller present on the
> -   Atheros AR71XX/AR7240 SoCs.
> -
>  config USB_OHCI_HCD_PPC_OF_BE
>   bool "OHCI support for OF platform bus (big endian)"
>   depends on PPC

For the EHCI and OHCI portions:

Acked-by: Alan Stern 

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

Re: [PATCH v3] USB: announce bcdDevice as well as idVendor, idProduct.

[Alan Stern]

On Sat, 24 Mar 2018, Benson Leung wrote:

> Print bcdDevice which is used by vendors to identify different versions
> of the same product (or different versions of firmware).
> 
> Adding this to the logs will be useful for support purposes.
> 
> Match the %2x.%02x formatting that's used by lsusb -v for this same value.
> 
> Signed-off-by: Benson Leung 
> ---
> 
> v3: Remove unnecessary whitespace changes.
> v2: Format for decimal output.
> 
>  drivers/usb/core/hub.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index aaeef03c0d83..779725836cf5 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2192,9 +2192,13 @@ static void show_string(struct usb_device *udev, char 
> *id, char *string)
>  
>  static void announce_device(struct usb_device *udev)
>  {
> - dev_info(>dev, "New USB device found, idVendor=%04x, 
> idProduct=%04x\n",
> + u16 bcdDevice = le16_to_cpu(udev->descriptor.bcdDevice);
> +
> + dev_info(>dev,
> + "New USB device found, idVendor=%04x, idProduct=%04x, 
> bcdDevice=%2x.%02x\n",
>   le16_to_cpu(udev->descriptor.idVendor),
> - le16_to_cpu(udev->descriptor.idProduct));
> + le16_to_cpu(udev->descriptor.idProduct),
> + bcdDevice >> 8, bcdDevice & 0xff);
>   dev_info(>dev,
>   "New USB device strings: Mfr=%d, Product=%d, SerialNumber=%d\n",
>   udev->descriptor.iManufacturer,

Acked-by: Alan Stern 

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

Re: [PATCH 1/2] usb: gadget: udc: core: update usb_ep_queue() documentation

[Yavuz, Tuba]

I agree with Alan that the spinlock must be dropped before calling 
usb_ep_queue. An example can be found in the ep0_queue function of the 
f_mass_storage driver.

Best,

Tuba Yavuz, Ph.D.
Assistant Professor
Electrical and Computer Engineering Department
University of Florida
Gainesville, FL 32611
Webpage: http://www.tuba.ece.ufl.edu/
Email: t...@ece.ufl.edu
Phone: (352) 846 0202


From: Alan Stern 
Sent: Monday, March 26, 2018 1:32 PM
To: Felipe Balbi
Cc: Greg Kroah-Hartman; Linux USB; Yavuz, Tuba
Subject: Re: [PATCH 1/2] usb: gadget: udc: core: update usb_ep_queue() 
documentation

On Mon, 26 Mar 2018, Felipe Balbi wrote:

> Mention that ->complete() should never be called from within
> usb_ep_queue().
>
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/gadget/udc/core.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 50988b21a21b..842814bc0e4f 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -238,6 +238,9 @@ EXPORT_SYMBOL_GPL(usb_ep_free_request);
>   * arranges to poll once per interval, and the gadget driver usually will
>   * have queued some data to transfer at that time.
>   *
> + * Note that @req's ->complete() callback must never be called from
> + * within usb_ep_queue() as that can create deadlock situations.
> + *

I think this is highly questionable.  Certainly it was not David
Brownell's original intention; his dummy-hcd driver will sometimes
give back a request from within usb_ep_queue() -- and I believe he
wrote it that way in order to emulate a feature of his net2280 driver.

In this particular case, the problem is that a driver acquires a
spinlock in its complete() routine, but then it holds that same
spinlock while submitting a request.  This is a bug; it should be fixed
in the driver.  The spinlock should be dropped while the request is
submitted.  I'm sure there are examples whether other drivers do this.

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 1/2] usb: gadget: udc: core: update usb_ep_queue() documentation

[Alan Stern]

On Mon, 26 Mar 2018, Felipe Balbi wrote:

> Mention that ->complete() should never be called from within
> usb_ep_queue().
> 
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/gadget/udc/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 50988b21a21b..842814bc0e4f 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -238,6 +238,9 @@ EXPORT_SYMBOL_GPL(usb_ep_free_request);
>   * arranges to poll once per interval, and the gadget driver usually will
>   * have queued some data to transfer at that time.
>   *
> + * Note that @req's ->complete() callback must never be called from
> + * within usb_ep_queue() as that can create deadlock situations.
> + *

I think this is highly questionable.  Certainly it was not David 
Brownell's original intention; his dummy-hcd driver will sometimes 
give back a request from within usb_ep_queue() -- and I believe he 
wrote it that way in order to emulate a feature of his net2280 driver.

In this particular case, the problem is that a driver acquires a 
spinlock in its complete() routine, but then it holds that same 
spinlock while submitting a request.  This is a bug; it should be fixed 
in the driver.  The spinlock should be dropped while the request is 
submitted.  I'm sure there are examples whether other drivers do this.

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: [PATCHv2] net/usb/qmi_wwan.c: Add USB id for lt4120 modem

[David Miller]

From: Torsten Hilbrich 
Date: Mon, 26 Mar 2018 07:19:57 +0200

> This is needed to support the modem found in HP EliteBook 820 G3.
> 
> Signed-off-by: Torsten Hilbrich 

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

Re: [PATCH] net: qmi_wwan: add BroadMobi BM806U 2020:2033

[David Miller]

From: Pawel Dembicki 
Date: Sat, 24 Mar 2018 22:08:14 +0100

> BroadMobi BM806U is an Qualcomm MDM9225 based 3G/4G modem.
> Tested hardware BM806U is mounted on D-Link DWR-921-C3 router.
> The USB id is added to qmi_wwan.c to allow QMI communication with
> the BM806U.
> 
> Tested on 4.14 kernel and OpenWRT.
> 
> Signed-off-by: Pawel Dembicki 

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

Re: [PATCH 4/4] drivers/net: Use octal not symbolic permissions

[David Miller]

From: Joe Perches 
Date: Fri, 23 Mar 2018 15:54:39 -0700

> Prefer the direct use of octal for permissions.
> 
> Done with checkpatch -f --types=SYMBOLIC_PERMS --fix-inplace
> and some typing.
> 
> Miscellanea:
> 
> o Whitespace neatening around these conversions.
> 
> Signed-off-by: Joe Perches 

Applied.
--
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: [PATCHv4] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4

[Dan Williams]

On Sun, 2018-03-25 at 13:52 -0700, Tony Lindgren wrote:
> * Pavel Machek  [180325 19:00]:
> > Hi!
> > 
> > > > Hmm. Interesting. Anyway, for me ttyUSB4 is interesting, as it
> > > > seems
> > > > to react to AT commands, and in particular reacts to ADT123; (;
> > > > is
> > > > important).
> > > 
> > > Is that to dial a voice call?
> > 
> > Yes. And it is ATD123; not ATD.
> 
> Strange, no semicolon is needed when using /dev/gsmtty to
> dial a voice call with my current pile of pending changes,
> just doing ATD123 dials..
> 
> Anyways, looks like qmi_wwan needs to be loaded before
> qcserial module, otherwise we get nine ttyUSB instances
> and ModemManager can't find any modems.

Use qcaux.c or option, unless the 6600 actually *does* have the same
layout as Gobi 1K/2K/etc devices.

If you're going to use qcaux or optoin, then you need to use some
variant of USB_DEVICE_AND_INTERFACE_INFO to lock the serial driver to
the specific USB interfaces that expose the TTYs and to ignore the QMI
interfaces and netdevs.

Dan

> With qcserial module loaded after qmi_wwan, it still takes
> a long time for ModemManager to find the modem.
> 
> Then unrelated to the qcserial module, also looks like I can
> no longer use the GPS with ModemManager:
> 
> $ mmcli -m 0 --enable
> $ mmcli -m 0 --location-enable-gps-raw
> 
> And then chmod a+r /dev/cdc-wdm0 and pointing gpsd to use
> /dev/cdc-wdm0 used to work, but now it seems that gpsd
> can no longer read it. Trying to start gpsd manually produces:
> 
> # gpsd -b -n -N /dev/cdc-wdm0
> gpsd:ERROR: SER: /dev/cdc-wdm0 already opened by another process
> gpsd:ERROR: initial GPS device /dev/cdc-wdm0 open failed
> gpsd:ERROR: can't run with neither control socket nor devices open
> 
> And lsof shows /usr/libexec/qmi-proxy having it open.
> 
> Anybody know what I might be doing wrong? Sounds like something
> now needs to be done with qmi-proxy to get access to GPS?
> 
> > > Anyway, "good" solution is to get ofonod running, then use ofone
> > > from
> > > here: https://github.com/pavelmachek/unicsy_demo
> 
> Thanks I'll take a look.
> 
> > > I think it's the cpcap based config to route voice call audio
> > > to SoC, Sebastian knows the details :)
> > > 
> > > The way to figure that one out is to dump the cpcap registers
> > > before and during voice call on android with cpcaprw, then
> > > diff the output for the audio registers. Probably some SoC
> > > registers need to be diffed too with rwmem or similar tool
> > > for the mcbsp instance(s) used.
> > 
> > That sounds like hard way to do it. There's source available, I'm
> > now
> > trying to understand it / fit it into Sebastian's driver.
> > 
> > https://raw.githubusercontent.com/NotKit/android_kernel_motorola_om
> > ap4-common/hybris-11.0/sound/soc/codecs/cpcap.c
> 
> Sure that hopefully helps too :)
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] drivers/net: Use octal not symbolic permissions

[Wei Liu]

On Fri, Mar 23, 2018 at 03:54:39PM -0700, Joe Perches wrote:
> Prefer the direct use of octal for permissions.
> 
> Done with checkpatch -f --types=SYMBOLIC_PERMS --fix-inplace
> and some typing.
> 
> Miscellanea:
> 
> o Whitespace neatening around these conversions.
> 
> Signed-off-by: Joe Perches 
> ---
>  drivers/net/xen-netback/xenbus.c   |  4 +-
>  drivers/net/xen-netfront.c |  6 +--

Reviewed-by: Wei Liu 
--
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 v7 2/6] Documentation: power: Initial effort to document power_supply ABI

[Greg Kroah-Hartman]

On Mon, Mar 26, 2018 at 12:40:06PM +, Adam Thomson wrote:
> On 26 March 2018 10:57, Greg Kroah-Hartman wrote:
> 
> > On Fri, Mar 23, 2018 at 10:12:21AM +, Adam Thomson wrote:
> > > This commit adds generic ABI information regarding power_supply
> > > properties. This is an initial attempt to try and align the usage
> > > of these properties between drivers. As part of this commit,
> > > common Battery and USB related properties have been listed.
> > >
> > > Signed-off-by: Adam Thomson 
> > > ---
> > >  Documentation/ABI/testing/sysfs-class-power | 443
> > 
> > >  MAINTAINERS |   1 +
> > >  2 files changed, 444 insertions(+)
> >
> > How was this not documented before?
> 
> As an FYI, there is some overall documentation under:
> 
>  Documentation/power/power_supply_class.txt
> 
> However, after discussions with Heikki, it was determined something more
> detailed would be useful to help clarify usage.

Oh I agree, it belongs in Documentation/ABI/ like you did here, thank
you so much for that.  I was just amazed it wasn't already there...

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 v7 2/6] Documentation: power: Initial effort to document power_supply ABI

[Adam Thomson]

On 26 March 2018 10:57, Greg Kroah-Hartman wrote:

> On Fri, Mar 23, 2018 at 10:12:21AM +, Adam Thomson wrote:
> > This commit adds generic ABI information regarding power_supply
> > properties. This is an initial attempt to try and align the usage
> > of these properties between drivers. As part of this commit,
> > common Battery and USB related properties have been listed.
> >
> > Signed-off-by: Adam Thomson 
> > ---
> >  Documentation/ABI/testing/sysfs-class-power | 443
> 
> >  MAINTAINERS |   1 +
> >  2 files changed, 444 insertions(+)
>
> How was this not documented before?

As an FYI, there is some overall documentation under:

 Documentation/power/power_supply_class.txt

However, after discussions with Heikki, it was determined something more
detailed would be useful to help clarify usage.

> Anyway, any objection to me taking this through the USB tree, as the
> typec patches depend on the 3/6 patch here.  If so, can I get an ack
> from the power supply maintainers?
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Regression] xhci: some hard drives cannot be seen using a JMicron JMS56x enclosure

[Mathias Nyman]

On 24.03.2018 06:37, Cyril Roelandt wrote:

Hello,

On 03/07/18 17:16, Mathias Nyman wrote:

I can try to write a workaround that sets dequeue pointers for both the stream 
we want, and
the current active stream for each URB canceling.


Is there a branch from which I could pull the workaround and test it?



I noticed some other issues while working on it. Streams set deq required a bit 
rework.
A branch with just streams deq rework can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git stream_set_tr_dq

If we're lucky that is enough, but your logs indicate that there might be 
something else.

If the rework alone doesn't work, then I have a simple fix on top of it.
It assumes streams aren't switched mid TD, i.e. that a non-active stream ring 
is not
left mid TD and controller doen't have cached TRBs for non-active streams.
This is pretty similar to how 4.12 kernel was handling it.
I haven't got confirmation from HW/xhci spec side if we can make this 
assumption.

Anyways, branch for this can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git 
stream_set_tr_dq_test1

If that still doesn't work, then a workaround that sets deq pointes for both 
the stream we want,
and the current active stream could be an option. I haven't written that yet.

Thanks
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 v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stübner]

Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay 
> Signed-off-by: Tomeu Vizoso 

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

Re: [PATCH 2/2] usb: dwc3: gadget: never call ->complete() from ->ep_queue()

[Greg Kroah-Hartman]

On Mon, Mar 26, 2018 at 01:14:47PM +0300, Felipe Balbi wrote:
> This is a requirement which has always existed but, somehow, wasn't
> reflected in the documentation and problems weren't found until now
> when Tuba Yavuz found a possible deadlock happening between dwc3 and
> f_hid. She described the situation as follows:
> 
> spin_lock_irqsave(>write_spinlock, flags); // first acquire
> /* we our function has been disabled by host */
> if (!hidg->req) {
>   free_ep_req(hidg->in_ep, hidg->req);
>   goto try_again;
> }
> 
> [...]
> 
> status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);
> =>
>   [...]
>   => usb_gadget_giveback_request
>   =>
>   f_hidg_req_complete
>   =>
>   spin_lock_irqsave(>write_spinlock, flags); // 
> second acquire
> 
> Note that this happens because dwc3 would call ->complete() on a
> failed usb_ep_queue() due to failed Start Transfer command. This is,
> anyway, a theoretical situation because dwc3 currently uses "No
> Response Update Transfer" command for Bulk and Interrupt endpoints.
> 
> It's still good to make this case impossible to happen even if the "No
> Reponse Update Transfer" command is changed.
> 
> Reported-by: Tuba Yavuz 
> Signed-off-by: Felipe Balbi 
> Cc: stable 
> ---
> 
> Greg, if you want to pick these two patches as they are, please go
> ahead. If you want, I can also add a Cc stable tag, your call.

I've added the stable tag and applied both of these patches now.

thanks,

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

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Amelie DELAUNAY]

Hi,

On 03/26/2018 11:00 AM, Tomeu Vizoso wrote:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus 
> supply")
> Cc: Amelie Delaunay 
> Signed-off-by: Tomeu Vizoso 
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>  Stübner )
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>  robot )
> ---
>   drivers/usb/dwc2/hcd.c | 13 -
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>   
>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>   {
> + int ret;
> +
>   hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> - if (IS_ERR(hsotg->vbus_supply))
> - return 0;
> + if (IS_ERR(hsotg->vbus_supply)) {
> + ret = PTR_ERR(hsotg->vbus_supply);
> + hsotg->vbus_supply = NULL;
> + return ret == -ENODEV ? 0 : ret;
> + }
>   
>   return regulator_enable(hsotg->vbus_supply);
>   }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>   
>   spin_unlock_irqrestore(>lock, flags);
>   
> - dwc2_vbus_supply_init(hsotg);
> -
> - return 0;
> + return dwc2_vbus_supply_init(hsotg);
>   }
>   
>   /*
> 

Reviewed-by: Amelie Delaunay 

Thanks,
Amelie

[PATCH 2/2] usb: dwc3: gadget: never call ->complete() from ->ep_queue()

[Felipe Balbi]

This is a requirement which has always existed but, somehow, wasn't
reflected in the documentation and problems weren't found until now
when Tuba Yavuz found a possible deadlock happening between dwc3 and
f_hid. She described the situation as follows:

spin_lock_irqsave(>write_spinlock, flags); // first acquire
/* we our function has been disabled by host */
if (!hidg->req) {
free_ep_req(hidg->in_ep, hidg->req);
goto try_again;
}

[...]

status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);
=>
[...]
=> usb_gadget_giveback_request
=>
f_hidg_req_complete
=>
spin_lock_irqsave(>write_spinlock, flags); // 
second acquire

Note that this happens because dwc3 would call ->complete() on a
failed usb_ep_queue() due to failed Start Transfer command. This is,
anyway, a theoretical situation because dwc3 currently uses "No
Response Update Transfer" command for Bulk and Interrupt endpoints.

It's still good to make this case impossible to happen even if the "No
Reponse Update Transfer" command is changed.

Reported-by: Tuba Yavuz 
Signed-off-by: Felipe Balbi 
---

Greg, if you want to pick these two patches as they are, please go
ahead. If you want, I can also add a Cc stable tag, your call.

I'll prepare patches for v4.18 which will cleanup a few extra details
about how dwc3 kicks transfers and that will make the case a lot
clearer as to how things should behave.

Tuba, thanks for the reports :-) 

 drivers/usb/dwc3/gadget.c | 43 +--
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 550ee952c0d1..8796a5ee9bb9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -166,18 +166,8 @@ static void dwc3_ep_inc_deq(struct dwc3_ep *dep)
dwc3_ep_inc_trb(>trb_dequeue);
 }
 
-/**
- * dwc3_gadget_giveback - call struct usb_request's ->complete callback
- * @dep: The endpoint to whom the request belongs to
- * @req: The request we're giving back
- * @status: completion code for the request
- *
- * Must be called with controller's lock held and interrupts disabled. This
- * function will unmap @req and call its ->complete() callback to notify upper
- * layers that it has completed.
- */
-void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
-   int status)
+void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep,
+   struct dwc3_request *req, int status)
 {
struct dwc3 *dwc = dep->dwc;
 
@@ -190,18 +180,35 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct 
dwc3_request *req,
 
if (req->trb)
usb_gadget_unmap_request_by_dev(dwc->sysdev,
-   >request, req->direction);
+   >request, req->direction);
 
req->trb = NULL;
-
trace_dwc3_gadget_giveback(req);
 
+   if (dep->number > 1)
+   pm_runtime_put(dwc->dev);
+}
+
+/**
+ * dwc3_gadget_giveback - call struct usb_request's ->complete callback
+ * @dep: The endpoint to whom the request belongs to
+ * @req: The request we're giving back
+ * @status: completion code for the request
+ *
+ * Must be called with controller's lock held and interrupts disabled. This
+ * function will unmap @req and call its ->complete() callback to notify upper
+ * layers that it has completed.
+ */
+void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
+   int status)
+{
+   struct dwc3 *dwc = dep->dwc;
+
+   dwc3_gadget_del_and_unmap_request(dep, req, status);
+
spin_unlock(>lock);
usb_gadget_giveback_request(>endpoint, >request);
spin_lock(>lock);
-
-   if (dep->number > 1)
-   pm_runtime_put(dwc->dev);
 }
 
 /**
@@ -1227,7 +1234,7 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep 
*dep)
if (req->trb)
memset(req->trb, 0, sizeof(struct dwc3_trb));
dep->queued_requests--;
-   dwc3_gadget_giveback(dep, req, ret);
+   dwc3_gadget_del_and_unmap_request(dep, req, ret);
return ret;
}
 
-- 
2.16.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: gadget: udc: core: update usb_ep_queue() documentation

[Felipe Balbi]

Mention that ->complete() should never be called from within
usb_ep_queue().

Signed-off-by: Felipe Balbi 
---
 drivers/usb/gadget/udc/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 50988b21a21b..842814bc0e4f 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -238,6 +238,9 @@ EXPORT_SYMBOL_GPL(usb_ep_free_request);
  * arranges to poll once per interval, and the gadget driver usually will
  * have queued some data to transfer at that time.
  *
+ * Note that @req's ->complete() callback must never be called from
+ * within usb_ep_queue() as that can create deadlock situations.
+ *
  * Returns zero, or a negative error code.  Endpoints that are not enabled
  * report errors; errors will also be
  * reported when the usb peripheral is disconnected.
-- 
2.16.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] usb: xhci-mtk: remove redundant MODULE_ALIAS entries

[sean.wang]

From: Sean Wang 

MODULE_ALIAS exports information to allow the module to be auto-loaded at
boot for the drivers registered using legacy platform registration.

However, currently the driver is always used by DT-only platform,
MODULE_ALIAS is redundant and should be removed properly.

Signed-off-by: Sean Wang 
---
 drivers/usb/host/xhci-mtk.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index b0ab4d5..2807c769 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -766,7 +766,6 @@ static struct platform_driver mtk_xhci_driver = {
.of_match_table = of_match_ptr(mtk_xhci_of_match),
},
 };
-MODULE_ALIAS("platform:xhci-mtk");
 
 static int __init xhci_mtk_init(void)
 {
-- 
2.7.4

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

Re: [PATCH v7 2/6] Documentation: power: Initial effort to document power_supply ABI

[Greg Kroah-Hartman]

On Fri, Mar 23, 2018 at 10:12:21AM +, Adam Thomson wrote:
> This commit adds generic ABI information regarding power_supply
> properties. This is an initial attempt to try and align the usage
> of these properties between drivers. As part of this commit,
> common Battery and USB related properties have been listed.
> 
> Signed-off-by: Adam Thomson 
> ---
>  Documentation/ABI/testing/sysfs-class-power | 443 
> 
>  MAINTAINERS |   1 +
>  2 files changed, 444 insertions(+)

How was this not documented before?

Anyway, any objection to me taking this through the USB tree, as the
typec patches depend on the 3/6 patch here.  If so, can I get an ack
from the power supply maintainers?

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: [RFC usb-next v2 1/2] usb: core: split usb_phy_roothub_{init,alloc}

[Roger Quadros]

On 24/03/18 16:21, Martin Blumenstingl wrote:
> Before this patch usb_phy_roothub_init served two purposes (from a
> caller's point of view - like hcd.c):
> - parsing the PHYs and allocating the list entries
> - calling phy_init on each list entry
> 
> While this worked so far it has one disadvantage: if we need to call
> phy_init for each PHY instance then the existing code cannot be re-used.
> Solve this by splitting off usb_phy_roothub_alloc which only parses the
> PHYs and allocates the list entries.
> usb_phy_roothub_init then gets a struct usb_phy_roothub and only calls
> phy_init on each PHY instance (along with the corresponding cleanup if
> that failed somewhere).
> 
> This is a preparation step for adding proper suspend support for some
> hardware that requires phy_exit to be called during suspend and phy_init
> to be called during resume.
> 
> Signed-off-by: Martin Blumenstingl 
> ---
>  drivers/usb/core/hcd.c | 10 +++---
>  drivers/usb/core/phy.c | 51 
> +-
>  drivers/usb/core/phy.h |  4 +++-
>  3 files changed, 35 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 777036ae6367..15b0418e3b6a 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2758,12 +2758,16 @@ int usb_add_hcd(struct usb_hcd *hcd,
>   }
>  
>   if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
> - hcd->phy_roothub = usb_phy_roothub_init(hcd->self.sysdev);
> + hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
>   if (IS_ERR(hcd->phy_roothub)) {
>   retval = PTR_ERR(hcd->phy_roothub);
> - goto err_phy_roothub_init;
> + goto err_phy_roothub_alloc;
>   }
>  
> + retval = usb_phy_roothub_init(hcd->phy_roothub);
> + if (retval)
> + goto err_phy_roothub_alloc;
> +
>   retval = usb_phy_roothub_power_on(hcd->phy_roothub);
>   if (retval)
>   goto err_usb_phy_roothub_power_on;
> @@ -2936,7 +2940,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
>   usb_phy_roothub_power_off(hcd->phy_roothub);
>  err_usb_phy_roothub_power_on:
>   usb_phy_roothub_exit(hcd->phy_roothub);
> -err_phy_roothub_init:
> +err_phy_roothub_alloc:
>   if (hcd->remove_phy && hcd->usb_phy) {
>   usb_phy_shutdown(hcd->usb_phy);
>   usb_put_phy(hcd->usb_phy);
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index f19aaa3c899c..d1861c5a74de 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -19,19 +19,6 @@ struct usb_phy_roothub {
>   struct list_headlist;
>  };
>  
> -static struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
> -{
> - struct usb_phy_roothub *roothub_entry;
> -
> - roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
> - if (!roothub_entry)
> - return ERR_PTR(-ENOMEM);
> -
> - INIT_LIST_HEAD(_entry->list);
> -
> - return roothub_entry;
> -}
> -
>  static int usb_phy_roothub_add_phy(struct device *dev, int index,
>  struct list_head *list)
>  {
> @@ -45,9 +32,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
> index,
>   return PTR_ERR(phy);
>   }
>  
> - roothub_entry = usb_phy_roothub_alloc(dev);
> - if (IS_ERR(roothub_entry))
> - return PTR_ERR(roothub_entry);
> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
> + if (!roothub_entry)
> + return -ENOMEM;
>  

We missed doing a
INIT_LIST_HEAD(_entry->list);

>   roothub_entry->phy = phy;
>  
> @@ -56,11 +43,9 @@ static int usb_phy_roothub_add_phy(struct device *dev, int 
> index,
>   return 0;
>  }
>  
> -struct usb_phy_roothub *usb_phy_roothub_init(struct device *dev)
> +struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev)
>  {
>   struct usb_phy_roothub *phy_roothub;
> - struct usb_phy_roothub *roothub_entry;
> - struct list_head *head;
>   int i, num_phys, err;
>  
>   num_phys = of_count_phandle_with_args(dev->of_node, "phys",
> @@ -68,16 +53,31 @@ struct usb_phy_roothub *usb_phy_roothub_init(struct 
> device *dev)
>   if (num_phys <= 0)
>   return NULL;
>  
> - phy_roothub = usb_phy_roothub_alloc(dev);
> - if (IS_ERR(phy_roothub))
> - return phy_roothub;
> + phy_roothub = devm_kzalloc(dev, sizeof(*phy_roothub), GFP_KERNEL);
> + if (!phy_roothub)
> + return ERR_PTR(-ENOMEM);
> +
> + INIT_LIST_HEAD(_roothub->list);
>  
>   for (i = 0; i < num_phys; i++) {
>   err = usb_phy_roothub_add_phy(dev, i, _roothub->list);
>   if (err)
> - goto err_out;
> + return 

Re: [RFC usb-next v2 2/2] usb: core: use phy_exit during suspend if wake up is not supported

[Roger Quadros]

Hi,

On 24/03/18 16:21, Martin Blumenstingl wrote:
> If the USB controller can wake up the system (which is the case for
> example with the Mediatek USB3 IP) then we must not call phy_exit during
> suspend to ensure that the USB controller doesn't have to re-enumerate
> the devices during resume.
> However, if the USB controller cannot wake up the system (which is the
> case for example on various TI platforms using a dwc3 controller) then
> we must call phy_exit during suspend. Otherwise the PHY driver keeps the
> clocks enabled, which prevents the system from entering the suspend
> state.

Actually it doesn't prevent the system from entering suspend but just prevents
the system from reaching lowest power levels in the suspend state.

> 
> Solve this by introducing two new functions in the PHY wrapper which are
> dedicated to the suspend and resume handling.
> If the controller can wake up the system the new usb_phy_roothub_suspend
> function will simply call usb_phy_roothub_power_off. However, if wake up
> is not supported by the controller it will also call
> usb_phy_roothub_exit.
> The also new usb_phy_roothub_resume function takes care of calling
> usb_phy_roothub_init (if the controller can't wake up the system) in
> addition to usb_phy_roothub_power_on.
> 
> Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the HCD")
> Fixes: 178a0bce05cbc1 ("usb: core: hcd: integrate the PHY wrapper into the 
> HCD core")
> Reported-by: Roger Quadros 
> Suggested-by: Roger Quadros 
> Suggested-by: Chunfeng Yun 
> Signed-off-by: Martin Blumenstingl 
> ---
>  drivers/usb/core/hcd.c |  8 +---
>  drivers/usb/core/phy.c | 37 +
>  drivers/usb/core/phy.h |  5 +
>  3 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 15b0418e3b6a..78bae4ecd68b 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2262,7 +2262,8 @@ int hcd_bus_suspend(struct usb_device *rhdev, 
> pm_message_t msg)
>   hcd->state = HC_STATE_SUSPENDED;
>  
>   if (!PMSG_IS_AUTO(msg))
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev,
> + hcd->phy_roothub);
>  
>   /* Did we race with a root-hub wakeup event? */
>   if (rhdev->do_remote_wakeup) {
> @@ -2302,7 +2303,8 @@ int hcd_bus_resume(struct usb_device *rhdev, 
> pm_message_t msg)
>   }
>  
>   if (!PMSG_IS_AUTO(msg)) {
> - status = usb_phy_roothub_power_on(hcd->phy_roothub);
> + status = usb_phy_roothub_resume(hcd->self.sysdev,
> + hcd->phy_roothub);
>   if (status)
>   return status;
>   }
> @@ -2344,7 +2346,7 @@ int hcd_bus_resume(struct usb_device *rhdev, 
> pm_message_t msg)
>   }
>   } else {
>   hcd->state = old_state;
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
>   dev_dbg(>dev, "bus %s fail, err %d\n",
>   "resume", status);
>   if (status != -ESHUTDOWN)
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index d1861c5a74de..e794cbee97e9 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -155,3 +155,40 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub 
> *phy_roothub)
>   phy_power_off(roothub_entry->phy);
>  }
>  EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
> +
> +int usb_phy_roothub_suspend(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub)
> +{
> + usb_phy_roothub_power_off(phy_roothub);
> +
> + /* keep the PHYs initialized so the device can wake up the system */
> + if (device_may_wakeup(controller_dev))
> + return 0;
> +
> + return usb_phy_roothub_exit(phy_roothub);
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
> +
> +int usb_phy_roothub_resume(struct device *controller_dev,
> +struct usb_phy_roothub *phy_roothub)
> +{
> + int err;
> +
> + /* if the device can't wake up the system _exit was called */
> + if (device_may_wakeup(controller_dev)) {
> + err = usb_phy_roothub_init(phy_roothub);
> + if (err)
> + return err;
> + }
> +
> + err = usb_phy_roothub_power_on(phy_roothub);
> + if (err) {
> + if (device_may_wakeup(controller_dev))
> + usb_phy_roothub_exit(phy_roothub);
> +
> + return err;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_resume);
> diff --git a/drivers/usb/core/phy.h 

Re: Possible deadlock due to double spinlock in /drivers/usb/gadget/function/f_hid

[Felipe Balbi]

Hi,

Greg KH  writes:
> For some reason, Tuba can't seem to post to linux-usb@vger, so I'm
> forwarding on his message below.
>
> Felipe, sorry if you have seen this 3+ times already :(
>
> thanks,
>
> greg k-h
>
> On Fri, Mar 23, 2018 at 01:05:28PM +, Yavuz, Tuba wrote:
>> 
>> Hello,
>> 
>> It looks like there is a deadlock possibility due to double locking of a 
>> spinlock in the f_hidg_write function of the f_hid driver.
>> 
>>spin_lock_irqsave(>write_spinlock, flags); // first acquire
>> 
>> /* we our function has been disabled by host */
>> if (!hidg->req) {
>> free_ep_req(hidg->in_ep, hidg->req);
>> /*
>>  * TODO
>>  * Should we fail with error here?
>>  */
>> goto try_again;
>> }
>> 
>> ...
>> 
>> status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);
>> =>
>>   ...
>>  => usb_gadget_giveback_request
>>  =>
>>  f_hidg_req_complete  
>> =>
>>   spin_lock_irqsave(>write_spinlock, 
>> flags); // second acquire 
>> 
>> The bug was introduced with commit
>> 749494b6bdbbaf0899aa1c62a1ad74cd747bce47.

Seems like the best idea is to teach dwc3 that it shouldn't call
->complete() on a failed ->queue(). Just unmap the request, delete from
dwc3's list and return error code.

I'll try to implement this and also update documentation to reflect this
requirement.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v1 3/3] usb: dwc2: Add High Bandwidth ISOC OUT support

[Dan Carpenter]

Hi Minas,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20180309]
[cannot apply to v4.16-rc4 v4.16-rc3 v4.16-rc2 v4.16-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Minas-Harutyunyan/usb-dwc2-gadget-Update-ISOC-DDMA-flow/20180318-134649

smatch warnings:
drivers/usb/dwc2/gadget.c:2250 dwc2_gadget_complete_isoc_request_ddma() warn: 
signed overflow undefined. 'index - (index / dpi) * dpi + 1 < dpi'
drivers/usb/dwc2/gadget.c:2270 dwc2_gadget_complete_isoc_request_ddma() warn: 
should this be a bitwise op?
drivers/usb/dwc2/gadget.c:2270 dwc2_gadget_complete_isoc_request_ddma() warn: 
should this be a bitwise op?

# 
https://github.com/0day-ci/linux/commit/ffb8a411a9d55bf8c15292a6ded7c8663a8ac290
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout ffb8a411a9d55bf8c15292a6ded7c8663a8ac290
vim +2250 drivers/usb/dwc2/gadget.c

5b7d70c6 drivers/usb/gadget/s3c-hsotg.c Ben Dooks 2009-06-02  2066  
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2067  
static void dwc2_hsotg_ep_stop_xfr(struct dwc2_hsotg *hsotg,
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2068  
   struct dwc2_hsotg_ep *hs_ep);
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2069  
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2070  /*
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2071   * 
dwc2_gadget_complete_isoc_request_ddma - complete an isoc request in DDMA
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2072   * 
@hs_ep: The endpoint the request was on.
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2073   *
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2074   * 
Get first request from the ep queue, determine descriptor on which complete
a19205da drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2075   * 
happened. SW discovers which descriptor currently in use by HW, adjusts
a19205da drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2076   * 
dma_address and calculates index of completed descriptor based on the value
a19205da drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2077   * 
of DEPDMA register. Update actual length of request, giveback to gadget.
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2078   */
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2079  
static void dwc2_gadget_complete_isoc_request_ddma(struct dwc2_hsotg_ep *hs_ep)
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2080  {
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2081  
struct dwc2_hsotg *hsotg = hs_ep->parent;
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2082  
struct dwc2_hsotg_req *hs_req;
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2083  
struct usb_request *ureq;
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2084  
int index, idx;
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2085  
dma_addr_t dma_addr;
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2086  
u32 dma_reg;
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2087  
u32 depdma;
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2088  
u32 desc_sts;
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2089  
u32 mask;
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2090  
int dpi;
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2091  
int ret;
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2092  
int sumofpid;
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2093  
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2094  
dpi = 1;
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2095  
if (!hs_ep->dir_in)
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2096  
dpi = hs_ep->mc;
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2097  
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2098  
hs_req = get_ep_head(hs_ep);
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  2016-11-14  2099  
if (!hs_req) {
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2100  
hs_ep->target_frame = TARGET_FRAME_INITIAL;
ffb8a411 drivers/usb/dwc2/gadget.c  Minas Harutyunyan 2018-03-17  2101  
dwc2_hsotg_ep_stop_xfr(hsotg, hs_ep);
540ccba0 drivers/usb/dwc2/gadget.c  Vahram Aharonyan  

[PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus 
supply")
Cc: Amelie Delaunay 
Signed-off-by: Tomeu Vizoso 

---

v2: Only overwrite the error in the pointer after checking it (Heiko
Stübner )
v3: Remove hunks that shouldn't be in this patch
v4: Don't overwrite the error code before returning it (kbuild test
robot )
---
 drivers/usb/dwc2/hcd.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 190f95964000..c51b73b3e048 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
+   int ret;
+
hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-   if (IS_ERR(hsotg->vbus_supply))
-   return 0;
+   if (IS_ERR(hsotg->vbus_supply)) {
+   ret = PTR_ERR(hsotg->vbus_supply);
+   hsotg->vbus_supply = NULL;
+   return ret == -ENODEV ? 0 : ret;
+   }
 
return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
spin_unlock_irqrestore(>lock, flags);
 
-   dwc2_vbus_supply_init(hsotg);
-
-   return 0;
+   return dwc2_vbus_supply_init(hsotg);
 }
 
 /*
-- 
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: UVC VGA WebCam: USB2.0 is listed as two video devices

[fademind]

> it have buttons? That looks like the input layer

This don't have any buttons.

Regards, FM

dmesg | grep uvc

[    3.991743] uvcvideo: Found UVC 1.00 device USB 2.0 UVC VGA WebCam
(13d3:5710)
[    3.998105] uvcvideo 1-1.3:1.0: Entity type for entity Extension 7
was not initialized!
[    3.998109] uvcvideo 1-1.3:1.0: Entity type for entity Processing 2
was not initialized!
[    3.998111] uvcvideo 1-1.3:1.0: Entity type for entity Camera 1 was
not initialized!
[    3.998113] uvcvideo 1-1.3:1.0: Entity type for entity Extension 4
was not initialized!
[    3.998272] usbcore: registered new interface driver uvcvideo


lsusb -v- full
Bus 002 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub
Device Descriptor:
  bLength    18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass    9 Hub
  bDeviceSubClass 0
  bDeviceProtocol 1 Single TT
  bMaxPacketSize0    64
  idVendor   0x8087 Intel Corp.
  idProduct  0x0024 Integrated Rate Matching Hub
  bcdDevice    0.00
  iManufacturer   0
  iProduct    0
  iSerial 0
  bNumConfigurations  1
  Configuration Descriptor:
    bLength 9
    bDescriptorType 2
    wTotalLength   25
    bNumInterfaces  1
    bConfigurationValue 1
    iConfiguration  0
    bmAttributes 0xe0
  Self Powered
  Remote Wakeup
    MaxPower    0mA
    Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber    0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 9 Hub
  bInterfaceSubClass  0
  bInterfaceProtocol  0 Full speed (or root) hub
  iInterface  0
  Endpoint Descriptor:
    bLength 7
    bDescriptorType 5
    bEndpointAddress 0x81  EP 1 IN
    bmAttributes    3
  Transfer Type    Interrupt
  Synch Type   None
  Usage Type   Data
    wMaxPacketSize 0x0001  1x 1 bytes
    bInterval  12
Hub Descriptor:
  bLength   9
  bDescriptorType  41
  nNbrPorts 6
  wHubCharacteristic 0x0009
    Per-port power switching
    Per-port overcurrent protection
    TT think time 8 FS bits
  bPwrOn2PwrGood   50 * 2 milli seconds
  bHubContrCurrent  0 milli Ampere
  DeviceRemovable    0x00
  PortPwrCtrlMask    0xff
 Hub Port Status:
   Port 1: .0100 power
   Port 2: .0100 power
   Port 3: .0100 power
   Port 4: .0100 power
   Port 5: .0100 power
   Port 6: .0100 power
Device Qualifier (for other device speed):
  bLength    10
  bDescriptorType 6
  bcdUSB   2.00
  bDeviceClass    9 Hub
  bDeviceSubClass 0
  bDeviceProtocol 0 Full speed (or root) hub
  bMaxPacketSize0    64
  bNumConfigurations  1
Device Status: 0x0001
  Self Powered

Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Device Descriptor:
  bLength    18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass    9 Hub
  bDeviceSubClass 0
  bDeviceProtocol 0 Full speed (or root) hub
  bMaxPacketSize0    64
  idVendor   0x1d6b Linux Foundation
  idProduct  0x0002 2.0 root hub
  bcdDevice    4.16
  iManufacturer   3 Linux 4.16.0-1-MANJARO ehci_hcd
  iProduct    2 EHCI Host Controller
  iSerial 1 :00:1d.0
  bNumConfigurations  1
  Configuration Descriptor:
    bLength 9
    bDescriptorType 2
    wTotalLength   25
    bNumInterfaces  1
    bConfigurationValue 1
    iConfiguration  0
    bmAttributes 0xe0
  Self Powered
  Remote Wakeup
    MaxPower    0mA
    Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber    0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 9 Hub
  bInterfaceSubClass  0
  bInterfaceProtocol  0 Full speed (or root) hub
  iInterface  0
  Endpoint Descriptor:
    bLength 7
    bDescriptorType 5
    bEndpointAddress 0x81  EP 1 IN
    bmAttributes    3
  Transfer Type    Interrupt
  Synch Type   None
  Usage Type   Data
    wMaxPacketSize 0x0004  1x 4 bytes
    bInterval  12
Hub Descriptor:
  bLength   9
  bDescriptorType  41
  nNbrPorts 2
  wHubCharacteristic 0x000a
    No power switching (usb 1.0)
    Per-port overcurrent protection
  bPwrOn2PwrGood   10 * 2 milli seconds
  bHubContrCurrent  0 milli Ampere
  DeviceRemovable    

Re: UVC VGA WebCam: USB2.0 is listed as two video devices

[Oliver Neukum]

Am Samstag, den 24.03.2018, 16:53 +0100 schrieb fademind:
> Manjaro Linux x86_64
> 
> linux416 4.16.r180324.g99fec39-1
> 
> Weird bug. My webcam is listed twice

Hi,

does it have buttons? That looks like the input layer
gets involved. Please post "lsusb -v".

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

Cześć słodka

[Wesley]

Am Wes ze Stanów Zjednoczonych, ale obecnie przebywa w Syrii na misji 
pokojowej. Obecnie szukam przyjaźni, która doprowadzi do związku, w którym 
znowu czuję się kochana ...

Chcę cię lepiej poznać, jeśli mogę być odważny. Uważam się za łatwego człowieka 
..

Proszę wybaczyć moje maniery nie są dobre, jeśli chodzi o Internet, ponieważ to 
nie jest moja dziedzina. Tutaj w Syrii nie wolno nam wychodzić, co sprawia, że 
bardzo się nudzę, więc myślę, że potrzebuję przyjaciela do rozmowy z zewnątrz, 
żeby mnie utrzymać ...

Chciałbym poznać "prawdziwego" ciebie jako przyjaciela. Twoje polubienia, 
nielubienia, twoje zainteresowania .. co cię wyróżnia.

Mój ulubiony kolor to niebieski. Moje ulubione jedzenie to BACON, mogłem z 
łatwością zostać wegetarianinem, gdyby nie było to na bekonie !!

Mam nadzieję, że możesz mi powiedzieć więcej szczegółów na temat twojej pracy, 
związku i przeszłości .



Mam nadzieję, że wkrótce skontaktuję się z Tobą .

Wes.
--
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 8466489ef5ba48272ba4fa4ea9f8f403306de4c7 breaks Renesas USB3 controller functionality

[Marc Zyngier]

On 25/03/18 15:39, Ard Biesheuvel wrote:
> 
> 
>> On 25 Mar 2018, at 15:14, Marc Zyngier  wrote:
>>
>> On Sun, 25 Mar 2018 14:26:58 +0100
>> Ard Biesheuvel  wrote:
>>
 On 25 March 2018 at 13:52, Marc Zyngier  wrote:
 On Sun, 25 Mar 2018 13:38:19 +0100,
 Ard Biesheuvel wrote:  
>
>> On 25 March 2018 at 13:31, Marc Zyngier  wrote:  
>> On Sun, 25 Mar 2018 12:57:55 +0100,
>> Ard Biesheuvel wrote:  
>>>
 On 25 March 2018 at 12:51, Marc Zyngier  wrote:  
 On Sun, 25 Mar 2018 11:48:35 +0100,
 Ard Biesheuvel wrote:

 Hi Ard,

 [...]

>> I finally found some time to work on this, and came up with an
>> alternative approach (it turns out that this chip is even more
>> braindead than I thought).
>>
>> It is slightly scary, in the sense that the USB controller seems to
>> perform memory accesses even when halted, and can generate faults,
>> but it works just fine on my system. And with this, we can drop the
>> hard reset at boot time. I'm still on the fence to limit it to 
>> systems
>> with an iommu though.
>>
>
> Hi Marc,
>
> I take it you tested this on Cello?  

 Tested on Cello indeed (I should have mentioned that the first place).

> There, it might make sense to
> limit this to systems with an IOMMU, but not in the general case, I
> think. The reason is that it is not guaranteed that the firmware will
> use 32-bit addressable allocations for these data structures, even if
> the kernel is able to without an IOMMU. (UEFI on arm64 will not prefer
> 32-bit addressable memory for PCI DMA if it is available, and usually
> serves heap allocations [such as the ones used for these data
> structures] starting at the top of DRAM)  

 My main worry is that this controller will happily try and DMA from
 zero as we wipe the 64bit registers, even when halted. On Seattle (and
 thus Cello), this is just fine as there is nothing there, and the
 controller aborts with the HSE bit set.

 On other systems, where memory actually exists at 0, who knows what
 this is going to do? On the other hand, this is not worse than the
 current situation, where we could end-up with any odd address...

>>>
>>> Is the PCI_COMMAND_MASTER bit enabled at this point? What happens if
>>> you clear it first?  
>>
>> Tried that. No difference whatsoever, as I still get a fault with the
>> device accessing address 0, and being caught by the iommu.
>>
>
> Wow so this device is more broken than I thought.  

 That's my impression as well. I came to the conclusion that the only
 way to make it behave is to crash it first, and then to reset it.

>>>
>>> OK, so what if it doesn't crash? Without an IOMMU, it is quite likely
>>> that putting zeroes in the lower half of a 64-bit memory address
>>> produces a physical address that is valid, and so the device will
>>> still misbehave in that case.
>>>
>>> If making it crash is what kicks it into submission, could we perhaps
>>> use U64_MAX instead?
>>
>> Just tried that. It gets into some even uglier state, and never
>> recovers. Even doing U64_MAX, fault, and then zero doesn't help. The
>> problem is that it dies with something in the top 32 bits, and that's
>> pretty final.
>>
>> If really feels that without an iommu in the path, this device is
>> completely unsafe, and should never be fed 64bit addresses.
>>
> 
> ... unless we do the pci reset in the exitbootservices handler in
> uefi, which is probably the most robust way of handling this (or wire
> up the iommu)
> 
> i have cello smmu patches if you’re interested

Could be worth a try.

M.
-- 
Jazz is not dead. It just smells funny...
--
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 -next] usb: roles: Fix return value check in intel_xhci_usb_probe()

[Hans de Goede]

Hi,

On 26-03-18 08:43, Wei Yongjun wrote:

In case of error, the function devm_ioremap_nocache() returns NULL
pointer not ERR_PTR(). The IS_ERR() test in the return value check
should be replaced with NULL test.

Fixes: f6fb9ec02be1 ("usb: roles: Add Intel xHCI USB role switch driver")
Signed-off-by: Wei Yongjun 


You're right, thank you for catching this.

Reviewed-by: Hans de Goede 

Regards,

Hans




---
  drivers/usb/roles/intel-xhci-usb-role-switch.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 58c1b60..de72eed 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -145,8 +145,8 @@ static int intel_xhci_usb_probe(struct platform_device 
*pdev)
  
  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

data->base = devm_ioremap_nocache(dev, res->start, resource_size(res));
-   if (IS_ERR(data->base))
-   return PTR_ERR(data->base);
+   if (!data->base)
+   return -ENOMEM;
  
  	for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)

if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",


--
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 -next] usb: roles: Fix return value check in intel_xhci_usb_probe()

[Wei Yongjun]

In case of error, the function devm_ioremap_nocache() returns NULL
pointer not ERR_PTR(). The IS_ERR() test in the return value check
should be replaced with NULL test.

Fixes: f6fb9ec02be1 ("usb: roles: Add Intel xHCI USB role switch driver")
Signed-off-by: Wei Yongjun 
---
 drivers/usb/roles/intel-xhci-usb-role-switch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 58c1b60..de72eed 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -145,8 +145,8 @@ static int intel_xhci_usb_probe(struct platform_device 
*pdev)
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
data->base = devm_ioremap_nocache(dev, res->start, resource_size(res));
-   if (IS_ERR(data->base))
-   return PTR_ERR(data->base);
+   if (!data->base)
+   return -ENOMEM;
 
for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",

--
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