Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection

[Heikki Krogerus]

Hi Hans,

On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 03-09-18 15:36, Heikki Krogerus wrote:
> > PI3USB30532 is used for muxing the port to DisplayPort on
> > CHT platforms, so changing the connection ID so that the
> > mux will get assigned to the alternate mode device and not
> > the port device. Connection ID "typec-mux" is now reserved
> > for Accessory Modes.
> > 
> > Signed-off-by: Heikki Krogerus 
> > ---
> >   drivers/platform/x86/intel_cht_int33fe.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
> > b/drivers/platform/x86/intel_cht_int33fe.c
> > index b0cef48f77af..f8c881f871f9 100644
> > --- a/drivers/platform/x86/intel_cht_int33fe.c
> > +++ b/drivers/platform/x86/intel_cht_int33fe.c
> > @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
> > data->connections[0].id = "typec-switch";
> > data->connections[1].endpoint[0] = "i2c-fusb302";
> > data->connections[1].endpoint[1] = "i2c-pi3usb30532";
> > -   data->connections[1].id = "typec-mux";
> > +   data->connections[1].id = "idff01m01";
> 
> Hmm, so the mux will start in open connection and needs to
> be moved from open to TYPEC_STATE_USB when doing USB3, I assume
> the alternative driver is only going to come into play when
> actually using a DP over Type-C capable device/dongle, so what
> will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ?

Ah, for some reason I assumed that the orientation switch will take
care of that, but of course it does not. So we'll keep the existing
connections, and just add a new one for the alt mode device.

Sorry for this.


Thanks,

-- 
heikki

Re: [PATCH] usb/typec: fix kernel-doc notation warning for typec_match_altmode

[Heikki Krogerus]

On Mon, Sep 03, 2018 at 12:58:35PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap 
> 
> Fix kernel-doc warning for missing function parameter 'mode' description:
> 
> ../drivers/usb/typec/bus.c:268: warning: Function parameter or member 'mode' 
> not described in 'typec_match_altmode'
> 
> Also fix typos for same function documentation.
> 
> Fixes: 8a37d87d72f0 ("usb: typec: Bus type for alternate modes")
> 
> Signed-off-by: Randy Dunlap 
> Cc: Heikki Krogerus 

Acked-by: Heikki Krogerus 

> ---
>  drivers/usb/typec/bus.c |7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> --- lnx-419-rc2.orig/drivers/usb/typec/bus.c
> +++ lnx-419-rc2/drivers/usb/typec/bus.c
> @@ -255,12 +255,13 @@ EXPORT_SYMBOL_GPL(typec_altmode_unregist
>  /* API for the port drivers */
>  
>  /**
> - * typec_match_altmode - Match SVID to an array of alternate modes
> + * typec_match_altmode - Match SVID and mode to an array of alternate modes
>   * @altmodes: Array of alternate modes
> - * @n: Number of elements in the array, or -1 for NULL termiated arrays
> + * @n: Number of elements in the array, or -1 for NULL terminated arrays
>   * @svid: Standard or Vendor ID to match with
> + * @mode: Mode to match with
>   *
> - * Return pointer to an alternate mode with SVID mathing @svid, or NULL when 
> no
> + * Return pointer to an alternate mode with SVID matching @svid, or NULL 
> when no
>   * match is found.
>   */
>  struct typec_altmode *typec_match_altmode(struct typec_altmode **altmodes,

-- 
heikki

Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection

[Hans de Goede]

HI,

On 04-09-18 09:37, Heikki Krogerus wrote:

Hi Hans,

On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote:

Hi,

On 03-09-18 15:36, Heikki Krogerus wrote:

PI3USB30532 is used for muxing the port to DisplayPort on
CHT platforms, so changing the connection ID so that the
mux will get assigned to the alternate mode device and not
the port device. Connection ID "typec-mux" is now reserved
for Accessory Modes.

Signed-off-by: Heikki Krogerus 
---
   drivers/platform/x86/intel_cht_int33fe.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index b0cef48f77af..f8c881f871f9 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[0].id = "typec-switch";
data->connections[1].endpoint[0] = "i2c-fusb302";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[1].id = "typec-mux";
+   data->connections[1].id = "idff01m01";


Hmm, so the mux will start in open connection and needs to
be moved from open to TYPEC_STATE_USB when doing USB3, I assume
the alternative driver is only going to come into play when
actually using a DP over Type-C capable device/dongle, so what
will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ?


Ah, for some reason I assumed that the orientation switch will take
care of that, but of course it does not. So we'll keep the existing
connections, and just add a new one for the alt mode device.


Ok, so I assume you are going to send a v3 for this ?

Regards,

Hans

Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection

[Heikki Krogerus]

On Tue, Sep 04, 2018 at 09:40:35AM +0200, Hans de Goede wrote:
> HI,
> 
> On 04-09-18 09:37, Heikki Krogerus wrote:
> > Hi Hans,
> > 
> > On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 03-09-18 15:36, Heikki Krogerus wrote:
> > > > PI3USB30532 is used for muxing the port to DisplayPort on
> > > > CHT platforms, so changing the connection ID so that the
> > > > mux will get assigned to the alternate mode device and not
> > > > the port device. Connection ID "typec-mux" is now reserved
> > > > for Accessory Modes.
> > > > 
> > > > Signed-off-by: Heikki Krogerus 
> > > > ---
> > > >drivers/platform/x86/intel_cht_int33fe.c | 2 +-
> > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
> > > > b/drivers/platform/x86/intel_cht_int33fe.c
> > > > index b0cef48f77af..f8c881f871f9 100644
> > > > --- a/drivers/platform/x86/intel_cht_int33fe.c
> > > > +++ b/drivers/platform/x86/intel_cht_int33fe.c
> > > > @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client 
> > > > *client)
> > > > data->connections[0].id = "typec-switch";
> > > > data->connections[1].endpoint[0] = "i2c-fusb302";
> > > > data->connections[1].endpoint[1] = "i2c-pi3usb30532";
> > > > -   data->connections[1].id = "typec-mux";
> > > > +   data->connections[1].id = "idff01m01";
> > > 
> > > Hmm, so the mux will start in open connection and needs to
> > > be moved from open to TYPEC_STATE_USB when doing USB3, I assume
> > > the alternative driver is only going to come into play when
> > > actually using a DP over Type-C capable device/dongle, so what
> > > will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ?
> > 
> > Ah, for some reason I assumed that the orientation switch will take
> > care of that, but of course it does not. So we'll keep the existing
> > connections, and just add a new one for the alt mode device.
> 
> Ok, so I assume you are going to send a v3 for this ?

Yes. Let me know if there is any other problems.


Thanks,

-- 
heikki

Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection

[Hans de Goede]

Hi,

On 04-09-18 09:44, Heikki Krogerus wrote:

On Tue, Sep 04, 2018 at 09:40:35AM +0200, Hans de Goede wrote:

HI,

On 04-09-18 09:37, Heikki Krogerus wrote:

Hi Hans,

On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote:

Hi,

On 03-09-18 15:36, Heikki Krogerus wrote:

PI3USB30532 is used for muxing the port to DisplayPort on
CHT platforms, so changing the connection ID so that the
mux will get assigned to the alternate mode device and not
the port device. Connection ID "typec-mux" is now reserved
for Accessory Modes.

Signed-off-by: Heikki Krogerus 
---
drivers/platform/x86/intel_cht_int33fe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index b0cef48f77af..f8c881f871f9 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[0].id = "typec-switch";
data->connections[1].endpoint[0] = "i2c-fusb302";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[1].id = "typec-mux";
+   data->connections[1].id = "idff01m01";


Hmm, so the mux will start in open connection and needs to
be moved from open to TYPEC_STATE_USB when doing USB3, I assume
the alternative driver is only going to come into play when
actually using a DP over Type-C capable device/dongle, so what
will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ?


Ah, for some reason I assumed that the orientation switch will take
care of that, but of course it does not. So we'll keep the existing
connections, and just add a new one for the alt mode device.


Ok, so I assume you are going to send a v3 for this ?


Yes. Let me know if there is any other problems.


Otherwise the series looks good to me.

Regards,

Hans

[PATCH] USB: change dev_WARN to dev_err triggerable from user space

[Oliver Neukum]

For those people who run with panic_on_warn a WARN() triggered
from user space is a DOS. It is worth returning to dev_err()

Signed-off-by: Oliver Neukum 
Fixes: 0cb54a3e47cb4baf0bc7463f0a64cfeae5e35697
Reported-by: syzbot+843efa30c8821bd69...@syzkaller.appspotmail.com
---
 drivers/usb/core/urb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index f51750bcd152..3fe65a774e6c 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -475,7 +475,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
 
/* Check that the pipe's type matches the endpoint's type */
if (usb_urb_ep_type_check(urb))
-   dev_WARN(&dev->dev, "BOGUS urb xfer, pipe %x != type %x\n",
+   dev_err(&dev->dev, "BOGUS urb xfer, pipe %x != type %x\n",
usb_pipetype(urb->pipe), pipetypes[xfertype]);
 
/* Check against a simple/standard policy */
@@ -499,7 +499,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
 
/* warn if submitter gave bogus flags */
if (allowed != urb->transfer_flags)
-   dev_WARN(&dev->dev, "BOGUS urb flags, %x --> %x\n",
+   dev_err(&dev->dev, "BOGUS urb flags, %x --> %x\n",
urb->transfer_flags, allowed);
 
/*
-- 
2.16.4

Re: Nothing in /sys/class/udc

[Felipe Balbi]

Hi,

Ranran  writes:
> On Mon, Sep 3, 2018 at 4:50 PM Ranran  wrote:
>>
>> On Mon, Sep 3, 2018 at 4:22 PM Ranran  wrote:
>> >
>> > On Mon, Sep 3, 2018 at 3:40 PM Greg KH  wrote:
>> > >
>> > > On Mon, Sep 03, 2018 at 09:39:14AM +0300, Ranran wrote:
>> > > > Hello,
>> > > >
>> > > > I try to add gadget configfs as described in:
>> > > > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt
>> > > > Yet, I find nothing in /sys/class/udc:
>> > > >
>> > > > user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al
>> > > > total 0
>> > > > drwxr-xr-x  2 root root 0 Sep  3 00:30 .
>> > > > drwxr-xr-x 58 root root 0 Sep  3 00:30 ..
>> > > >
>> > > > I also don't have dwc2, but dwc3:
>> > > > user@user-VirtualBox:~/tegra$ lsmod | grep dw
>> > > > dwc3   90112  0
>> > > > ulpi   16384  1 dwc3
>> > > > udc_core   24576  2 dwc3,libcomposite
>> > > > user@user-VirtualBox:~/tegra$
>> > > >
>> > > > Kernel is 4.4.50.
>> > >
>> > > Why do you think the dwc3 driver will work here?  Do you really have
>> > > this hardware being emulated?
>> > >
>> > Hi,
>> >
>> > I check it with Intel's E3900.
>> > In datasheet, I can't find it being described in details in datasheet,
>> > yet being linked with another document ("see Section Little-Endian and
>> > Big-Endian in the DWC SuperSpeed USB 3.0 Controller User Guide" )
>> > https://www.mouser.com/ds/2/612/atom-e3800-family-datasheet-915661.pdf
>> > I have installed module dwc3 and also dwc2 (both compiled as modules),
>> > but nothing appear in /sys/class/udc
>> > I also tried to add all options in device-drivers->usb->usb
>> > gadget->usb peripheral control into kernel, (not as modules, but built
>> > into kernel).
>> > Yet, on checking
>> > ls /sys/class/udc
>> > I only find dummy_udc.0 , and nothing else.
>> >
>>
>> I suspect that there is no device controller supported with this com
>> express (congatec type 10), probably this is the only explanations for
>> the empty  /sys/class/udc.
>>
>
> A fix for my last comment:
> usb device controller should be supported with conga-MA5 (contains
> Intel's appolo lake), so I am still not sure why I find /sys/class/udc
> empty.
> In congatec MA-5 document it is said:
> "The conga-MA5 offers six USB 2.0 interfaces on the COM Express
> connector including one USB 2.0 Dual Role port. The xHCI host
> controller in the SoC complies with USB standard 1.1 and 2.0 with
> high-speed, full-speed and low-speed USB signalling. Note USB Dual
> Role is only supported under Linux. The port is a standard USB Host
> port under Windows."
> Maybe some configuration in my .config is missing ? (I enabled all
> controllers in device-drivers->usb->usb-> gadget->usb peripheral
> control , so I am not sure what is still missing)

do you have the PCI device enabled? what do you get with lspci - ?

-- 
balbi

Re: [PATCH] USB: change dev_WARN to dev_err triggerable from user space

[Johan Hovold]

On Tue, Sep 04, 2018 at 10:44:41AM +0200, Oliver Neukum wrote:
> For those people who run with panic_on_warn a WARN() triggered
> from user space is a DOS. It is worth returning to dev_err()

I think this should be dev_warn() unless you want to bring back the
returning of errors on these conditions as well (i.e. as was the case
prior to 0cb54a3e47cb ("USB: debugging code shouldn't alter control
flow")).

> Signed-off-by: Oliver Neukum 
> Fixes: 0cb54a3e47cb4baf0bc7463f0a64cfeae5e35697

This should be:

Fixes: 0cb54a3e47cb ("USB: debugging code shouldn't alter control flow")

> Reported-by: syzbot+843efa30c8821bd69...@syzkaller.appspotmail.com
> ---
>  drivers/usb/core/urb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> index f51750bcd152..3fe65a774e6c 100644
> --- a/drivers/usb/core/urb.c
> +++ b/drivers/usb/core/urb.c
> @@ -475,7 +475,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
>  
>   /* Check that the pipe's type matches the endpoint's type */
>   if (usb_urb_ep_type_check(urb))
> - dev_WARN(&dev->dev, "BOGUS urb xfer, pipe %x != type %x\n",
> + dev_err(&dev->dev, "BOGUS urb xfer, pipe %x != type %x\n",
>   usb_pipetype(urb->pipe), pipetypes[xfertype]);
>  
>   /* Check against a simple/standard policy */
> @@ -499,7 +499,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
>  
>   /* warn if submitter gave bogus flags */
>   if (allowed != urb->transfer_flags)
> - dev_WARN(&dev->dev, "BOGUS urb flags, %x --> %x\n",
> + dev_err(&dev->dev, "BOGUS urb flags, %x --> %x\n",
>   urb->transfer_flags, allowed);
>  
>   /*

Johan

Re: [PATCH] USB: change dev_WARN to dev_err triggerable from user space

[Oliver Neukum]

On Di, 2018-09-04 at 11:31 +0200, Johan Hovold wrote:
> On Tue, Sep 04, 2018 at 10:44:41AM +0200, Oliver Neukum wrote:
> > For those people who run with panic_on_warn a WARN() triggered
> > from user space is a DOS. It is worth returning to dev_err()
> 
> I think this should be dev_warn() unless you want to bring back the
> returning of errors on these conditions as well (i.e. as was the case
> prior to 0cb54a3e47cb ("USB: debugging code shouldn't alter control
> flow")).

Should I? A warning in syslog is pretty hardcore, so I have no idea
whether dev_warn() is enough.

Regards
Oliver

Re: [PATCH] USB: change dev_WARN to dev_err triggerable from user space

[Johan Hovold]

On Tue, Sep 04, 2018 at 12:21:09PM +0200, Oliver Neukum wrote:
> On Di, 2018-09-04 at 11:31 +0200, Johan Hovold wrote:
> > On Tue, Sep 04, 2018 at 10:44:41AM +0200, Oliver Neukum wrote:
> > > For those people who run with panic_on_warn a WARN() triggered
> > > from user space is a DOS. It is worth returning to dev_err()
> > 
> > I think this should be dev_warn() unless you want to bring back the
> > returning of errors on these conditions as well (i.e. as was the case
> > prior to 0cb54a3e47cb ("USB: debugging code shouldn't alter control
> > flow")).
> 
> Should I? A warning in syslog is pretty hardcore, so I have no idea
> whether dev_warn() is enough.

Perhaps there are two sides to this. If something really should not be
happening and needs to be addressed (i.e. it's a driver bug) that
dev_WARN is warranted. If user space can be pass in bogus flags that
gets propagated to USB core, perhaps those need to be sanitised sooner
(in the vain of "don't trust anything coming from user space").

In general though, I believe dev_warn() is more appropriate for cases
where something odd is happening, but we try to recover and proceed
anyway (e.g. by sanitising the flags without bailing out as is the case
here) instead of aborting.

Johan

Fw: Aw: Re: keyboard-problem on bpi-r2 since 4.17

[Frank Wunderlich]

Hi, thank you for quick answer.
i will try it, but afaik git bisect resets full git tree to the commit, right?
i need to do some adjustments to get system booting (config,swap mmc in dts).

 
is there another way to show more debug-messages?
 
i have a working 4.16-tree, maybe i can merge newer versions (go forward from 
it instead of backwards).
 
regards frank

Gesendet: Dienstag, 04. September 2018 um 07:26 Uhr
Von: "Greg KH" 
An: "Frank Wunderlich" 
Cc: linux-usb@vger.kernel.org
Betreff: Re: Fw: keyboard-problem on bpi-r2 since 4.17
On Tue, Sep 04, 2018 at 06:39:20AM +0200, Frank Wunderlich wrote:
>
>  
>
> Hi,
>
> i have problems with usb-keyboard on bananapi-r2 since 4.17. same keyboard 
> works till 4.16.
> In 4.19-rc1 same issue occours. Keyboard is recognized and on keypress it is 
> disconnected
> and connected again without anything written to console.
>
> dmesg (printk-debuglevel=8) looks like this:
>
> [ 77.292068] usb 1-1: USB disconnect, device number 2
> [ 77.292068] usb 1-1: USB disconnect, device number 2
> [ 77.472554] evbug: Disconnected device: input0
> [ 77.472554] evbug: Disconnected device: input0
> [ 77.632390] evbug: Disconnected device: input1
> [ 77.632390] evbug: Disconnected device: input1
> [ 77.773255] evbug: Disconnected device: input2
> [ 77.773255] evbug: Disconnected device: input2
> [ 78.342590] usb 1-1: new low-speed USB device number 3 using xhci-mtk
> [ 78.342590] usb 1-1: new low-speed USB device number 3 using xhci-mtk
> [ 78.545576] input: USB USB Keykoard as
> /devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4
> [ 78.545576] input: USB USB Keykoard as
> /devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4
> [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at
> usb-1a1c.usb-1/input0)
> [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID
> v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0
> [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at
> usb-1a1c.usb-1/input0)
> [ 78.655044] input: USB USB Keykoard Consumer Control as
> /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5
> [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID
> v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0
> [ 78.655044] input: USB USB Keykoard Consumer Control as
> /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5
> [ 78.734340] evbug: Connected device: input5 (USB USB Keykoard
> Consumer Control at usb-1a1c.usb-1/input1)
> [ 78.746118] input: USB USB Keykoard System Control as
> /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6
> [ 78.734340] e[ 78.760069] evbug: Connected device: input6 (USB USB
> Keykoard System Control at usb-1a1c.usb-1/input1)
> vbug: Connected [ 78.770893] hid-generic 0003:1C4F:0002.0004:
> input,hidraw1: USB HID v1.10 Device [USB USB Keykoard] on
> usb-1a1c.usb-1/inpu
> t1
> device: input5 (USB USB Keykoard Consumer Control at
> usb-1a1c.usb-1/input1)
> [ 78.746118] input: USB USB Keykoard System Control as
> /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6
> [ 78.760069] evbug: Connected device: input6 (USB USB Keykoard System
> Control at usb-1a1c.usb-1/input1)
> [ 78.770893] hid-generic 0003:1C4F:0002.0004: input,hidraw1: USB HID
> v1.10 Device [USB USB Keykoard] on usb-1a1c.usb-1/input1
>
> can i debug this further?
>
> i see that mtk_xhci-driver was rewritten to use new api
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/host/xhci-mtk.c?h=v4.17-rc1&id=6ae9f5062aa6f5a301c16715c601c05bc9aa450e
>
> maybe this is the reason, but i don't know how to debug it. i tried
> disabling acpi and apic via cmdline to exclude anything acpi-related.
>
> maybe you have an idea...

Can you use 'git bisect' to track down the offending commit?

thanks,

greg k-h

[PATCH v3 02/10] usb: roles: Handle driver reference counting

[Heikki Krogerus]

This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/common/roles.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..3d8a776e55ee 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
device_connection *con, int ep,
  */
 struct usb_role_switch *usb_role_switch_get(struct device *dev)
 {
-   return device_connection_find_match(dev, "usb-role-switch", NULL,
-   usb_role_switch_match);
+   struct usb_role_switch *sw;
+
+   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+   if (!IS_ERR(sw))
+   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+   return sw;
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get);
 
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);
  */
 void usb_role_switch_put(struct usb_role_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
put_device(&sw->dev);
+   module_put(sw->dev.parent->driver->owner);
+   }
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_put);
 
-- 
2.18.0

[PATCH v3 03/10] platform: x86: intel_cht_int33fe: Add dependency on muxes

[Heikki Krogerus]

The connections create clear dependency on the muxes.
fusb302 fails to probe unless we have the mux drivers
available.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0c1aa6c314f5..bdac939de223 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -867,6 +867,8 @@ config INTEL_CHT_INT33FE
tristate "Intel Cherry Trail ACPI INT33FE Driver"
depends on X86 && ACPI && I2C && REGULATOR
depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
+   depends on USB_ROLES_INTEL_XHCI=y || (USB_ROLES_INTEL_XHCI=m && m)
+   depends on TYPEC_MUX_PI3USB30532=y || (TYPEC_MUX_PI3USB30532=m && m)
---help---
  This driver add support for the INT33FE ACPI device found on
  some Intel Cherry Trail devices.
-- 
2.18.0

[PATCH v3 00/10] usb: typec: A few more improvements for Intel CHT

[Heikki Krogerus]

Hi,

These patches will introduce a few improvements to the USB Type-C
support on Intel CHT platform. In this series I'm preparing Intel CHT
mux handling for DisplayPort Alt Mode support, but please note that,
after these patches the DisplayPort alternate mode will still not work
out of the box on CHT platform. Changes to the fusb302.c, and possibly
tcpm.c are still needed.

This version will only fix the issue Hans pointed out in v2. Instead
of replacing the connection that assigned the mux to the USB Type-C
port with a connection that assigns the mux to the alternate mode
device, we keep all the existing connections and add a new one for the
alternate mode device. That way USB3 devices continue to be enumerated
as USB3 devices instead of USB2 devices.

The origin thread can be read here:
https://www.spinics.net/lists/linux-usb/msg172033.html


Heikki Krogerus (10):
  usb: typec: Take care of driver module reference counting
  usb: roles: Handle driver reference counting
  platform: x86: intel_cht_int33fe: Add dependency on muxes
  drivers: base: Helpers for adding device connection descriptions
  platform: x86: intel_cht_int33fe: Register all connections at once
  platform: x86: intel_cht_int33fe: Add connection for the DP alt mode
  platform: x86: intel_cht_int33fe: Add connections for the USB Type-C
port
  usb: typec: class: Don't use port parent for getting mux handles
  platform: x86: intel_cht_int33fe: Remove the old connections for the
muxes
  usb: typec: fusb302: reorganizing the probe function a little

 drivers/platform/x86/Kconfig |  2 ++
 drivers/platform/x86/intel_cht_int33fe.c | 27 -
 drivers/usb/common/roles.c   | 15 --
 drivers/usb/typec/class.c| 38 ++--
 drivers/usb/typec/fusb302/fusb302.c  | 25 ++--
 drivers/usb/typec/mux.c  | 17 ---
 include/linux/device.h   | 24 +++
 7 files changed, 87 insertions(+), 61 deletions(-)

-- 
2.18.0

[PATCH v3 01/10] usb: typec: Take care of driver module reference counting

[Heikki Krogerus]

Functions typec_mux_get() and typec_switch_get() already
make sure that the mux device reference count is
incremented, but the same must be done to the driver module
as well to prevent the drivers from being unloaded in the
middle of operation.

This fixes a potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: 93dd2112c7b2 ("usb: typec: mux: Get the mux identifier from function 
parameter")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/mux.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index ddaac63ecf12..d990aa510fab 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -49,8 +50,10 @@ struct typec_switch *typec_switch_get(struct device *dev)
mutex_lock(&switch_lock);
sw = device_connection_find_match(dev, "typec-switch", NULL,
  typec_switch_match);
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   WARN_ON(!try_module_get(sw->dev->driver->owner));
get_device(sw->dev);
+   }
mutex_unlock(&switch_lock);
 
return sw;
@@ -65,8 +68,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get);
  */
 void typec_switch_put(struct typec_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   module_put(sw->dev->driver->owner);
put_device(sw->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_switch_put);
 
@@ -136,8 +141,10 @@ struct typec_mux *typec_mux_get(struct device *dev, const 
char *name)
 
mutex_lock(&mux_lock);
mux = device_connection_find_match(dev, name, NULL, typec_mux_match);
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   WARN_ON(!try_module_get(mux->dev->driver->owner));
get_device(mux->dev);
+   }
mutex_unlock(&mux_lock);
 
return mux;
@@ -152,8 +159,10 @@ EXPORT_SYMBOL_GPL(typec_mux_get);
  */
 void typec_mux_put(struct typec_mux *mux)
 {
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   module_put(mux->dev->driver->owner);
put_device(mux->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_mux_put);
 
-- 
2.18.0

[PATCH v3 05/10] platform: x86: intel_cht_int33fe: Register all connections at once

[Heikki Krogerus]

We can register all device connection descriptors with a
single call to device_connections_add().

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 39d4100c60a2..b0cef48f77af 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,7 @@ struct cht_int33fe_data {
struct i2c_client *fusb302;
struct i2c_client *pi3usb30532;
/* Contain a list-head must be per device */
-   struct device_connection connections[3];
+   struct device_connection connections[4];
 };
 
 /*
@@ -184,9 +184,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
 
-   device_connection_add(&data->connections[0]);
-   device_connection_add(&data->connections[1]);
-   device_connection_add(&data->connections[2]);
+   device_connections_add(data->connections);
 
memset(&board_info, 0, sizeof(board_info));
strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -217,9 +215,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(&data->connections[2]);
-   device_connection_remove(&data->connections[1]);
-   device_connection_remove(&data->connections[0]);
+   device_connections_remove(data->connections);
 
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
 }
@@ -233,9 +229,7 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(&data->connections[2]);
-   device_connection_remove(&data->connections[1]);
-   device_connection_remove(&data->connections[0]);
+   device_connections_remove(data->connections);
 
return 0;
 }
-- 
2.18.0

[PATCH v3 09/10] platform: x86: intel_cht_int33fe: Remove the old connections for the muxes

[Heikki Krogerus]

USB Type-C class driver now expects the muxes to be always
assigned to the ports and not controllers, so the
connections for the mux and fusb302 can be removed.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 419ce8c8ffb5..a26f410800c2 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,7 @@ struct cht_int33fe_data {
struct i2c_client *fusb302;
struct i2c_client *pi3usb30532;
/* Contain a list-head must be per device */
-   struct device_connection connections[8];
+   struct device_connection connections[5];
 };
 
 /*
@@ -174,29 +174,19 @@ static int cht_int33fe_probe(struct i2c_client *client)
return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
}
 
-   data->connections[0].endpoint[0] = "i2c-fusb302";
+   data->connections[0].endpoint[0] = "port0";
data->connections[0].endpoint[1] = "i2c-pi3usb30532";
data->connections[0].id = "typec-switch";
-   data->connections[1].endpoint[0] = "i2c-fusb302";
+   data->connections[1].endpoint[0] = "port0";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
-   data->connections[2].endpoint[0] = "i2c-fusb302";
+   data->connections[2].endpoint[0] = "port0";
data->connections[2].endpoint[1] = "i2c-pi3usb30532";
data->connections[2].id = "idff01m01";
data->connections[3].endpoint[0] = "i2c-fusb302";
data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[3].id = "usb-role-switch";
 
-   data->connections[4].endpoint[0] = "port0";
-   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[4].id = "typec-switch";
-   data->connections[5].endpoint[0] = "port0";
-   data->connections[5].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[5].id = "typec-mux";
-   data->connections[6].endpoint[0] = "port0";
-   data->connections[6].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[6].id = "idff01m01";
-
device_connections_add(data->connections);
 
memset(&board_info, 0, sizeof(board_info));
-- 
2.18.0

[PATCH v3 04/10] drivers: base: Helpers for adding device connection descriptions

[Heikki Krogerus]

Introducing helpers for adding and removing multiple device
connection descriptions at once.

Signed-off-by: Heikki Krogerus 
---
 include/linux/device.h | 24 
 1 file changed, 24 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..3f1066a9e1c3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -773,6 +773,30 @@ struct device *device_connection_find(struct device *dev, 
const char *con_id);
 void device_connection_add(struct device_connection *con);
 void device_connection_remove(struct device_connection *con);
 
+/**
+ * device_connections_add - Add multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_add(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_add(c);
+}
+
+/**
+ * device_connections_remove - Remove multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_remove(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_remove(c);
+}
+
 /**
  * enum device_link_state - Device link states.
  * @DL_STATE_NONE: The presence of the drivers is not being tracked.
-- 
2.18.0

[PATCH v3 08/10] usb: typec: class: Don't use port parent for getting mux handles

[Heikki Krogerus]

It is not possible to use the parent of the port device when
requesting mux handles as the parent may be a multiport USB
Type-C or PD controller. The muxes must be assigned to the
ports, not the controllers.

This will also move the requesting of the muxes after the
port device is initialized.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/class.c | 38 +++---
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c202975f8097..5b969339d1b3 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1501,7 +1501,7 @@ typec_port_register_altmode(struct typec_port *port,
 
sprintf(id, "id%04xm%02x", desc->svid, desc->mode);
 
-   mux = typec_mux_get(port->dev.parent, id);
+   mux = typec_mux_get(&port->dev, id);
if (IS_ERR(mux))
return ERR_CAST(mux);
 
@@ -1541,18 +1541,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
return ERR_PTR(id);
}
 
-   port->sw = typec_switch_get(cap->fwnode ? &port->dev : parent);
-   if (IS_ERR(port->sw)) {
-   ret = PTR_ERR(port->sw);
-   goto err_switch;
-   }
-
-   port->mux = typec_mux_get(parent, "typec-mux");
-   if (IS_ERR(port->mux)) {
-   ret = PTR_ERR(port->mux);
-   goto err_mux;
-   }
-
switch (cap->type) {
case TYPEC_PORT_SRC:
port->pwr_role = TYPEC_SOURCE;
@@ -1593,13 +1581,26 @@ struct typec_port *typec_register_port(struct device 
*parent,
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
 
+   device_initialize(&port->dev);
port->dev.class = typec_class;
port->dev.parent = parent;
port->dev.fwnode = cap->fwnode;
port->dev.type = &typec_port_dev_type;
dev_set_name(&port->dev, "port%d", id);
 
-   ret = device_register(&port->dev);
+   port->sw = typec_switch_get(&port->dev);
+   if (IS_ERR(port->sw)) {
+   put_device(&port->dev);
+   return ERR_CAST(port->sw);
+   }
+
+   port->mux = typec_mux_get(&port->dev, "typec-mux");
+   if (IS_ERR(port->mux)) {
+   put_device(&port->dev);
+   return ERR_CAST(port->mux);
+   }
+
+   ret = device_add(&port->dev);
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
put_device(&port->dev);
@@ -1607,15 +1608,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
}
 
return port;
-
-err_mux:
-   typec_switch_put(port->sw);
-
-err_switch:
-   ida_simple_remove(&typec_index_ida, port->id);
-   kfree(port);
-
-   return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(typec_register_port);
 
-- 
2.18.0

[PATCH v3 07/10] platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port

[Heikki Krogerus]

Assigning the mux to the USB Type-C port on top of fusb302.
That will prepare this driver for the change in the USB
Type-C class code, where the class driver will assume the
muxes to be always assigned to the ports and not the
controllers.

Once the USB Type-C class driver has been updated, the
connections between the mux and fusb302 can be dropped.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 424064187124..419ce8c8ffb5 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,7 @@ struct cht_int33fe_data {
struct i2c_client *fusb302;
struct i2c_client *pi3usb30532;
/* Contain a list-head must be per device */
-   struct device_connection connections[5];
+   struct device_connection connections[8];
 };
 
 /*
@@ -187,6 +187,16 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[3].id = "usb-role-switch";
 
+   data->connections[4].endpoint[0] = "port0";
+   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[4].id = "typec-switch";
+   data->connections[5].endpoint[0] = "port0";
+   data->connections[5].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[5].id = "typec-mux";
+   data->connections[6].endpoint[0] = "port0";
+   data->connections[6].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[6].id = "idff01m01";
+
device_connections_add(data->connections);
 
memset(&board_info, 0, sizeof(board_info));
-- 
2.18.0

[PATCH v3 10/10] usb: typec: fusb302: reorganizing the probe function a little

[Heikki Krogerus]

The debugfs needs to be initialized as the last step in
probe in this case. The struct dentry *rootdir can't be
pointing to anything unless driver probe really finishes
successfully.

It is also not necessary to clear the i2c clientdata if the
probe fails, so removing the extra label used for that.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/fusb302/fusb302.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index 82bed9810be6..5f5864273e16 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1730,7 +1730,6 @@ static int fusb302_probe(struct i2c_client *client,
return -ENOMEM;
 
chip->i2c_client = client;
-   i2c_set_clientdata(client, chip);
chip->dev = &client->dev;
chip->tcpc_config = fusb302_tcpc_config;
chip->tcpc_dev.config = &chip->tcpc_config;
@@ -1756,22 +1755,17 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
 
-   fusb302_debugfs_init(chip);
+   chip->vbus = devm_regulator_get(chip->dev, "vbus");
+   if (IS_ERR(chip->vbus))
+   return PTR_ERR(chip->vbus);
 
chip->wq = create_singlethread_workqueue(dev_name(chip->dev));
-   if (!chip->wq) {
-   ret = -ENOMEM;
-   goto clear_client_data;
-   }
+   if (!chip->wq)
+   return -ENOMEM;
+
INIT_DELAYED_WORK(&chip->bc_lvl_handler, fusb302_bc_lvl_handler_work);
init_tcpc_dev(&chip->tcpc_dev);
 
-   chip->vbus = devm_regulator_get(chip->dev, "vbus");
-   if (IS_ERR(chip->vbus)) {
-   ret = PTR_ERR(chip->vbus);
-   goto destroy_workqueue;
-   }
-
if (client->irq) {
chip->gpio_int_n_irq = client->irq;
} else {
@@ -1797,15 +1791,15 @@ static int fusb302_probe(struct i2c_client *client,
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);
+   fusb302_debugfs_init(chip);
+   i2c_set_clientdata(client, chip);
+
return ret;
 
 tcpm_unregister_port:
tcpm_unregister_port(chip->tcpm_port);
 destroy_workqueue:
destroy_workqueue(chip->wq);
-clear_client_data:
-   i2c_set_clientdata(client, NULL);
-   fusb302_debugfs_exit(chip);
 
return ret;
 }
@@ -1816,7 +1810,6 @@ static int fusb302_remove(struct i2c_client *client)
 
tcpm_unregister_port(chip->tcpm_port);
destroy_workqueue(chip->wq);
-   i2c_set_clientdata(client, NULL);
fusb302_debugfs_exit(chip);
 
return 0;
-- 
2.18.0

[PATCH v3 06/10] platform: x86: intel_cht_int33fe: Add connection for the DP alt mode

[Heikki Krogerus]

Adding a connection for the DisplayPort alternate mode.
PI3USB30532 is used for muxing the port to DisplayPort on
CHT platforms. The connection allows the alternate mode
device to get handle to the mux, and therefore make it
possible to use the USB Type-C connector as DisplayPort.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index b0cef48f77af..424064187124 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,7 @@ struct cht_int33fe_data {
struct i2c_client *fusb302;
struct i2c_client *pi3usb30532;
/* Contain a list-head must be per device */
-   struct device_connection connections[4];
+   struct device_connection connections[5];
 };
 
 /*
@@ -181,8 +181,11 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
data->connections[2].endpoint[0] = "i2c-fusb302";
-   data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
-   data->connections[2].id = "usb-role-switch";
+   data->connections[2].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[2].id = "idff01m01";
+   data->connections[3].endpoint[0] = "i2c-fusb302";
+   data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch";
+   data->connections[3].id = "usb-role-switch";
 
device_connections_add(data->connections);
 
-- 
2.18.0

gadgetfs & endpoints

[Ranran]

Hello,

In gadgetfs documentation I find no mention of endpoints:
https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt

Is it possible to create endpoints using gadgetfs ?

Thank you,
ranran

Re: Nothing in /sys/class/udc

[Ranran]

On Tue, Sep 4, 2018 at 12:12 PM Felipe Balbi
 wrote:
>
>
> Hi,
>
> Ranran  writes:
> > On Mon, Sep 3, 2018 at 4:50 PM Ranran  wrote:
> >>
> >> On Mon, Sep 3, 2018 at 4:22 PM Ranran  wrote:
> >> >
> >> > On Mon, Sep 3, 2018 at 3:40 PM Greg KH  
> >> > wrote:
> >> > >
> >> > > On Mon, Sep 03, 2018 at 09:39:14AM +0300, Ranran wrote:
> >> > > > Hello,
> >> > > >
> >> > > > I try to add gadget configfs as described in:
> >> > > > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt
> >> > > > Yet, I find nothing in /sys/class/udc:
> >> > > >
> >> > > > user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al
> >> > > > total 0
> >> > > > drwxr-xr-x  2 root root 0 Sep  3 00:30 .
> >> > > > drwxr-xr-x 58 root root 0 Sep  3 00:30 ..
> >> > > >
> >> > > > I also don't have dwc2, but dwc3:
> >> > > > user@user-VirtualBox:~/tegra$ lsmod | grep dw
> >> > > > dwc3   90112  0
> >> > > > ulpi   16384  1 dwc3
> >> > > > udc_core   24576  2 dwc3,libcomposite
> >> > > > user@user-VirtualBox:~/tegra$
> >> > > >
> >> > > > Kernel is 4.4.50.
> >> > >
> >> > > Why do you think the dwc3 driver will work here?  Do you really have
> >> > > this hardware being emulated?
> >> > >
> >> > Hi,
> >> >
> >> > I check it with Intel's E3900.
> >> > In datasheet, I can't find it being described in details in datasheet,
> >> > yet being linked with another document ("see Section Little-Endian and
> >> > Big-Endian in the DWC SuperSpeed USB 3.0 Controller User Guide" )
> >> > https://www.mouser.com/ds/2/612/atom-e3800-family-datasheet-915661.pdf
> >> > I have installed module dwc3 and also dwc2 (both compiled as modules),
> >> > but nothing appear in /sys/class/udc
> >> > I also tried to add all options in device-drivers->usb->usb
> >> > gadget->usb peripheral control into kernel, (not as modules, but built
> >> > into kernel).
> >> > Yet, on checking
> >> > ls /sys/class/udc
> >> > I only find dummy_udc.0 , and nothing else.
> >> >
> >>
> >> I suspect that there is no device controller supported with this com
> >> express (congatec type 10), probably this is the only explanations for
> >> the empty  /sys/class/udc.
> >>
> >
> > A fix for my last comment:
> > usb device controller should be supported with conga-MA5 (contains
> > Intel's appolo lake), so I am still not sure why I find /sys/class/udc
> > empty.
> > In congatec MA-5 document it is said:
> > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express
> > connector including one USB 2.0 Dual Role port. The xHCI host
> > controller in the SoC complies with USB standard 1.1 and 2.0 with
> > high-speed, full-speed and low-speed USB signalling. Note USB Dual
> > Role is only supported under Linux. The port is a standard USB Host
> > port under Windows."
> > Maybe some configuration in my .config is missing ? (I enabled all
> > controllers in device-drivers->usb->usb-> gadget->usb peripheral
> > control , so I am not sure what is still missing)
>
> do you have the PCI device enabled? what do you get with lspci - ?
>

Yes, here it is:

00:00.0 Host bridge [0600]: Intel Corporation Celeron N3350/Pentium
N4200/Atom E3900 Series Host Bridge [8086:5af0] (rev 0b)
00:02.0 VGA compatible controller [0300]: Intel Corporation Device
[8086:5a85] (rev 0b)
00:0e.0 Audio device [0403]: Intel Corporation Celeron N3350/Pentium
N4200/Atom E3900 Series Audio Cluster [8086:5a98] (rev 0b)
00:0f.0 Communication controller [0780]: Intel Corporation Celeron
N3350/Pentium N4200/Atom E3900 Series Trusted Execution Engine
[8086:5a9a] (rev 0b)
00:12.0 SATA controller [0106]: Intel Corporation Celeron
N3350/Pentium N4200/Atom E3900 Series SATA AHCI Controller [8086:5ae3]
(rev 0b)
00:13.0 PCI bridge [0604]: Intel Corporation Celeron N3350/Pentium
N4200/Atom E3900 Series PCI Express Port A #1 [8086:5ad8] (rev fb)
00:14.0 PCI bridge [0604]: Intel Corporation Celeron N3350/Pentium
N4200/Atom E3900 Series PCI Express Port B #1 [8086:5ad6] (rev fb)
00:15.0 USB controller [0c03]: Intel Corporation Celeron N3350/Pentium
N4200/Atom E3900 Series USB xHCI [8086:5aa8] (rev 0b)
00:1b.0 SD Host controller [0805]: Intel Corporation Celeron
N3350/Pentium N4200/Atom E3900 Series SDXC/MMC Host Controller
[8086:5aca] (rev 0b)
00:1c.0 SD Host controller [0805]: Intel Corporation Celeron
N3350/Pentium N4200/Atom E3900 Series eMMC Controller [8086:5acc] (rev
0b)
00:1f.0 ISA bridge [0601]: Intel Corporation Celeron N3350/Pentium
N4200/Atom E3900 Series Low Pin Count Interface [8086:5ae8] (rev 0b)
00:1f.1 SMBus [0c05]: Intel Corporation Celeron N3350/Pentium
N4200/Atom E3900 Series SMBus Controller [8086:5ad4] (rev 0b)
02:00.0 Ethernet controller [0200]: Intel Corporation I210 Gigabit
Network Connection [8086:157b] (rev 03)

Thank you,
ranran

> --
> balbi

Re: [PATCH v3 00/10] usb: typec: A few more improvements for Intel CHT

[Andy Shevchenko]

On Tue, Sep 4, 2018 at 2:22 PM Heikki Krogerus
 wrote:
>
> Hi,
>
> These patches will introduce a few improvements to the USB Type-C
> support on Intel CHT platform. In this series I'm preparing Intel CHT
> mux handling for DisplayPort Alt Mode support, but please note that,
> after these patches the DisplayPort alternate mode will still not work
> out of the box on CHT platform. Changes to the fusb302.c, and possibly
> tcpm.c are still needed.
>
> This version will only fix the issue Hans pointed out in v2. Instead
> of replacing the connection that assigned the mux to the USB Type-C
> port with a connection that assigns the mux to the alternate mode
> device, we keep all the existing connections and add a new one for the
> alternate mode device. That way USB3 devices continue to be enumerated
> as USB3 devices instead of USB2 devices.
>
> The origin thread can be read here:
> https://www.spinics.net/lists/linux-usb/msg172033.html
>
>

It seems you forgot my tag

Acked-by: Andy Shevchenko 

for PDx86 bits on condition that Hans is okay with them.

> Heikki Krogerus (10):
>   usb: typec: Take care of driver module reference counting
>   usb: roles: Handle driver reference counting
>   platform: x86: intel_cht_int33fe: Add dependency on muxes
>   drivers: base: Helpers for adding device connection descriptions
>   platform: x86: intel_cht_int33fe: Register all connections at once
>   platform: x86: intel_cht_int33fe: Add connection for the DP alt mode
>   platform: x86: intel_cht_int33fe: Add connections for the USB Type-C
> port
>   usb: typec: class: Don't use port parent for getting mux handles
>   platform: x86: intel_cht_int33fe: Remove the old connections for the
> muxes
>   usb: typec: fusb302: reorganizing the probe function a little
>
>  drivers/platform/x86/Kconfig |  2 ++
>  drivers/platform/x86/intel_cht_int33fe.c | 27 -
>  drivers/usb/common/roles.c   | 15 --
>  drivers/usb/typec/class.c| 38 ++--
>  drivers/usb/typec/fusb302/fusb302.c  | 25 ++--
>  drivers/usb/typec/mux.c  | 17 ---
>  include/linux/device.h   | 24 +++
>  7 files changed, 87 insertions(+), 61 deletions(-)
>
> --
> 2.18.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: Nothing in /sys/class/udc

[Felipe Balbi]

hi,

Ranran  writes:
> > A fix for my last comment:
>> > usb device controller should be supported with conga-MA5 (contains
>> > Intel's appolo lake), so I am still not sure why I find /sys/class/udc
>> > empty.
>> > In congatec MA-5 document it is said:
>> > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express
>> > connector including one USB 2.0 Dual Role port. The xHCI host
>> > controller in the SoC complies with USB standard 1.1 and 2.0 with
>> > high-speed, full-speed and low-speed USB signalling. Note USB Dual
>> > Role is only supported under Linux. The port is a standard USB Host
>> > port under Windows."
>> > Maybe some configuration in my .config is missing ? (I enabled all
>> > controllers in device-drivers->usb->usb-> gadget->usb peripheral
>> > control , so I am not sure what is still missing)
>>
>> do you have the PCI device enabled? what do you get with lspci - ?
>>
>
> Yes, here it is:

it's not. APL's dwc3 ID 0x5aaa. See dwc3-pci.c:

#define PCI_DEVICE_ID_INTEL_APL 0x5aaa

you only have xhci enabled, not dwc3.

-- 
balbi

Re: Nothing in /sys/class/udc

[Ranran]

On Tue, Sep 4, 2018 at 3:26 PM Felipe Balbi
 wrote:
>
>
> hi,
>
> Ranran  writes:
> > > A fix for my last comment:
> >> > usb device controller should be supported with conga-MA5 (contains
> >> > Intel's appolo lake), so I am still not sure why I find /sys/class/udc
> >> > empty.
> >> > In congatec MA-5 document it is said:
> >> > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express
> >> > connector including one USB 2.0 Dual Role port. The xHCI host
> >> > controller in the SoC complies with USB standard 1.1 and 2.0 with
> >> > high-speed, full-speed and low-speed USB signalling. Note USB Dual
> >> > Role is only supported under Linux. The port is a standard USB Host
> >> > port under Windows."
> >> > Maybe some configuration in my .config is missing ? (I enabled all
> >> > controllers in device-drivers->usb->usb-> gadget->usb peripheral
> >> > control , so I am not sure what is still missing)
> >>
> >> do you have the PCI device enabled? what do you get with lspci - ?
> >>
> >
> > Yes, here it is:
>
> it's not. APL's dwc3 ID 0x5aaa. See dwc3-pci.c:
>
> #define PCI_DEVICE_ID_INTEL_APL 0x5aaa
>
> you only have xhci enabled, not dwc3.
>
Right, I made big confusion in things. sorry !
So, it seems that I don't have any usb peripheral controller (xhci is
only host and there is no other usb device controller listed above),
Right ?

> --
> balbi

Re: [PATCH v3] USB: serial: ftdi_sio: Add support for CBUS GPIO

[Johan Hovold]

On Sat, Aug 04, 2018 at 12:17:15PM +0200, Loic Poulain wrote:
> Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS
> pins, allowing host to control them via simple USB control transfers.
> To make use of a CBUS pin in Bit Bang mode, the pin must be configured
> to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular
> USB to Serial function.
> 
> In this implementation, a GPIO controller is registered on FTDI probe
> if at least one CBUS pin is configured for I/O mode. For now, only
> FTX devices are supported.
> 
> This patch is based on previous Stefan Agner implementation tentative
> on LKML [1].
> 
> [1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch
> 
> Signed-off-by: Loic Poulain 
> ---
> v2: Use message-id for LKML reference
> Rework read_eeprom according to Andy's comment and return read count
> Remove noisy messages
> Comment style alignment
> Add defines for magic values
> Cannot use devm, because gpiochip is linked to the port not to the udev
> v3:
>Fix typo in CBUS mask description comment

First, thanks for looking into this; seems like we're finally about to
get support for the CBUS gpios.

But now I get not one, but two, competing implementations for this
posted in one month:

https://lkml.kernel.org/r/20180825204744.2307-1-pa...@pados.hu

And it looks like you both have been considering at least some of the
earlier attempts, which is great.

I've reviewed both patches and it seems to me that Karoly's version is
closer to what I'd like to see as the end-result. Specifically this
means having:

 - fixed offsets for the physical pins rather than having the offsets
   depend on eeprom configuration

 - better type abstraction (we want to add support for ft232r and ft232h
   eventually as well)

The other patch is also more complete in that it:

 - considers the initial pin state
 - implements a request() callback
 - implements a get_direction() callback

In contrast, with this implementation as it stands, we could end up with
a driven pin being reported as an input (corner case, but still).

Implementation-wise, the other patch is also closer to what I'd like to
see (e.g. not using atomic bit ops, and getting the error handling right
from the start).

There are some issues that needs to be addressed in the other patch as
well, and in doing so it would be wise to look at your patch for
inspiration (e.g. naming issues and adding an eeprom helper to only read
the couple of configuration words needed).

In the end, going with the other patch means less work for me, even if
you both (by unfortunate timing) have forced me to do more than twice
the amount of review work already.

Hopefully we can all work together on getting the missing bits,
including further device support, in place.

For completeness, I've included my review comments below.

Thanks,
Johan


>  drivers/usb/serial/Kconfig|   9 ++
>  drivers/usb/serial/ftdi_sio.c | 238 
> ++
>  drivers/usb/serial/ftdi_sio.h |  83 +++
>  3 files changed, 330 insertions(+)
> 
> diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig
> index 533f127..64c9f2e 100644
> --- a/drivers/usb/serial/Kconfig
> +++ b/drivers/usb/serial/Kconfig
> @@ -181,6 +181,15 @@ config USB_SERIAL_FTDI_SIO
> To compile this driver as a module, choose M here: the
> module will be called ftdi_sio.
>  
> +config USB_SERIAL_FTDI_SIO_CBUS_GPIO
> + bool "USB FDTI CBUS GPIO support"
> + depends on USB_SERIAL_FTDI_SIO
> + depends on GPIOLIB
> + help
> +   Say yes here to add support for the CBUS bit-bang mode, allowing CBUS
> +   pins to act as GPIOs. Note that pins must first be configured for GPIO
> +   in the device's EEPROM. The FT232R and FT-X series support this mode.

I understand you're referring to hardware, but this is ambiguous as you
don't implement support for FT232R in the driver.

> +
>  config USB_SERIAL_VISOR
>   tristate "USB Handspring Visor / Palm m50x / Sony Clie Driver"
>   help
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index b5cef32..e90cae6 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -40,12 +41,21 @@
>  #include 
>  #include 
>  #include 
> +

Unrelated change.

>  #include "ftdi_sio.h"
>  #include "ftdi_sio_ids.h"
>  
>  #define DRIVER_AUTHOR "Greg Kroah-Hartman , Bill Ryder 
> , Kuba Ober , Andreas Mohr, Johan 
> Hovold "
>  #define DRIVER_DESC "USB FTDI Serial Converters Driver"
>  
> +#define FTDI_CBUS_PIN_COUNT 4
> +
> +struct ftdi_gpiochip {
> + struct gpio_chip gc;
> + struct usb_serial_port *port;
> + unsigned int cbus_map[FTDI_CBUS_PIN_COUNT];

Looks like u8 will do here since it's just an index, right? But see more
about this mapping below first.

>

Re: gadgetfs & endpoints

[Ranran]

On Tue, Sep 4, 2018 at 2:36 PM Ranran  wrote:
>
> Hello,
>
> In gadgetfs documentation I find no mention of endpoints:
> https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt
>
> Is it possible to create endpoints using gadgetfs ?
>

The question need to be phrased differently:
Is it possible to create HID using gadgetfs.
I see that it is possible to create usb device with endpoints as described here:
http://www.linux-usb.org/gadget/usb.c
But what about HID device ? How can we implement HID device using gadget ?

Best Regards,
ranran




> Thank you,
> ranran

Re: Fw: Aw: Re: keyboard-problem on bpi-r2 since 4.17

[Frank Wunderlich]

Hi,

i got an info that my issue was caused by this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=217491487c43892318c18b6dcafe33b6353efd40

Patch is in progress...

Regards Frank

Am 4. September 2018 13:16:12 MESZ schrieb Frank Wunderlich :
>Hi, thank you for quick answer.
>i will try it, but afaik git bisect resets full git tree to the commit,
>right?
>i need to do some adjustments to get system booting (config,swap mmc in
>dts).
>
> 
>is there another way to show more debug-messages?
> 
>i have a working 4.16-tree, maybe i can merge newer versions (go
>forward from it instead of backwards).
> 
>regards frank
>
>Gesendet: Dienstag, 04. September 2018 um 07:26 Uhr
>Von: "Greg KH" 
>An: "Frank Wunderlich" 
>Cc: linux-usb@vger.kernel.org
>Betreff: Re: Fw: keyboard-problem on bpi-r2 since 4.17
>On Tue, Sep 04, 2018 at 06:39:20AM +0200, Frank Wunderlich wrote:
>>
>>  
>>
>> Hi,
>>
>> i have problems with usb-keyboard on bananapi-r2 since 4.17. same
>keyboard works till 4.16.
>> In 4.19-rc1 same issue occours. Keyboard is recognized and on
>keypress it is disconnected
>> and connected again without anything written to console.
>>
>> dmesg (printk-debuglevel=8) looks like this:
>>
>> [ 77.292068] usb 1-1: USB disconnect, device number 2
>> [ 77.292068] usb 1-1: USB disconnect, device number 2
>> [ 77.472554] evbug: Disconnected device: input0
>> [ 77.472554] evbug: Disconnected device: input0
>> [ 77.632390] evbug: Disconnected device: input1
>> [ 77.632390] evbug: Disconnected device: input1
>> [ 77.773255] evbug: Disconnected device: input2
>> [ 77.773255] evbug: Disconnected device: input2
>> [ 78.342590] usb 1-1: new low-speed USB device number 3 using
>xhci-mtk
>> [ 78.342590] usb 1-1: new low-speed USB device number 3 using
>xhci-mtk
>> [ 78.545576] input: USB USB Keykoard as
>>
>/devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4
>> [ 78.545576] input: USB USB Keykoard as
>>
>/devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4
>> [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at
>> usb-1a1c.usb-1/input0)
>> [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID
>> v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0
>> [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at
>> usb-1a1c.usb-1/input0)
>> [ 78.655044] input: USB USB Keykoard Consumer Control as
>>
>/devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5
>> [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID
>> v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0
>> [ 78.655044] input: USB USB Keykoard Consumer Control as
>>
>/devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5
>> [ 78.734340] evbug: Connected device: input5 (USB USB Keykoard
>> Consumer Control at usb-1a1c.usb-1/input1)
>> [ 78.746118] input: USB USB Keykoard System Control as
>>
>/devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6
>> [ 78.734340] e[ 78.760069] evbug: Connected device: input6 (USB USB
>> Keykoard System Control at usb-1a1c.usb-1/input1)
>> vbug: Connected [ 78.770893] hid-generic 0003:1C4F:0002.0004:
>> input,hidraw1: USB HID v1.10 Device [USB USB Keykoard] on
>> usb-1a1c.usb-1/inpu
>> t1
>> device: input5 (USB USB Keykoard Consumer Control at
>> usb-1a1c.usb-1/input1)
>> [ 78.746118] input: USB USB Keykoard System Control as
>>
>/devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6
>> [ 78.760069] evbug: Connected device: input6 (USB USB Keykoard System
>> Control at usb-1a1c.usb-1/input1)
>> [ 78.770893] hid-generic 0003:1C4F:0002.0004: input,hidraw1: USB HID
>> v1.10 Device [USB USB Keykoard] on usb-1a1c.usb-1/input1
>>
>> can i debug this further?
>>
>> i see that mtk_xhci-driver was rewritten to use new api
>>
>>
>https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/host/xhci-mtk.c?h=v4.17-rc1&id=6ae9f5062aa6f5a301c16715c601c05bc9aa450e
>>
>> maybe this is the reason, but i don't know how to debug it. i tried
>> disabling acpi and apic via cmdline to exclude anything acpi-related.
>>
>> maybe you have an idea...
>
>Can you use 'git bisect' to track down the offending commit?
>
>thanks,
>
>greg k-h


Mit freundlichen Grüßen
Frank Wunderlich

Re: [PATCH] dwc3: Remove wait for wait_end_transfer event.

[Pierre Le Magourou]

Hello,

Le jeu. 19 juil. 2018 à 09:24, Felipe Balbi
 a écrit :
>
>
> Hi,
>
> lemag...@gmail.com writes:
> > From: Pierre Le Magourou 
> >
> > We can't wait for END_OF_TRANSFER event in dwc3_gadget_ep_dequeue()
> > because it can be called in an interruption context.
> >
> > TRBs are now cleared after the END_OF_TRANSFER completion, in the
> > interrupt handler.
> >
> > Signed-off-by: Pierre Le Magourou 
> > ---
>
> you forgot to Cc linux-usb
>
> >  drivers/usb/dwc3/core.h   |  14 ++
> >  drivers/usb/dwc3/gadget.c | 122 
> > +++---
> >  2 files changed, 85 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> > index 7a217a36c36b..5e2070183e3a 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -627,6 +627,7 @@ struct dwc3_event_buffer {
> >   * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer 
> > complete
> >   * @lock: spinlock for endpoint request queue traversal
> >   * @regs: pointer to first endpoint register
> > + * @pending_trb: pointer to pending TRB structure
> >   * @trb_pool: array of transaction buffers
> >   * @trb_pool_dma: dma address of @trb_pool
> >   * @trb_enqueue: enqueue 'pointer' into TRB array
> > @@ -654,6 +655,7 @@ struct dwc3_ep {
> >   void __iomem*regs;
> >
> >   struct dwc3_trb *trb_pool;
> > + struct dwc3_pending_trb *pending_trb;
> >   dma_addr_t  trb_pool_dma;
> >   struct dwc3 *dwc;
> >
> > @@ -777,6 +779,18 @@ struct dwc3_trb {
> >   u32 ctrl;
> >  } __packed;
> >
> > +/**
> > + * struct dwc3_pending_trb
> > + * @dirty_trb: pointer to TRB waiting for END_TRANSFER completion
> > + * @extra_trb: true if extra TRB was set to align transfer
> > + * @num_pending_sgs: number of pending SGs
> > + */
> > +struct dwc3_pending_trb {
> > + struct dwc3_trb *dirty_trb;
> > + bool extra_trb;
> > + unsigned int num_pending_sgs;
> > +};
> > +
> >  /**
> >   * struct dwc3_hwparams - copy of HWPARAMS registers
> >   * @hwparams0: GHWPARAMS0
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 69bf137aab37..dd1f2b74723b 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -1341,6 +1341,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
> >
> >   struct dwc3_ep  *dep = to_dwc3_ep(ep);
> >   struct dwc3 *dwc = dep->dwc;
> > + struct dwc3_pending_trb *p_trb;
> >
> >   unsigned long   flags;
> >   int ret = 0;
> > @@ -1367,63 +1368,29 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
> >* If request was already started, this means we had 
> > to
> >* stop the transfer. With that we also need to ignore
> >* all TRBs used by the request, however TRBs can only
> > -  * be modified after completion of END_TRANSFER
> > -  * command. So what we do here is that we wait for
> > -  * END_TRANSFER completion and only after that, we 
> > jump
> > -  * over TRBs by clearing HWO and incrementing dequeue
> > -  * pointer.
> > +  * be modified after completion of END_TRANSFER 
> > command.
> >*
> > -  * Note that we have 2 possible types of transfers 
> > here:
> > -  *
> > -  * i) Linear buffer request
> > -  * ii) SG-list based request
> > -  *
> > -  * SG-list based requests will have r->num_pending_sgs
> > -  * set to a valid number (> 0). Linear requests,
> > -  * normally use a single TRB.
> > -  *
> > -  * For each of these two cases, if r->unaligned flag 
> > is
> > -  * set, one extra TRB has been used to align transfer
> > -  * size to wMaxPacketSize.
> > -  *
> > -  * All of these cases need to be taken into
> > -  * consideration so we don't mess up our TRB ring
> > -  * pointers.
> > +  * Don't wait for END_TRANSFER completion here because
> > +  * dwc3_gadget_ep_dequeue() can be called from within
> > +  * the controller's interrupt handler. Save the TRB
> > +  * pointer, the number of pending sgs, and the
> > +  * extra_trb flag, so that TRBs HWO can be cleared and
> > +  * dequeue pointer incremented when the END_TRANSFER
> > +  * completion is received.
> >*/
> > - wait_event_lock_irq(de

Re: Nothing in /sys/class/udc

[Felipe Balbi]

Hi,

Ranran  writes:
>> Ranran  writes:
>> > > A fix for my last comment:
>> >> > usb device controller should be supported with conga-MA5 (contains
>> >> > Intel's appolo lake), so I am still not sure why I find /sys/class/udc
>> >> > empty.
>> >> > In congatec MA-5 document it is said:
>> >> > "The conga-MA5 offers six USB 2.0 interfaces on the COM Express
>> >> > connector including one USB 2.0 Dual Role port. The xHCI host
>> >> > controller in the SoC complies with USB standard 1.1 and 2.0 with
>> >> > high-speed, full-speed and low-speed USB signalling. Note USB Dual
>> >> > Role is only supported under Linux. The port is a standard USB Host
>> >> > port under Windows."
>> >> > Maybe some configuration in my .config is missing ? (I enabled all
>> >> > controllers in device-drivers->usb->usb-> gadget->usb peripheral
>> >> > control , so I am not sure what is still missing)
>> >>
>> >> do you have the PCI device enabled? what do you get with lspci - ?
>> >>
>> >
>> > Yes, here it is:
>>
>> it's not. APL's dwc3 ID 0x5aaa. See dwc3-pci.c:
>>
>> #define PCI_DEVICE_ID_INTEL_APL 0x5aaa
>>
>> you only have xhci enabled, not dwc3.
>>
> Right, I made big confusion in things. sorry !
> So, it seems that I don't have any usb peripheral controller (xhci is
> only host and there is no other usb device controller listed above),
> Right ?

right

-- 
balbi

Re: Fw: Aw: Re: keyboard-problem on bpi-r2 since 4.17

[Mathias Nyman]

On 04.09.2018 16:14, Frank Wunderlich wrote:

Hi,

i got an info that my issue was caused by this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=217491487c43892318c18b6dcafe33b6353efd40

Patch is in progress...


That patch has been there since 4.13
If 4.16 works for you then it's probably something else

  
is there another way to show more debug-messages?
  
i have a working 4.16-tree, maybe i can merge newer versions (go

forward from it instead of backwards).
  


xhci debugging can be added by:

mount -t debugfs none /sys/kernel/debug
echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control


and tracing with: (before pressing keyboard key)

(mount -t debugfs none /sys/kernel/debug)
echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb
echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable

after pressing key traces are found at:
/sys/kernel/debug/tracing/trace

-Mathias

Re: [PATCH v3 00/10] usb: typec: A few more improvements for Intel CHT

[Hans de Goede]

Hi,

On 04-09-18 14:13, Andy Shevchenko wrote:

On Tue, Sep 4, 2018 at 2:22 PM Heikki Krogerus
 wrote:


Hi,

These patches will introduce a few improvements to the USB Type-C
support on Intel CHT platform. In this series I'm preparing Intel CHT
mux handling for DisplayPort Alt Mode support, but please note that,
after these patches the DisplayPort alternate mode will still not work
out of the box on CHT platform. Changes to the fusb302.c, and possibly
tcpm.c are still needed.

This version will only fix the issue Hans pointed out in v2. Instead
of replacing the connection that assigned the mux to the USB Type-C
port with a connection that assigns the mux to the alternate mode
device, we keep all the existing connections and add a new one for the
alternate mode device. That way USB3 devices continue to be enumerated
as USB3 devices instead of USB2 devices.

The origin thread can be read here:
https://www.spinics.net/lists/linux-usb/msg172033.html




It seems you forgot my tag

Acked-by: Andy Shevchenko 

for PDx86 bits on condition that Hans is okay with them.


I'm ok with them now.

The entire series is:

Reviewed-by: Hans de Goede 


Greg, I believe that the intent was for the entire series
to be merged through your tree and I believe that this
series is ready for merging now.

Regards,

Hans






Heikki Krogerus (10):
   usb: typec: Take care of driver module reference counting
   usb: roles: Handle driver reference counting
   platform: x86: intel_cht_int33fe: Add dependency on muxes
   drivers: base: Helpers for adding device connection descriptions
   platform: x86: intel_cht_int33fe: Register all connections at once
   platform: x86: intel_cht_int33fe: Add connection for the DP alt mode
   platform: x86: intel_cht_int33fe: Add connections for the USB Type-C
 port
   usb: typec: class: Don't use port parent for getting mux handles
   platform: x86: intel_cht_int33fe: Remove the old connections for the
 muxes
   usb: typec: fusb302: reorganizing the probe function a little

  drivers/platform/x86/Kconfig |  2 ++
  drivers/platform/x86/intel_cht_int33fe.c | 27 -
  drivers/usb/common/roles.c   | 15 --
  drivers/usb/typec/class.c| 38 ++--
  drivers/usb/typec/fusb302/fusb302.c  | 25 ++--
  drivers/usb/typec/mux.c  | 17 ---
  include/linux/device.h   | 24 +++
  7 files changed, 87 insertions(+), 61 deletions(-)

--
2.18.0

Re: musb_hdrc HNP?

[Bin Liu]

Hi,

On Fri, Aug 31, 2018 at 06:35:50AM +, Takashi Matsuzawa wrote:
> Hello.
> I just confirmed what I wanted to see.
> I could do lsusb to list A-device (from b_host) and B-device (from a_host).
> Suspending from either side kicked role change between A-device and B-device 
> (in both direction).
> 
> I needed to wait 20ms after B-device seeing MUSB_INTR_CONNECT and before 
> calling musb_host_poke_root_hub().
> 
> I suppose seeing CONNECT inturrupt B-device can expect that A-device is 
> reset, but it may take some time and B-device may need to wait.
> On technial nodes, this is mentioned as something like this:
> 
> "Reset Signaling. If the RESET bit in the POWER register (bit 3) is set while 
> the controller is in Host mode, it will generate Reset signaling on the bus. 
> If the HSENAB bit in the POWER register (bit 5) was set, it will also try to 
> negotiate for high-speed operation. The software should keep the RESET bit 
> set for at least 20 ms to ensure correct resetting of the target device."
> 
> Note I still see errors like below after role change (b_host -> 
> b_peripheral), perhaps some necessary cleanup is not there.
> But anyway they role-switched in both direction..

Is commit [1] reverted or not in this experiment?

[1] 0a9134bd733b usb: musb: disable otg protocol support

> 
> [ 1204.225843] usb 2-1: USB disconnect, device number 7
> [ 1204.274238] hub 2-0:1.0: USB hub found
> [ 1204.282564] hub 2-0:1.0: 1 port detected
> [ 1204.496301] musb-hdrc musb-hdrc.1: Babble
> [ 1204.622799] musb_h_ep0_irq 1192: no URB for end 0
> [ 1208.474661] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1210.063965] zero gadget: high-speed config #3: source/sink
> [ 1212.566697] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1212.573607] usb usb2-port1: attempt power cycle
> [ 1216.974544] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1221.066539] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1221.073328] usb usb2-port1: unable to enumerate USB device
> debian@beaglebone:~/wk-b$ 

Regards,
-Bin.

> ===
> 
> 
> From: linux-usb-ow...@vger.kernel.org  on 
> behalf of Takashi Matsuzawa 
> Sent: Friday, August 31, 2018 1:50 PM
> To: Bin Liu
> Cc: linux-usb@vger.kernel.org
> Subject: Re: musb_hdrc HNP?
> 
> Hello.
> FYI.  I made a progress on this, but no solution yet.
> 
> >The smartphone does use HNP, it is not iPhone Carplay, correct?
> 
> At this point, I am trying to see original HNP behavior between two 
> pocketbeagles.
> (After seeing it works, I plan to replace B-device with a phone, and so 
> customization on A-device stack.)
> 
> >1. MUSB uses one register bit to report babble and reset event, so driver 
> >bug could report bus reset as babble if it doesn't trace the controller role 
> >correctly;
> 
> As mentioned below, it may be related to how driver on B-device responds to 
> RESET/BABBLE interrupts (or its sideeffets).
> 
> 1) I could see CONFIG_USB_OTG was not set on "Debian 9.4 2018-06-17" image so 
> I turned it on.
> 
> This improved the situation a bit, like A-device side b_hnp_enable flag 
> (which is set when B-device b_hnp_enable is set.)
> 
> 2) Now I can see A-device and B-device turns to expected modes.
> 
> A-device:
> 
> a_host -> a_peripheral
> After suspending port, sees DISCONNECT and RESET events.
> 
> B-device:
> 
> b_peripheral -> b_host
> Sees SUSPEND, CONNECT events.
> 
> 3) But I do not see B-device enumerate A-device at this point, and instead on 
> B-device (now b_host) RESET(or BUBBLE) events are seen.
> Then after that, immediately, SUSPEND is seen on A-device, looks like now 
> A-device is resuming as a_host and B-device back to b_peripheral.
> 
> 4) I expect at 2) B-device should enumerate A-device and both stays in new 
> mode (and I can, say do lsusb on B-device and see A-device listed), but it 
> does not happen.
> 
> Ignoring RESET(BUBBLE) events on B-device (b_host) at 3) did not improve the 
> situgation.  (Now I see SUSPEND on B-device instead of DISCONNECT.)
> 
> It may be that driver behavior after 2) (to be initiated as a_peripheral and 
> b_host and restarting) having some problem.
> 
> 
> From: linux-usb-ow...@vger.kernel.org  on 
> behalf of Bin Liu 
> Sent: Monday, August 27, 2018 10:33 PM
> To: Takashi Matsuzawa
> Cc: linux-usb@vger.kernel.org
> Subject: Re: musb_hdrc HNP?
> 
> Hi,
> 
> On Mon, Aug 27, 2018 at 12:57:55AM +, Takashi Matsuzawa wrote:
> > Thank you for your suggestion.
> > Yes, I am aware that full-OTG support code is being wiped out of the
> > latest mainline kernels.
> 
> Okay. Let me know if reverting that patch can magically make HNP works.
> 
> > I am trying this for smartphone connectivity where OTG based
> > role-switch is being used, which may not be of interest of everyone.
> 
> The smartphone does use HNP, it is not iPhone Carplay, correct?
> 
> > I picked pocketbeagle since 

[PATCH] usb: Don't die twice if PCI xhci host is not responding in resume

[Mathias Nyman]

usb_hc_died() should only be called once, and with the primary HCD
as parameter. It will mark both primary and secondary hcd's dead.

Remove the extra call to usb_cd_died with the shared hcd as parameter.

Fixes: ff9d78b36f76 ("USB: Set usb_hcd->state and flags for shared roothubs")
Signed-off-by: Mathias Nyman 
---
 drivers/usb/core/hcd-pci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 66fe1b7..0343246 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -515,8 +515,6 @@ static int resume_common(struct device *dev, int event)
event == PM_EVENT_RESTORE);
if (retval) {
dev_err(dev, "PCI post-resume error %d!\n", retval);
-   if (hcd->shared_hcd)
-   usb_hc_died(hcd->shared_hcd);
usb_hc_died(hcd);
}
}
-- 
2.7.4

Aw: Re: Fw: Re: keyboard-problem on bpi-r2 since 4.17

[Frank Wunderlich]

this fixes the issue:

adding the following lines to drivers/usb/host/xhci-mem.c line:1616

if (xhci->quirks & XHCI_MTK_HOST) {
in_ep_ctx->reserved[0] = out_ep_ctx->reserved[0];
in_ep_ctx->reserved[1] = out_ep_ctx->reserved[1];
}
 
commit below is wrong...i got the response:
 
This issue is caused by the commit : 27082e2654dc (“xhci: Clear the host side 
toggle manually when endpoint is ‘soft reset’”)...
 
i think this commit is meant: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f5249461b504d35aa1a40140983b7ec415807d9e

sorry for confusion, had searched and not compared the result again :(

pushed to my github: 
https://github.com/frank-w/BPI-R2-4.14/commit/c9acc5a4285d0de71171c7c1fb3ae7dc0bffec34

official patch will be send by mediatek

regards Frank
 
 

Gesendet: Dienstag, 04. September 2018 um 15:45 Uhr
Von: "Mathias Nyman" 
An: "Frank Wunderlich" , linux-usb@vger.kernel.org
Betreff: Re: Fw: Aw: Re: keyboard-problem on bpi-r2 since 4.17
On 04.09.2018 16:14, Frank Wunderlich wrote:
> Hi,
>
> i got an info that my issue was caused by this commit:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=217491487c43892318c18b6dcafe33b6353efd40
>
> Patch is in progress...

That patch has been there since 4.13
If 4.16 works for you then it's probably something else

>>
>> is there another way to show more debug-messages?
>>
>> i have a working 4.16-tree, maybe i can merge newer versions (go
>> forward from it instead of backwards).
>>

xhci debugging can be added by:

mount -t debugfs none /sys/kernel/debug
echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control


and tracing with: (before pressing keyboard key)

(mount -t debugfs none /sys/kernel/debug)
echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb
echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable

after pressing key traces are found at:
/sys/kernel/debug/tracing/trace

-Mathias

Re: Aw: Re: Fw: Re: keyboard-problem on bpi-r2 since 4.17

[Mathias Nyman]

On 04.09.2018 17:51, Frank Wunderlich wrote:

this fixes the issue:

adding the following lines to drivers/usb/host/xhci-mem.c line:1616

if (xhci->quirks & XHCI_MTK_HOST) {
in_ep_ctx->reserved[0] = out_ep_ctx->reserved[0];
in_ep_ctx->reserved[1] = out_ep_ctx->reserved[1];
}
  
commit below is wrong...i got the response:
  
This issue is caused by the commit : 27082e2654dc (“xhci: Clear the host side toggle manually when endpoint is ‘soft reset’”)...
  
i think this commit is meant: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f5249461b504d35aa1a40140983b7ec415807d9e


sorry for confusion, had searched and not compared the result again :(

pushed to my github: 
https://github.com/frank-w/BPI-R2-4.14/commit/c9acc5a4285d0de71171c7c1fb3ae7dc0bffec34

official patch will be send by mediatek

regards Frank
  


Ok, thanks, makes sense.

I'll wait for that patch

I'm offline rest of the week so responses will be delayed

-Mathias

[PATCH v3 2/6] doc: usb: ci-hdrc-usb2: Add pinctrl properties definition

[Loic Poulain]

Some hardware implementations require to configure pins differently
according to the USB role (host/device), this can be an update of the
pins routing or a simple GPIO value change.

This patch introduces new optional "host" and "device" pinctrls.
If these pinctrls are defined by the device, they are respectively
selected on host/device role start.

Signed-off-by: Loic Poulain 
---
 v2: Add new pin modes documentation (host, device)
 v3: rebase on usb-next

 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt 
b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 2e93181..529e518 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -80,6 +80,8 @@ Optional properties:
   controller. It's expected that a mux state of 0 indicates device mode and a
   mux state of 1 indicates host mode.
 - mux-control-names: Shall be "usb_switch" if mux-controls is specified.
+- pinctrl-names: Names for optional pin modes in "default", "host", "device"
+- pinctrl-n: alternate pin modes
 
 i.mx specific properties
 - fsl,usbmisc: phandler of non-core register device, with one
-- 
2.7.4

[PATCH v3 1/6] usb: chipidea: Add dynamic pinctrl selection

[Loic Poulain]

Some hardware implementations require to configure pins differently
according to the USB role (host/device), this can be an update of the
pins routing or a simple GPIO value change.

This patch introduces new optional "host" and "device" pinctrls.
If these pinctrls are defined by the device, they are respectively
selected on host/device role start.

If a default pinctrl exist, it is restored on host/device role stop.

Signed-off-by: Loic Poulain 
---
 v2: includes ordering
 v3: no change

 drivers/usb/chipidea/core.c  | 19 +++
 drivers/usb/chipidea/host.c  |  9 +
 drivers/usb/chipidea/udc.c   |  9 +
 include/linux/usb/chipidea.h |  6 ++
 4 files changed, 43 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 85fc6db..7bfcbb2 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -53,6 +53,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -723,6 +724,24 @@ static int ci_get_platdata(struct device *dev,
else
cable->connected = false;
}
+
+   platdata->pctl = devm_pinctrl_get(dev);
+   if (!IS_ERR(platdata->pctl)) {
+   struct pinctrl_state *p;
+
+   p = pinctrl_lookup_state(platdata->pctl, "default");
+   if (!IS_ERR(p))
+   platdata->pins_default = p;
+
+   p = pinctrl_lookup_state(platdata->pctl, "host");
+   if (!IS_ERR(p))
+   platdata->pins_host = p;
+
+   p = pinctrl_lookup_state(platdata->pctl, "device");
+   if (!IS_ERR(p))
+   platdata->pins_device = p;
+   }
+
return 0;
 }
 
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 4638d9b..d858a82 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../host/ehci.h"
 
@@ -153,6 +154,10 @@ static int host_start(struct ci_hdrc *ci)
}
}
 
+   if (ci->platdata->pins_host)
+   pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_host);
+
ret = usb_add_hcd(hcd, 0, 0);
if (ret) {
goto disable_reg;
@@ -197,6 +202,10 @@ static void host_stop(struct ci_hdrc *ci)
}
ci->hcd = NULL;
ci->otg.host = NULL;
+
+   if (ci->platdata->pins_host && ci->platdata->pins_default)
+   pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
 }
 
 
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 9852ec5..829e947 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1965,6 +1966,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
 
 static int udc_id_switch_for_device(struct ci_hdrc *ci)
 {
+   if (ci->platdata->pins_device)
+   pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_device);
+
if (ci->is_otg)
/* Clear and enable BSV irq */
hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
@@ -1983,6 +1988,10 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);
 
ci->vbus_active = 0;
+
+   if (ci->platdata->pins_device && ci->platdata->pins_default)
+   pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
 }
 
 /**
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 07f9936..63758c3 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -77,6 +77,12 @@ struct ci_hdrc_platform_data {
struct ci_hdrc_cablevbus_extcon;
struct ci_hdrc_cableid_extcon;
u32 phy_clkgate_delay_us;
+
+   /* pins */
+   struct pinctrl *pctl;
+   struct pinctrl_state *pins_default;
+   struct pinctrl_state *pins_host;
+   struct pinctrl_state *pins_device;
 };
 
 /* Default offset of capability registers */
-- 
2.7.4

[PATCH v3 3/6] usb: chipidea: Prevent unbalanced IRQ disable

[Loic Poulain]

The ChipIdea IRQ is disabled before scheduling the otg work and
re-enabled on otg work completion. However if the job is already
scheduled we have to undo the effect of disable_irq int order to
balance the IRQ disable-depth value.

Fixes: be6b0c1bd0be ("usb: chipidea: using one inline function to cover queue 
work operations")
Signed-off-by: Loic Poulain 
---
 v2: v3: no change

 drivers/usb/chipidea/otg.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 7e7428e..4f8b817 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -17,7 +17,8 @@ void ci_handle_vbus_change(struct ci_hdrc *ci);
 static inline void ci_otg_queue_work(struct ci_hdrc *ci)
 {
disable_irq_nosync(ci->irq);
-   queue_work(ci->wq, &ci->work);
+   if (queue_work(ci->wq, &ci->work) == false)
+   enable_irq(ci->irq);
 }
 
 #endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
-- 
2.7.4

[PATCH v3 5/6] phy: qcom-usb-hs: Fix unbalanced notifier registration

[Loic Poulain]

Phy power on/off cycle can happen several times during device life.
We then need to balance the extcon notifier registration accordingly.

Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
Signed-off-by: Loic Poulain 
---
 v2: don't use devres version (power-on always followed by power-off)
 v3: no change

 drivers/phy/qualcomm/phy-qcom-usb-hs.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c 
b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index abbbe75..7153dde 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -160,8 +160,8 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
/* setup initial state */
qcom_usb_hs_phy_vbus_notifier(&uphy->vbus_notify, state,
  uphy->vbus_edev);
-   ret = devm_extcon_register_notifier(&ulpi->dev, uphy->vbus_edev,
-   EXTCON_USB, &uphy->vbus_notify);
+   ret = extcon_register_notifier(uphy->vbus_edev, EXTCON_USB,
+  &uphy->vbus_notify);
if (ret)
goto err_ulpi;
}
@@ -182,6 +182,11 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
 {
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
 
+   if (uphy->vbus_edev) {
+   extcon_unregister_notifier(uphy->vbus_edev, EXTCON_USB,
+  &uphy->vbus_notify);
+   }
+
regulator_disable(uphy->v3p3);
regulator_disable(uphy->v1p8);
clk_disable_unprepare(uphy->sleep_clk);
-- 
2.7.4

[PATCH v3 4/6] usb: chipidea: Fix otg event handler

[Loic Poulain]

At OTG work running time, it's possible that several events need to be
addressed (e.g. ID and VBUS events). The current implementation handles
only one event at a time which leads to ignoring the other one. Fix it.

Signed-off-by: Loic Poulain 
---
 v2: v3: no change

 drivers/usb/chipidea/otg.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index db4ceff..f25d482 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -203,14 +203,17 @@ static void ci_otg_work(struct work_struct *work)
}
 
pm_runtime_get_sync(ci->dev);
+
if (ci->id_event) {
ci->id_event = false;
ci_handle_id_switch(ci);
-   } else if (ci->b_sess_valid_event) {
+   }
+
+   if (ci->b_sess_valid_event) {
ci->b_sess_valid_event = false;
ci_handle_vbus_change(ci);
-   } else
-   dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
+   }
+
pm_runtime_put_sync(ci->dev);
 
enable_irq(ci->irq);
-- 
2.7.4

[PATCH v3 6/6] arm: dts: qcom: db410c: Enable USB OTG support

[Loic Poulain]

The Dragonboard-410c is able to act either as USB Host or Device.
The role can be determined at runtime via the USB_HS_ID pin which is
derived from the micro-usb port VBUS pin.

In Host role, SoC USB D+/D- are routed to the onboard USB 2.0 HUB.
In Device role, SoC USB D+/D- are routed to the USB 2.0 micro B port.
Routing is selected via USB_SW_SEL_PM gpio.

In device role USB HUB can be held in reset.

chipidea driver expects two extcon device pointers, one for the
EXTCON_USB event and one for the EXTCON_USB_HOST event. Since
the extcon-usb-gpio device is capable of generating both these
events, point two times to this extcon device.

Signed-off-by: Loic Poulain 
---
 v2: no change
 v3: Point two times to the same extcon-usb-device

 arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi | 20 
 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi   | 11 ++-
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi 
b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
index ec2f0de..99787cc 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
@@ -8,6 +8,16 @@
pinconf {
pins = "gpio3";
function = PMIC_GPIO_FUNC_NORMAL;
+   input-disable;
+   output-high;
+   };
+   };
+
+   usb_hub_reset_pm_device: usb_hub_reset_pm_device {
+   pinconf {
+   pins = "gpio3";
+   function = PMIC_GPIO_FUNC_NORMAL;
+   input-disable;
output-low;
};
};
@@ -22,6 +32,16 @@
};
};
 
+   usb_sw_sel_pm_device: usb_sw_sel_pm_device {
+   pinconf {
+   pins = "gpio4";
+   function = PMIC_GPIO_FUNC_NORMAL;
+   power-source = ;
+   input-disable;
+   output-low;
+   };
+   };
+
pm8916_gpios_leds: pm8916_gpios_leds {
pinconf {
pins = "gpio1", "gpio2";
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi 
b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 78ce397..1f7dc1c 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -366,14 +366,15 @@
};
 
usb@78d9000 {
-   extcon = <&usb_id>;
+   extcon = <&usb_id>, <&usb_id>;
status = "okay";
adp-disable;
hnp-disable;
srp-disable;
-   dr_mode = "host";
-   pinctrl-names = "default";
-   pinctrl-0 = <&usb_sw_sel_pm>;
+   dr_mode = "otg";
+   pinctrl-names = "default", "device";
+   pinctrl-0 = <&usb_sw_sel_pm &usb_hub_reset_pm>;
+   pinctrl-1 = <&usb_sw_sel_pm_device 
&usb_hub_reset_pm_device>;
ulpi {
phy {
v1p8-supply = <&pm8916_l7>;
@@ -512,7 +513,7 @@
 
usb_id: usb-id {
compatible = "linux,extcon-usb-gpio";
-   vbus-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
+   id-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&usb_id_default>;
};
-- 
2.7.4

Re: A few questions about gadgetfs

[Andrey Konovalov]

Hi Alan,

Another question. According to the USB spec:

"""
The Data stage of a control transfer from an endpoint to the host is
complete when the endpoint does one of the following:
* Has transferred exactly the amount of data specified during the Setup stage
* Transfers a packet with a payload size less than wMaxPacketSize or
transfers a zero-length packet
"""

AFAIU, this means that a device only needs to send a zero-length
packet when the last data packet happened to be equal to
wMaxPacketSize to indicate that the data stage is over. Gadgetfs seems
to be doing something different, it sets the req->zero flag (for the
response passed to usb_ep_queue) when value < w_length, where value is
the length of the response being submitted and w_length is the value
of the wLength field of the control request. Is there something I
misunderstand?

Thanks!

Re: [PATCH] USB: change dev_WARN to dev_err triggerable from user space

[Alan Stern]

On Tue, 4 Sep 2018, Johan Hovold wrote:

> On Tue, Sep 04, 2018 at 12:21:09PM +0200, Oliver Neukum wrote:
> > On Di, 2018-09-04 at 11:31 +0200, Johan Hovold wrote:
> > > On Tue, Sep 04, 2018 at 10:44:41AM +0200, Oliver Neukum wrote:
> > > > For those people who run with panic_on_warn a WARN() triggered
> > > > from user space is a DOS. It is worth returning to dev_err()
> > > 
> > > I think this should be dev_warn() unless you want to bring back the
> > > returning of errors on these conditions as well (i.e. as was the case
> > > prior to 0cb54a3e47cb ("USB: debugging code shouldn't alter control
> > > flow")).
> > 
> > Should I? A warning in syslog is pretty hardcore, so I have no idea
> > whether dev_warn() is enough.
> 
> Perhaps there are two sides to this. If something really should not be
> happening and needs to be addressed (i.e. it's a driver bug) that
> dev_WARN is warranted. If user space can be pass in bogus flags that
> gets propagated to USB core, perhaps those need to be sanitised sooner
> (in the vain of "don't trust anything coming from user space").

I'd go along with this.  The usbfs code should fix or reject URBs 
submitted from userspace with bogus flags or an incorrect pipe value.  
(In fact, we already sanitize the flags to some extent, but we could do 
more: ISO_ASAP should apply only to isochronous URBs, and ZERO_PACKET 
should apply only to bulk-OUT URBS.)

Similar errors coming from kernel drivers should be reported as actual 
bugs.

Alan

> In general though, I believe dev_warn() is more appropriate for cases
> where something odd is happening, but we try to recover and proceed
> anyway (e.g. by sanitising the flags without bailing out as is the case
> here) instead of aborting.
> 
> Johan

Re: gadgetfs & endpoints

[Alan Stern]

On Tue, 4 Sep 2018, Ranran wrote:

> On Tue, Sep 4, 2018 at 2:36 PM Ranran  wrote:
> >
> > Hello,
> >
> > In gadgetfs documentation I find no mention of endpoints:
> > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt
> >
> > Is it possible to create endpoints using gadgetfs ?
> >
> 
> The question need to be phrased differently:
> Is it possible to create HID using gadgetfs.

Yes, it is (as far as I know -- I haven't actually tried to do it).

> I see that it is possible to create usb device with endpoints as described 
> here:
> http://www.linux-usb.org/gadget/usb.c
> But what about HID device ? How can we implement HID device using gadget ?

That's a complicated question.  You can't implement an HID gadget
without first understanding the HID specification.  Have you read the 
specification and do you understand it?

Assuming you have, are there any specific questions you need answers 
to?

Alan Stern

Re: [PATCH] usb: Don't die twice if PCI xhci host is not responding in resume

[Alan Stern]

On Tue, 4 Sep 2018, Mathias Nyman wrote:

> usb_hc_died() should only be called once, and with the primary HCD
> as parameter. It will mark both primary and secondary hcd's dead.
> 
> Remove the extra call to usb_cd_died with the shared hcd as parameter.
> 
> Fixes: ff9d78b36f76 ("USB: Set usb_hcd->state and flags for shared roothubs")
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/core/hcd-pci.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 66fe1b7..0343246 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -515,8 +515,6 @@ static int resume_common(struct device *dev, int event)
>   event == PM_EVENT_RESTORE);
>   if (retval) {
>   dev_err(dev, "PCI post-resume error %d!\n", retval);
> - if (hcd->shared_hcd)
> - usb_hc_died(hcd->shared_hcd);
>   usb_hc_died(hcd);
>   }
>   }

Acked-by: Alan Stern 

Re: A few questions about gadgetfs

[Alan Stern]

On Tue, 4 Sep 2018, Andrey Konovalov wrote:

> Hi Alan,
> 
> Another question. According to the USB spec:
> 
> """
> The Data stage of a control transfer from an endpoint to the host is
> complete when the endpoint does one of the following:
> * Has transferred exactly the amount of data specified during the Setup stage
> * Transfers a packet with a payload size less than wMaxPacketSize or
> transfers a zero-length packet
> """
> 
> AFAIU, this means that a device only needs to send a zero-length
> packet when the last data packet happened to be equal to
> wMaxPacketSize to indicate that the data stage is over.

And if the total reply size is less than wLength in the Setup stage.

>  Gadgetfs seems
> to be doing something different, it sets the req->zero flag (for the
> response passed to usb_ep_queue) when value < w_length, where value is
> the length of the response being submitted and w_length is the value
> of the wLength field of the control request. Is there something I
> misunderstand?

No, you got it.  But you're missing an important fact: The USB 
controller drivers will ignore the ->zero flag in cases where the 
message length isn't an exact multiple of wMaxPacketSize.  This is what 
makes gadgetfs's behavior correct.

Alan Stern

Re: [PATCH v2] usb: Avoid use-after-free by flushing endpoints early in usb_set_interface()

[Alan Stern]

On Mon, 3 Sep 2018, Mathias Nyman wrote:

> The steps taken by usb core to set a new interface is very different from
> what is done on the xHC host side.
> 
> xHC hardware will do everything in one go. One command is used to set up
> new endpoints, free old endpoints, check bandwidth, and run the new
> endpoints.
> 
> All this is done by xHC when usb core asks the hcd to check for
> available bandwidth. At this point usb core has not yet flushed the old
> endpoints, which will cause use-after-free issues in xhci driver as
> queued URBs are cancelled on a re-allocated endpoint.
> 
> To resolve this add a call to usb_disable_interface() which will flush
> the endpoints before calling usb_hcd_alloc_bandwidth()
> 
> Additional checks in xhci driver will also be implemented to gracefully
> handle stale URB cancel on freed and re-allocated endpoints
> 
> Cc: 
> Reported-by: Sudip Mukherjee 
> Signed-off-by: Mathias Nyman 
> ---
>   v2: update kerneldoc as well
> ---
>  drivers/usb/core/message.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 228672f..bfa5eda 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1341,6 +1341,11 @@ void usb_enable_interface(struct usb_device *dev,
>   * is submitted that needs that bandwidth.  Some other operating systems
>   * allocate bandwidth early, when a configuration is chosen.
>   *
> + * xHCI reserves bandwidth and configures the alternate setting in
> + * usb_hcd_alloc_bandwidth(). If it fails the original interface altsetting
> + * may be disabled. Drivers cannot rely on any particular alternate
> + * setting being in effect after a failure.
> + *
>   * This call is synchronous, and may not be used in an interrupt context.
>   * Also, drivers must not change altsettings while urbs are scheduled for
>   * endpoints in that interface; all such urbs must first be completed
> @@ -1376,6 +1381,12 @@ int usb_set_interface(struct usb_device *dev, int 
> interface, int alternate)
>alternate);
>   return -EINVAL;
>   }
> + /*
> +  * usb3 hosts configure the interface in usb_hcd_alloc_bandwidth,
> +  * including freeing dropped endpoint ring buffers.
> +  * Make sure the interface endpoints are flushed before that
> +  */
> + usb_disable_interface(dev, iface, false);
>  
>   /* Make sure we have enough bandwidth for this alternate interface.
>* Remove the current alt setting and add the new alt setting.

Acked-by: Alan Stern 

[hid:for-4.20/multitouch 1/1] drivers/hid/hid-multitouch.c:1327:5: error: 'struct hid_input' has no member named 'application'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git 
for-4.20/multitouch
head:   93c61d2e6bc4849a5527e32568479ff67154870e
commit: 93c61d2e6bc4849a5527e32568479ff67154870e [1/1] HID: multitouch: 
simplify the application retrieval
config: x86_64-randconfig-x018-201835 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 93c61d2e6bc4849a5527e32568479ff67154870e
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/hid/hid-multitouch.c: In function 'mt_input_mapping':
>> drivers/hid/hid-multitouch.c:1327:5: error: 'struct hid_input' has no member 
>> named 'application'
  hi->application = HID_DG_STYLUS;
^~
   drivers/hid/hid-multitouch.c: In function 'mt_input_configured':
   drivers/hid/hid-multitouch.c:1527:12: error: 'struct hid_input' has no 
member named 'application'
 switch (hi->application) {
   ^~

vim +1327 drivers/hid/hid-multitouch.c

  1257  
  1258  #define mt_map_key_clear(c) hid_map_usage_clear(hi, usage, bit, \
  1259  max, EV_KEY, (c))
  1260  static int mt_input_mapping(struct hid_device *hdev, struct hid_input 
*hi,
  1261  struct hid_field *field, struct hid_usage *usage,
  1262  unsigned long **bit, int *max)
  1263  {
  1264  struct mt_device *td = hid_get_drvdata(hdev);
  1265  struct mt_application *application;
  1266  struct mt_report_data *rdata;
  1267  
  1268  rdata = mt_find_report_data(td, field->report);
  1269  if (!rdata) {
  1270  hid_err(hdev, "failed to allocate data for report\n");
  1271  return 0;
  1272  }
  1273  
  1274  application = rdata->application;
  1275  
  1276  /*
  1277   * If mtclass.export_all_inputs is not set, only map fields from
  1278   * TouchScreen or TouchPad collections. We need to ignore fields
  1279   * that belong to other collections such as Mouse that might 
have
  1280   * the same GenericDesktop usages.
  1281   */
  1282  if (!td->mtclass.export_all_inputs &&
  1283  field->application != HID_DG_TOUCHSCREEN &&
  1284  field->application != HID_DG_PEN &&
  1285  field->application != HID_DG_TOUCHPAD &&
  1286  field->application != HID_GD_KEYBOARD &&
  1287  field->application != HID_GD_SYSTEM_CONTROL &&
  1288  field->application != HID_CP_CONSUMER_CONTROL &&
  1289  field->application != HID_GD_WIRELESS_RADIO_CTLS &&
  1290  field->application != HID_GD_SYSTEM_MULTIAXIS &&
  1291  !(field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS &&
  1292application->quirks & MT_QUIRK_ASUS_CUSTOM_UP))
  1293  return -1;
  1294  
  1295  /*
  1296   * Some Asus keyboard+touchpad devices have the hotkeys defined 
in the
  1297   * touchpad report descriptor. We need to treat these as an 
array to
  1298   * map usages to input keys.
  1299   */
  1300  if (field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS &&
  1301  application->quirks & MT_QUIRK_ASUS_CUSTOM_UP &&
  1302  (usage->hid & HID_USAGE_PAGE) == HID_UP_CUSTOM) {
  1303  set_bit(EV_REP, hi->input->evbit);
  1304  if (field->flags & HID_MAIN_ITEM_VARIABLE)
  1305  field->flags &= ~HID_MAIN_ITEM_VARIABLE;
  1306  switch (usage->hid & HID_USAGE) {
  1307  case 0x10: mt_map_key_clear(KEY_BRIGHTNESSDOWN);
break;
  1308  case 0x20: mt_map_key_clear(KEY_BRIGHTNESSUP);  
break;
  1309  case 0x35: mt_map_key_clear(KEY_DISPLAY_OFF);   
break;
  1310  case 0x6b: mt_map_key_clear(KEY_F21);   
break;
  1311  case 0x6c: mt_map_key_clear(KEY_SLEEP); 
break;
  1312  default:
  1313  return -1;
  1314  }
  1315  return 1;
  1316  }
  1317  
  1318  if (rdata->is_mt_collection)
  1319  return mt_touch_input_mapping(hdev, hi, field, usage, 
bit, max,
  1320application);
  1321  
  1322  /*
  1323   * some egalax touchscreens have "application == DG_TOUCHSCREEN"
  1324   * for the stylus. Overwrite the hid_input application
  1325   */
  1326  if (field->physical == HID_DG_STYLUS)
> 1327  hi->application = HID_DG_STYLUS;
  1328  
  1329  /* let hid-core decide for the others */
  1330  return 0;
  1331  }
  1332  

---
0-DAY kernel test infrastructureOpen Source Technol

Re: [hid:for-4.20/multitouch 1/1] drivers/hid/hid-multitouch.c:1327:5: error: 'struct hid_input' has no member named 'application'

[Jiri Kosina]

On Wed, 5 Sep 2018, kbuild test robot wrote:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git 
> for-4.20/multitouch
> head:   93c61d2e6bc4849a5527e32568479ff67154870e
> commit: 93c61d2e6bc4849a5527e32568479ff67154870e [1/1] HID: multitouch: 
> simplify the application retrieval
> config: x86_64-randconfig-x018-201835 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
> git checkout 93c61d2e6bc4849a5527e32568479ff67154870e
> # save the attached .config to linux build tree
> make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>drivers/hid/hid-multitouch.c: In function 'mt_input_mapping':
> >> drivers/hid/hid-multitouch.c:1327:5: error: 'struct hid_input' has no 
> >> member named 'application'
>   hi->application = HID_DG_STYLUS;
> ^~
>drivers/hid/hid-multitouch.c: In function 'mt_input_configured':
>drivers/hid/hid-multitouch.c:1527:12: error: 'struct hid_input' has no 
> member named 'application'
>  switch (hi->application) {
>^~

That is my mismerge. I've fixed it now.

Thanks for the report and sorry for the noise.

-- 
Jiri Kosina
SUSE Labs

Re: musb_hdrc HNP?

[Takashi Matsuzawa]

Hello.

>Is commit [1] reverted or not in this experiment?

In fact I noticed I am looking into TI kernel which is distributed in official 
debian image for pocketbeagle.
So, the behavior may not be the same as mainline kernel code.
I think I continue look into what I have now, and later check mainline status 
when I need to support (or try) other boards as well.

From: linux-usb-ow...@vger.kernel.org  on 
behalf of Bin Liu 
Sent: Tuesday, September 4, 2018 11:00 PM
To: Takashi Matsuzawa
Cc: linux-usb@vger.kernel.org
Subject: Re: musb_hdrc HNP?

Hi,

On Fri, Aug 31, 2018 at 06:35:50AM +, Takashi Matsuzawa wrote:
> Hello.
> I just confirmed what I wanted to see.
> I could do lsusb to list A-device (from b_host) and B-device (from a_host).
> Suspending from either side kicked role change between A-device and B-device 
> (in both direction).
>
> I needed to wait 20ms after B-device seeing MUSB_INTR_CONNECT and before 
> calling musb_host_poke_root_hub().
>
> I suppose seeing CONNECT inturrupt B-device can expect that A-device is 
> reset, but it may take some time and B-device may need to wait.
> On technial nodes, this is mentioned as something like this:
>
> "Reset Signaling. If the RESET bit in the POWER register (bit 3) is set while 
> the controller is in Host mode, it will generate Reset signaling on the bus. 
> If the HSENAB bit in the POWER register (bit 5) was set, it will also try to 
> negotiate for high-speed operation. The software should keep the RESET bit 
> set for at least 20 ms to ensure correct resetting of the target device."
>
> Note I still see errors like below after role change (b_host -> 
> b_peripheral), perhaps some necessary cleanup is not there.
> But anyway they role-switched in both direction..

Is commit [1] reverted or not in this experiment?

[1] 0a9134bd733b usb: musb: disable otg protocol support

> 
> [ 1204.225843] usb 2-1: USB disconnect, device number 7
> [ 1204.274238] hub 2-0:1.0: USB hub found
> [ 1204.282564] hub 2-0:1.0: 1 port detected
> [ 1204.496301] musb-hdrc musb-hdrc.1: Babble
> [ 1204.622799] musb_h_ep0_irq 1192: no URB for end 0
> [ 1208.474661] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1210.063965] zero gadget: high-speed config #3: source/sink
> [ 1212.566697] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1212.573607] usb usb2-port1: attempt power cycle
> [ 1216.974544] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1221.066539] usb usb2-port1: Cannot enable. Maybe the USB cable is bad?
> [ 1221.073328] usb usb2-port1: unable to enumerate USB device
> debian@beaglebone:~/wk-b$

Regards,
-Bin.

> ===
>
> 
> From: linux-usb-ow...@vger.kernel.org  on 
> behalf of Takashi Matsuzawa 
> Sent: Friday, August 31, 2018 1:50 PM
> To: Bin Liu
> Cc: linux-usb@vger.kernel.org
> Subject: Re: musb_hdrc HNP?
>
> Hello.
> FYI.  I made a progress on this, but no solution yet.
>
> >The smartphone does use HNP, it is not iPhone Carplay, correct?
>
> At this point, I am trying to see original HNP behavior between two 
> pocketbeagles.
> (After seeing it works, I plan to replace B-device with a phone, and so 
> customization on A-device stack.)
>
> >1. MUSB uses one register bit to report babble and reset event, so driver 
> >bug could report bus reset as babble if it doesn't trace the controller role 
> >correctly;
>
> As mentioned below, it may be related to how driver on B-device responds to 
> RESET/BABBLE interrupts (or its sideeffets).
>
> 1) I could see CONFIG_USB_OTG was not set on "Debian 9.4 2018-06-17" image so 
> I turned it on.
>
> This improved the situation a bit, like A-device side b_hnp_enable flag 
> (which is set when B-device b_hnp_enable is set.)
>
> 2) Now I can see A-device and B-device turns to expected modes.
>
> A-device:
>
> a_host -> a_peripheral
> After suspending port, sees DISCONNECT and RESET events.
>
> B-device:
>
> b_peripheral -> b_host
> Sees SUSPEND, CONNECT events.
>
> 3) But I do not see B-device enumerate A-device at this point, and instead on 
> B-device (now b_host) RESET(or BUBBLE) events are seen.
> Then after that, immediately, SUSPEND is seen on A-device, looks like now 
> A-device is resuming as a_host and B-device back to b_peripheral.
>
> 4) I expect at 2) B-device should enumerate A-device and both stays in new 
> mode (and I can, say do lsusb on B-device and see A-device listed), but it 
> does not happen.
>
> Ignoring RESET(BUBBLE) events on B-device (b_host) at 3) did not improve the 
> situgation.  (Now I see SUSPEND on B-device instead of DISCONNECT.)
>
> It may be that driver behavior after 2) (to be initiated as a_peripheral and 
> b_host and restarting) having some problem.
>
> 
> From: linux-usb-ow...@vger.kernel.org  on 
> behalf of Bin Liu 
> Sent: Monday, August 27, 2018 10:33 PM
> To: Takashi Matsuzawa
> Cc: linux-usb@vger.kernel.org
>

RE: [PATCH v3 4/6] usb: chipidea: Fix otg event handler

[Peter Chen]

 
>  v2: v3: no change
> 
>  drivers/usb/chipidea/otg.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> db4ceff..f25d482 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -203,14 +203,17 @@ static void ci_otg_work(struct work_struct *work)
>   }
> 
>   pm_runtime_get_sync(ci->dev);
> +
>   if (ci->id_event) {
>   ci->id_event = false;
>   ci_handle_id_switch(ci);
> - } else if (ci->b_sess_valid_event) {
> + }
> +
> + if (ci->b_sess_valid_event) {
>   ci->b_sess_valid_event = false;
>   ci_handle_vbus_change(ci);
> - } else
> - dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
> + }
> +
>   pm_runtime_put_sync(ci->dev);
> 
>   enable_irq(ci->irq);
> --

For chipidea changes, I will apply them. Thanks.

Peter

Re: [PATCH v3] USB: serial: ftdi_sio: Add support for CBUS GPIO

[Loic Poulain]

Hi Johan,

On 4 September 2018 at 14:47, Johan Hovold  wrote:
> On Sat, Aug 04, 2018 at 12:17:15PM +0200, Loic Poulain wrote:
>> Some FTDI devices like FTX or FT232R support CBUS Bit Bang mode on CBUS
>> pins, allowing host to control them via simple USB control transfers.
>> To make use of a CBUS pin in Bit Bang mode, the pin must be configured
>> to I/O mode in the FTDI EEPROM. This mode perfectly coexists with regular
>> USB to Serial function.
>>
>> In this implementation, a GPIO controller is registered on FTDI probe
>> if at least one CBUS pin is configured for I/O mode. For now, only
>> FTX devices are supported.
>>
>> This patch is based on previous Stefan Agner implementation tentative
>> on LKML [1].
>>
>> [1] Message-Id: 1434838377-8042-1-git-send-email-ste...@agner.ch
>>
>> Signed-off-by: Loic Poulain 
>> ---
>> v2: Use message-id for LKML reference
>> Rework read_eeprom according to Andy's comment and return read count
>> Remove noisy messages
>> Comment style alignment
>> Add defines for magic values
>> Cannot use devm, because gpiochip is linked to the port not to the udev
>> v3:
>>Fix typo in CBUS mask description comment
>
> First, thanks for looking into this; seems like we're finally about to
> get support for the CBUS gpios.
>
> But now I get not one, but two, competing implementations for this
> posted in one month:
>
> https://lkml.kernel.org/r/20180825204744.2307-1-pa...@pados.hu
>
> And it looks like you both have been considering at least some of the
> earlier attempts, which is great.
>
> I've reviewed both patches and it seems to me that Karoly's version is
> closer to what I'd like to see as the end-result. Specifically this
> means having:
>
>  - fixed offsets for the physical pins rather than having the offsets
>depend on eeprom configuration
>
>  - better type abstraction (we want to add support for ft232r and ft232h
>eventually as well)
>
> The other patch is also more complete in that it:
>
>  - considers the initial pin state
>  - implements a request() callback
>  - implements a get_direction() callback
>
> In contrast, with this implementation as it stands, we could end up with
> a driven pin being reported as an input (corner case, but still).
>
> Implementation-wise, the other patch is also closer to what I'd like to
> see (e.g. not using atomic bit ops, and getting the error handling right
> from the start).
>
> There are some issues that needs to be addressed in the other patch as
> well, and in doing so it would be wise to look at your patch for
> inspiration (e.g. naming issues and adding an eeprom helper to only read
> the couple of configuration words needed).
>
> In the end, going with the other patch means less work for me, even if
> you both (by unfortunate timing) have forced me to do more than twice
> the amount of review work already.

Thanks for review, I'm fine with that and I'll review the other patch.
Karoly can use chunks of my patch if useful.

Regards,
Loic

Re: [PATCH 3/4 next] net: lan78xx: Switch to SPDX identifier

[Stefan Wahren]

Hi Raghu,


Am 04.09.2018 um 06:29 schrieb raghuramchary.jallipa...@microchip.com:
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/* Copyright (C) 2015 Microchip Technology */
> Can we merge both in single comment line?

i don't think we can do this. I'm simply following this guideline [1].

How about this:

// SPDX-License-Identifier: GPL-2.0+
/*
 * Copyright (C) 2015 Microchip Technology
 */

[1] - https://www.kernel.org/doc/html/v4.18/process/license-rules.html

>
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/* Copyright (C) 2015 Microchip Technology */
> Here too.
>
> Thanks,
> -Raghu

RE: [PATCH 3/4 next] net: lan78xx: Switch to SPDX identifier

[RaghuramChary.Jallipalli]

> i don't think we can do this. I'm simply following this guideline [1].
> 
> How about this:
> 
> // SPDX-License-Identifier: GPL-2.0+
> /*
>  * Copyright (C) 2015 Microchip Technology
>  */
> 
> [1] - https://www.kernel.org/doc/html/v4.18/process/license-rules.html
> 
Thank you.
Looks good to me.

Thanks,
-Raghu

Re: [PATCH] linux/mod_devicetable.h: fix kernel-doc missing notation for typec_device_id

[Heikki Krogerus]

On Mon, Sep 03, 2018 at 12:51:59PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap 
> 
> Fix kernel-doc warning for missing struct member description:
> 
> ../include/linux/mod_devicetable.h:763: warning: Function parameter or member 
> 'driver_data' not described in 'typec_device_id'
> 
> Fixes: 8a37d87d72f0c ("usb: typec: Bus type for alternate modes")
> 
> Signed-off-by: Randy Dunlap 

Reviewed-by: Heikki Krogerus 

> ---
>  include/linux/mod_devicetable.h |1 +
>  1 file changed, 1 insertion(+)
> 
> --- lnx-419-rc2.orig/include/linux/mod_devicetable.h
> +++ lnx-419-rc2/include/linux/mod_devicetable.h
> @@ -754,6 +754,7 @@ struct tb_service_id {
>   * struct typec_device_id - USB Type-C alternate mode identifiers
>   * @svid: Standard or Vendor ID
>   * @mode: Mode index
> + * @driver_data: Driver specific data
>   */
>  struct typec_device_id {
>   __u16 svid;
> 

Thanks,

-- 
heikki

RE: [PATCH v2 0/8] usb: dwc3: Fix broken BULK stream support to dwc3 gadget driver

[Anurag Kumar Vulisha]

Hi All,

Ping!
Just thought of giving a friendly remainder for this patch series. Does these 
patches looks okay, can we proceed with them? Please provide me your valuable 
suggestions.

Thanks,
Anurag Kumar Vulisha

>-Original Message-
>From: Anurag Kumar Vulisha [mailto:anurag.kumar.vuli...@xilinx.com]
>Sent: Friday, August 17, 2018 5:55 PM
>To: ba...@kernel.org; gre...@linuxfoundation.org
>Cc: v.anuragku...@gmail.com; linux-usb@vger.kernel.org; linux-
>ker...@vger.kernel.org; Anurag Kumar Vulisha 
>Subject: [PATCH v2 0/8] usb: dwc3: Fix broken BULK stream support to dwc3 
>gadget
>driver
>
>These patch series fixes the broken BULK streaming support in
>dwc3 gadget driver.
>
>Changes in v2:
>   1. Added "usb: dwc3:" in subject heading
>
>Anurag Kumar Vulisha (8):
>  usb: dwc3: Correct the logic for checking TRB full in
>__dwc3_prepare_one_trb()
>  usb: dwc3: update stream id in depcmd
>  usb: dwc3: make controller clear transfer resources after complete
>  usb: dwc3: implement stream transfer timeout
>  usb: dwc3: don't issue no-op trb for stream capable endpoints
>  usb: dwc3: check for requests in started list for stream capable
>endpoints
>  usb: dwc3: Check for IOC/LST bit in both event->status and TRB->ctrl
>fields
>  usb: dwc3: Check MISSED ISOC bit only for ISOC endpoints
>
> drivers/usb/dwc3/core.h   |  7 +
> drivers/usb/dwc3/gadget.c | 78 ++-
>
> 2 files changed, 78 insertions(+), 7 deletions(-)
>
>--
>2.1.1

Re: [PATCH 01/25] tty: Change return type to void

[Sergei Shtylyov]

Hello!

On 9/4/2018 5:44 AM, Jaejoong Kim wrote:


Many drivers with tty use the tty_stand_install(). But, there is no
need to handle the error, since it always returns 0. So, change the
return type of tty_standard_install() and tty_port_install() to void
type and remove unnecessary exception handling where we use these
functions.

Signed-off-by: Jaejoong Kim 
---
  drivers/tty/tty_io.c   | 10 ++
  drivers/tty/tty_port.c |  4 ++--
  include/linux/tty.h|  4 ++--
  3 files changed, 10 insertions(+), 8 deletions(-)


[...]

diff --git a/include/linux/tty.h b/include/linux/tty.h
index c56e397..63cdac1 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -556,7 +556,7 @@ extern struct tty_struct *tty_init_dev(struct tty_driver 
*driver, int idx);
  extern void tty_release_struct(struct tty_struct *tty, int idx);
  extern int tty_release(struct inode *inode, struct file *filp);
  extern void tty_init_termios(struct tty_struct *tty);
-extern int tty_standard_install(struct tty_driver *driver,
+extern void tty_standard_install(struct tty_driver *driver,
struct tty_struct *tty);
  
  extern struct mutex tty_mutex;

@@ -688,7 +688,7 @@ extern int tty_port_close_start(struct tty_port *port,
  extern void tty_port_close_end(struct tty_port *port, struct tty_struct *tty);
  extern void tty_port_close(struct tty_port *port,
struct tty_struct *tty, struct file *filp);
-extern int tty_port_install(struct tty_port *port, struct tty_driver *driver,
+extern void tty_port_install(struct tty_port *port, struct tty_driver *driver,
struct tty_struct *tty);


   You need to update all the callers in the same patch -- the kernel must 
remain buildable after each patch but you seem to have spread that update 
among a lot of patches..


[...]

MBR, Sergei

Re: [PATCH] USB: serial: ftdi_sio: implement GPIO support for FT230X

[Johan Hovold]

On Sat, Aug 25, 2018 at 10:47:44PM +0200, Karoly Pados wrote:
> This patch uses the CBUS bitbang mode of the device, so there is
> no conflict between the GPIO and VCP functionality.

Please make the commit message self-contained (e.g. don't rely on the
summary, or mail Subject, even if you feel like you're repeating
yourself.)

And you can be a bit more verbose when describing your patch here.

> Signed-off-by: Karoly Pados 
> ---
> 
> Tested on an FT230X device. Though there is no copied code from other 
> sources, libftdi was used as a reference.
> 
> I noticed there have been numerous historic attempts by other people at 
> adding GPIO support to ftdi_sio. One tried adding it in some very 
> application-specific way, another one tried to use the mfd framework, the
> tried using a GPIO mode which is mutually exclusive to VCP, another
> one did not implement the review results etc... I hope I learned from those 
> attempts and am planning to respond to reviews. Also, this patch follows 
> the rough structure of the GPIO support for cp210x devices that have 
> already been accepted, in the hope that it is a good starting point.
> 
> This patch uses CBUS bitbanging mode, which works nicely in parallel with
> the VCP function. The other modes do not, and so IMHO it does not make
> sense to try adding them to this same module.
>
> For this device, whenever changing the state of any single pin, all the
> others need to be written too. This means in order to change any pin, we 
> need to know the current state of all the others. So when using GPIO,
> we need to have a known starting state for all pins, but there seems to 
> be no way to retrieve the existing GPIO configuration (directions and
> output register values). The way I handle this in this patch is that when
> requesting a GPIO for the first time, the module initializes all pins to 
> a known default state (to inputs). Input was chosen, because a potentially
> floating pin is better than a potential driver conflict, because the latter
> could result in hardware damage. However, if the user does not request a 
> GPIO, the CBUS pins are not initialized, avoiding unnecessarily changing 
> hardware state. I figured I cannot rely on the default power-on state of
> the device for this as the user might have used libftdi before loading 
> our module. When the module is unloaded, CBUS bitbanging mode is exited 
> if it were us who entered it earlier.

This paragraph seems like it belongs in the commit message in some form.

But your approach seems reasonable, even if it means all pins may change
state when requesting the first.

Actually, after thinking about this some more, it may be better to just
configure all pins during probe. The device is managed by the kernel,
and we can't really consider what user space may have done before, at
least if we can't query the device.  Better to be in a consistent state
while the driver is bound.

> checkpatch.pl emits a bunch of warnings, recommending const for structs. 
> I am unsure if this makes sense, existing code in this same and also
> other modules does not adhere to this rule either. I fixed everything else.

Really? Checkpatch doesn't complain here. Were you using the --strict
option, perhaps? Don't do that...

>  drivers/usb/serial/ftdi_sio.c | 284 +-
>  drivers/usb/serial/ftdi_sio.h |  25 +++
>  2 files changed, 308 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index b5cef322826f..d1ff4a372b6e 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -40,6 +40,9 @@
>  #include 
>  #include 
>  #include 
> +#ifdef CONFIG_GPIOLIB
> +#include 
> +#endif

No ifdef.

>  #include "ftdi_sio.h"
>  #include "ftdi_sio_ids.h"
>  
> @@ -72,6 +75,14 @@ struct ftdi_private {
>   unsigned int latency;   /* latency setting in use */
>   unsigned short max_packet_size;
>   struct mutex cfg_lock; /* Avoid mess by parallel calls of config 
> ioctl() and change_speed() */
> +#ifdef CONFIG_GPIOLIB
> + struct gpio_chip gc;
> + boolgpio_registered;  /* is the gpiochip in kernel registered */
> + boolgpio_used;/* true if the user requested a gpio */
> + u8  gpio_altfunc; /* which pins are in gpio mode */
> + u8  gpio_input;   /* pin directions cache */
> + u8  gpio_value;   /* pin value for outputs */
> +#endif
>  };
>  
>  /* struct ftdi_sio_quirk is used by devices requiring special attention. */
> @@ -1766,6 +1777,268 @@ static void remove_sysfs_attrs(struct usb_serial_port 
> *port)
>  
>  }
>  
> +#ifdef CONFIG_GPIOLIB
> +
> +static int ftdi_sio_set_cbus(struct usb_serial_port *port,

Please drop the "_sio" infix from your functions, "ftdi_" as prefix is
enough.

> +  u8 direction,
> +  u8 value,
> +  u8 mode)
> +{
> + struct ftdi_private *priv =

Re: [PATCH 1/6] usb: xhci-mtk: resume USB3 roothub first

[Mathias Nyman]

On 30.08.2018 04:09, Chunfeng Yun wrote:

Hi,
On Wed, 2018-08-29 at 12:13 +0300, Sergei Shtylyov wrote:

On 8/29/2018 5:55 AM, Chunfeng Yun wrote:


Give USB3 devices a better chance to enumerate at USB3 speeds if
they are connected to a suspended host.
Porting from "671ffdf xhci: resume USB 3 roothub first"


 From commit 671ffdff5b13 ("xhci: resume USB 3 roothub first").


Ok, Thanks



adding to queue (I'll fix that commit ID)

-Mathias

[PATCH V2 2/4 next] net: lan78xx: Drop unnecessary strcpy in lan78xx_probe

[Stefan Wahren]

There is no need for this strcpy because alloc_etherdev() already
does this job.

Signed-off-by: Stefan Wahren 
Reviewed-by: Raghuram Chary Jallipalli 
---
 drivers/net/usb/lan78xx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 3f70b94..3d505c2 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -3766,7 +3766,6 @@ static int lan78xx_probe(struct usb_interface *intf,
ret = lan78xx_bind(dev, intf);
if (ret < 0)
goto out2;
-   strcpy(netdev->name, "eth%d");
 
if (netdev->mtu > (dev->hard_mtu - netdev->hard_header_len))
netdev->mtu = dev->hard_mtu - netdev->hard_header_len;
-- 
2.7.4

[PATCH V2 0/4 next] net: lan78xx: Minor improvements

[Stefan Wahren]

This patch series contains some minor improvements for the lan78xx
driver.

Changes in V2:
- Keep Copyright comment as multi-line
- Add Raghuram's Reviewed-by

Stefan Wahren (4):
  net: lan78xx: Bail out if lan78xx_get_endpoints fails
  net: lan78xx: Drop unnecessary strcpy in lan78xx_probe
  net: lan78xx: Switch to SPDX identifier
  net: lan78xx: Make declaration style consistent

 drivers/net/usb/lan78xx.c | 34 +-
 drivers/net/usb/lan78xx.h | 14 +-
 2 files changed, 14 insertions(+), 34 deletions(-)

-- 
2.7.4

[PATCH V2 1/4 next] net: lan78xx: Bail out if lan78xx_get_endpoints fails

[Stefan Wahren]

We need to bail out if lan78xx_get_endpoints() fails, otherwise the
result is overwritten.

Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 
Ethernet")
Signed-off-by: Stefan Wahren 
Reviewed-by: Raghuram Chary Jallipalli 
---
 drivers/net/usb/lan78xx.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index a9991c5..3f70b94 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2952,6 +2952,11 @@ static int lan78xx_bind(struct lan78xx_net *dev, struct 
usb_interface *intf)
int i;
 
ret = lan78xx_get_endpoints(dev, intf);
+   if (ret) {
+   netdev_warn(dev->net, "lan78xx_get_endpoints failed: %d\n",
+   ret);
+   return ret;
+   }
 
dev->data[0] = (unsigned long)kzalloc(sizeof(*pdata), GFP_KERNEL);
 
-- 
2.7.4

[PATCH V2 3/4 next] net: lan78xx: Switch to SPDX identifier

[Stefan Wahren]

Adopt the SPDX license identifier headers to ease license compliance
management.

Signed-off-by: Stefan Wahren 
---
 drivers/net/usb/lan78xx.c | 14 +-
 drivers/net/usb/lan78xx.h | 14 +-
 2 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 3d505c2..e1c055d 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1,18 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0+
 /*
  * Copyright (C) 2015 Microchip Technology
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see .
  */
 #include 
 #include 
diff --git a/drivers/net/usb/lan78xx.h b/drivers/net/usb/lan78xx.h
index 25aa546..968e5e5 100644
--- a/drivers/net/usb/lan78xx.h
+++ b/drivers/net/usb/lan78xx.h
@@ -1,18 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
 /*
  * Copyright (C) 2015 Microchip Technology
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see .
  */
 #ifndef _LAN78XX_H
 #define _LAN78XX_H
-- 
2.7.4

[PATCH V2 4/4 next] net: lan78xx: Make declaration style consistent

[Stefan Wahren]

This patch makes some declaration more consistent.

Signed-off-by: Stefan Wahren 
Reviewed-by: Raghuram Chary Jallipalli 
---
 drivers/net/usb/lan78xx.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index e1c055d..331bc99 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1015,7 +1015,7 @@ static int lan78xx_dataport_write(struct lan78xx_net 
*dev, u32 ram_select,
 static void lan78xx_set_addr_filter(struct lan78xx_priv *pdata,
int index, u8 addr[ETH_ALEN])
 {
-   u32 temp;
+   u32 temp;
 
if ((pdata) && (index > 0) && (index < NUM_OF_MAF)) {
temp = addr[3];
@@ -2690,7 +2690,7 @@ static void lan78xx_terminate_urbs(struct lan78xx_net 
*dev)
 
 static int lan78xx_stop(struct net_device *net)
 {
-   struct lan78xx_net  *dev = netdev_priv(net);
+   struct lan78xx_net *dev = netdev_priv(net);
 
if (timer_pending(&dev->stat_monitor))
del_timer_sync(&dev->stat_monitor);
@@ -3073,7 +3073,7 @@ static void lan78xx_rx_vlan_offload(struct lan78xx_net 
*dev,
 
 static void lan78xx_skb_return(struct lan78xx_net *dev, struct sk_buff *skb)
 {
-   int status;
+   int status;
 
if (test_bit(EVENT_RX_PAUSED, &dev->flags)) {
skb_queue_tail(&dev->rxq_pause, skb);
@@ -3633,10 +3633,10 @@ static void intr_complete(struct urb *urb)
 
 static void lan78xx_disconnect(struct usb_interface *intf)
 {
-   struct lan78xx_net  *dev;
-   struct usb_device   *udev;
-   struct net_device   *net;
-   struct phy_device   *phydev;
+   struct lan78xx_net *dev;
+   struct usb_device *udev;
+   struct net_device *net;
+   struct phy_device *phydev;
 
dev = usb_get_intfdata(intf);
usb_set_intfdata(intf, NULL);
-- 
2.7.4

Re: [PATCH] USB: serial: ftdi_sio: implement GPIO support for FT230X

[Karoly Pados]

Thanks for the review, I am on the road most of this week, but I will submit
a new patch this weekend with all your feedback incorporated. Until then here
are my inputs / answers to some of your points. 

> Actually, after thinking about this some more, it may be better to just
> configure all pins during probe. The device is managed by the kernel,
> and we can't really consider what user space may have done before, at
> least if we can't query the device. Better to be in a consistent state
> while the driver is bound.

The CBUS pins do not affect the UART communication in any way, so if we're
not using the GPIO, I don't see any reason why we should destroy the CBUS
state just for the sake of knowing what value they have, even though we don't
set or use or need them and there is also no interaction. But if you wish so,
I'll set them during probe, I just think we are disabling a possible use-case
with no added gain.

>> checkpatch.pl emits a bunch of warnings, recommending const for structs.
>> I am unsure if this makes sense, existing code in this same and also
>> other modules does not adhere to this rule either. I fixed everything else.
> 
> Really? Checkpatch doesn't complain here. Were you using the --strict
> option, perhaps? Don't do that...
> 

Yeah, that's what happened. I'll drop that flag.

>> + /* device's direction polarity is different from kernel's */
> 
> Why so? You could just replace gpio_input with gpio_output. I think that
> may be preferred.

That doesn't change the fact that for the kernel (for example in
ftdi_gpio_direction_input or ftdi_gpio_direction_output) '1' is still input.
Yes I know we can do the inversion in those functions "for free" by setting
instead of clearing and vice-versa, I just thought it is better if I choose
to stay with the kernel convention and turn it device-specific at the deepest
level possible. Not arguing, just explaining what my motivation was. I'll
change it as you requested.

>> + if (!priv->gpio_used) {
>> + /* Set default pin states, as we cannot get them from device */
>> + priv->gpio_input = 0xff;
>> + priv->gpio_value = 0x00;
>> + result = ftdi_sio_set_cbus(port,
>> + priv->gpio_input,
>> + priv->gpio_value,
>> + FTDI_SIO_GPIO_MODE_CBUS);
>> + if (result)
>> + return result;
>> +
>> + priv->gpio_used = true;
>> + }
> 
> So I think this initialisation should be done at probe instead.

See my point further above.

> Factor this out to an eeprom helper that can be reused by other chip
> types.

Yep, I'll consult the other gpio patch regarding this as you suggested
in the other's review. By the way, sorry for the parallel submission,
I wasn't aware of Poulain's patch in this same month.

>> + priv->gc.ngpio = 4;
> 
> Shouldn't this be handled the other way round? IIRC there are two FTX
> device types with four pins, and one type where only one pin is
> accessible.
> 

There are 4 devices with 1 GPIO, 1 device with 2 GPIOs, 2 devices with 4 GPIOs,
and 1 device with 6 GPIOs. Source: http://www.ftdichip.com/FT-X.htm (2nd table)
Do you still want me turn this over?

>> +#else
>> +
>> +static int ftdi_sio_gpio_init(struct usb_serial *serial)
> 
> As the test robot reported, these should take a port as their argument.

Yes, I already sent in a v2 for that. So my patch incorporating your feedback
will be v3.

Thanks again,
Karoly

Re: [PATCH] USB: serial: ftdi_sio: implement GPIO support for FT230X

[Karoly Pados]

>>> + priv->gc.ngpio = 4;
>> 
>> Shouldn't this be handled the other way round? IIRC there are two FTX
>> device types with four pins, and one type where only one pin is
>> accessible.
> 
> There are 4 devices with 1 GPIO, 1 device with 2 GPIOs, 2 devices with 4 
> GPIOs,
> and 1 device with 6 GPIOs. Source: http://www.ftdichip.com/FT-X.htm (2nd 
> table)
> Do you still want me turn this over?
> 

Aaah, but those other devices are not UART bridges. I see what you mean now.
The point for me when developing th original patch

KASAN: slab-out-of-bounds Read in vhci_hub_control

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit:420f51f4ab6b Merge tag 'arm64-fixes' of git://git.kernel.o..
git tree:   upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=126a6f0e40
kernel config:  https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
dashboard link: https://syzkaller.appspot.com/bug?extid=bccc1fe10b70fadc78d0
compiler:   gcc (GCC) 8.0.1 20180413 (experimental)
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=121caa4640
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14ed8ab640

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+bccc1fe10b70fadc7...@syzkaller.appspotmail.com

random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
vhci_hcd: invalid port number 132
==
BUG: KASAN: slab-out-of-bounds in vhci_hub_control+0x1b88/0x1bf0  
drivers/usb/usbip/vhci_hcd.c:441

Read of size 4 at addr 8801ce615ebc by task syz-executor741/4647

CPU: 1 PID: 4647 Comm: syz-executor741 Not tainted 4.19.0-rc1+ #217
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011

Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
 print_address_description+0x6c/0x20b mm/kasan/report.c:256
 kasan_report_error mm/kasan/report.c:354 [inline]
 kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
 __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432
 vhci_hub_control+0x1b88/0x1bf0 drivers/usb/usbip/vhci_hcd.c:441
 rh_call_control drivers/usb/core/hcd.c:679 [inline]
 rh_urb_enqueue drivers/usb/core/hcd.c:838 [inline]
 usb_hcd_submit_urb+0x184a/0x2160 drivers/usb/core/hcd.c:1651
 usb_submit_urb+0x895/0x14d0 drivers/usb/core/urb.c:570
 usb_start_wait_urb+0x140/0x360 drivers/usb/core/message.c:57
 usb_internal_control_msg drivers/usb/core/message.c:101 [inline]
 usb_control_msg+0x332/0x4e0 drivers/usb/core/message.c:152
 proc_control+0x99b/0xef0 drivers/usb/core/devio.c:1106
 usbdev_do_ioctl+0x1eb4/0x3b30 drivers/usb/core/devio.c:2394
 usbdev_ioctl+0x25/0x30 drivers/usb/core/devio.c:2551
 vfs_ioctl fs/ioctl.c:46 [inline]
 file_ioctl fs/ioctl.c:501 [inline]
 do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
 __do_sys_ioctl fs/ioctl.c:709 [inline]
 __se_sys_ioctl fs/ioctl.c:707 [inline]
 __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x449329
Code: e8 ac b7 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 ab d6 fb ff c3 66 2e 0f 1f 84 00 00 00 00

RSP: 002b:7f8653ff9da8 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 006dac28 RCX: 00449329
RDX: 2100 RSI: c0185500 RDI: 0003
RBP: 006dac20 R08: 006dac2c R09: 
R10: 7f8653ffa700 R11: 0246 R12: 006dac2c
R13: 2330302f6273752f R14: 7375622f7665642f R15: 006dad2c

Allocated by task 1:
 save_stack+0x43/0xd0 mm/kasan/kasan.c:448
 set_track mm/kasan/kasan.c:460 [inline]
 kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
 kmem_cache_alloc_trace+0x152/0x730 mm/slab.c:3620
 kmalloc include/linux/slab.h:513 [inline]
 kzalloc include/linux/slab.h:707 [inline]
 usb_set_configuration+0x10e9/0x19f0 drivers/usb/core/message.c:1834
 generic_probe+0xb6/0x110 drivers/usb/core/generic.c:174
 usb_probe_device+0xaf/0x110 drivers/usb/core/driver.c:266
 really_probe+0x5be/0x850 drivers/base/dd.c:500
 driver_probe_device+0x108/0x210 drivers/base/dd.c:662
 __device_attach_driver+0x25a/0x2d0 drivers/base/dd.c:758
 bus_for_each_drv+0x16b/0x1f0 drivers/base/bus.c:461
 __device_attach+0x2a1/0x430 drivers/base/dd.c:815
 device_initial_probe+0x1a/0x20 drivers/base/dd.c:862
 bus_probe_device+0x1fb/0x2a0 drivers/base/bus.c:521
 device_add+0x93e/0x17b0 drivers/base/core.c:1927
 usb_new_device+0x8ac/0x12b0 drivers/usb/core/hub.c:2485
 register_root_hub drivers/usb/core/hcd.c:1105 [inline]
 usb_add_hcd+0xb1f/0x1910 drivers/usb/core/hcd.c:2882
 vhci_hcd_probe+0xfb/0x240 drivers/usb/usbip/vhci_hcd.c:1316
 platform_drv_probe+0x96/0x160 drivers/base/platform.c:579
 really_probe+0x5be/0x850 drivers/base/dd.c:500
 driver_probe_device+0x108/0x210 drivers/base/dd.c:662
 __device_attach_driver+0x25a/0x2d0 drivers/base/dd.c:758
 bus_for_each_drv+0x16b/0x1f0 drivers/base/bus.c:461
 __device_attach+0x2a1/0x430 drivers/base/dd.c:815
 device_initial_probe+0x1a/0x20 drivers/base/dd.c:862
 bus_probe_device+0x1fb/0x2a0 drivers/base/bus.c:521
 device_add+0x93e/0x17b0 drivers/base/core.c:1927
 platform_device_add+0x36e/0x6f0 drivers/base/platform.c:417
 vhci_hcd_init+0x386/0x4e0 drivers/usb/u

Re: [PATCH net-next] net: usbnet: mark expected switch fall-through

[David Miller]

From: "Gustavo A. R. Silva" 
Date: Mon, 3 Sep 2018 13:48:45 -0500

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
> 
> Addresses-Coverity-ID: 1077614 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva 

Applied, thank you.

[PATCH v2 0/2] Add Embedded USB Debugger (EUD) driver

[Prakruthi Deepak Heragu]

This is a series of patches that implements a driver for the control
peripheral, EUD (Embedded USB Debugger). The EUD is a mini-USB hub 
implemented on chip to support the USB-based debug and trace capabilities.
Apart from debug capabilities, EUD has a control peripheral. Control 
Peripheral is on when EUD is on and gets signals like USB attach, pet EUD,
charge phone etc. EUD driver listens to events like USB attach or detach 
and charger enable or disable and then notifies the USB driver or PMIC 
driver respectively about these events via EXTCON. At regular intervals, 
the EUD driver receives an interrupt to pet the driver indicating that 
the software is functional.

Changes since v1:
- Remove EUD_NR as it is an unused macro
Changes since v0:
- Remove select SERIAL_CORE from Kconfig as this patch doesn't involve
  anything related to serial console
- Changed the dt-bindings to remove extcon and replace it with graphs
  to represent a connection with client

Prakruthi Deepak Heragu (2):
  dt-bindings: Documentation for qcom,eud
  Embedded USB Debugger (EUD) driver

 .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt  |  41 +++
 drivers/soc/qcom/Kconfig   |  12 +
 drivers/soc/qcom/Makefile  |   1 +
 drivers/soc/qcom/eud.c | 338 +
 4 files changed, 392 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
 create mode 100644 drivers/soc/qcom/eud.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 2/2] Embedded USB Debugger (EUD) driver

[Prakruthi Deepak Heragu]

Add support for control peripheral of EUD (Embedded USB Debugger) to
listen to events such as USB attach/detach, charger enable/disable, pet
EUD to indicate software is functional.

Signed-off-by: Satya Durga Srinivasu Prabhala 
Signed-off-by: Prakruthi Deepak Heragu 
---
 drivers/soc/qcom/Kconfig  |  12 ++
 drivers/soc/qcom/Makefile |   1 +
 drivers/soc/qcom/eud.c| 338 ++
 3 files changed, 351 insertions(+)
 create mode 100644 drivers/soc/qcom/eud.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 5856e79..12669ec 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -136,4 +136,16 @@ config QCOM_APR
   application processor and QDSP6. APR is
   used by audio driver to configure QDSP6
   ASM, ADM and AFE modules.
+
+config QCOM_EUD
+   tristate "QTI Embedded USB Debugger (EUD)"
+   depends on ARCH_QCOM
+   help
+ The Embedded USB Debugger (EUD) driver is a driver for the
+ control peripheral which waits on events like USB attach/detach
+ and charger enable/disable. The control peripheral further helps
+ support the USB-based debug and trace capabilities.
+ This module enables support for Qualcomm Technologies, Inc.
+ Embedded USB Debugger (EUD).
+ If unsure, say N.
 endmenu
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 19dcf95..dd4701b 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_QCOM_SMP2P)  += smp2p.o
 obj-$(CONFIG_QCOM_SMSM)+= smsm.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
 obj-$(CONFIG_QCOM_APR) += apr.o
+obj-$(CONFIG_QCOM_EUD) += eud.o
diff --git a/drivers/soc/qcom/eud.c b/drivers/soc/qcom/eud.c
new file mode 100644
index 000..16a0e7b
--- /dev/null
+++ b/drivers/soc/qcom/eud.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define EUD_ENABLE_CMD 1
+#define EUD_DISABLE_CMD 0
+
+#define EUD_REG_INT1_EN_MASK   0x0024
+#define EUD_REG_INT_STATUS_1   0x0044
+#define EUD_REG_CTL_OUT_1  0x0074
+#define EUD_REG_VBUS_INT_CLR   0x0080
+#define EUD_REG_CHGR_INT_CLR   0x0084
+#define EUD_REG_CSR_EUD_EN 0x1014
+#define EUD_REG_SW_ATTACH_DET  0x1018
+
+#define EUD_INT_VBUS   BIT(2)
+#define EUD_INT_CHGR   BIT(3)
+#define EUD_INT_SAFE_MODE  BIT(4)
+
+struct eud_chip {
+   struct device   *dev;
+   int eud_irq;
+   unsigned intextcon_id;
+   unsigned intint_status;
+   boolusb_attach;
+   boolchgr_enable;
+   void __iomem*eud_reg_base;
+   struct extcon_dev   *extcon;
+   struct work_struct  eud_work;
+};
+
+static const unsigned int eud_extcon_cable[] = {
+   EXTCON_USB,
+   EXTCON_CHG_USB_SDP,
+   EXTCON_NONE,
+};
+
+/*
+ * On the kernel command line specify eud.enable=1 to enable EUD.
+ * EUD is disabled by default.
+ */
+static int enable;
+static bool eud_ready;
+static struct eud_chip *eud_private;
+
+static void enable_eud(struct eud_chip *priv)
+{
+   struct power_supply *usb_psy = NULL;
+   union power_supply_propval pval = {0};
+   union power_supply_propval tval = {0};
+   int ret;
+
+   usb_psy = power_supply_get_by_name("usb");
+   if (!usb_psy)
+   return;
+
+   ret = power_supply_get_property(usb_psy,
+   POWER_SUPPLY_PROP_PRESENT, &pval);
+   if (ret)
+   return;
+
+   ret = power_supply_get_property(usb_psy,
+   POWER_SUPPLY_PROP_REAL_TYPE, &tval);
+   if (ret)
+   return;
+
+   if (pval.intval && (tval.intval == POWER_SUPPLY_TYPE_USB ||
+   tval.intval == POWER_SUPPLY_TYPE_USB_CDP)) {
+   /* write into CSR to enable EUD */
+   writel_relaxed(BIT(0), priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
+   /* Enable vbus, chgr & safe mode warning interrupts */
+   writel_relaxed(EUD_INT_VBUS | EUD_INT_CHGR | EUD_INT_SAFE_MODE,
+   priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
+
+   /* Ensure Register Writes Complete */
+   wmb();
+
+   /*
+* Set the default cable state to usb connect and charger
+* enable
+*/
+   ret = extcon_set_state_sync(priv->extcon, EXTCON_USB, true);
+   if (ret)
+   return;
+   ret = extcon_set_state_sync(priv->extcon,
+   EXTCON_CHG_USB_SDP, true);
+   

[PATCH v2 1/2] dt-bindings: Documentation for qcom,eud

[Prakruthi Deepak Heragu]

Documentation for Embedded USB Debugger (EUD) device tree bindings.

Signed-off-by: Satya Durga Srinivasu Prabhala 
Signed-off-by: Prakruthi Deepak Heragu 
---
 .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt  | 41 ++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt 
b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
new file mode 100644
index 000..a03021a
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
@@ -0,0 +1,41 @@
+* Qualcomm Technologies Inc Embedded USB Debugger (EUD)
+
+The EUD (Embedded USB Debugger) is a mini-USB hub implemented
+on chip to support the USB-based debug and trace capabilities.
+
+Required properties:
+
+ - compatible:  Should be "qcom,msm-eud"
+ - interrupts:  Interrupt number
+ - reg: Should be address and size of EUD register space
+
+Driver notifies clients for VBUS attach/detach and charger enable/disable
+events. The link between client and EUD is established via a directed
+graph. EUD driver has one endpoint of the graph link mentioning EUD as an
+output link and clients registered for these notifications from the EUD
+should have the other endpoint of the graph link as an input link. Each of
+these endpoints should contain a 'remote-endpoint' phandle property that
+points to the corresponding endpoint in the port of the remote device.
+
+An example for EUD device node:
+
+   eud: qcom,msm-eud@88e {
+   compatible = "qcom,msm-eud";
+   interrupts = ;
+   reg = <0x88e 0x4000>;
+   port {
+   eud_output: endpoint {
+   remote-endpoint = <&usb3_input>;
+};
+   };
+   };
+
+An example for EUD client:
+
+   usb3 {
+   port {
+   usb3_input: endpoint {
+   remote-endpoint = <&eud_output>;
+};
+   };
+   };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/8] usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb()

[Thinh Nguyen]

Hi Anurag,

On 8/17/2018 5:25 AM, Anurag Kumar Vulisha wrote:
> Availability of TRB's are calculated using dwc3_calc_trbs_left(), which
> determines available TRB's based on the HWO bit set in a TRB.
>
> __dwc3_prepare_one_trb() is called with a TRB which needs to be prepared
> for transfer. This __dwc3_prepare_one_trb() calls dwc3_calc_trbs_left()
> to determine total available TRBs and set IOC bit if the total available
> TRBs are zero. Since the present working TRB(which is passed as an
> argument to __dwc3_prepare_one_trb() )  doesn't have HWO bit already set,
> there are chances where dwc3_calc_trbs_left() wrongly calculates this
> present working TRB as free(since the HWO bit is not yet set). This could
> be a problem. This patch correct this issue by setting HWO bit before
> calling dwc3_calc_trbs_left()
>
> Signed-off-by: Anurag Kumar Vulisha 
> ---
>  Changes in v2:
>   1. Changed the commit message
> ---
>  drivers/usb/dwc3/gadget.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 69bf137..f73d219 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -990,6 +990,8 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, 
> struct dwc3_trb *trb,
>   trb->ctrl |= DWC3_TRB_CTRL_ISP_IMI;
>   }
>  
> + trb->ctrl |= DWC3_TRB_CTRL_HWO;
> +
>   if ((!no_interrupt && !chain) ||
>   (dwc3_calc_trbs_left(dep) == 0))
>   trb->ctrl |= DWC3_TRB_CTRL_IOC;
> @@ -1000,8 +1002,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, 
> struct dwc3_trb *trb,
>   if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable)
>   trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id);
>  
> - trb->ctrl |= DWC3_TRB_CTRL_HWO;
> -
>   trace_dwc3_prepare_trb(dep, trb);
>  }
>  
How do you reproduce this issue? We should not set HWO before setting
other trb->ctrl bits. Can you provide a driver tracepoint? If there's an
issue with the check if ((!no_interrupt && !chain) ||
dwc3_calc_trbs_left == 0), then we may need to fix the check there.

BR,
Thinh

Re: [PATCH v2 3/8] usb: dwc3: make controller clear transfer resources after complete

[Thinh Nguyen]

On 8/17/2018 5:26 AM, Anurag Kumar Vulisha wrote:
> To start transfer with another stream id, controller needs to free
> previously allocated transfer resource. This will be automatically
> done by the controller at the time of XferComplete Event. This
> patch updates the code to issue XferComplete event once all transfers
> are done by setting LST bit in the ctrl field of the last TRB.
>
> Signed-off-by: Anurag Kumar Vulisha 
> ---
>  Changes in v2:
>   1. None
> ---
>  drivers/usb/dwc3/gadget.c | 19 ---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index efc6e13..b3e9e7f 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -571,7 +571,8 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep, 
> unsigned int action)
>  
>   if (usb_ss_max_streams(comp_desc) && usb_endpoint_xfer_bulk(desc)) {
>   params.param1 |= DWC3_DEPCFG_STREAM_CAPABLE
> - | DWC3_DEPCFG_STREAM_EVENT_EN;
> + | DWC3_DEPCFG_STREAM_EVENT_EN
> + | DWC3_DEPCFG_XFER_COMPLETE_EN;
>   dep->stream_capable = true;
>   }
>  
> @@ -999,6 +1000,15 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, 
> struct dwc3_trb *trb,
>   if (chain)
>   trb->ctrl |= DWC3_TRB_CTRL_CHN;
>  
> + /*
> +  * To issue start transfer on another stream, controller need to free
> +  * previously acquired transfer resource. Setting the LST bit in
> +  * last TRB makes the controller clear transfer resource for that
> +  * endpoint, allowing to start another stream on that endpoint.
> +  */
> + else if (dep->stream_capable)
> + trb->ctrl |= DWC3_TRB_CTRL_LST;
> +
>   if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable)
>   trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id);
>  
> @@ -2268,7 +2278,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct 
> dwc3_ep *dep,
>   if (event->status & DEPEVT_STATUS_SHORT && !chain)
>   return 1;
>  
> - if (event->status & DEPEVT_STATUS_IOC)
> + if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST))
>   return 1;
>  
>   return 0;
> @@ -2457,6 +2467,10 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
>   }
>  
>   switch (event->endpoint_event) {
> + case DWC3_DEPEVT_XFERCOMPLETE:
> + if (!dep->stream_capable)
> + break;
> + dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
 Add this:  /* Falls Through */
>   case DWC3_DEPEVT_XFERINPROGRESS:
>   dwc3_gadget_endpoint_transfer_in_progress(dep, event);
>   break;
> @@ -2472,7 +2486,6 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
>   }
>   break;
>   case DWC3_DEPEVT_STREAMEVT:
> - case DWC3_DEPEVT_XFERCOMPLETE:
>   case DWC3_DEPEVT_RXTXFIFOEVT:
>   break;
>   }

Thinh

Re: [PATCH 01/25] tty: Change return type to void

[Jiri Slaby]

On 09/05/2018, 03:08 AM, Jaejoong Kim wrote:
> > @@ -688,7 +688,7 @@ extern int tty_port_close_start(struct
> tty_port *port,
> >   extern void tty_port_close_end(struct tty_port *port, struct
> tty_struct *tty);
> >   extern void tty_port_close(struct tty_port *port,
> >                               struct tty_struct *tty, struct file
> *filp);
> > -extern int tty_port_install(struct tty_port *port, struct
> tty_driver *driver,
> > +extern void tty_port_install(struct tty_port *port, struct
> tty_driver *driver,
> >                               struct tty_struct *tty);
> 
>     You need to update all the callers in the same patch -- the
> kernel must
> remain buildable after each patch but you seem to have spread that
> update
> among a lot of patches..
> 
> 
> OK. I will make several patches as one patch.

You don't have to. Just reorder the patches as suggested by Sam. (First,
make everybody ignore the return value, then change these return types
to void.)

BTW you can mention in this commit log, that this change is possible
after a3123fd0a4a5. That commit made tty_standard_install to always
return 0.

thanks,
-- 
js
suse labs

Re: [PATCH v2 4/8] usb: dwc3: implement stream transfer timeout

[Thinh Nguyen]

On 8/17/2018 5:25 AM, Anurag Kumar Vulisha wrote:
> According to dwc3 databook when streams are used, it may be possible
> for the host and device become out of sync, where device may wait for
> host to issue prime transcation and host may wait for device to issue
> erdy. To avoid such deadlock, timeout needs to be implemented. After
> timeout occurs, device will first stop transfer and restart the transfer
> again. This patch does the same.
>
> Signed-off-by: Anurag Kumar Vulisha 
> ---
>  Changes in v2:
>   1. Changed STREAM_TIMEOUT to STREAM_TIMEOUT_MS as suggested by
>  Andy Shevchenko
> ---
>  drivers/usb/dwc3/core.h   |  7 +++
>  drivers/usb/dwc3/gadget.c | 39 +++
>  2 files changed, 46 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 285ce0e..f58640f 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -619,6 +619,11 @@ struct dwc3_event_buffer {
>  
>  #define DWC3_TRB_NUM 256
>  
> +/*
> + * Timeout value in msecs used by stream_timeout_timer when streams are 
> enabled
> + */
> +#define STREAM_TIMEOUT_MS50
> +
>  /**
>   * struct dwc3_ep - device side endpoint representation
>   * @endpoint: usb endpoint
> @@ -642,6 +647,7 @@ struct dwc3_event_buffer {
>   * @name: a human readable name e.g. ep1out-bulk
>   * @direction: true for TX, false for RX
>   * @stream_capable: true when streams are enabled
> + * @stream_timeout_timer: timer used to aviod deadlock when streams are used
>   */
>  struct dwc3_ep {
>   struct usb_ep   endpoint;
> @@ -691,6 +697,7 @@ struct dwc3_ep {
>  
>   unsigneddirection:1;
>   unsignedstream_capable:1;
> + struct timer_list   stream_timeout_timer;
>  };
>  
>  enum dwc3_phy {
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index b3e9e7f..8cef488 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -254,6 +254,7 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc, 
> unsigned cmd, u32 param)
>  }
>  
>  static int __dwc3_gadget_wakeup(struct dwc3 *dwc);
> +static void stream_timeout_function(struct timer_list *arg);
>  
>  /**
>   * dwc3_send_gadget_ep_cmd - issue an endpoint command
> @@ -574,6 +575,8 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep, 
> unsigned int action)
>   | DWC3_DEPCFG_STREAM_EVENT_EN
>   | DWC3_DEPCFG_XFER_COMPLETE_EN;
>   dep->stream_capable = true;
> + timer_setup(&dep->stream_timeout_timer,
> + stream_timeout_function, 0);
>   }
>  
>   if (!usb_endpoint_xfer_control(desc))
> @@ -730,6 +733,9 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
>  
>   trace_dwc3_gadget_ep_disable(dep);
>  
> + if (dep->stream_capable)
> + del_timer(&dep->stream_timeout_timer);
> +
>   dwc3_remove_requests(dwc, dep);
>  
>   /* make sure HW endpoint isn't stalled */
> @@ -1257,6 +1263,12 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep 
> *dep)
>   return ret;
>   }
>  
> + if (starting && dep->stream_capable) {
> + dep->stream_timeout_timer.expires = jiffies +
> + msecs_to_jiffies(STREAM_TIMEOUT_MS);
> + add_timer(&dep->stream_timeout_timer);
> + }
> +
>   return 0;
>  }
>  
> @@ -2403,6 +2415,13 @@ static void 
> dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
>   stop = true;
>   }
>  
> + /*
> +  * Delete the timer that was started in __dwc3_gadget_kick_transfer()
> +  * for stream capable endpoints.
> +  */
> + if (dep->stream_capable)
> + del_timer(&dep->stream_timeout_timer);
> +
>   dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
>  
>   if (stop) {
> @@ -2486,6 +2505,14 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
>   }
>   break;
>   case DWC3_DEPEVT_STREAMEVT:
> + switch (event->status) {
> + case DEPEVT_STREAMEVT_FOUND:
> + del_timer(&dep->stream_timeout_timer);
> + break;
> + case DEPEVT_STREAMEVT_NOTFOUND:
> + default:
> + dev_err(dwc->dev, "unable to find suitable stream");
> + }
Add break after this. Also, it's probably easier to read if we use
if-else case here.

>   case DWC3_DEPEVT_RXTXFIFOEVT:
>   break;
>   }
> @@ -2587,6 +2614,18 @@ static void dwc3_stop_active_transfer(struct dwc3_ep 
> *dep, bool force)
>   }
>  }
>  
> +static void stream_timeout_function(struct timer_list *arg)
> +{
> + struct dwc3_ep *dep = from_timer(dep, arg, stream_timeout_timer);
> + struct dwc3 *dwc = dep->dwc;
> + unsigned long   flags;
> +
> + spin_lock_irqsave(&dwc->lock, flags);