RE: [PATCH v6 07/12] usb: otg: add OTG/dual-role core

[Yoshihiro Shimoda]

Hi,

> From: Peter Chen
> Sent: Friday, April 22, 2016 12:34 PM
> 
> On Fri, Apr 22, 2016 at 09:26:46AM +0800, Peter Chen wrote:
> > On Fri, Apr 15, 2016 at 10:03:16AM +, Yoshihiro Shimoda wrote:
> > > Hi,
> > >
> > > > From: Yoshihiro Shimoda
> > > > Sent: Friday, April 15, 2016 6:59 PM
> > > >
> > > > Hi,
> > > >
> > > > > From: Roger Quadros
> > > > > Sent: Thursday, April 14, 2016 8:32 PM
> > > > >
> > > > > On 14/04/16 14:15, Yoshihiro Shimoda wrote:
> > > > > > Hi,
> > > > > >
> > > > < snip >
> > > > > >>> @@ -865,7 +867,8 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, 
> > > > > >>> unsigned int irqnum,
> > > > > >>>* we're ready only if we have shared HCD
> > > > > >>>* or we don't need shared HCD.
> > > > > >>>*/
> > > > > >>> - if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
> > > > > >>> + if (otg->shared_hcd.hcd || (!otg->caps->needs_companion &&
> > > > > >>> + !otg->primary_hcd.hcd->shared_hcd)) 
> > > > > >>> {
> > > > > >>>   otg->host = hcd_to_bus(hcd);
> > > > > >>>   /* FIXME: set bus->otg_port if this is true OTG port 
> > > > > >>> with HNP */
> > > > > >>>
> > > > > >>
> > > > > >> These changes look good to me. Thanks.
> > > > > >
> > > > > > Thank you for the comment.
> > > > > > If we change the "needs_companion" place to the otg_config,
> > > > > > do we need to add a flag into the otg, instead of otg->caps?
> > > > >
> > > > > Yes we can add a flag in struct usb_otg.
> > > >
> > > > Thank you for the comment.
> > > >
> > > > I made a fixed patch.
> > > > So, should I send this patch to ML after you sent v7 patches?
> > > > Or, would you apply this patch before you send v7 patches?
> > >
> > > Oops, I sent this email without my patch...
> > >
> > > ---
> > > Subject: [PATCH] usb: otg: add hcd companion support
> > >
> > > Since some host controller (e.g. EHCI) needs a companion host controller
> > > (e.g. OHCI), this patch adds such a configuration to use it in the OTG
> > > core.
> > >
> > > Signed-off-by: Yoshihiro Shimoda 
> > > ---
> > >  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
> > >  drivers/usb/common/usb-otg.c  | 17 +
> > >  include/linux/usb/otg.h   |  7 ++-
> > >  3 files changed, 22 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
> > > b/Documentation/devicetree/bindings/usb/generic.txt
> > > index f6866c1..1db1c33 100644
> > > --- a/Documentation/devicetree/bindings/usb/generic.txt
> > > +++ b/Documentation/devicetree/bindings/usb/generic.txt
> > > @@ -27,6 +27,9 @@ Optional properties:
> > >   - otg-controller: phandle to otg controller. Host or gadget controllers 
> > > can
> > >   contain this property to link it to a particular OTG
> > >   controller.
> > > + - hcd-needs-companion: must be present if otg controller is dealing with
> > > + EHCI host controller that needs a companion OHCI host
> > > + controller.
> > >
> > >  This is an attribute to a USB controller such as:
> > >
> > > diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> > > index 41e762a..83c8c96 100644
> > > --- a/drivers/usb/common/usb-otg.c
> > > +++ b/drivers/usb/common/usb-otg.c
> > > @@ -20,6 +20,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -600,6 +601,10 @@ struct usb_otg *usb_otg_register(struct device *dev,
> > >   else
> > >   INIT_WORK(>work, usb_otg_work);
> > >
> > > + if (of_find_property(dev->of_node, "hcd-needs-companion", NULL) ||
> > > + config->hcd_needs_companion)/* needs comanion ? */
> > > + otg->flags |= OTG_FLAG_HCD_NEEDS_COMPANION;
> > > +
> > >   otg->wq = create_singlethread_workqueue("usb_otg");
> > >   if (!otg->wq) {
> > >   dev_err(dev, "otg: %s: can't create workqueue\n",
> > > @@ -823,13 +828,15 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, 
> > > unsigned int irqnum,
> > >   /* HCD will be started by OTG fsm when needed */
> > >   mutex_lock(>fsm.lock);
> > >   if (otg->primary_hcd.hcd) {
> > > - /* probably a shared HCD ? */
> > > - if (usb_otg_hcd_is_primary_hcd(hcd)) {
> > > + /* probably a shared HCD or a companion OHCI HCD ? */
> > > + if (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
> > > + usb_otg_hcd_is_primary_hcd(hcd)) {
> > >   dev_err(otg_dev, "otg: primary host already 
> > > registered\n");
> > >   goto err;
> > >   }
> > >
> > > - if (hcd->shared_hcd == otg->primary_hcd.hcd) {
> > > + if (otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION ||
> > > + (hcd->shared_hcd == otg->primary_hcd.hcd)) {
> > >   if (otg->shared_hcd.hcd) {
> > >   

[PATCH] usb: core: Do not use sizeof on pointer type

[Vaishali Thakkar]

When sizeof is applied to a pointer typed expression, it gives
the size of the pointer. So, do not use sizeof on pointer type.

Problem found using Coccinelle.

Signed-off-by: Vaishali Thakkar 
---
 drivers/usb/core/hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 2ca2cef..2aa352d 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1386,7 +1386,7 @@ static int hcd_alloc_coherent(struct usb_bus *bus,
return -EFAULT;
}
 
-   vaddr = hcd_buffer_alloc(bus, size + sizeof(vaddr),
+   vaddr = hcd_buffer_alloc(bus, size + sizeof(*vaddr),
 mem_flags, dma_handle);
if (!vaddr)
return -ENOMEM;
-- 
2.1.4

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

Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core

[Peter Chen]

On Fri, Apr 22, 2016 at 09:26:46AM +0800, Peter Chen wrote:
> On Fri, Apr 15, 2016 at 10:03:16AM +, Yoshihiro Shimoda wrote:
> > Hi,
> > 
> > > From: Yoshihiro Shimoda
> > > Sent: Friday, April 15, 2016 6:59 PM
> > > 
> > > Hi,
> > > 
> > > > From: Roger Quadros
> > > > Sent: Thursday, April 14, 2016 8:32 PM
> > > >
> > > > On 14/04/16 14:15, Yoshihiro Shimoda wrote:
> > > > > Hi,
> > > > >
> > > < snip >
> > > > >>> @@ -865,7 +867,8 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, 
> > > > >>> unsigned int irqnum,
> > > > >>>  * we're ready only if we have shared HCD
> > > > >>>  * or we don't need shared HCD.
> > > > >>>  */
> > > > >>> -   if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
> > > > >>> +   if (otg->shared_hcd.hcd || (!otg->caps->needs_companion &&
> > > > >>> +   !otg->primary_hcd.hcd->shared_hcd)) 
> > > > >>> {
> > > > >>> otg->host = hcd_to_bus(hcd);
> > > > >>> /* FIXME: set bus->otg_port if this is true OTG port 
> > > > >>> with HNP */
> > > > >>>
> > > > >>
> > > > >> These changes look good to me. Thanks.
> > > > >
> > > > > Thank you for the comment.
> > > > > If we change the "needs_companion" place to the otg_config,
> > > > > do we need to add a flag into the otg, instead of otg->caps?
> > > >
> > > > Yes we can add a flag in struct usb_otg.
> > > 
> > > Thank you for the comment.
> > > 
> > > I made a fixed patch.
> > > So, should I send this patch to ML after you sent v7 patches?
> > > Or, would you apply this patch before you send v7 patches?
> > 
> > Oops, I sent this email without my patch...
> > 
> > ---
> > Subject: [PATCH] usb: otg: add hcd companion support
> > 
> > Since some host controller (e.g. EHCI) needs a companion host controller
> > (e.g. OHCI), this patch adds such a configuration to use it in the OTG
> > core.
> > 
> > Signed-off-by: Yoshihiro Shimoda 
> > ---
> >  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
> >  drivers/usb/common/usb-otg.c  | 17 +
> >  include/linux/usb/otg.h   |  7 ++-
> >  3 files changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
> > b/Documentation/devicetree/bindings/usb/generic.txt
> > index f6866c1..1db1c33 100644
> > --- a/Documentation/devicetree/bindings/usb/generic.txt
> > +++ b/Documentation/devicetree/bindings/usb/generic.txt
> > @@ -27,6 +27,9 @@ Optional properties:
> >   - otg-controller: phandle to otg controller. Host or gadget controllers 
> > can
> > contain this property to link it to a particular OTG
> > controller.
> > + - hcd-needs-companion: must be present if otg controller is dealing with
> > +   EHCI host controller that needs a companion OHCI host
> > +   controller.
> >  
> >  This is an attribute to a USB controller such as:
> >  
> > diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> > index 41e762a..83c8c96 100644
> > --- a/drivers/usb/common/usb-otg.c
> > +++ b/drivers/usb/common/usb-otg.c
> > @@ -20,6 +20,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -600,6 +601,10 @@ struct usb_otg *usb_otg_register(struct device *dev,
> > else
> > INIT_WORK(>work, usb_otg_work);
> >  
> > +   if (of_find_property(dev->of_node, "hcd-needs-companion", NULL) ||
> > +   config->hcd_needs_companion)/* needs comanion ? */
> > +   otg->flags |= OTG_FLAG_HCD_NEEDS_COMPANION;
> > +
> > otg->wq = create_singlethread_workqueue("usb_otg");
> > if (!otg->wq) {
> > dev_err(dev, "otg: %s: can't create workqueue\n",
> > @@ -823,13 +828,15 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, 
> > unsigned int irqnum,
> > /* HCD will be started by OTG fsm when needed */
> > mutex_lock(>fsm.lock);
> > if (otg->primary_hcd.hcd) {
> > -   /* probably a shared HCD ? */
> > -   if (usb_otg_hcd_is_primary_hcd(hcd)) {
> > +   /* probably a shared HCD or a companion OHCI HCD ? */
> > +   if (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
> > +   usb_otg_hcd_is_primary_hcd(hcd)) {
> > dev_err(otg_dev, "otg: primary host already 
> > registered\n");
> > goto err;
> > }
> >  
> > -   if (hcd->shared_hcd == otg->primary_hcd.hcd) {
> > +   if (otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION ||
> > +   (hcd->shared_hcd == otg->primary_hcd.hcd)) {
> > if (otg->shared_hcd.hcd) {
> > dev_err(otg_dev, "otg: shared host already 
> > registered\n");
> > goto err;
> > @@ -865,7 +872,9 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
> > int irqnum,
> >  * we're ready 

Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core

[Peter Chen]

On Fri, Apr 15, 2016 at 10:03:16AM +, Yoshihiro Shimoda wrote:
> Hi,
> 
> > From: Yoshihiro Shimoda
> > Sent: Friday, April 15, 2016 6:59 PM
> > 
> > Hi,
> > 
> > > From: Roger Quadros
> > > Sent: Thursday, April 14, 2016 8:32 PM
> > >
> > > On 14/04/16 14:15, Yoshihiro Shimoda wrote:
> > > > Hi,
> > > >
> > < snip >
> > > >>> @@ -865,7 +867,8 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, 
> > > >>> unsigned int irqnum,
> > > >>>* we're ready only if we have shared HCD
> > > >>>* or we don't need shared HCD.
> > > >>>*/
> > > >>> - if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
> > > >>> + if (otg->shared_hcd.hcd || (!otg->caps->needs_companion &&
> > > >>> + !otg->primary_hcd.hcd->shared_hcd)) 
> > > >>> {
> > > >>>   otg->host = hcd_to_bus(hcd);
> > > >>>   /* FIXME: set bus->otg_port if this is true OTG port 
> > > >>> with HNP */
> > > >>>
> > > >>
> > > >> These changes look good to me. Thanks.
> > > >
> > > > Thank you for the comment.
> > > > If we change the "needs_companion" place to the otg_config,
> > > > do we need to add a flag into the otg, instead of otg->caps?
> > >
> > > Yes we can add a flag in struct usb_otg.
> > 
> > Thank you for the comment.
> > 
> > I made a fixed patch.
> > So, should I send this patch to ML after you sent v7 patches?
> > Or, would you apply this patch before you send v7 patches?
> 
> Oops, I sent this email without my patch...
> 
> ---
> Subject: [PATCH] usb: otg: add hcd companion support
> 
> Since some host controller (e.g. EHCI) needs a companion host controller
> (e.g. OHCI), this patch adds such a configuration to use it in the OTG
> core.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  Documentation/devicetree/bindings/usb/generic.txt |  3 +++
>  drivers/usb/common/usb-otg.c  | 17 +
>  include/linux/usb/otg.h   |  7 ++-
>  3 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/generic.txt 
> b/Documentation/devicetree/bindings/usb/generic.txt
> index f6866c1..1db1c33 100644
> --- a/Documentation/devicetree/bindings/usb/generic.txt
> +++ b/Documentation/devicetree/bindings/usb/generic.txt
> @@ -27,6 +27,9 @@ Optional properties:
>   - otg-controller: phandle to otg controller. Host or gadget controllers can
>   contain this property to link it to a particular OTG
>   controller.
> + - hcd-needs-companion: must be present if otg controller is dealing with
> + EHCI host controller that needs a companion OHCI host
> + controller.
>  
>  This is an attribute to a USB controller such as:
>  
> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> index 41e762a..83c8c96 100644
> --- a/drivers/usb/common/usb-otg.c
> +++ b/drivers/usb/common/usb-otg.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -600,6 +601,10 @@ struct usb_otg *usb_otg_register(struct device *dev,
>   else
>   INIT_WORK(>work, usb_otg_work);
>  
> + if (of_find_property(dev->of_node, "hcd-needs-companion", NULL) ||
> + config->hcd_needs_companion)/* needs comanion ? */
> + otg->flags |= OTG_FLAG_HCD_NEEDS_COMPANION;
> +
>   otg->wq = create_singlethread_workqueue("usb_otg");
>   if (!otg->wq) {
>   dev_err(dev, "otg: %s: can't create workqueue\n",
> @@ -823,13 +828,15 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
> int irqnum,
>   /* HCD will be started by OTG fsm when needed */
>   mutex_lock(>fsm.lock);
>   if (otg->primary_hcd.hcd) {
> - /* probably a shared HCD ? */
> - if (usb_otg_hcd_is_primary_hcd(hcd)) {
> + /* probably a shared HCD or a companion OHCI HCD ? */
> + if (!(otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION) &&
> + usb_otg_hcd_is_primary_hcd(hcd)) {
>   dev_err(otg_dev, "otg: primary host already 
> registered\n");
>   goto err;
>   }
>  
> - if (hcd->shared_hcd == otg->primary_hcd.hcd) {
> + if (otg->flags & OTG_FLAG_HCD_NEEDS_COMPANION ||
> + (hcd->shared_hcd == otg->primary_hcd.hcd)) {
>   if (otg->shared_hcd.hcd) {
>   dev_err(otg_dev, "otg: shared host already 
> registered\n");
>   goto err;
> @@ -865,7 +872,9 @@ int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned 
> int irqnum,
>* we're ready only if we have shared HCD
>* or we don't need shared HCD.
>*/
> - if (otg->shared_hcd.hcd || !otg->primary_hcd.hcd->shared_hcd) {
> + if (otg->shared_hcd.hcd ||
> + (!(otg->flags & 

Re: [PATCH] usb: musb: jz4740: fix error check of usb_get_phy()

[Vladimir Zapolskiy]

Hi Bin,

On 09.03.2016 02:57, Vladimir Zapolskiy wrote:
> The usb_get_phy() function returns either a valid pointer to phy or
> ERR_PTR() error, check for NULL always fails and may lead to oops on
> error path, fix this issue.
> 
> Signed-off-by: Vladimir Zapolskiy 
> ---
>  drivers/usb/musb/jz4740.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/musb/jz4740.c b/drivers/usb/musb/jz4740.c
> index 5e5a8fa..bc88899 100644
> --- a/drivers/usb/musb/jz4740.c
> +++ b/drivers/usb/musb/jz4740.c
> @@ -83,9 +83,9 @@ static int jz4740_musb_init(struct musb *musb)
>  {
>   usb_phy_generic_register();
>   musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
> - if (!musb->xceiv) {
> + if (IS_ERR(musb->xceiv)) {
>   pr_err("HS UDC: no transceiver configured\n");
> - return -ENODEV;
> + return PTR_ERR(musb->xceiv);
>   }
>  
>   /* Silicon does not implement ConfigData register.
> 

congratulations with new responsibilities regarding MUSB maintenance,
please could you check the status of this change?

Thank you in advance.

--
With best wishes,
Vladimir
--
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

Bug 116821 - Regression: System freeze when unplugging unmounted USB flash drive from powered hub

[John]

jbMacAZ  2016-04-20 22:39:07 UTC

System hangs requiring hard reboot. System behaves normally with earlier 
kernels upto 4.6-rc3. Reverting all edits to drivers/USB/ between rc3 
and rc4 does NOT fix the problem. Problem first appears at 
4.6-rc3-next20160412, still present 4.6-rc4-next20160420.


To reproduce, attach a powered USB hub to USB3 port on an ASUS T100CHI. 
Attach a USB thumb drive (mine was 32GB Samsung USB3.0) to powered hub. 
Boot into latest kernel, unmount flash drive. Unplug it and the system 
freezes. May freeze when plugging in a flash drive, but not as 
consistent. Simplest workaround is to use older kernel (like -rc3).


Other details: Ubuntu 16.04 (i686) 2GB Ram, Intel baytrail atom Z3775, 
generic USB2 4-port hub (BC 1.1 charging) OTG cable micro USB3.0 to USB-A.


Thanks for looking,
John

[reply ] 
[−]  Comment 1 
 Greg 
Kroah-Hartman  2016-04-21 02:27:34 UTC


On Wed, Apr 20, 2016 at 10:39:07PM +,bugzilla-dae...@bugzilla.kernel.org  
  wrote:

https://bugzilla.kernel.org/show_bug.cgi?id=116821

Bug ID: 116821
   Summary: Regression: System freeze when unplugging unmounted
USB flash drive from powered hub


Please send to thelinux-...@vger.kernel.org   
 mailing list.

--
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: What are Shared HCD and Companion Controller(HCD?)

[Alan Stern]

On Thu, 21 Apr 2016, Peter Chen wrote:

> I have one more question, if hcd codes takes primary hcd as USB2 and
> shared hcd as USB3, are there any problems if EHCI (or OHCI) as primary
> hcd and OHCI (EHCI) as shared hcd like [1], current hcd code seems to
> use hcd->shared_hcd at drivers/usb/core/port.c
> 
> [1] http://www.spinics.net/lists/linux-usb/msg139344.html

Neither EHCI nor OHCI can ever be a shared_hcd; only xHCI has a 
shared_hcd.  EHCI and OHCI can be companions, not shared.

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: Microsoft wireless usb mouse frequently fails to detect in ubuntu 15.10/16.04

[Alan Stern]

On Thu, 21 Apr 2016, rootsr wrote:

> Hello all,
> 
> I am using ubuntu 16.04 (upgraded yesterday from 15.10) on Dell
> inspiron 15 3000 series (core i7). While using Microsoft wireless
> mouse, it fails to detect most of the time. The issue is not there
> when I use the same mouse in Windows / other linux distro / TV / PS4
> etc.

Do you run those other tests on the Dell Inspiron or on other machines?

> Here is the Kernel log for the failure. 
> 
> [   74.186391] usb 1-1.3: new full-speed USB device number 7 using ehci-pci
> [   74.258396] usb 1-1.3: device descriptor read/64, error -32
> [   74.434356] usb 1-1.3: device descriptor read/64, error -32
> [   74.610315] usb 1-1.3: new full-speed USB device number 8 using ehci-pci
> [   74.682322] usb 1-1.3: device descriptor read/64, error -32
> [   74.858322] usb 1-1.3: device descriptor read/64, error -32
> [   75.034320] usb 1-1.3: new full-speed USB device number 9 using ehci-pci
> [   75.446330] usb 1-1.3: device not accepting address 9, error -32
> [   75.518334] usb 1-1.3: new full-speed USB device number 10 using ehci-pci
> [   75.926331] usb 1-1.3: device not accepting address 10, error -32
> [   75.926400] usb 1-1-port3: unable to enumerate USB device
> [   93.835769] usb 1-1.3: new full-speed USB device number 11 using ehci-pci
> [   93.908028] usb 1-1.3: device descriptor read/64, error -32
> [   94.084569] usb 1-1.3: device descriptor read/64, error -32
> [   94.261110] usb 1-1.3: new full-speed USB device number 12 using ehci-pci
> [   94.37] usb 1-1.3: device descriptor read/64, error -32
> [   94.509893] usb 1-1.3: device descriptor read/64, error -32
> [   94.686342] usb 1-1.3: new full-speed USB device number 13 using ehci-pci
> [   95.095548] usb 1-1.3: device not accepting address 13, error -32
> [   95.167686] usb 1-1.3: new full-speed USB device number 14 using ehci-pci
> [   95.576740] usb 1-1.3: device not accepting address 14, error -32
> [   95.576848] usb 1-1-port3: unable to enumerate USB device
> -
> 
> After multiple connect/disconnect of the wireless dongle, it finally detects. 
> Here is the detection log.
> --
> [  110.221269] usb 1-1.3: new full-speed USB device number 15 using ehci-pci
> [  110.319427] usb 1-1.3: New USB device found, idVendor=045e, idProduct=0745
> [  110.319431] usb 1-1.3: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=0
> [  110.319433] usb 1-1.3: Product: Microsoft� 2.4GHz Transceiver v8.0
> [  110.319435] usb 1-1.3: Manufacturer: Microsoft
> [  110.361146] usbcore: registered new interface driver usbhid
> [  110.361151] usbhid: USB HID core driver
> [  110.380274] input: Microsoft Microsoft� 2.4GHz Transceiver v8.0 as 
> /devices/pci:00/:00:1d.0/usb1/1-1/1-1.3/1-1.3:1.0/0003:045E:0745.0002/input/input14
> [  110.433886] hid-generic 0003:045E:0745.0002: input,hidraw1: USB HID v1.11 
> Keyboard [Microsoft Microsoft� 2.4GHz Transceiver v8.0] on 
> usb-:00:1d.0-1.3/input0
> [  110.434274] input: Microsoft Microsoft� 2.4GHz Transceiver v8.0 as 
> /devices/pci:00/:00:1d.0/usb1/1-1/1-1.3/1-1.3:1.1/0003:045E:0745.0003/input/input15
> [  110.489634] hid-generic 0003:045E:0745.0003: input,hidraw2: USB HID v1.11 
> Mouse [Microsoft Microsoft� 2.4GHz Transceiver v8.0] on 
> usb-:00:1d.0-1.3/input1
> [  110.500335] input: Microsoft Microsoft� 2.4GHz Transceiver v8.0 as 
> /devices/pci:00/:00:1d.0/usb1/1-1/1-1.3/1-1.3:1.2/0003:045E:0745.0004/input/input16
> [  110.553640] hid-generic 0003:045E:0745.0004: input,hiddev0,hidraw3: USB 
> HID v1.11 Device [Microsoft Microsoft� 2.4GHz Transceiver v8.0] on 
> usb-:00:1d.0-1.3/input2
> -
> 
> I am not sure of this weird behaviour. Is there some issue with
> ubuntu kernel for input device? or is there a problem with microsoft
> mouse driver?

As far as I can tell, this is a hardware problem.  Not a software 
problem.

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: xhci-rcar: Avoid long wait in xhci_reset()

[Felipe Balbi]

Hi,

Geert Uytterhoeven  writes:
> On Thu, Apr 21, 2016 at 12:27 PM, Yoshihiro Shimoda
>  wrote:
>>> > [1.565605] xhci-hcd ee00.usb: xHCI Host Controller
>>> > [1.570636] xhci-hcd ee00.usb: new USB bus registered, assigned 
>>> > bus number 5
>>> > [   22.270160] xhci-hcd ee00.usb: can't setup: -110
>>> > [   22.274931] xhci-hcd ee00.usb: USB bus 5 deregistered
>>> > [   22.280158] xhci-hcd: probe of ee00.usb failed with error -110
>>> >
>>> > The timestamp is strange to me. But, logs of R-Car H3 (ES1.0) and
>>> > R-Car H2 were the same.
>>>
>>> yeah, seems like your system timer is counting twice for each tick.
>>
>> Yes, I will investigate this later.
>
> The main clock crystal on Salvator-X is half of the expected value. But
> despite the correct value being in the DTS, there's some timer code that
> doesn't take this into account.

cool, thanks for the note. One problem down.

-- 
balbi


signature.asc
Description: PGP signature

Re: devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

[Felipe Balbi]

Hi,

Rob Herring  writes:
> On Thu, Apr 21, 2016 at 6:20 AM, Felipe Balbi
>  wrote:
>>
>> Hi,
>>
>> Heikki Krogerus  writes:
>>> @@ -197,7 +196,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>>   }
>>>
>>>   xhci = hcd_to_xhci(hcd);
>>> - match = of_match_node(usb_xhci_of_match, node);
>>> + match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);
>>
>> Rob, it's weird that OF-based drivers have to redo the same matching
>> which was already done by drivers/base/platform.c::platform_match() just
>> to get match->data. If we know we matched, couldn't we just cache a
>> pointer to match->data in struct device_node.data ? Something like
>> below? (completely untested)
>
> Yes, it is. AIUI, there is some sort of race condition in doing what
> you suggest though. IIRC, Grant did that and reverted it if you look
> at the git history.

looking at drivers/base/platform.c I can't find anything along these
lines. Adding Grant.

Grant, any memory left of this race ?

-- 
balbi


signature.asc
Description: PGP signature

Re: devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

[Rob Herring]

On Thu, Apr 21, 2016 at 6:20 AM, Felipe Balbi
 wrote:
>
> Hi,
>
> Heikki Krogerus  writes:
>> @@ -197,7 +196,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>   }
>>
>>   xhci = hcd_to_xhci(hcd);
>> - match = of_match_node(usb_xhci_of_match, node);
>> + match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);
>
> Rob, it's weird that OF-based drivers have to redo the same matching
> which was already done by drivers/base/platform.c::platform_match() just
> to get match->data. If we know we matched, couldn't we just cache a
> pointer to match->data in struct device_node.data ? Something like
> below? (completely untested)

Yes, it is. AIUI, there is some sort of race condition in doing what
you suggest though. IIRC, Grant did that and reverted it if you look
at the git history.

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

Re: [PATCH] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Geert Uytterhoeven]

On Thu, Apr 21, 2016 at 12:27 PM, Yoshihiro Shimoda
 wrote:
>> > [1.565605] xhci-hcd ee00.usb: xHCI Host Controller
>> > [1.570636] xhci-hcd ee00.usb: new USB bus registered, assigned bus 
>> > number 5
>> > [   22.270160] xhci-hcd ee00.usb: can't setup: -110
>> > [   22.274931] xhci-hcd ee00.usb: USB bus 5 deregistered
>> > [   22.280158] xhci-hcd: probe of ee00.usb failed with error -110
>> >
>> > The timestamp is strange to me. But, logs of R-Car H3 (ES1.0) and
>> > R-Car H2 were the same.
>>
>> yeah, seems like your system timer is counting twice for each tick.
>
> Yes, I will investigate this later.

The main clock crystal on Salvator-X is half of the expected value. But
despite the correct value being in the DTS, there's some timer code that
doesn't take this into account.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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

[PATCHv3] usb: Add driver for UCSI

[Heikki Krogerus]

USB Type-C Connector System Software Interface (UCSI) is
specification that defines the registers and data structures
that can be used to control USB Type-C ports on a system.
UCSI is used on several Intel Broxton SoC based platforms.
Things that UCSI can be used to control include at least USB
Data Role swapping, Power Role swapping and controlling of
Alternate Modes on top of providing general details about
the port and the partners that are attached to it.

The initial purpose of the UCSI driver is to make sure USB
is in host mode on desktop and server systems that are USB
dual role capable, and provide UCSI interface.

The goal is to integrate the driver later to an USB Type-C
framework for Linux kernel, and at the same time add support
for more extensive USB Type-C port control that UCSI offers,
for example data role swapping, power role swapping,
Alternate Mode control etc.

The UCSI specification is public can be obtained from here:
http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/misc/Kconfig  |  26 +++
 drivers/usb/misc/Makefile |   1 +
 drivers/usb/misc/ucsi.c   | 478 ++
 drivers/usb/misc/ucsi.h   | 215 +
 4 files changed, 720 insertions(+)
 create mode 100644 drivers/usb/misc/ucsi.c
 create mode 100644 drivers/usb/misc/ucsi.h

Changes since v2:
- Polling the reset complete bit of CCI in ucsi_reset_ppm
- Added back the mutex as suggested by Mathias

Changes since v1:
- Now just always using host mode. Got rid of the modules parameter.
- Replaced the atomic_t with bitop "flags". It's used to sync
  communication with the PPMs.
- Refactored the notifier handler based on suggestions from Felipe
  and Andy
- Added data structure for CCI as suggested by Felipe


diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index f7a7fc2..e9e5ae5 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -268,3 +268,29 @@ config USB_CHAOSKEY
 
  To compile this driver as a module, choose M here: the
  module will be called chaoskey.
+
+config UCSI
+   tristate "USB Type-C Connector System Software Interface driver"
+   depends on ACPI
+   help
+ UCSI driver is meant to be used as a convenience tool for desktop and
+ server systems that are not equipped to handle USB in device mode. It
+ will always select USB host role for the USB Type-C ports on systems
+ that provide UCSI interface.
+
+ USB Type-C Connector System Software Interface (UCSI) is a
+ specification for an interface that allows the Operating System to
+ control the USB Type-C ports on a system. Things the need controlling
+ include the USB Data Role (host or device), and when USB Power
+ Delivery is supported, the Power Role (source or sink). With USB
+ Type-C connectors, when two dual role capable devices are attached
+ together, the data role is selected randomly. Therefore it is
+ important to give the OS a way to select the role. Otherwise the user
+ would have to unplug and replug in order in order to attempt to swap
+ the data and power roles.
+
+ The UCSI specification can be downloaded from:
+ 
http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html
+
+ To compile the driver as a module, choose M here: the module will be
+ called ucsi.
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 45fd4ac..2769cf6 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_USB_SEVSEG)  += usbsevseg.o
 obj-$(CONFIG_USB_YUREX)+= yurex.o
 obj-$(CONFIG_USB_HSIC_USB3503) += usb3503.o
 obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o
+obj-$(CONFIG_UCSI) += ucsi.o
 
 obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/
 obj-$(CONFIG_USB_LINK_LAYER_TEST)  += lvstest.o
diff --git a/drivers/usb/misc/ucsi.c b/drivers/usb/misc/ucsi.c
new file mode 100644
index 000..07397bd
--- /dev/null
+++ b/drivers/usb/misc/ucsi.c
@@ -0,0 +1,478 @@
+/*
+ * USB Type-C Connector System Software Interface driver
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Author: Heikki Krogerus 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "ucsi.h"
+
+/* Double the time defined by MIN_TIME_TO_RESPOND_WITH_BUSY */
+#define UCSI_TIMEOUT_MS 20
+
+enum ucsi_status {
+   UCSI_IDLE = 0,
+   UCSI_BUSY,
+   UCSI_ERROR,
+};
+
+struct ucsi_connector {
+   int num;
+   struct ucsi *ucsi;
+   

Re: 4.5.1: UBSAN: Undefined behaviour in ./arch/x86/include/asm/atomic.h:156:9

[Greg KH]

On Thu, Apr 21, 2016 at 01:17:19PM +0200, Martin MOKREJŠ wrote:
> Greg KH wrote:
> > On Thu, Apr 21, 2016 at 11:18:16AM +0200, Martin MOKREJŠ wrote:
> > > Hi Greg,
> > > 
> > > thank you for your answer.
> > > 
> > > Greg KH wrote:
> > > > On Thu, Apr 21, 2016 at 12:22:49AM +0200, Martin MOKREJŠ wrote:
> > > > > Hi,
> > > > >   I am not certain to to forward this to, so I am trying linux-usb 
> > > > > and ext4. Please add relevant people/lists, thank you.
> > > > > 
> > > > > # dmesg | grep "UBSAN: Undefined behaviour"
> > > > > [2.638843] UBSAN: Undefined behaviour in 
> > > > > drivers/usb/host/ehci-hub.c:873:47
> > > > > [8.553620] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:2612:15
> > > > > [   25.341048] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:1274:11
> > > > > [   41.460542] UBSAN: Undefined behaviour in 
> > > > > ./arch/x86/include/asm/atomic.h:156:9
> > > > > #
> > > > > 
> > > > > Full dmesg is attached.
> > > > > 
> > > > > Hope this helps,
> > > > > Martin
> > > > 
> > > > > [0.00] Linux version 4.5.1-default-pciehp (root@vostro) (gcc 
> > > > > version 5.3.0 (Gentoo 5.3.0 p1.0, pie-0.6.5) ) #1 SMP Thu Apr 14 
> > > > > 14:17:48 CEST 2016
> > > > 
> > > > Please try 4.6-rc4, a number of UBSAN issues have been fixed there.
> > > 
> > > They are still present:
> > > 
> > > # dmesg | grep "UBSAN: Undefined behaviour"
> > > [3.113405] UBSAN: Undefined behaviour in 
> > > drivers/usb/host/ehci-hub.c:877:47
> > > [5.678949] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:2619:15
> > > [   18.345599] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:1281:11
> > > [   34.544363] UBSAN: Undefined behaviour in 
> > > ./arch/x86/include/asm/atomic.h:156:9
> > 
> > Ah, sorry about that, turns out this is a bug in the tool, not in the
> > code, see the thread from a few weeks ago in the linux-usb archives with
> > the subject:
> > Subject: function ehci_hub_control in ehci-hub.c
> > for all of the details.
> 
> Thank you. I am not a programmer so I only infer from this thread I should 
> disable
> the UBSAN in .config. The kernel code is technically correct but triggers 
> false alarm.
> Maybe the UBSAN could have a whitelist of falsely matching locations?
> 
> https://www.mail-archive.com/linux-usb@vger.kernel.org/msg72438.html

It probably should be fixed to not report false things like this.

> Did anybody check the ext4 code or is it also just believed the UBSAN 
> complaint is
> just broken and code changes are not necessary?

I'm not an ext4 developer, I'll leave that to them  :)

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

devicetree: avoid duplicated matching code (was: Re: [PATCH 1/3] xhci: plat: adapt to unified device property interface)

[Felipe Balbi]

Hi,

Heikki Krogerus  writes:
> @@ -197,7 +196,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   }
>  
>   xhci = hcd_to_xhci(hcd);
> - match = of_match_node(usb_xhci_of_match, node);
> + match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);

Rob, it's weird that OF-based drivers have to redo the same matching
which was already done by drivers/base/platform.c::platform_match() just
to get match->data. If we know we matched, couldn't we just cache a
pointer to match->data in struct device_node.data ? Something like
below? (completely untested)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index b299de2b3afa..9b44caa38f7c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -981,6 +981,8 @@ const struct of_device_id *of_match_node(const struct 
of_device_id *matches,
 
raw_spin_lock_irqsave(_lock, flags);
match = __of_match_node(matches, node);
+   if (match)
+   node->data = match->data;
raw_spin_unlock_irqrestore(_lock, flags);
return match;
 }

I tried to find users of device_node.data but couldn't find any. If this
patch is acceptable, we can remove 160 occurences of of_match_node with
some variance of node->data.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 0/3] xhci: get rid of platform data

[Heikki Krogerus]

On Thu, Apr 21, 2016 at 02:02:38PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Heikki Krogerus  writes:
> > Hi,
> >
> > This depends on API change the device property framework in Rafael's
> > linux-pm tree.
> >
> > Branch linux-next in
> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
> 
> same comment as other series: should we wait for v4.8 ?

v4.8 works for me.

-- 
heikki
--
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] xhci: Cleanup only when releasing primary hcd.

[Mathias Nyman]

On 19.04.2016 21:13, Gabriel Krisman Bertazi wrote:

Under stress occasions some TI devices might not return early when
reading the status register during the quirk invocation of xhci_irq made
by usb_hcd_pci_remove.  This means that instead of returning, we end up
handling this interruption in the middle of a shutdown.  Since
xhci->event_ring has already been freed in xhci_mem_cleanup, we end up
accessing freed memory, causing the Oops below.

commit 8c24d6d7b09d ("usb: xhci: stop everything on the first call to
xhci_stop") is the one that changed the instant in which we clean up the
event queue when stopping a device.  Before, we didn't call
xhci_mem_cleanup at the first time xhci_stop is executed (for the shared
HCD), instead, we only did it after the invocation for the primary HCD,
much later at the removal path.  The code flow for this oops looks like
this:

xhci_pci_remove()
usb_remove_hcd(xhci->shared)
xhci_stop(xhci->shared)
xhci_halt()
xhci_mem_cleanup(xhci);  // Free the event_queue
usb_hcd_pci_remove(primary)
xhci_irq()  // Access the event_queue if STS_EINT is set. Crash.
xhci_stop()
xhci_halt()
// return early

The fix modifies xhci_stop to only cleanup the xhci data when releasing
the primary HCD.  This way, we still have the event_queue configured
when invoking xhci_irq.  We still halt the device on the first call to
xhci_stop, though.

I could reproduce this issue several times on the mainline kernel by
doing a bind-unbind stress test with a specific storage gadget attached.
I also ran the same test over-night with my patch applied and didn't
observe the issue anymore.



Nice catch, and moving xhci_mem_cleanup() until second hcd (primary) is
removed is one way to solve it.

But I don't think we should even try to handle the interrupt at this stage 
anymore.
The host is already halted and normally the handler should not be called 
anymore.

How about handling interrupts for a halted host in the same way as a dying host?
Does something like this work for your TI devices?


diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 99b4ff4..2cdc027 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2728,8 +2728,9 @@ hw_died:
writel(irq_pending, >ir_set->irq_pending);
}
 
-   if (xhci->xhc_state & XHCI_STATE_DYING) {

-   xhci_dbg(xhci, "xHCI dying, ignoring interrupt. "
+   if ((xhci->xhc_state & XHCI_STATE_DYING) ||
+   (xhci->xhc_state & XHCI_STATE_HALTED)) {
+   xhci_dbg(xhci, "xHCI halted or dying, ignoring interrupt. "
"Shouldn't IRQs be disabled?\n");
/* Clear the event handler busy flag (RW1C);
 * the event ring should be empty.


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: 4.5.1: UBSAN: Undefined behaviour in ./arch/x86/include/asm/atomic.h:156:9

[Martin MOKREJŠ]

Greg KH wrote:

On Thu, Apr 21, 2016 at 11:18:16AM +0200, Martin MOKREJŠ wrote:

Hi Greg,

thank you for your answer.

Greg KH wrote:

On Thu, Apr 21, 2016 at 12:22:49AM +0200, Martin MOKREJŠ wrote:

Hi,
  I am not certain to to forward this to, so I am trying linux-usb and ext4. 
Please add relevant people/lists, thank you.

# dmesg | grep "UBSAN: Undefined behaviour"
[2.638843] UBSAN: Undefined behaviour in drivers/usb/host/ehci-hub.c:873:47
[8.553620] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:2612:15
[   25.341048] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:1274:11
[   41.460542] UBSAN: Undefined behaviour in 
./arch/x86/include/asm/atomic.h:156:9
#

Full dmesg is attached.

Hope this helps,
Martin



[0.00] Linux version 4.5.1-default-pciehp (root@vostro) (gcc version 
5.3.0 (Gentoo 5.3.0 p1.0, pie-0.6.5) ) #1 SMP Thu Apr 14 14:17:48 CEST 2016


Please try 4.6-rc4, a number of UBSAN issues have been fixed there.


They are still present:

# dmesg | grep "UBSAN: Undefined behaviour"
[3.113405] UBSAN: Undefined behaviour in drivers/usb/host/ehci-hub.c:877:47
[5.678949] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:2619:15
[   18.345599] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:1281:11
[   34.544363] UBSAN: Undefined behaviour in 
./arch/x86/include/asm/atomic.h:156:9


Ah, sorry about that, turns out this is a bug in the tool, not in the
code, see the thread from a few weeks ago in the linux-usb archives with
the subject:
Subject: function ehci_hub_control in ehci-hub.c
for all of the details.


Thank you. I am not a programmer so I only infer from this thread I should 
disable
the UBSAN in .config. The kernel code is technically correct but triggers false 
alarm.
Maybe the UBSAN could have a whitelist of falsely matching locations?

https://www.mail-archive.com/linux-usb@vger.kernel.org/msg72438.html

Did anybody check the ext4 code or is it also just believed the UBSAN complaint 
is
just broken and code changes are not necessary?
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: dwc3: pci: use build-in properties instead of platform data

[Heikki Krogerus]

On Thu, Apr 21, 2016 at 02:01:47PM +0300, Felipe Balbi wrote:
> > @@ -169,20 +161,21 @@ static int dwc3_pci_probe(struct pci_dev *pci,
> > return ret;
> > }
> >  
> > -   pci_set_drvdata(pci, dwc3);
> > -   ret = dwc3_pci_quirks(pci);
> > -   if (ret)
> > -   goto err;
> > -
> > dwc3->dev.parent = dev;
> > ACPI_COMPANION_SET(>dev, ACPI_COMPANION(dev));
> >  
> > +   ret = dwc3_pci_quirks(pci, dwc3);
> > +   if (ret)
> > +   goto err;
> > +
> 
> do you mind moving this hunk ...
> 
> > ret = platform_device_add(dwc3);
> > if (ret) {
> > dev_err(dev, "failed to register dwc3 device\n");
> > goto err;
> > }
> >  
> > +   pci_set_drvdata(pci, dwc3);
> 
> and this hunk to a separate patch ? They look like extra noise here
> which takes away from the actual work of $subject

Sure thing.

Thanks,

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

Re: [PATCH 0/2] usb: dwc3: remove platform data

[Heikki Krogerus]

On Thu, Apr 21, 2016 at 01:58:43PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Heikki Krogerus  writes:
> > Hi,
> >
> > The API changes to the device property framework that I've been
> > waiting are now part of Rafael's linux-pm tree:
> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
> >
> > So these depend on, I guess linux-next branch of that tree.
> >
> >
> > Heikki Krogerus (2):
> >   usb: dwc3: pci: use build-in properties instead of platform data
> >   usb: dwc3: remove handling of platform data
> 
> how should we handle this ? I guess the best way would be to delay this
> series to v4.8 ?

I'm fine with that.

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

Re: [PATCH 0/3] xhci: get rid of platform data

[Felipe Balbi]

Hi,

Heikki Krogerus  writes:
> Hi,
>
> This depends on API change the device property framework in Rafael's
> linux-pm tree.
>
> Branch linux-next in
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git

same comment as other series: should we wait for v4.8 ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 1/2] usb: dwc3: pci: use build-in properties instead of platform data

[Felipe Balbi]

Hi,

Heikki Krogerus  writes:
> This should allow the core driver to drop handling of
> platform data and expect the platform specific details to
> always come from properties.
>
> The ACPI companion needs to be set before any build-in
> properties are applied as setting it would otherwise
> override the build-in properties. Therefore setting the
> companion before dwc3_pci_quirks() is called.
>
> Signed-off-by: Heikki Krogerus 
> Cc: Huang Rui 
> CC: John Youn 
> ---
>  drivers/usb/dwc3/dwc3-pci.c | 85 
> +
>  1 file changed, 39 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index adc1e8a..d7e77b0 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -23,8 +23,7 @@
>  #include 
>  #include 
>  #include 
> -
> -#include "platform_data.h"
> +#include 
>  
>  #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3  0xabcd
>  #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI  0xabce
> @@ -47,38 +46,33 @@ static const struct acpi_gpio_mapping 
> acpi_dwc3_byt_gpios[] = {
>   { },
>  };
>  
> -static int dwc3_pci_quirks(struct pci_dev *pdev)
> +static int dwc3_pci_quirks(struct pci_dev *pdev, struct platform_device 
> *dwc3)
>  {
>   if (pdev->vendor == PCI_VENDOR_ID_AMD &&
>   pdev->device == PCI_DEVICE_ID_AMD_NL_USB) {
> - struct dwc3_platform_data pdata;
> -
> - memset(, 0, sizeof(pdata));
> -
> - pdata.has_lpm_erratum = true;
> - pdata.lpm_nyet_threshold = 0xf;
> -
> - pdata.u2exit_lfps_quirk = true;
> - pdata.u2ss_inp3_quirk = true;
> - pdata.req_p1p2p3_quirk = true;
> - pdata.del_p1p2p3_quirk = true;
> - pdata.del_phy_power_chg_quirk = true;
> - pdata.lfps_filter_quirk = true;
> - pdata.rx_detect_poll_quirk = true;
> -
> - pdata.tx_de_emphasis_quirk = true;
> - pdata.tx_de_emphasis = 1;
> -
> - /*
> -  * FIXME these quirks should be removed when AMD NL
> -  * taps out
> -  */
> - pdata.disable_scramble_quirk = true;
> - pdata.dis_u3_susphy_quirk = true;
> - pdata.dis_u2_susphy_quirk = true;
> -
> - return platform_device_add_data(pci_get_drvdata(pdev), ,
> - sizeof(pdata));
> + struct property_entry properties[] = {
> + PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
> + PROPERTY_ENTRY_U8("snps,lpm-nyet-threshold", 0xf),
> + PROPERTY_ENTRY_BOOL("snps,u2exit_lfps_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,u2ss_inp3_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,req_p1p2p3_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,del_p1p2p3_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,del_phy_power_chg_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,lfps_filter_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,rx_detect_poll_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,tx_de_emphasis_quirk"),
> + PROPERTY_ENTRY_U8("snps,tx_de_emphasis", 1),
> + /*
> +  * FIXME these quirks should be removed when AMD NL
> +  * tapes out
> +  */
> + PROPERTY_ENTRY_BOOL("snps,disable_scramble_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
> + PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
> + { },
> + };
> +
> + return platform_device_add_properties(dwc3, properties);
>   }
>  
>   if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> @@ -115,16 +109,14 @@ static int dwc3_pci_quirks(struct pci_dev *pdev)
>   (pdev->device == PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3 ||
>pdev->device == PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI ||
>pdev->device == PCI_DEVICE_ID_SYNOPSYS_HAPSUSB31)) {
> -
> - struct dwc3_platform_data pdata;
> -
> - memset(, 0, sizeof(pdata));
> - pdata.usb3_lpm_capable = true;
> - pdata.has_lpm_erratum = true;
> - pdata.dis_enblslpm_quirk = true;
> -
> - return platform_device_add_data(pci_get_drvdata(pdev), ,
> - sizeof(pdata));
> + struct property_entry properties[] = {
> + PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"),
> + PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
> + PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"),
> + { },
> + };
> +
> + return 

Re: [PATCH 0/2] usb: dwc3: remove platform data

[Felipe Balbi]

Hi,

Heikki Krogerus  writes:
> Hi,
>
> The API changes to the device property framework that I've been
> waiting are now part of Rafael's linux-pm tree:
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
>
> So these depend on, I guess linux-next branch of that tree.
>
>
> Heikki Krogerus (2):
>   usb: dwc3: pci: use build-in properties instead of platform data
>   usb: dwc3: remove handling of platform data

how should we handle this ? I guess the best way would be to delay this
series to v4.8 ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 0/7] usb: host: add support for threaded IRQs

[Felipe Balbi]

Hi,

Felipe Balbi  writes:
> Hi,
>
> Greg Kroah-Hartman  writes:
>> On Mon, Apr 11, 2016 at 03:44:09PM +0300, Felipe Balbi wrote:
>>> Hi guys,
>>> 
>>> this patchset introduces support for threaded IRQs
>>> for host controllers drivers to use. Right now, only
>>> XHCI has been converted, but more drivers could
>>> easily be converted as well.
>>> 
>>> With this series we can, eventually, spend much less
>>> time with IRQs disabled. Note that, because we're
>>> masking XHCI's IRQs, we could run our thread
>>> handlers with IRQs enabled, but I haven't tested
>>> that yet.
>>
>> Does it really benifit anything?  Do you have any measurements?  Why the
>
> I have measurements showing that it doesn't have a negative impact.
>
>> added work for no real need, and increased latency?
>
> the latency is negligible. XHCI's irqs are used for completions and
> aborts, not to start transfers. Also, it's a generally a good idea to
> spend less time in hardirq context. Not to mention that this is a good
> stepping stone for further optimization; we could (and probably should)
> have a per-device-slot irq thread and also per-device-slot locks and
> demote the controller's lock to only the parts which actually need to
> access controller-wide resources (for the most part we have
> slot-specific registers, anyway).
>
> Now, the benefit of the work yet to come is that we could be processing
> N slots concurrently given we have N cpu cores.
>
> Note, also, that we're not forcing anybody to use this but given that
> XHCI has these separate units referred to as 'device slots', we might as
> well exploit that, right ?

Any further comments here ? Greg ? Mathias ?

-- 
balbi


signature.asc
Description: PGP signature

Re: 4.5.1: UBSAN: Undefined behaviour in ./arch/x86/include/asm/atomic.h:156:9

[Greg KH]

On Thu, Apr 21, 2016 at 11:18:16AM +0200, Martin MOKREJŠ wrote:
> Hi Greg,
> 
> thank you for your answer.
> 
> Greg KH wrote:
> > On Thu, Apr 21, 2016 at 12:22:49AM +0200, Martin MOKREJŠ wrote:
> > > Hi,
> > >   I am not certain to to forward this to, so I am trying linux-usb and 
> > > ext4. Please add relevant people/lists, thank you.
> > > 
> > > # dmesg | grep "UBSAN: Undefined behaviour"
> > > [2.638843] UBSAN: Undefined behaviour in 
> > > drivers/usb/host/ehci-hub.c:873:47
> > > [8.553620] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:2612:15
> > > [   25.341048] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:1274:11
> > > [   41.460542] UBSAN: Undefined behaviour in 
> > > ./arch/x86/include/asm/atomic.h:156:9
> > > #
> > > 
> > > Full dmesg is attached.
> > > 
> > > Hope this helps,
> > > Martin
> > 
> > > [0.00] Linux version 4.5.1-default-pciehp (root@vostro) (gcc 
> > > version 5.3.0 (Gentoo 5.3.0 p1.0, pie-0.6.5) ) #1 SMP Thu Apr 14 14:17:48 
> > > CEST 2016
> > 
> > Please try 4.6-rc4, a number of UBSAN issues have been fixed there.
> 
> They are still present:
> 
> # dmesg | grep "UBSAN: Undefined behaviour"
> [3.113405] UBSAN: Undefined behaviour in 
> drivers/usb/host/ehci-hub.c:877:47
> [5.678949] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:2619:15
> [   18.345599] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:1281:11
> [   34.544363] UBSAN: Undefined behaviour in 
> ./arch/x86/include/asm/atomic.h:156:9

Ah, sorry about that, turns out this is a bug in the tool, not in the
code, see the thread from a few weeks ago in the linux-usb archives with
the subject:
Subject: function ehci_hub_control in ehci-hub.c
for all of the details.

thanks,

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

RE: [PATCH] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Yoshihiro Shimoda]

Hi Felipe,

> From: Felipe Balbi
> Sent: Thursday, April 21, 2016 7:19 PM
> 
> Yoshihiro Shimoda  writes:
> 
> > Hi Felipe,
> >
> >> From: Felipe Balbi
> >> Sent: Thursday, April 21, 2016 7:05 PM
> >>
> >> Yoshihiro Shimoda  writes:
> >>
> >> > If kernel configuration is CONFIG_USB_XHCI_PLATFORM=y and
> >> > CONFIG_USB_XHCI_RCAR is not set, R-Car Gen2/3 will cause long wait
> >> > in xhci_reset() because such SoCs need specific initialization.
> >>
> >> where is the delay coming from exactly ?
> >
> > The delay is coming from the following code:
> >
> > ret = xhci_handshake(>op_regs->command,
> > CMD_RESET, 0, 10 * 1000 * 1000);
> > if (ret)
> > return ret;
> 
> okay, and why does reset fail ?

Oops, I don't know why. So, I will investigate it.

> > And, kernel log is the following:
> >
> > [1.565605] xhci-hcd ee00.usb: xHCI Host Controller
> > [1.570636] xhci-hcd ee00.usb: new USB bus registered, assigned bus 
> > number 5
> > [   22.270160] xhci-hcd ee00.usb: can't setup: -110
> > [   22.274931] xhci-hcd ee00.usb: USB bus 5 deregistered
> > [   22.280158] xhci-hcd: probe of ee00.usb failed with error -110
> >
> > The timestamp is strange to me. But, logs of R-Car H3 (ES1.0) and
> > R-Car H2 were the same.
> 
> yeah, seems like your system timer is counting twice for each tick.

Yes, I will investigate this later.

> > Should I revise the commit log in detail?
> 
> Sure, but let's first why this is the case. It's unclear, to me at
> least, why reset fails.

I understood it. I will investigate why reset fails first.

Best regards,
Yoshihiro Shimoda

> --
> balbi
--
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: xhci-rcar: Avoid long wait in xhci_reset()

[Felipe Balbi]

Yoshihiro Shimoda  writes:

> Hi Felipe,
>
>> From: Felipe Balbi
>> Sent: Thursday, April 21, 2016 7:05 PM
>> 
>> Yoshihiro Shimoda  writes:
>> 
>> > If kernel configuration is CONFIG_USB_XHCI_PLATFORM=y and
>> > CONFIG_USB_XHCI_RCAR is not set, R-Car Gen2/3 will cause long wait
>> > in xhci_reset() because such SoCs need specific initialization.
>> 
>> where is the delay coming from exactly ?
>
> The delay is coming from the following code:
>
>   ret = xhci_handshake(>op_regs->command,
>   CMD_RESET, 0, 10 * 1000 * 1000);
>   if (ret)
>   return ret;

okay, and why does reset fail ?

> And, kernel log is the following:
>
> [1.565605] xhci-hcd ee00.usb: xHCI Host Controller
> [1.570636] xhci-hcd ee00.usb: new USB bus registered, assigned bus 
> number 5
> [   22.270160] xhci-hcd ee00.usb: can't setup: -110
> [   22.274931] xhci-hcd ee00.usb: USB bus 5 deregistered
> [   22.280158] xhci-hcd: probe of ee00.usb failed with error -110
>
> The timestamp is strange to me. But, logs of R-Car H3 (ES1.0) and
> R-Car H2 were the same.

yeah, seems like your system timer is counting twice for each tick.

> Should I revise the commit log in detail?

Sure, but let's first why this is the case. It's unclear, to me at
least, why reset fails.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Yoshihiro Shimoda]

Hi Felipe,

> From: Felipe Balbi
> Sent: Thursday, April 21, 2016 7:05 PM
> 
> Yoshihiro Shimoda  writes:
> 
> > If kernel configuration is CONFIG_USB_XHCI_PLATFORM=y and
> > CONFIG_USB_XHCI_RCAR is not set, R-Car Gen2/3 will cause long wait
> > in xhci_reset() because such SoCs need specific initialization.
> 
> where is the delay coming from exactly ?

The delay is coming from the following code:

ret = xhci_handshake(>op_regs->command,
CMD_RESET, 0, 10 * 1000 * 1000);
if (ret)
return ret;

And, kernel log is the following:

[1.565605] xhci-hcd ee00.usb: xHCI Host Controller
[1.570636] xhci-hcd ee00.usb: new USB bus registered, assigned bus 
number 5
[   22.270160] xhci-hcd ee00.usb: can't setup: -110
[   22.274931] xhci-hcd ee00.usb: USB bus 5 deregistered
[   22.280158] xhci-hcd: probe of ee00.usb failed with error -110

The timestamp is strange to me. But, logs of R-Car H3 (ES1.0) and R-Car H2 were 
the same.

Should I revise the commit log in detail?

Best regards,
Yoshihiro Shimoda

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

Re: [PATCHv2] usb: Add driver for UCSI

[Heikki Krogerus]

On Thu, Apr 21, 2016 at 11:37:34AM +0300, Mathias Nyman wrote:
> >+static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
> >+{
> >+struct ucsi *ucsi = data;
> >+struct ucsi_cci *cci;
> >+
> >+spin_lock(>dev_lock);
> >+
> >+ucsi->status = UCSI_IDLE;
> >+cci = >data->cci;
> >+
> >+/*
> >+ * REVISIT: This is not documented behavior, but all known PPMs ACK
> >+ * asynchronous events by sending notification with cleared CCI.
> >+ */
> >+if (!ucsi->data->raw_cci) {
> >+if (test_bit(EVENT_PENDING, >flags))
> >+complete(>complete);
> >+else
> >+dev_WARN(ucsi->dev, "spurious notification\n");
> >+goto out_unlock;
> >+}
> >+
> >+if (test_bit(COMMAND_PENDING, >flags)) {
> >+if (cci->busy) {
> 
> So if I understood correctly there is no risk of missing a connector change
> here because a cci->busy bit is set there can be no other ones, right?

Exactly. No other bits can be set together bit the busy bit.

> >+ucsi->status = UCSI_BUSY;
> >+complete(>complete);
> >+
> >+goto out_unlock;
> >+} else if (cci->ack_complete || cci->cmd_complete) {
> >+/* Error Indication is only valid with commands */
> >+if (cci->error && cci->cmd_complete)
> >+ucsi->status = UCSI_ERROR;
> >+
> >+ucsi->data->ctrl.raw_cmd = 0;
> >+complete(>complete);
> >+}
> >+}
> >+
> >+if (cci->connector_change) {
> >+struct ucsi_connector *con;
> >+
> >+/*
> >+ * This is workaround for buggy PPMs that create asynchronous
> >+ * event notifications before OPM has enabled them.
> >+ */
> >+if (!ucsi->connector)
> >+goto out_unlock;
> >+
> >+con = ucsi->connector + (cci->connector_change - 1);
> >+
> >+/*
> >+ * PPM will not clear the connector specific bit in Connector
> >+ * Change Indication field of CCI until the driver has ACK it,
> >+ * and the driver can not ACK it before it has been processed.
> >+ * The PPM will not generate new events before the first has
> >+ * been acknowledged, even if they are for an other connector.
> >+ * So only one event at a time.
> >+ */
> >+if (!test_and_set_bit(EVENT_PENDING, >flags))
> >+schedule_work(>work);
> 
> Was there any possibility that we miss fast consecutive connection change, 
> such
> as a new connection change work not being scheduled if the previous is not yet
> ACKed and EVENT_PENDING flag cleared?
> Or does hw/fw make sure no new connection change notification is queued before
> previous is ACKed?

The FW/HW part is called PPM (Platform Policy Manager) in UCSI terms
(straight from USB PD I guess). The OS end, our driver in practice, is
called OPM (OS Policy Manager).

Yes, the PPM will queue an asynchronous event that is received while
OPM is still processing the first one.

> >+}
> >+out_unlock:
> >+spin_unlock(>dev_lock);
> >+}
> >+
> >+static int ucsi_ack(struct ucsi *ucsi, u8 cmd)
> >+{
> >+struct ucsi_control ctrl;
> >+int ret;
> >+
> >+ctrl.cmd.cmd = UCSI_ACK_CC_CI;
> >+ctrl.cmd.length = 0;
> >+ctrl.cmd.data = cmd;
> >+ret = ucsi_acpi_cmd(ucsi, );
> >+if (ret)
> >+return ret;
> >+
> >+/* Waiting for ACK also with ACK CMD for now */
> >+ret = wait_for_completion_timeout(>complete,
> >+  msecs_to_jiffies(UCSI_TIMEOUT_MS));
> >+if (!ret)
> >+return -ETIMEDOUT;
> >+return 0;
> >+}
> >+
> >+static int ucsi_run_cmd(struct ucsi *ucsi, struct ucsi_control *ctrl,
> >+void *data, size_t size)
> >+{
> >+int timeout = UCSI_TIMEOUT_MS;
> >+u16 err_value = 0;
> >+int ret;
> >+
> >+while (test_and_set_bit(COMMAND_PENDING, >flags) && timeout--)
> >+usleep_range(1000, 2000);
> 
> Basically a home made mutex, but as the flags is needed anyways I guess this
> works as well.

Well, I'll bring back the separate mutex I had before in any case.

> >+
> >+if (!timeout)
> >+return -ETIMEDOUT;
> >+
> >+ret = ucsi_acpi_cmd(ucsi, ctrl);
> >+if (ret)
> >+goto err_clear_flag;
> >+
> >+ret = wait_for_completion_timeout(>complete,
> >+  msecs_to_jiffies(UCSI_TIMEOUT_MS));
> >+if (!ret) {
> >+ret = -ETIMEDOUT;
> >+goto err_clear_flag;
> >+}
> >+
> >+switch (ucsi->status) {
> >+case UCSI_IDLE:
> >+if (data)
> >+memcpy(data, ucsi->data->message_in, size);
> >+
> >+ret = ucsi_ack(ucsi, UCSI_ACK_CMD);
> >+break;
> >+case 

Re: [PATCH] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Felipe Balbi]

Yoshihiro Shimoda  writes:

> If kernel configuration is CONFIG_USB_XHCI_PLATFORM=y and
> CONFIG_USB_XHCI_RCAR is not set, R-Car Gen2/3 will cause long wait
> in xhci_reset() because such SoCs need specific initialization.

where is the delay coming from exactly ?

> So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
> to exit the probe function immediately.
>
> Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 
> xHCI controllers)
> Cc:  # v3.17+
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/usb/host/xhci-rcar.h | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
> index 2941a25..2afed68 100644
> --- a/drivers/usb/host/xhci-rcar.h
> +++ b/drivers/usb/host/xhci-rcar.h
> @@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
>  
>  static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
>  {
> - return 0;
> + /*
> +  * To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
> +  * disabled, this function fails.
> +  */
> + return -ENODEV;
>  }
>  #endif
>  #endif /* _XHCI_RCAR_H */
> -- 
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] usb: host: xhci-rcar: Avoid long wait in xhci_reset()

[Yoshihiro Shimoda]

If kernel configuration is CONFIG_USB_XHCI_PLATFORM=y and
CONFIG_USB_XHCI_RCAR is not set, R-Car Gen2/3 will cause long wait
in xhci_reset() because such SoCs need specific initialization.
So, this patch modifies the xhci_rcar_init_quirk() in xhci-rcar.h
to exit the probe function immediately.

Fixes: 4ac8918f3a7 (usb: host: xhci-plat: add support for the R-Car H2 and M2 
xHCI controllers)
Cc:  # v3.17+
Signed-off-by: Yoshihiro Shimoda 
---
 drivers/usb/host/xhci-rcar.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
index 2941a25..2afed68 100644
--- a/drivers/usb/host/xhci-rcar.h
+++ b/drivers/usb/host/xhci-rcar.h
@@ -24,7 +24,11 @@ static inline void xhci_rcar_start(struct usb_hcd *hcd)
 
 static inline int xhci_rcar_init_quirk(struct usb_hcd *hcd)
 {
-   return 0;
+   /*
+* To avoid wait and timeout in xhci_reset() if CONFIG_XHCI_RCAR is
+* disabled, this function fails.
+*/
+   return -ENODEV;
 }
 #endif
 #endif /* _XHCI_RCAR_H */
-- 
1.9.1

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

Re: 4.5.1: UBSAN: Undefined behaviour in ./arch/x86/include/asm/atomic.h:156:9

[Martin MOKREJŠ]

Hi Greg,

thank you for your answer.

Greg KH wrote:

On Thu, Apr 21, 2016 at 12:22:49AM +0200, Martin MOKREJŠ wrote:

Hi,
  I am not certain to to forward this to, so I am trying linux-usb and ext4. 
Please add relevant people/lists, thank you.

# dmesg | grep "UBSAN: Undefined behaviour"
[2.638843] UBSAN: Undefined behaviour in drivers/usb/host/ehci-hub.c:873:47
[8.553620] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:2612:15
[   25.341048] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:1274:11
[   41.460542] UBSAN: Undefined behaviour in 
./arch/x86/include/asm/atomic.h:156:9
#

Full dmesg is attached.

Hope this helps,
Martin



[0.00] Linux version 4.5.1-default-pciehp (root@vostro) (gcc version 
5.3.0 (Gentoo 5.3.0 p1.0, pie-0.6.5) ) #1 SMP Thu Apr 14 14:17:48 CEST 2016


Please try 4.6-rc4, a number of UBSAN issues have been fixed there.


They are still present:

# dmesg | grep "UBSAN: Undefined behaviour"
[3.113405] UBSAN: Undefined behaviour in drivers/usb/host/ehci-hub.c:877:47
[5.678949] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:2619:15
[   18.345599] UBSAN: Undefined behaviour in fs/ext4/mballoc.c:1281:11
[   34.544363] UBSAN: Undefined behaviour in 
./arch/x86/include/asm/atomic.h:156:9
#

Updated dmesg is attached.
Martin
[0.00] Linux version 4.6.0-rc4-default-pciehp (root@vostro) (gcc 
version 5.3.0 (Gentoo 5.3.0 p1.0, pie-0.6.5) ) #1 SMP Thu Apr 21 10:24:31 CEST 
2016
[0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-4.6-rc4 root=/dev/sda5 ro 
slub_debug=AFPZ pciehp.pciehp_debug=1 pciehp_debug=1 intel_idle.max_cstate=c3 
i915.i915_enable_rc6=1 usbcore.autosuspend=-1
[0.00] x86/fpu: xstate_offset[2]:  576, xstate_sizes[2]:  256
[0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point 
registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
[0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
[0.00] x86/fpu: Enabled xstate features 0x7, context size is 832 bytes, 
using 'standard' format.
[0.00] x86/fpu: Using 'eager' FPU context switches.
[0.00] e820: BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0009d3ff] usable
[0.00] BIOS-e820: [mem 0x0009d400-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000e-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0x1fff] usable
[0.00] BIOS-e820: [mem 0x2000-0x201f] reserved
[0.00] BIOS-e820: [mem 0x2020-0x3fff] usable
[0.00] BIOS-e820: [mem 0x4000-0x401f] reserved
[0.00] BIOS-e820: [mem 0x4020-0xda4e4fff] usable
[0.00] BIOS-e820: [mem 0xda4e5000-0xda527fff] ACPI NVS
[0.00] BIOS-e820: [mem 0xda528000-0xda792fff] usable
[0.00] BIOS-e820: [mem 0xda793000-0xda966fff] reserved
[0.00] BIOS-e820: [mem 0xda967000-0xdaa88fff] usable
[0.00] BIOS-e820: [mem 0xdaa89000-0xdad67fff] reserved
[0.00] BIOS-e820: [mem 0xdad68000-0xdafe7fff] ACPI NVS
[0.00] BIOS-e820: [mem 0xdafe8000-0xdaff] ACPI data
[0.00] BIOS-e820: [mem 0xdb80-0xdf9f] reserved
[0.00] BIOS-e820: [mem 0xf800-0xfbff] reserved
[0.00] BIOS-e820: [mem 0xfec0-0xfec00fff] reserved
[0.00] BIOS-e820: [mem 0xfed0-0xfed03fff] reserved
[0.00] BIOS-e820: [mem 0xfed1c000-0xfed1] reserved
[0.00] BIOS-e820: [mem 0xfee0-0xfee00fff] reserved
[0.00] BIOS-e820: [mem 0xff00-0x] reserved
[0.00] BIOS-e820: [mem 0x0001-0x00041fdf] usable
[0.00] NX (Execute Disable) protection: active
[0.00] SMBIOS 2.6 present.
[0.00] DMI: Dell Inc. Vostro 3550/, BIOS A12 02/18/2014
[0.00] e820: update [mem 0x-0x0fff] usable ==> reserved
[0.00] e820: remove [mem 0x000a-0x000f] usable
[0.00] e820: last_pfn = 0x41fe00 max_arch_pfn = 0x4
[0.00] MTRR default type: uncachable
[0.00] MTRR fixed ranges enabled:
[0.00]   0-9 write-back
[0.00]   A-B uncachable
[0.00]   C-C write-protect
[0.00]   D-E7FFF uncachable
[0.00]   E8000-F write-protect
[0.00] MTRR variable ranges enabled:
[0.00]   0 base 0 mask C write-back
[0.00]   1 base 4 mask FE000 write-back
[0.00]   2 base 0DB80 mask FFF80 uncachable
[0.00]   3 base 0DC00 mask FFC00 uncachable
[0.00]   4 base 0E000 mask FE000 uncachable
[0.00]   5 

Re: [PATCH 1/2] usb: chipidea: add flag CI_HDRC_DP_ALWAYS_PULLUP

[Peter Chen]

On Tue, Apr 19, 2016 at 12:20:18PM +0530, maitysancha...@gmail.com wrote:
> Hello Peter,
> 
> On 16-04-19 10:40:20, Peter Chen wrote:
> > On Mon, Apr 18, 2016 at 02:36:06PM +0530, maitysancha...@gmail.com wrote:
> > > Hello Peter,
> > > 
> > > I tested this on a Colibri Vybrid VF61 module with the patches applied
> > > on top of for-next branch.
> > > 
> > > root@colibri-vf:~# uname -a
> > > Linux colibri-vf 4.6.0-rc1-00044-gc76529e-dirty #120 Mon Apr 18 13:46:34 
> > > IST 2016 armv7l GNU/Linux
> > > 
> > > Host and client mode work only on boot up. Trying to change the mode by 
> > > disconnecting/reconnecting
> > > the cable or plugging in a USB device does not work. Also when the USB 
> > > client mode is working on
> > > boot up, disconnecting the cable results in the below stack trace.
> > > 
> > > root@colibri-vf:~# ping 192.168.11.234
> > > 
> > > PING 192.168.11.234 (192.168.11.234): 56 data bytes
> > > 64 bytes from 192.168.11.234: seq=0 ttl=64 time=5.399 ms
> > > ^C
> > > --- 192.168.11.234 ping statistics ---
> > > 1 packets transmitted, 1 packets received, 0% packet loss
> > > round-trip min/avg/max = 5.399/5.399/5.399 ms
> > > - On Cable disconnection-
> > > root@colibri-vf:~# [   23.923607] irq 35: nobody cared (try booting with 
> > > the "irqpoll" option)
> > > [   23.930354] CPU: 0 PID: 0 Comm: swapper Not tainted 
> > > 4.6.0-rc1-00044-gc76529e-dirty #120
> > > [   23.938359] Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
> > > [   23.944807] Backtrace: 
> > > [   23.947327] [<8010b56c>] (dump_backtrace) from [<8010b764>] 
> > > (show_stack+0x18/0x1c)
> > > [   23.954900]  r7:0023 r6: r5: r4:8f4ad240
> > > [   23.960644] [<8010b74c>] (show_stack) from [<803a1c00>] 
> > > (dump_stack+0x24/0x28)
> > > [   23.967893] [<803a1bdc>] (dump_stack) from [<80148150>] 
> > > (__report_bad_irq+0x30/0xb8)
> > > [   23.975655] [<80148120>] (__report_bad_irq) from [<801484a8>] 
> > > (note_interrupt+0x260/0x2b0)
> > > [   23.983918]  r5: r4:8f4ad240
> > > [   23.987538] [<80148248>] (note_interrupt) from [<801460cc>] 
> > > (handle_irq_event_percpu+0xe0/0x154)
> > > [   23.996328]  r10: r9:80b35683 r8:8f4ad240 r7:0023 
> > > r6: r5:
> > > [   24.004242]  r4: r3:
> > > [   24.007860] [<80145fec>] (handle_irq_event_percpu) from [<80146170>] 
> > > (handle_irq_event+0x30/0x44)
> > > [   24.016732]  r10:80b0210c r9:90003100 r8:8f406000 r7:80b01f10 
> > > r6: r5:80b14ed0
> > > [   24.024646]  r4:8f4ad240
> > > [   24.027206] [<80146140>] (handle_irq_event) from [<80148d2c>] 
> > > (handle_fasteoi_irq+0xa8/0x170)
> > > [   24.035737]  r5:80b14ed0 r4:8f4ad240
> > > [   24.039363] [<80148c84>] (handle_fasteoi_irq) from [<801457a0>] 
> > > (generic_handle_irq+0x2c/0x3c)
> > > [   24.047973]  r7:80b01f10 r6: r5:0023 r4:80b14d8c
> > > [   24.053706] [<80145774>] (generic_handle_irq) from [<80145a30>] 
> > > (__handle_domain_irq+0x5c/0xb0)
> > > [   24.062420] [<801459d4>] (__handle_domain_irq) from [<801013d0>] 
> > > (gic_handle_irq+0x50/0x84)
> > > [   24.070771]  r9:90003100 r8:80b01e00 r7:90002100 r6:9000210c 
> > > r5:80b023b0 r4:80b14ec0
> > > [   24.078611] [<80101380>] (gic_handle_irq) from [<8010c214>] 
> > > (__irq_svc+0x54/0x70)
> > > [   24.086101] Exception stack(0x80b01e00 to 0x80b01e48)
> > > [   24.091173] 1e00: 80b360c0 0020 80b36080  0002 
> > > 0010  
> > > [   24.099363] 1e20: 8f406000 90003100 80b0210c 80b01eac 0020 
> > > 80b01e50 8011cf18 8011caec
> > > [   24.107545] 1e40: 600c0113 
> > > [   24.111039]  r9:90003100 r8:8f406000 r7:80b01e34 r6: 
> > > r5:600c0113 r4:8011caec
> > > [   24.118892] [<8011ca4c>] (__do_softirq) from [<8011cf18>] 
> > > (irq_exit+0xb8/0xf4)
> > > [   24.126114]  r10:80b0210c r9:90003100 r8:8f406000 r7: 
> > > r6: r5:0010
> > > [   24.134028]  r4:80b14d8c
> > > [   24.136587] [<8011ce60>] (irq_exit) from [<80145a34>] 
> > > (__handle_domain_irq+0x60/0xb0)
> > > [   24.144429] [<801459d4>] (__handle_domain_irq) from [<801013d0>] 
> > > (gic_handle_irq+0x50/0x84)
> > > [   24.152782]  r9:90003100 r8:80b01f10 r7:90002100 r6:9000210c 
> > > r5:80b023b0 r4:80b14ec0
> > > [   24.160621] [<80101380>] (gic_handle_irq) from [<8010c214>] 
> > > (__irq_svc+0x54/0x70)
> > > [   24.168110] Exception stack(0x80b01f10 to 0x80b01f58)
> > > [   24.173175] 1f00: 0001 
> > >   80115ac0
> > > [   24.181365] 1f20:  80b0210c  80b29c78 80b02114 
> > > 80b0 80b0210c 80b01f6c
> > > [   24.189554] 1f40: 80b01f70 80b01f60 80108200 80108204 600c0013 
> > > [   24.196173]  r9:80b0 r8:80b02114 r7:80b01f44 r6: 
> > > r5:600c0013 r4:80108204
> > > [   24.204026] [<801081c4>] (arch_cpu_idle) from 

Re: [PATCHv2] usb: Add driver for UCSI

[Mathias Nyman]

On 20.04.2016 13:19, Heikki Krogerus wrote:

USB Type-C Connector System Software Interface (UCSI) is
specification that defines the registers and data structures
that can be used to control USB Type-C ports on a system.
UCSI is used on several Intel Broxton SoC based platforms.
Things that UCSI can be used to control include at least USB
Data Role swapping, Power Role swapping and controlling of
Alternate Modes on top of providing general details about
the port and the partners that are attached to it.

The initial purpose of the UCSI driver is to make sure USB
is in host mode on desktop and server systems that are USB
dual role capable, and provide UCSI interface.

The goal is to integrate the driver later to an USB Type-C
framework for Linux kernel, and at the same time add support
for more extensive USB Type-C port control that UCSI offers,
for example data role swapping, power role swapping,
Alternate Mode control etc.

The UCSI specification is public can be obtained from here:
http://www.intel.com/content/www/us/en/io/universal-serial-bus/usb-type-c-ucsi-spec.html

Signed-off-by: Heikki Krogerus 


Some small comments inline.


---
  drivers/usb/misc/Kconfig  |  26 +++
  drivers/usb/misc/Makefile |   1 +
  drivers/usb/misc/ucsi.c   | 467 ++
  drivers/usb/misc/ucsi.h   | 215 +
  4 files changed, 709 insertions(+)
  create mode 100644 drivers/usb/misc/ucsi.c
  create mode 100644 drivers/usb/misc/ucsi.h

Changes since v1:
- Now just always using host mode. Got rid of the modules parameter.
- Replaced the atomic_t with bitop "flags". It's used to sync
   communication with the PPMs.
- Refactored the notifier handler based on suggestions from Felipe
   and Andy
- Added data structure for CCI as suggested by Felipe


...



+static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
+{
+   struct ucsi *ucsi = data;
+   struct ucsi_cci *cci;
+
+   spin_lock(>dev_lock);
+
+   ucsi->status = UCSI_IDLE;
+   cci = >data->cci;
+
+   /*
+* REVISIT: This is not documented behavior, but all known PPMs ACK
+* asynchronous events by sending notification with cleared CCI.
+*/
+   if (!ucsi->data->raw_cci) {
+   if (test_bit(EVENT_PENDING, >flags))
+   complete(>complete);
+   else
+   dev_WARN(ucsi->dev, "spurious notification\n");
+   goto out_unlock;
+   }
+
+   if (test_bit(COMMAND_PENDING, >flags)) {
+   if (cci->busy) {


So if I understood correctly there is no risk of missing a connector change
here because a cci->busy bit is set there can be no other ones, right?


+   ucsi->status = UCSI_BUSY;
+   complete(>complete);
+
+   goto out_unlock;
+   } else if (cci->ack_complete || cci->cmd_complete) {
+   /* Error Indication is only valid with commands */
+   if (cci->error && cci->cmd_complete)
+   ucsi->status = UCSI_ERROR;
+
+   ucsi->data->ctrl.raw_cmd = 0;
+   complete(>complete);
+   }
+   }
+
+   if (cci->connector_change) {
+   struct ucsi_connector *con;
+
+   /*
+* This is workaround for buggy PPMs that create asynchronous
+* event notifications before OPM has enabled them.
+*/
+   if (!ucsi->connector)
+   goto out_unlock;
+
+   con = ucsi->connector + (cci->connector_change - 1);
+
+   /*
+* PPM will not clear the connector specific bit in Connector
+* Change Indication field of CCI until the driver has ACK it,
+* and the driver can not ACK it before it has been processed.
+* The PPM will not generate new events before the first has
+* been acknowledged, even if they are for an other connector.
+* So only one event at a time.
+*/
+   if (!test_and_set_bit(EVENT_PENDING, >flags))
+   schedule_work(>work);


Was there any possibility that we miss fast consecutive connection change, such
as a new connection change work not being scheduled if the previous is not yet
ACKed and EVENT_PENDING flag cleared?
Or does hw/fw make sure no new connection change notification is queued before
previous is ACKed?  


+   }
+out_unlock:
+   spin_unlock(>dev_lock);
+}
+
+static int ucsi_ack(struct ucsi *ucsi, u8 cmd)
+{
+   struct ucsi_control ctrl;
+   int ret;
+
+   ctrl.cmd.cmd = UCSI_ACK_CC_CI;
+   ctrl.cmd.length = 0;
+   ctrl.cmd.data = cmd;
+   ret = ucsi_acpi_cmd(ucsi, );
+   if (ret)
+   return ret;
+
+   /* Waiting for ACK also with ACK CMD 

Re: dwc3 initiated xhci probe problem in arm64 4.4 kernel due to DMA setup

[Felipe Balbi]

Hi,

Grygorii Strashko  writes:
> On 04/15/2016 02:31 PM, Felipe Balbi wrote:
>> Catalin Marinas  writes:
>>> On Fri, Apr 15, 2016 at 01:30:01PM +0300, Felipe Balbi wrote:
 Catalin Marinas  writes:
> On Fri, Apr 15, 2016 at 11:01:08AM +0100, Catalin Marinas wrote:
>> On Fri, Apr 15, 2016 at 12:49:15PM +0300, Felipe Balbi wrote:
>>> Catalin Marinas  writes:
 On Thu, Apr 14, 2016 at 12:46:47PM +, David Fisher wrote:
> dwc3 is in dual-role, with "synopsys,dwc3" specified in DT.
>
> When xhci is probed, initiated from dwc3/host.c (not DT), we get :
> xhci-hcd: probe of xhci-hcd.7.auto failed with error -5
> This -EIO error originated from inside dma_set_mask() down in 
> include/asm-generic/dma-mapping-common.h
>
> If "generic-xhci" is specified in DT instead, it probes fine as a 
> host-only dwc3
> The difference between DT initiated probe and dwc3 initiated probe is 
> that
> when DT initiated probe gets to dma_supported, dma_supported is
> implemented by swiotlb_dma_supported (previously set up by a DT call 
> to arch_dma_setup_ops).
> Whereas when dwc3 initiated xhci probe gets to dma_supported, 
> arch_dma_setup_ops has not been called
> and dma_supported is only implemented by __dummy_dma_supported, 
> returning 0.
>
> Bisecting finds the "bad" commit as
> 1dccb598df549d892b6450c261da54cdd7af44b4  (inbetween 4.4-rc1 and 
> 4.4-rc2)
> --- a/arch/arm64/include/asm/dma-mapping.h
> --- a/arch/arm64/mm/dma-mapping.c
>
> Previous to this commit, dma_ops = _dma_ops was done in 
> arm64_dma_init
> After this commit, the assignment is only done in arch_dma_setup_ops.

 This restriction was added on purpose and the arm64 __generic_dma_ops()
 now has a comment:

/*
 * We expect no ISA devices, and all other DMA masters are 
 expected to
 * have someone call arch_setup_dma_ops at device creation time.
 */
>>>
>>> how ?
>>
>> Usually by calling arch_setup_dma_ops().
>
> Or of_dma_configure(), I forgot to mention this (see the
> pci_dma_configure() function as an example).

 the device is manually created, there's not OF/DT for it ;-)
>>>
>>> As for PCI, the created device doesn't have a node but the bridge does.
>>> Something like below, completely untested:
>>>
>>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
>>> index c679f63783ae..96d8babd7f23 100644
>>> --- a/drivers/usb/dwc3/host.c
>>> +++ b/drivers/usb/dwc3/host.c
>>> @@ -32,7 +32,10 @@ int dwc3_host_init(struct dwc3 *dwc)
>>> return -ENOMEM;
>>> }
>>>   
>>> -   dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask);
>>> +   if (IS_ENABLED(CONFIG_OF) && dwc->dev->of_node)
>>> +   of_dma_configure(>dev, dwc->dev->of_node);
>>> +   else
> {
>>> +   dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask);
>
>xhci->dev.dma_mask  = dwc->dev->dma_mask;
>  xhci->dev.dma_parms = dwc->dev->dma_parms;
> }
>
>> 
>> assuming this works fine for all users, I don't have an issue with
>> it. Grygorii has just been working on DMA setup for k2 and dwc3, so I
>> want to make sure this won't regress those devices.
>> 
>
> And this is the same approach I've proposed here:
> https://lkml.org/lkml/2016/3/31/609
>
> :)
>
> So, final change could be:
>
> ---
> From c68225e97e8c9505aca4ceab19a0d8e4dde31b73 Mon Sep 17 00:00:00 2001
> From: Grygorii Strashko 
> Date: Thu, 31 Mar 2016 19:40:52 +0300
> Subject: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev
>
> Now not all DMA paremters configured properly for "xhci-hcd" platform
> device which is created manually. For example: dma_pfn_offset, dam_ops
> and iommu configuration will not corresponds "dwc3" devices
> configuration. As result, this will cause problems like wrong DMA
> addresses translation on platforms with LPAE enabled like Keystone 2.
>
> When platform is using DT boot mode the DMA configuration will be
> parsed and applied from DT, so, to fix this issue, reuse
> of_dma_configure() API and retrieve DMA configuartion for "xhci-hcd"
> from DWC3 device node.
>
> Signed-off-by: Grygorii Strashko 
> ---
>  drivers/usb/dwc3/host.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index c679f63..93c8ef9 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -17,6 +17,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #include "core.h"
>  
> @@ -32,12 +33,7 @@ int dwc3_host_init(struct 

Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core

[Peter Chen]

On Tue, Apr 05, 2016 at 05:05:12PM +0300, Roger Quadros wrote:
> It provides APIs for the following tasks
> 
> - Registering an OTG/dual-role capable controller
> - Registering Host and Gadget controllers to OTG core
> - Providing inputs to and kicking the OTG state machine
> 
> Provide a dual-role device (DRD) state machine.
> DRD mode is a reduced functionality OTG mode. In this mode
> we don't support SRP, HNP and dynamic role-swap.
> 
> In DRD operation, the controller mode (Host or Peripheral)
> is decided based on the ID pin status. Once a cable plug (Type-A
> or Type-B) is attached the controller selects the state
> and doesn't change till the cable in unplugged and a different
> cable type is inserted.
> 
> As we don't need most of the complex OTG states and OTG timers
> we implement a lean DRD state machine in usb-otg.c.
> The DRD state machine is only interested in 2 hardware inputs
> 'id' and 'b_sess_vld'.
> 
> Signed-off-by: Roger Quadros 
> ---
>  drivers/usb/common/Makefile  |2 +-
>  drivers/usb/common/usb-otg.c | 1058 
> ++
>  drivers/usb/common/usb-otg.h |   71 +++
>  drivers/usb/core/Kconfig |2 +-
>  include/linux/usb/gadget.h   |2 +
>  include/linux/usb/hcd.h  |1 +
>  include/linux/usb/otg-fsm.h  |7 +
>  include/linux/usb/otg.h  |  154 +-
>  8 files changed, 1292 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/usb/common/usb-otg.c
>  create mode 100644 drivers/usb/common/usb-otg.h
> 
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index f8f2c88..730d928 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -7,5 +7,5 @@ usb-common-y+= common.o
>  usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>  
>  obj-$(CONFIG_USB_ULPI_BUS)   += ulpi.o
> -usbotg-y := usb-otg-fsm.o
> +usbotg-y := usb-otg.o usb-otg-fsm.o
>  obj-$(CONFIG_USB_OTG)+= usbotg.o
> diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
> new file mode 100644
> index 000..41e762a
> --- /dev/null
> +++ b/drivers/usb/common/usb-otg.c
> @@ -0,0 +1,1058 @@
> +/**
> + * drivers/usb/common/usb-otg.c - USB OTG core
> + *
> + * Copyright (C) 2016 Texas Instruments Incorporated - http://www.ti.com
> + * Author: Roger Quadros 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "usb-otg.h"
> +
> +struct otg_gcd {
> + struct usb_gadget *gadget;
> + struct otg_gadget_ops *ops;
> +};
> +
> +/* OTG device list */
> +LIST_HEAD(otg_list);
> +static DEFINE_MUTEX(otg_list_mutex);
> +
> +/* Hosts and Gadgets waiting for OTG controller */
> +struct otg_wait_data {
> + struct device *dev; /* OTG controller device */
> +
> + struct otg_hcd primary_hcd;
> + struct otg_hcd shared_hcd;
> + struct otg_gcd gcd;
> + struct list_head list;
> +};
> +
> +LIST_HEAD(wait_list);
> +static DEFINE_MUTEX(wait_list_mutex);
> +
> +static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
> +{
> + if (!hcd->primary_hcd)
> + return 1;
> + return hcd == hcd->primary_hcd;
> +}

Just find there is already a usb_hcd_is_primary_hcd at hcd.c,
would you use it directly?

Peter
> +
> +/**
> + * Check if the OTG device is in our wait list and return
> + * otg_wait_data, else NULL.
> + *
> + * wait_list_mutex must be held.
> + */
> +static struct otg_wait_data *usb_otg_get_wait(struct device *otg_dev)
> +{
> + struct otg_wait_data *wait;
> +
> + if (!otg_dev)
> + return NULL;
> +
> + /* is there an entry for this otg_dev ?*/
> + list_for_each_entry(wait, _list, list) {
> + if (wait->dev == otg_dev)
> + return wait;
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + * Add the hcd to our wait list
> + */
> +static int usb_otg_hcd_wait_add(struct device *otg_dev, struct usb_hcd *hcd,
> + unsigned int irqnum, unsigned long irqflags,
> + struct otg_hcd_ops *ops)
> +{
> + struct otg_wait_data *wait;
> + int ret = -EINVAL;
> +
> + mutex_lock(_list_mutex);
> +
> + wait = usb_otg_get_wait(otg_dev);
> + if (!wait) {
> + /* Not yet in wait list? allocate and add */
> + wait = kzalloc(sizeof(*wait), GFP_KERNEL);
> + if (!wait) {
> + ret = -ENOMEM;
> +  

Re: What are Shared HCD and Companion Controller(HCD?)

[Peter Chen]

On Tue, Apr 19, 2016 at 10:20:33AM -0400, Alan Stern wrote:
> On Tue, 19 Apr 2016, Felipe Balbi wrote:
> 
> > Hi,
> > 
> > Peter Chen  writes:
> > > Hi all,
> > >
> > > When I review the patch [1] for adding companion controller support for
> > > OTG framework, I am puzzled several concepts, like shared hcd and
> > > companion controller, companion hcd, companion port, would someone
> > > please explain them? And why EHCI/OHCI do not use shared hcd, but
> > > xHCI uses it? Thanks.
> > 
> > xHCI is modeled as two separate buses (High-speed and Super-speed) which
> > *share* the same IP block. In the case of EHCI/OHCI, there is a port
> > being handed over to the a completely separate IP. OHCI is EHCI's
> > companion for non-HS signalling.
> 
> That's right.  In more detail:
> 
> Companion controller and companion hcd are the same thing.  They exist
> because EHCI can only handle high speed connections; it can't handle
> full speed or low speed.  When a USB-1.1 device is plugged into an EHCI
> controller, the EHCI controller can't handle it and so the connection
> is handed over to the companion controller, which is either OHCI or
> UHCI.
> 

I have one more question, if hcd codes takes primary hcd as USB2 and
shared hcd as USB3, are there any problems if EHCI (or OHCI) as primary
hcd and OHCI (EHCI) as shared hcd like [1], current hcd code seems to
use hcd->shared_hcd at drivers/usb/core/port.c

[1] http://www.spinics.net/lists/linux-usb/msg139344.html
-- 

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

RE: [PATCH v6 09/12] usb: gadget: udc: adapt to OTG core

[Jun Li]

Hi,

...
> 
>  /**
> + * usb_gadget_start - start the usb gadget controller and connect to
> +bus
> + * @gadget: the gadget device to start
> + *
> + * This is external API for use by OTG core.
> + *
> + * Start the usb device controller and connect to bus (enable pull).
> + */
> +static int usb_gadget_start(struct usb_gadget *gadget) {
> + int ret;
> + struct usb_udc *udc = NULL;
> +
> + dev_dbg(>dev, "%s\n", __func__);
> + mutex_lock(_lock);
> + list_for_each_entry(udc, _list, list)
> + if (udc->gadget == gadget)
> + goto found;
> +
> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> + __func__);
> + mutex_unlock(_lock);
> + return -EINVAL;
> +
> +found:
> + ret = usb_gadget_udc_start(udc);
> + if (ret)
> + dev_err(>dev, "USB Device Controller didn't start: %d\n",
> + ret);
> + else
> + usb_udc_connect_control(udc);

For drd, it's fine, but for real otg, gadget connect should be done
by loc_conn() instead of gadget start.

> +
> + mutex_unlock(_lock);
> +
> + return ret;
> +}
> +
> +/**
> + * usb_gadget_stop - disconnect from bus and stop the usb gadget
> + * @gadget: The gadget device we want to stop
> + *
> + * This is external API for use by OTG core.
> + *
> + * Disconnect from the bus (disable pull) and stop the
> + * gadget controller.
> + */
> +static int usb_gadget_stop(struct usb_gadget *gadget) {
> + struct usb_udc *udc = NULL;
> +
> + dev_dbg(>dev, "%s\n", __func__);
> + mutex_lock(_lock);
> + list_for_each_entry(udc, _list, list)
> + if (udc->gadget == gadget)
> + goto found;
> +
> + dev_err(gadget->dev.parent, "%s: gadget not registered.\n",
> + __func__);
> + mutex_unlock(_lock);
> + return -EINVAL;
> +
> +found:
> + usb_gadget_disconnect(udc->gadget);

Likewise, gadget disconnect also should be done by loc_conn()
instead of gadget stop.

> + udc->driver->disconnect(udc->gadget);
> + usb_gadget_udc_stop(udc);
> + mutex_unlock(_lock);
> +
> + return 0;
> +}
> +

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