Re: [PATCH] USB: usbtmc: Add support for missing USBTMC-USB488 spec

[dave penkler]

Hi Oliver,

On Wed, Oct 14, 2015 at 3:33 PM, Oliver Neukum  wrote:
> On Wed, 2015-10-14 at 15:13 +0200, dave penkler wrote:
>
> Hi,
>
> just a few remarks.

thank you.

>
>>
>> +static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
>> + unsigned long arg)
>> +{
>> + u8 *buffer;
>> + struct device *dev;
>> + int rv;
>> + u8 tag, stb;
>> +
>> + dev = >intf->dev;
>> +
>> + dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
>> + data->iin_ep_present);
>> +
>> + buffer = kmalloc(8, GFP_KERNEL);
>> + if (!buffer)
>> + return -ENOMEM;
>> +
>> +
>> + atomic_set(>iin_data_valid, 0);
>> +
>> + /* must issue read_stb before using poll or select */
>> + atomic_set(>srq_asserted, 0);
>> +
>> + rv = usb_control_msg(data->usb_dev,
>> + usb_rcvctrlpipe(data->usb_dev, 0),
>> + USBTMC488_REQUEST_READ_STATUS_BYTE,
>> + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
>> + data->iin_bTag,
>> + data->ifnum,
>> + buffer, 0x03, USBTMC_TIMEOUT);
>> +
>> + if (rv < 0) {
>> + dev_err(dev, "stb usb_control_msg returned %d\n", rv);
>> + goto exit;
>> + }
>> +
>> + if (buffer[0] != USBTMC_STATUS_SUCCESS) {
>> + dev_err(dev, "control status returned %x\n",
>> + buffer[0]);
>> + rv = -EIO;
>> + goto exit;
>> + }
>> +
>> + if (data->iin_ep_present) {
>> +
>> + rv = wait_event_interruptible_timeout(
>> + data->waitq,
>> + (atomic_read(>iin_data_valid) != 0),
>> + USBTMC_TIMEOUT
>> + );
>> +
>> + if (rv < 0) {
>> + dev_dbg(dev, "wait interrupted %d\n", rv);
>> + goto exit;
>> + }
>
> If you do this, yet the transfer succeeds, how do you keep the tag
> between host and device consistent?
>

The next message will just use the same tag value as before if rv <= 0
which is not a problem - one could do it either way. I'll move the bump
after exit: for V2

>> +
>> + if (rv == 0) {
>> + dev_dbg(dev, "wait timed out\n");
>> + rv = -ETIME;
>> + goto exit;
>> + }
>> +
>> + tag = data->bNotify1 & 0x7f;
>> +
>> + if (tag != data->iin_bTag) {
>> + dev_err(dev, "expected bTag %x got %x\n",
>> + data->iin_bTag, tag);
>> + }
>> +
>> + stb = data->bNotify2;
>> + } else {
>> + stb = buffer[2];
>> + }
>> +
>> + /* bump interrupt bTag */
>> + data->iin_bTag += 1;
>> + if (data->iin_bTag > 127)
>> + data->iin_bTag = 2;
>> +
>> + rv = copy_to_user((void *)arg, , sizeof(stb));
>> + if (rv)
>> + rv = -EFAULT;
>> +
>> + exit:
>> + kfree(buffer);
>> + return rv;
>> +
>> +}
>> +
>> +static unsigned int usbtmc_poll(struct file *file, poll_table *wait)
>> +{
>> + struct usbtmc_device_data *data = file->private_data;
>> + unsigned int mask = 0;
>> +
>> + mutex_lock(>io_mutex);
>> +
>> + if (data->zombie)
>> + goto no_poll;
>> +
>> + poll_wait(file, >waitq, wait);
>
> Presumably the waiters should be woken when the device is disconnected.
>

Yes the mask should be set to POLLHUP etc on the first zombie test above.
After the poll_wait it should simply read
  mask = (atomic_read(>srq_asserted)) ? POLLIN | POLLRDNORM : 0;
Will revise for V2
thank you

>> +
>> + mask = data->zombie ? POLLHUP | POLLERR :
>> + (atomic_read(>srq_asserted)) ? POLLIN | POLLRDNORM : 0;
>> +
>> +no_poll:
>> + mutex_unlock(>io_mutex);
>> + return mask;
>> +}
>
> Regards
> Oliver
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: gadget: function: acm: return zlp for OUT setup

[jaswinder . singh]

From: Jassi Brar 

We must return 0 for any OUT setup request, otherwise
protocol error may occur.

Signed-off-by: Jassi Brar 
---
 drivers/usb/gadget/function/f_acm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index aad8165..14d9e28 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -364,7 +364,7 @@ static int acm_setup(struct usb_function *f, const struct 
usb_ctrlrequest *ctrl)
|| w_index != acm->ctrl_id)
goto invalid;
 
-   value = w_length;
+   value = 0;
cdev->gadget->ep0->driver_data = acm;
req->complete = acm_complete_set_line_coding;
break;
-- 
1.9.1

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

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Jassi Brar]

On Wed, Oct 14, 2015 at 8:42 PM, Felipe Balbi  wrote:
>
> Hi,
>
> jaswinder.si...@linaro.org writes:
>> From: Jassi Brar 
>>
>> We must return 0 for any OUT setup request, otherwise
>> protocol error may occur.
>
> have you seen any such errors ?
>
Yes. While testing my new udc driver.


> Care to describe what problems you have seen and which UDC you were using,
> what's the exact scenario. How have you tested this ?
>
The udc I am writing the driver for is not yet public. To test my
driver at lowest level possible, I use 'usbmon' to capture and analyze
traffic since I don't have any hardware probe.

I see the following on gadget side
---
configfs-gadget gadget: non-core control req21.20 v i0001 l7
configfs-gadget gadget: acm ttyGS0 req21.20 v i0001 l7
configfs-gadget gadget: acm ttyGS0 completion, err -71
---

and the following on host side usbmon capture
---
88041ed01f00 540296433 S Co:3:023:0 s 21 20  0001 0007 7 =
8025 08
88041ed01f00 540296790 C Co:3:023:0 -71 0
---

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

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Felipe Balbi]

Hi,

Jassi Brar  writes:
> On Wed, Oct 14, 2015 at 8:42 PM, Felipe Balbi  wrote:
>>
>> Hi,
>>
>> jaswinder.si...@linaro.org writes:
>>> From: Jassi Brar 
>>>
>>> We must return 0 for any OUT setup request, otherwise
>>> protocol error may occur.
>>
>> have you seen any such errors ?
>>
> Yes. While testing my new udc driver.
>
>
>> Care to describe what problems you have seen and which UDC you were using,
>> what's the exact scenario. How have you tested this ?
>>
> The udc I am writing the driver for is not yet public. To test my
> driver at lowest level possible, I use 'usbmon' to capture and analyze
> traffic since I don't have any hardware probe.
>
> I see the following on gadget side
> ---
> configfs-gadget gadget: non-core control req21.20 v i0001 l7
> configfs-gadget gadget: acm ttyGS0 req21.20 v i0001 l7
> configfs-gadget gadget: acm ttyGS0 completion, err -71
> ---
>
> and the following on host side usbmon capture
> ---
> 88041ed01f00 540296433 S Co:3:023:0 s 21 20  0001 0007 7 =
> 8025 08
> 88041ed01f00 540296790 C Co:3:023:0 -71 0
> ---

and you did you even consider this could be a bug in your new UDC driver
at all ? This works fine with other UDCs.

How far are you in developing this new UDC driver ? Did you run USBCV at
all ? Are you sure you're implementing EP0 handling correctly ? What
sort of tests have you done with this UDC ? Is it working with testusb
against a known good host (EHCI and XHCI should be good for that) ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] USB: usbtmc: Add support for missing USBTMC-USB488 spec

[kbuild test robot]

Hi dave,

[auto build test WARNING on usb/usb-next -- if it's inappropriate base, please 
suggest rules for selecting the more suitable base]

url:
https://github.com/0day-ci/linux/commits/dave-penkler/USB-usbtmc-Add-support-for-missing-USBTMC-USB488-spec/20151014-211711
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/usb/class/usbtmc.c:476:28: sparse: incorrect type in argument 1 
>> (different address spaces)
   drivers/usb/class/usbtmc.c:476:28:expected void [noderef] *to
   drivers/usb/class/usbtmc.c:476:28:got void *
>> drivers/usb/class/usbtmc.c:507:44: sparse: incorrect type in argument 2 
>> (different address spaces)
   drivers/usb/class/usbtmc.c:507:44:expected void const [noderef] 
*from
   drivers/usb/class/usbtmc.c:507:44:got void *
   drivers/usb/class/usbtmc.c:1248:40: sparse: incorrect type in argument 1 
(different address spaces)
   drivers/usb/class/usbtmc.c:1248:40:expected void [noderef] *to
   drivers/usb/class/usbtmc.c:1248:40:got void *

vim +476 drivers/usb/class/usbtmc.c

   470  
   471  /* bump interrupt bTag */
   472  data->iin_bTag += 1;
   473  if (data->iin_bTag > 127)
   474  data->iin_bTag = 2;
   475  
 > 476  rv = copy_to_user((void *)arg, , sizeof(stb));
   477  if (rv)
   478  rv = -EFAULT;
   479  
   480   exit:
   481  kfree(buffer);
   482  return rv;
   483  
   484  }
   485  
   486  static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
   487  unsigned long arg,
   488  unsigned int cmd)
   489  {
   490  u8 *buffer;
   491  struct device *dev;
   492  int rv;
   493  unsigned int val;
   494  u16 wValue;
   495  
   496  dev = >intf->dev;
   497  
   498  if (0 == (data->usb488_caps & USBTMC488_CAPABILITY_SIMPLE))
   499  return -EINVAL;
   500  
   501  buffer = kmalloc(8, GFP_KERNEL);
   502  if (!buffer)
   503  return -ENOMEM;
   504  
   505  
   506  if (cmd == USBTMC488_REQUEST_REN_CONTROL) {
 > 507  rv = copy_from_user(, (void *)arg, sizeof(val));
   508  if (rv) {
   509  rv = -EFAULT;
   510  goto exit;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: musb: communication issue with more than 12 FTDI ports

[Bin Liu]

Hi,

On 10/13/2015 01:22 PM, Felipe Balbi wrote:

Yegor Yefremov  writes:

On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
 wrote:

We have a problem, when using more than 12 FTDI ports. Kernels tried:
3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz

Below the USB topology:

# lsusb -t
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
 |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
 |__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
 |__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 12M
 |__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M


How many EPs does each FTDI device require? at least one INT EP, right? 
If I read it right, the topology above has 2 hubs, and 16 high-speed 
FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT 
EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP 
configuration used by default. I am wondering how those devices got 
enumerated properly.




When using 12 ports and performing serial test (a pair of ports is
connected via null-modem cable and a rather short string ca. 90
characters will be sent alternating at 1200 and 115200b/s, testing
scripts are written in Python and running as own processes per a pair
of ports) there are no timeouts, i.e. all sent characters will be
received. As soon as I open ports 13 and 14 I start to get arbitrary
timeouts  (from test software point of view) on all ports.

In order to check, if ftdi_sio has primary to do with this issue, I've
performed the same test on a PC and PandaBoard Rev. A2 (EHCI port) and
there were no issues with 16 ports. So it seems to have something to
do with am335x + musb + number of end points.

Any idea? Let me know, if you need our test script.


 From time to time I get following warnings (4.3.0-rc5):

musb_host_rx 1915: RX1 dma busy, csr 2020
musb_host_rx 1915: RX4 dma busy, csr 2020
musb_host_rx 1915: RX7 dma busy, csr 2220
musb_host_rx 1915: RX1 dma busy, csr 2020

Though they are not timely related to serial test timeouts.


yeah, I don't think MUSB can easily handle that. IIRC, endpoint
scheduling in MUSB is rather bad. While we have enough endpoints to
handle this case, you might be running into some IP (or driver) issues.

Bin, have you ever tested this many serial devices on AM335x ?


No, I never tested this many devices. It could be resource limitation, 
but the log above is about CPPI, so I recommend to test with CPPI 
disabled to isolate if this is MUSB issue or CPPI.


Regards,
-Bin.




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

Re: musb: communication issue with more than 12 FTDI ports

[Bin Liu]

On 10/14/2015 10:56 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

Hi,

On 10/13/2015 01:22 PM, Felipe Balbi wrote:

Yegor Yefremov  writes:

On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
 wrote:

We have a problem, when using more than 12 FTDI ports. Kernels tried:
3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz

Below the USB topology:

# lsusb -t
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
  |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
  |__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
  |__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 12M
  |__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M


How many EPs does each FTDI device require? at least one INT EP, right?
If I read it right, the topology above has 2 hubs, and 16 high-speed
FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT
EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP
configuration used by default. I am wondering how those devices got
enumerated properly.


dynamic EP allocation, but that has its own limitations.


MUSB does not support dynamic EP allocation for INT/ISOCH.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: musb: communication issue with more than 12 FTDI ports

[Felipe Balbi]

Hi,

Bin Liu  writes:
> Hi,
>
> On 10/13/2015 01:22 PM, Felipe Balbi wrote:
>> Yegor Yefremov  writes:
>>> On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
>>>  wrote:
 We have a problem, when using more than 12 FTDI ports. Kernels tried:
 3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz

 Below the USB topology:

 # lsusb -t
 /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
 /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
  |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
  |__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
  |__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 12M
  |__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
  |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
  |__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M
>
> How many EPs does each FTDI device require? at least one INT EP, right? 
> If I read it right, the topology above has 2 hubs, and 16 high-speed 
> FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT 
> EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP 
> configuration used by default. I am wondering how those devices got 
> enumerated properly.

dynamic EP allocation, but that has its own limitations.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Felipe Balbi]

Hi,

jaswinder.si...@linaro.org writes:
> From: Jassi Brar 
>
> We must return 0 for any OUT setup request, otherwise
> protocol error may occur.

have you seen any such errors ? composite.c treats >=0 as success:

if (value >= 0 && value != USB_GADGET_DELAYED_STATUS) {
req->length = value;
req->context = cdev;
req->zero = value < w_length;
value = composite_ep0_queue(cdev, req, GFP_ATOMIC);
if (value < 0) {
DBG(cdev, "ep_queue --> %d\n", value);
req->status = 0;
composite_setup_complete(gadget->ep0, req);
}
} else if 

We actually NEED to return w_length to cope with the need for ZLPs. Care
to describe what problems you have seen and which UDC you were using,
what's the exact scenario. How have you tested this ?

-- 
balbi


signature.asc
Description: PGP signature

Re: Fwd: [Bug 105901] OOPS using symbolserial

[Stephan Althaus]

Am 13.10.2015 um 21:08 schrieb Johan Hovold:

On Tue, Oct 13, 2015 at 08:37:58PM +0200, Stephan Althaus wrote:

 Weitergeleitete Nachricht 
Betreff:[Bug 105901] OOPS using symbolserial
Datum:  Tue, 13 Oct 2015 18:05:06 +
Von:bugzilla-dae...@bugzilla.kernel.org
An: stephan.alth...@duedinghausen.eu



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

--- Comment #1 from Greg Kroah-Hartman  ---
On Tue, Oct 13, 2015 at 05:46:13PM +, bugzilla-dae...@bugzilla.kernel.org
wrote:

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

 Bug ID: 105901
Summary: OOPS using symbolserial
Product: Drivers
Version: 2.5
 Kernel Version: 4.1.5
   Hardware: All
 OS: Linux
   Tree: Mainline
 Status: NEW
   Severity: normal
   Priority: P1
  Component: USB
   Assignee: g...@kroah.com
   Reporter: stephan.alth...@duedinghausen.eu
 Regression: No

Created attachment 190191
   --> https://bugzilla.kernel.org/attachment.cgi?id=190191=edit
OOPS trace from /var/log/syslog

I'm testing the Barcodescanner SYMBOL DS670
and don't want to use USBHID but in serial mode.
I'm getting OOPS

Please send this to the linux-usb@vger.kernel.org mailing list.

Please try using a more recent kernel than 4.1.5. The oops you're
hitting should already have been fixed in mainline (and the fix has also
been backported to 4.1.8).

Johan

Thanks for the hint.
Kernel 4.2.3 works perfectly :-)

But, at work i will need to do this on an IGEL,
with kernel 3.13.11.
Unfortunately i will not be able to upgrade the kernel entirely.

I would like to build the kernel defective modules on my PC
an copy them over to the IGEL device.

Can you give me a hint, which files have been affected?
symbolserial.c seems not the only responsible file for this OOPS..

Thank you!

Stephan

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

Re: [RFT PATCH] xhci: don't finish a TD if we get a short transfer event mid TD

[Mathias Nyman]

On 13.10.2015 23:23, Alexey Brodkin wrote:

Hi Mathias,

On Fri, 2015-09-18 at 22:45 +0300, Alexey Brodkin wrote:

Hi Mathias,

On Fri, 2015-09-18 at 18:07 +0300, Mathias Nyman wrote:

If the difference is big enough between the bytes asked and received
in a bulk tranfer we can get a short transfer event pointing to a TRB in
the middle of the TD. We don't want to handle the TD yet as we will anyway
receive a new event for the last TRB in the TD.

Hold off removing TD from list and finishing it before we reveive a event
for the last TRB in the TD

Signed-off-by: Mathias Nyman 
---


I tried your patch and even though I I cannot see literally any changes
in usbmon output I see [probably too] many things reported in syslog.

I'm not sure if that's ok to post such a long logs here but I'll do it now.
If that's not ok please let me know how to pass those long logs to you.

Also before that log an observation - USB device gets reconnected
even though I don't touch it. In the log below you'll see it happens twice.

Note below the first part of the log is for just device connection and
some more time after that. And only then logs for 2 attempts to get
data from USB device.



I'm wondering if there's any chance to continue this investigation?



Yes, we should, but I need to take some time to properly review the Mediatek
xhci driver that has been pending for far too long already.

In the meantime could you try reproducing it without Powermanagement, 
(CONFIG_PM).
It's a long shot but it would help ruling out PM.

-Mathias  


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

Re: Enumerating the USB device more than the capability of Host in the Linux Platform

[Peter Stuge]

Mathias Nyman wrote:
> it's possible that the failing "0" returned by xhci_alloc_dev() is
> not properly propageated/translated through usb core, the used usb
> libarary and whatever other userspace components on top.

Does any userspace API receive notification of this failure?

I don't think so. I don't think there will be any netlink message
from the kernel nor from udev following this failure, there would
only be one later if the device is allocated successfully. Am I wrong?


Thanks

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

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Felipe Balbi]

Hi,

Alan Stern  writes:
> On Wed, 14 Oct 2015, Jassi Brar wrote:
>
>> BTW, should the gadget stack ever queue a Non-ZLP as reply to some
>> setup request that has USB_DIR_IN not set?
>
> Yes.  If USB_DIR_IN is not set then the control transfer is OUT, so the
> gadget needs to queue a request to receive some data from the host.  
> That request will obviously need to be a non-ZLP.  In fact, it's hard
> to think of a situation where a gadget would ever want to submit a
> zero-length OUT request.  Isn't the UDC driver supposed to handle the
> status stage of a control-IN transfer automatically?

yes and no. :-) If USB_GADGET_DELAYED_STATUS is returned, we need to
wait for the gadget driver to queue a request.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] usb: dwc2: host: Fix use after free w/ simultaneous irqs

[Douglas Anderson]

While plugging / unplugging on a DWC2 host port with "slub_debug=FZPUA"
enabled, I found a crash that was quite obviously a use after free.

It appears that in some cases when we handle the various sub-cases of
HCINT we may end up freeing the QTD.  If there is more than one bit set
in HCINT we may then end up continuing to use the QTD, which is bad.
Let's be paranoid and check for this after each sub-case.  We only do
this if there are still further bits to process, so the overhead should
be small.  This should be safe since we officially have the
"hsotg->lock" (it was grabbed in dwc2_handle_hcd_intr).

The specific crash I found was:
 Unable to handle kernel paging request at virtual address 6b6b6b9f

At the time of the crash, the kernel reported:
 (dwc2_hc_nak_intr+0x5c/0x198)
 (dwc2_handle_hcd_intr+0xa84/0xbf8)
 (_dwc2_hcd_irq+0x1c/0x20)
 (usb_hcd_irq+0x34/0x48)

Popping into kgdb found that "*qtd" was filled with "0x6b", AKA qtd had
been freed and filled with slub_debug poison.

kgdb gave a little better stack crawl:
 0 dwc2_hc_nak_intr (hsotg=hsotg@entry=0xec42e058,
 chan=chan@entry=0xec546dc0, chnum=chnum@entry=4,
 qtd=qtd@entry=0xec679600) at drivers/usb/dwc2/hcd_intr.c:1237
 1 dwc2_hc_n_intr (chnum=4, hsotg=0xec42e058) at
 drivers/usb/dwc2/hcd_intr.c:2041
 2 dwc2_hc_intr (hsotg=0xec42e058) at drivers/usb/dwc2/hcd_intr.c:2078
 3 dwc2_handle_hcd_intr (hsotg=0xec42e058) at
 drivers/usb/dwc2/hcd_intr.c:2128
 4 _dwc2_hcd_irq (hcd=) at drivers/usb/dwc2/hcd.c:2837
 5 usb_hcd_irq (irq=, __hcd=) at
 drivers/usb/core/hcd.c:2353

Popping up to frame #1 (dwc2_hc_n_intr) found:
 (gdb) print /x hcint
 $12 = 0x12

AKA:
 #define HCINTMSK_CHHLTD  (1 << 1)
 #define HCINTMSK_NAK (1 << 4)

Further debugging found that by simulating receiving those two
interrupts at the same time it was trivial to replicate the
use-after-free.  See  for a patch and
instructions.  This lead to getting the following stack crawl of the
actual free:
 0  arch_kgdb_breakpoint () at arch/arm/include/asm/outercache.h:103
 1  kgdb_breakpoint () at kernel/debug/debug_core.c:1054
 2  dwc2_hcd_qtd_unlink_and_free (hsotg=, qh=, qtd=0xe4479a00) at drivers/usb/dwc2/hcd.h:488
 3  dwc2_deactivate_qh (free_qtd=, qh=0xe5efa280,
  hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:671
 4  dwc2_release_channel (hsotg=hsotg@entry=0xed424618,
  chan=chan@entry=0xed5be000, qtd=,
  halt_status=) at drivers/usb/dwc2/hcd_intr.c:742
 5  dwc2_halt_channel (hsotg=0xed424618, chan=0xed5be000, qtd=, halt_status=) at
  drivers/usb/dwc2/hcd_intr.c:804
 6  dwc2_complete_non_periodic_xfer (chnum=,
  halt_status=, qtd=, chan=, hsotg=) at drivers/usb/dwc2/hcd_intr.c:889
 7  dwc2_hc_xfercomp_intr (hsotg=hsotg@entry=0xed424618,
  chan=chan@entry=0xed5be000, chnum=chnum@entry=6,
  qtd=qtd@entry=0xe4479a00) at drivers/usb/dwc2/hcd_intr.c:1065
 8  dwc2_hc_chhltd_intr_dma (qtd=0xe4479a00, chnum=6, chan=0xed5be000,
  hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:1823
 9  dwc2_hc_chhltd_intr (qtd=0xe4479a00, chnum=6, chan=0xed5be000,
  hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:1944
 10 dwc2_hc_n_intr (chnum=6, hsotg=0xed424618) at
  drivers/usb/dwc2/hcd_intr.c:2052
 11 dwc2_hc_intr (hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:2097
 12 dwc2_handle_hcd_intr (hsotg=0xed424618) at
  drivers/usb/dwc2/hcd_intr.c:2147
 13 _dwc2_hcd_irq (hcd=) at drivers/usb/dwc2/hcd.c:2837
 14 usb_hcd_irq (irq=, __hcd=) at
  drivers/usb/core/hcd.c:2353

Though we could add specific code to handle this case, adding the
general purpose code to check for all cases where qtd might be freed
seemed safer.

Signed-off-by: Douglas Anderson 
---
 drivers/usb/dwc2/hcd_intr.c | 80 +++--
 1 file changed, 70 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index f70c970..57e71f9d 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -1949,6 +1949,25 @@ static void dwc2_hc_chhltd_intr(struct dwc2_hsotg *hsotg,
}
 }
 
+/*
+ * Check if the given qtd is still the top of the list (and thus valid).
+ *
+ * If dwc2_hcd_qtd_unlink_and_free() has been called since we grabbed
+ * the qtd from the top of the list, this will return NULL.  Otherwise
+ * it will be passed back qtd.
+ */
+struct dwc2_qtd *dwc2_check_qtd_still_ok(struct dwc2_qtd *qtd,
+   struct list_head *qtd_list)
+{
+   struct dwc2_qtd *cur_head;
+
+   cur_head = list_first_entry(qtd_list, struct dwc2_qtd, qtd_list_entry);
+   if (cur_head == qtd)
+   return qtd;
+
+   return NULL;
+}
+
 /* Handles interrupt for a specific Host Channel */
 static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum)
 {
@@ -2031,26 +2050,67 @@ static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, 
int chnum)
 */
hcint &= 

Re: Fwd: [Bug 105901] OOPS using symbolserial

[Johan Hovold]

On Wed, Oct 14, 2015 at 06:08:23PM +0200, Stephan Althaus wrote:
> Am 13.10.2015 um 21:08 schrieb Johan Hovold:

> > Please try using a more recent kernel than 4.1.5. The oops you're
> > hitting should already have been fixed in mainline (and the fix has also
> > been backported to 4.1.8).

> Thanks for the hint.
> Kernel 4.2.3 works perfectly :-)
> 
> But, at work i will need to do this on an IGEL,
> with kernel 3.13.11.
> Unfortunately i will not be able to upgrade the kernel entirely.
> 
> I would like to build the kernel defective modules on my PC
> an copy them over to the IGEL device.
> 
> Can you give me a hint, which files have been affected?
> symbolserial.c seems not the only responsible file for this OOPS..

You should only need to backport the following commit:

951d3793bbfc ("USB: symbolserial: Use usb_get_serial_port_data")

but if you run into any problems you'll have to take this up with
whoever is forcing you to use such an old kernel as 3.13 is not
supported by the community.

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

Re: musb: communication issue with more than 12 FTDI ports

[Felipe Balbi]

Hi,

Bin Liu  writes:
> On 10/14/2015 10:56 AM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Bin Liu  writes:
>>> Hi,
>>>
>>> On 10/13/2015 01:22 PM, Felipe Balbi wrote:
 Yegor Yefremov  writes:
> On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
>  wrote:
>> We have a problem, when using more than 12 FTDI ports. Kernels tried:
>> 3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz
>>
>> Below the USB topology:
>>
>> # lsusb -t
>> /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
>> /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
>>   |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
>>   |__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
>>   |__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 12M
>>   |__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
>>   |__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M
>>>
>>> How many EPs does each FTDI device require? at least one INT EP, right?
>>> If I read it right, the topology above has 2 hubs, and 16 high-speed
>>> FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT
>>> EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP
>>> configuration used by default. I am wondering how those devices got
>>> enumerated properly.
>>
>> dynamic EP allocation, but that has its own limitations.
>>
> MUSB does not support dynamic EP allocation for INT/ISOCH.

I remember isoc doesn't, not sure about int. Do you remember where that
part of the code is off the top of your head ?

-- 
balbi


signature.asc
Description: PGP signature

Re: musb: communication issue with more than 12 FTDI ports

[Felipe Balbi]

Hi,

Bin Liu  writes:
> On 10/14/2015 12:05 PM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Bin Liu  writes:
>>> Felipe,
>>>
>>> On 10/14/2015 11:25 AM, Felipe Balbi wrote:

 Hi,

 Bin Liu  writes:
> On 10/14/2015 10:56 AM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Bin Liu  writes:
>>> Hi,
>>>
>>> On 10/13/2015 01:22 PM, Felipe Balbi wrote:
 Yegor Yefremov  writes:
> On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
>  wrote:
>> We have a problem, when using more than 12 FTDI ports. Kernels tried:
>> 3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz
>>
>> Below the USB topology:
>>
>> # lsusb -t
>> /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
>> /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
>> |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
>> |__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
>> |__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 
>> 12M
>> |__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 
>> 480M
>> |__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 
>> 480M
>> |__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 
>> 480M
>> |__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 
>> 480M
>> |__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 
>> 480M
>> |__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 
>> 480M
>> |__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 
>> 480M
>> |__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 
>> 480M
>> |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
>> |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
>> |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
>> |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
>> |__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
>> |__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
>> |__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
>> |__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M
>>>
>>> How many EPs does each FTDI device require? at least one INT EP, right?
>>> If I read it right, the topology above has 2 hubs, and 16 high-speed
>>> FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT
>>> EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP
>>> configuration used by default. I am wondering how those devices got
>>> enumerated properly.
>>
>> dynamic EP allocation, but that has its own limitations.
>>
> MUSB does not support dynamic EP allocation for INT/ISOCH.

 I remember isoc doesn't, not sure about int. Do you remember where that
 part of the code is off the top of your head ?

>>>
>>> The MUSB EP allocation is in musb_schedule() in musb_host.c.
>>>
>>> It does not have specific policy for INT/ISOCH, but the issue is that
>>> for periodic EP, it got allocated during device enumeration but freed
>>> only when the device is disconnected. So practically there is no dynamic
>>> EP allocation for INT/ISOCH.
>>
>> This is not exactly what I can see when trying things out:
>>
>> minicom.cap:56:[   90.909917] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
>> dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
>> minicom.cap:66:[   91.175860] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
>> dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
>> minicom.cap:100:[   91.697827] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
>> dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
>> minicom.cap:106:[   91.818066] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
>> df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
>> minicom.cap:149:[   92.475792] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
>> dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
>> minicom.cap:162:[   92.736808] musb-hdrc musb-hdrc.1.auto: qh dc07d5c0 urb 
>> dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
>> minicom.cap:207:[   93.703046] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
>> df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
>> minicom.cap:215:[   93.977574] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
>> df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
>> minicom.cap:240:[   94.388472] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
>> df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
>> minicom.cap:289:[   95.422325] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
>> df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
>> 

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Alan Stern]

On Wed, 14 Oct 2015, Jassi Brar wrote:

> BTW, should the gadget stack ever queue a Non-ZLP as reply to some
> setup request that has USB_DIR_IN not set?

Yes.  If USB_DIR_IN is not set then the control transfer is OUT, so the
gadget needs to queue a request to receive some data from the host.  
That request will obviously need to be a non-ZLP.  In fact, it's hard
to think of a situation where a gadget would ever want to submit a
zero-length OUT request.  Isn't the UDC driver supposed to handle the
status stage of a control-IN transfer automatically?

Could this cause the problem you're seeing?  The host tries to send 
more data than the gadget is ready to receive?  (Although then the 
error code on the gadget side should be -75, not -71.)

Alan Stern

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

Re: musb: communication issue with more than 12 FTDI ports

[Bin Liu]

On 10/14/2015 12:19 PM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

On 10/14/2015 12:05 PM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

Felipe,

On 10/14/2015 11:25 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

On 10/14/2015 10:56 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

Hi,

On 10/13/2015 01:22 PM, Felipe Balbi wrote:

Yegor Yefremov  writes:

On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
 wrote:

We have a problem, when using more than 12 FTDI ports. Kernels tried:
3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz

Below the USB topology:

# lsusb -t
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
 |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
 |__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
 |__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 12M
 |__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
 |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
 |__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M


How many EPs does each FTDI device require? at least one INT EP, right?
If I read it right, the topology above has 2 hubs, and 16 high-speed
FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT
EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP
configuration used by default. I am wondering how those devices got
enumerated properly.


dynamic EP allocation, but that has its own limitations.


MUSB does not support dynamic EP allocation for INT/ISOCH.


I remember isoc doesn't, not sure about int. Do you remember where that
part of the code is off the top of your head ?



The MUSB EP allocation is in musb_schedule() in musb_host.c.

It does not have specific policy for INT/ISOCH, but the issue is that
for periodic EP, it got allocated during device enumeration but freed
only when the device is disconnected. So practically there is no dynamic
EP allocation for INT/ISOCH.


This is not exactly what I can see when trying things out:

minicom.cap:56:[   90.909917] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:66:[   91.175860] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:100:[   91.697827] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:106:[   91.818066] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:149:[   92.475792] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:162:[   92.736808] musb-hdrc musb-hdrc.1.auto: qh dc07d5c0 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:207:[   93.703046] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:215:[   93.977574] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:240:[   94.388472] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:289:[   95.422325] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:305:[   95.688207] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:335:[   96.291453] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:410:[   97.696976] musb-hdrc musb-hdrc.1.auto: qh df6b70c0 urb 
dc011f40 dev11 ep1in-intr, hw_ep 2, dd842080/8
minicom.cap:56:[   90.909917] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:66:[   91.175860] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:100:[   

Re: musb: communication issue with more than 12 FTDI ports

[Bin Liu]

On 10/14/2015 12:05 PM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

Felipe,

On 10/14/2015 11:25 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

On 10/14/2015 10:56 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

Hi,

On 10/13/2015 01:22 PM, Felipe Balbi wrote:

Yegor Yefremov  writes:

On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
 wrote:

We have a problem, when using more than 12 FTDI ports. Kernels tried:
3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz

Below the USB topology:

# lsusb -t
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
|__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
|__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
|__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 12M
|__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M


How many EPs does each FTDI device require? at least one INT EP, right?
If I read it right, the topology above has 2 hubs, and 16 high-speed
FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT
EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP
configuration used by default. I am wondering how those devices got
enumerated properly.


dynamic EP allocation, but that has its own limitations.


MUSB does not support dynamic EP allocation for INT/ISOCH.


I remember isoc doesn't, not sure about int. Do you remember where that
part of the code is off the top of your head ?



The MUSB EP allocation is in musb_schedule() in musb_host.c.

It does not have specific policy for INT/ISOCH, but the issue is that
for periodic EP, it got allocated during device enumeration but freed
only when the device is disconnected. So practically there is no dynamic
EP allocation for INT/ISOCH.


This is not exactly what I can see when trying things out:

minicom.cap:56:[   90.909917] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:66:[   91.175860] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:100:[   91.697827] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:106:[   91.818066] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:149:[   92.475792] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:162:[   92.736808] musb-hdrc musb-hdrc.1.auto: qh dc07d5c0 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:207:[   93.703046] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:215:[   93.977574] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:240:[   94.388472] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:289:[   95.422325] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:305:[   95.688207] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:335:[   96.291453] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:410:[   97.696976] musb-hdrc musb-hdrc.1.auto: qh df6b70c0 urb 
dc011f40 dev11 ep1in-intr, hw_ep 2, dd842080/8
minicom.cap:56:[   90.909917] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:66:[   91.175860] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:100:[   91.697827] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1

Re: musb: communication issue with more than 12 FTDI ports

[Felipe Balbi]

Hi,

Bin Liu  writes:
> Felipe,
>
> On 10/14/2015 11:25 AM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Bin Liu  writes:
>>> On 10/14/2015 10:56 AM, Felipe Balbi wrote:

 Hi,

 Bin Liu  writes:
> Hi,
>
> On 10/13/2015 01:22 PM, Felipe Balbi wrote:
>> Yegor Yefremov  writes:
>>> On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
>>>  wrote:
 We have a problem, when using more than 12 FTDI ports. Kernels tried:
 3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz

 Below the USB topology:

 # lsusb -t
 /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
 /:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
|__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
|__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
|__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 12M
|__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
|__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
|__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M
>
> How many EPs does each FTDI device require? at least one INT EP, right?
> If I read it right, the topology above has 2 hubs, and 16 high-speed
> FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT
> EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP
> configuration used by default. I am wondering how those devices got
> enumerated properly.

 dynamic EP allocation, but that has its own limitations.

>>> MUSB does not support dynamic EP allocation for INT/ISOCH.
>>
>> I remember isoc doesn't, not sure about int. Do you remember where that
>> part of the code is off the top of your head ?
>>
>
> The MUSB EP allocation is in musb_schedule() in musb_host.c.
>
> It does not have specific policy for INT/ISOCH, but the issue is that 
> for periodic EP, it got allocated during device enumeration but freed 
> only when the device is disconnected. So practically there is no dynamic 
> EP allocation for INT/ISOCH.

This is not exactly what I can see when trying things out:

minicom.cap:56:[   90.909917] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:66:[   91.175860] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:100:[   91.697827] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:106:[   91.818066] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:149:[   92.475792] musb-hdrc musb-hdrc.1.auto: qh df62d240 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:162:[   92.736808] musb-hdrc musb-hdrc.1.auto: qh dc07d5c0 urb 
dc4ebec0 dev3 ep1in-intr, hw_ep 10, df700680/1
minicom.cap:207:[   93.703046] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:215:[   93.977574] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:240:[   94.388472] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:289:[   95.422325] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:305:[   95.688207] musb-hdrc musb-hdrc.1.auto: qh dc4eb340 urb 
df5b7e40 dev4 ep1in-intr, hw_ep 11, df5c3400/1
minicom.cap:335:[   96.291453] musb-hdrc musb-hdrc.1.auto: qh df551cc0 urb 
df65ee40 dev7 ep1in-intr, hw_ep 12, df6a2bc0/1
minicom.cap:410:[   97.696976] musb-hdrc musb-hdrc.1.auto: qh df6b70c0 urb 
dc011f40 dev11 ep1in-intr, hw_ep 2, dd842080/8
minicom.cap:56:[   90.909917] musb-hdrc 

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Felipe Balbi]

Hi,

Jassi Brar  writes:
> On Wed, Oct 14, 2015 at 9:18 PM, Felipe Balbi  wrote:
>>
>> Hi,
>>
>> Jassi Brar  writes:
>>> On Wed, Oct 14, 2015 at 8:42 PM, Felipe Balbi  wrote:

 Hi,

 jaswinder.si...@linaro.org writes:
> From: Jassi Brar 
>
> We must return 0 for any OUT setup request, otherwise
> protocol error may occur.

 have you seen any such errors ?

>>> Yes. While testing my new udc driver.
>>>
>>>
 Care to describe what problems you have seen and which UDC you were using,
>>>n> what's the exact scenario. How have you tested this ?

>>> The udc I am writing the driver for is not yet public. To test my
>>> driver at lowest level possible, I use 'usbmon' to capture and analyze
>>> traffic since I don't have any hardware probe.
>>>
>>> I see the following on gadget side
>>> ---
>>> configfs-gadget gadget: non-core control req21.20 v i0001 l7
>>> configfs-gadget gadget: acm ttyGS0 req21.20 v i0001 l7
>>> configfs-gadget gadget: acm ttyGS0 completion, err -71
>>> ---
>>>
>>> and the following on host side usbmon capture
>>> ---
>>> 88041ed01f00 540296433 S Co:3:023:0 s 21 20  0001 0007 7 =
>>> 8025 08
>>> 88041ed01f00 540296790 C Co:3:023:0 -71 0
>>> ---
>>
>> and you did you even consider this could be a bug in your new UDC driver
>> at all ? This works fine with other UDCs.
>>
> I was happy too until I decided to look closely at the annoying print
> on gadget side. This is a non-fatal error and visible only when
> debugging is enabled, so every udc seems 'fine'

I tried with my sniffer and saw no stalls, nothing. My setup request to
set line coding to 9600 baud worked just fine.

>> How far are you in developing this new UDC driver ?
>>
> Its done and tested for fsg+acm composite and each alone.

stress tests ? Meaning, did you leave it running for a couple of weeks
at least ?

>> Did you run USBCV at all ?
>>
> No USBCV not yet. I borrowed Windows machine to test FSG but forgot
> USBCV since it's been years I wrote last udc driver. Will give it a
> try tomorrow but I don't think it could emulate the sequence I hit
> with f_acm.

USBCV might already have some ACM test cases and it should exercise all
mandatory setup requests.

>> Are you sure you're implementing EP0 handling correctly ? What
>> sort of tests have you done with this UDC ? Is it working with testusb
>> against a known good host (EHCI and XHCI should be good for that) ?
>>
> Well as I said I have been looking at low level transfers and
> everything is good.

still, run testusb with the acompanying shell script and keep it running
for at least 2 weeks.

> BTW, should the gadget stack ever queue a Non-ZLP as reply to some
> setup request that has USB_DIR_IN not set?

What phase of the setup are we talking about ? If it has a DATA phase,
then data can have non-ZLP transfers and it can also require a ZLP to
end the data phase itself (if wLength % wMaxPacketSize == 0). Status
phase is always zlp.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v6 2/2] usb: dwc2: refactor common low-level hw code to platform.c

[Marek Szyprowski]

DWC2 module on some platforms needs three additional hardware
resources: phy controller, clock and power supply. All of them must be
enabled/activated to properly initialize and operate. This was initially
handled in s3c-hsotg driver, which has been converted to 'gadget' part
of dwc2 driver. Unfortunately, not all of this code got moved to common
platform code, what resulted in accessing DWC2 registers without
enabling low-level hardware resources. This fails for example on Exynos
SoCs. This patch moves all the code for managing those resources to
common platform.c file and provides convenient wrappers for controlling
them.

Signed-off-by: Marek Szyprowski 
Acked-by: John Youn 
Tested-by: John Youn 
---
Changelog:
v6:
- fixed typo pointed by John Youn
- added Acked-by and Tested-by tags

v5:
- added separate patch removing init_mutex, which is no longer needed

v4:
- fixed broken conditional compilation and adjusted comments in dwc2_hsotg
  structure documentation

v3:
- rebased onto latest 'testing/next' from Felipe Balbi (includes
  s3c_hsotg -> dwc2 rename)

v2:
- moved setting of ll_hw_enabled flag to enable/disable functions,
  as suggested by John Youn
- moved setting of phy width to dwc2_lowlevel_init function
---
 drivers/usb/dwc2/core.h |  24 +++--
 drivers/usb/dwc2/gadget.c   | 186 --
 drivers/usb/dwc2/platform.c | 216 
 3 files changed, 210 insertions(+), 216 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 89091db..a66d3cb 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -579,6 +579,15 @@ struct dwc2_hregs_backup {
  *  - USB_DR_MODE_PERIPHERAL
  *  - USB_DR_MODE_HOST
  *  - USB_DR_MODE_OTG
+ * @hcd_enabledHost mode sub-driver initialization indicator.
+ * @gadget_enabled Peripheral mode sub-driver initialization indicator.
+ * @ll_hw_enabled  Status of low-level hardware resources.
+ * @phy:The otg phy transceiver structure for phy control.
+ * @uphy:   The otg phy transceiver structure for old USB phy 
control.
+ * @plat:   The platform specific configuration data. This can be 
removed once
+ *  all SoCs support usb transceiver.
+ * @supplies:   Definition of USB power supplies
+ * @phyif:  PHY interface width
  * @lock:  Spinlock that protects all the driver data structures
  * @priv:  Stores a pointer to the struct usb_hcd
  * @queuing_high_bandwidth: True if multiple packets of a high-bandwidth
@@ -671,12 +680,6 @@ struct dwc2_hregs_backup {
  * These are for peripheral mode:
  *
  * @driver: USB gadget driver
- * @phy:The otg phy transceiver structure for phy control.
- * @uphy:   The otg phy transceiver structure for old USB phy 
control.
- * @plat:   The platform specific configuration data. This can be 
removed once
- *  all SoCs support usb transceiver.
- * @supplies:   Definition of USB power supplies
- * @phyif:  PHY interface width
  * @dedicated_fifos:Set if the hardware has dedicated IN-EP fifos.
  * @num_of_eps: Number of available EPs (excluding EP0)
  * @debug_root: Root directrory for debugfs.
@@ -706,10 +709,13 @@ struct dwc2_hsotg {
enum usb_dr_mode dr_mode;
unsigned int hcd_enabled:1;
unsigned int gadget_enabled:1;
+   unsigned int ll_hw_enabled:1;
 
struct phy *phy;
struct usb_phy *uphy;
+   struct dwc2_hsotg_plat *plat;
struct regulator_bulk_data 
supplies[ARRAY_SIZE(dwc2_hsotg_supply_names)];
+   u32 phyif;
 
spinlock_t lock;
void *priv;
@@ -812,9 +818,6 @@ struct dwc2_hsotg {
 #if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || 
IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
/* Gadget structures */
struct usb_gadget_driver *driver;
-   struct dwc2_hsotg_plat *plat;
-
-   u32 phyif;
int fifo_mem;
unsigned int dedicated_fifos:1;
unsigned char num_of_eps;
@@ -1103,7 +1106,8 @@ extern void dwc2_set_all_params(struct dwc2_core_params 
*params, int value);
 
 extern int dwc2_get_hwparams(struct dwc2_hsotg *hsotg);
 
-
+extern int dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg);
+extern int dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg);
 
 /*
  * Dump core registers and SPRAM
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 79d9f3b..0abf73c 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -25,15 +25,11 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
-#include 
 
 #include 
 #include 
 #include 
-#include 
 
 #include "core.h"
 #include "hw.h"
@@ -3077,50 +3073,6 @@ static struct usb_ep_ops 

[PATCH v6 1/2] usb: dwc2: remove no longer needed init_mutex

[Marek Szyprowski]

init_mutex is a leftover from the time, when s3c-hsotg driver did not
implement proper pull up/down control and emulated it by enabling
enabling/disabling usb phy. Proper pull up/down control has been added
by commit 5b9451f8c4fbaf0549139755fb45ff2b57975b7f ("usb: dwc2: gadget:
use soft-disconnect udc feature in pullup() method"), so init_muxtex can
be removed now to avoid potential deadlocks with other locks.

Signed-off-by: Marek Szyprowski 
Acked-by: John Youn 
Tested-by: John Youn 
---
 drivers/usb/dwc2/core.h |  1 -
 drivers/usb/dwc2/gadget.c   | 17 -
 drivers/usb/dwc2/platform.c |  1 -
 3 files changed, 19 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index ebf2504..89091db 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -712,7 +712,6 @@ struct dwc2_hsotg {
struct regulator_bulk_data 
supplies[ARRAY_SIZE(dwc2_hsotg_supply_names)];
 
spinlock_t lock;
-   struct mutex init_mutex;
void *priv;
int irq;
struct clk *clk;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7e5670c..79d9f3b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3193,7 +3193,6 @@ static int dwc2_hsotg_udc_start(struct usb_gadget *gadget,
return -EINVAL;
}
 
-   mutex_lock(>init_mutex);
WARN_ON(hsotg->driver);
 
driver->driver.bus = NULL;
@@ -3220,12 +3219,9 @@ static int dwc2_hsotg_udc_start(struct usb_gadget 
*gadget,
 
dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
 
-   mutex_unlock(>init_mutex);
-
return 0;
 
 err:
-   mutex_unlock(>init_mutex);
hsotg->driver = NULL;
return ret;
 }
@@ -3246,8 +3242,6 @@ static int dwc2_hsotg_udc_stop(struct usb_gadget *gadget)
if (!hsotg)
return -ENODEV;
 
-   mutex_lock(>init_mutex);
-
/* all endpoints should be shutdown */
for (ep = 1; ep < hsotg->num_of_eps; ep++) {
if (hsotg->eps_in[ep])
@@ -3270,8 +3264,6 @@ static int dwc2_hsotg_udc_stop(struct usb_gadget *gadget)
 
regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies), hsotg->supplies);
 
-   mutex_unlock(>init_mutex);
-
return 0;
 }
 
@@ -3307,7 +3299,6 @@ static int dwc2_hsotg_pullup(struct usb_gadget *gadget, 
int is_on)
return 0;
}
 
-   mutex_lock(>init_mutex);
spin_lock_irqsave(>lock, flags);
if (is_on) {
hsotg->enabled = 1;
@@ -3321,7 +3312,6 @@ static int dwc2_hsotg_pullup(struct usb_gadget *gadget, 
int is_on)
 
hsotg->gadget.speed = USB_SPEED_UNKNOWN;
spin_unlock_irqrestore(>lock, flags);
-   mutex_unlock(>init_mutex);
 
return 0;
 }
@@ -3832,8 +3822,6 @@ int dwc2_hsotg_suspend(struct dwc2_hsotg *hsotg)
if (hsotg->lx_state != DWC2_L0)
return ret;
 
-   mutex_lock(>init_mutex);
-
if (hsotg->driver) {
int ep;
 
@@ -3861,8 +3849,6 @@ int dwc2_hsotg_suspend(struct dwc2_hsotg *hsotg)
clk_disable(hsotg->clk);
}
 
-   mutex_unlock(>init_mutex);
-
return ret;
 }
 
@@ -3874,8 +3860,6 @@ int dwc2_hsotg_resume(struct dwc2_hsotg *hsotg)
if (hsotg->lx_state == DWC2_L2)
return ret;
 
-   mutex_lock(>init_mutex);
-
if (hsotg->driver) {
dev_info(hsotg->dev, "resuming usb gadget %s\n",
 hsotg->driver->driver.name);
@@ -3892,7 +3876,6 @@ int dwc2_hsotg_resume(struct dwc2_hsotg *hsotg)
dwc2_hsotg_core_connect(hsotg);
spin_unlock_irqrestore(>lock, flags);
}
-   mutex_unlock(>init_mutex);
 
return ret;
 }
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index b920e43..581e9ca 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -252,7 +252,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
}
 
spin_lock_init(>lock);
-   mutex_init(>init_mutex);
 
/* Detect config values from hardware */
retval = dwc2_get_hwparams(hsotg);
-- 
1.9.2

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

Re: [PATCH 14/20] target: Remove the write_pending_status() callback function

[Sagi Grimberg]

On 10/12/2015 8:28 PM, Felipe Balbi wrote:


Hi,

Bart Van Assche  writes:

Due to the previous patch the write_pending_status() callback
function is no longer called. Hence remove it.

Signed-off-by: Bart Van Assche 


Looks Good

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

Re: [PATCH v4 2/2] usb: chipidea: imx: refine clock operations to adapt for all platforms

[Peter Chen]

On Wed, Oct 14, 2015 at 10:07:59AM +0200, Marc Kleine-Budde wrote:
> On 10/14/2015 03:57 AM, Peter Chen wrote:
> > Some i.mx platforms need three clocks to let controller work, but
> > others only need one, refine clock operation to adapt for all
> > platforms, it fixes a regression found at i.mx27.
> > 
> > Signed-off-by: Peter Chen 
> > Tested-by: Fabio Estevam 
> > Cc:  #v4.1+
> > ---
> >  drivers/usb/chipidea/ci_hdrc_imx.c | 132 
> > -
> >  1 file changed, 114 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
> > b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 6ccbf60..82b1dfe 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -84,6 +84,12 @@ struct ci_hdrc_imx_data {
> > struct imx_usbmisc_data *usbmisc_data;
> > bool supports_runtime_pm;
> > bool in_lpm;
> > +   /* SoC before i.mx6 (except imx23/imx28) needs three clks */
> > +   bool need_three_clks;
> > +   struct clk *clk_ipg;
> > +   struct clk *clk_ahb;
> > +   struct clk *clk_per;
> > +   /* - */
> >  };
> >  
> >  /* Common functions shared by usbmisc drivers */
> > @@ -135,6 +141,103 @@ static struct imx_usbmisc_data 
> > *usbmisc_get_init_data(struct device *dev)
> >  }
> >  
> >  /* End of common functions shared by usbmisc drivers*/
> > +static int imx_get_clks(struct device *dev)
> > +{
> > +   struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> > +   int ret = 0;
> > +
> > +   data->clk_ipg = devm_clk_get(dev, "ipg");
> > +   if (IS_ERR(data->clk_ipg)) {
> > +   /* If the platform only needs one clocks */
> > +   data->clk = devm_clk_get(dev, NULL);
> > +   if (IS_ERR(data->clk)) {
> > +   ret = PTR_ERR(data->clk);
> > +   dev_err(dev,
> > +   "Failed to get clock, err=%d\n", ret);
> > +   dev_err(dev,
> > +   "Failed to get ipg clock, err=%d\n", ret);
> 
> Nitpick, one error message should be enough.

Yes, I will show both error value for clk_ipg and NULL.

> 
> > +   return ret;
> > +   }
> > +   return ret;
> > +   }
> > +
> > +   data->clk_ahb = devm_clk_get(dev, "ahb");
> > +   if (IS_ERR(data->clk_ahb)) {
> > +   ret = PTR_ERR(data->clk_ahb);
> > +   dev_err(dev,
> > +   "Failed to get ahb clock, err=%d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   data->clk_per = devm_clk_get(dev, "per");
> > +   if (IS_ERR(data->clk_per)) {
> > +   ret = PTR_ERR(data->clk_per);
> > +   dev_err(dev,
> > +   "Failed to get per clock, err=%d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   data->need_three_clks = true;
> > +   return ret;
> > +}
> > +
> > +static int imx_prepare_enable_clks(struct device *dev)
> > +{
> > +   struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> > +   int ret = 0;
> > +
> > +   if (data->need_three_clks) {
> > +   ret = clk_prepare_enable(data->clk_ipg);
> > +   if (ret) {
> > +   dev_err(dev,
> > +   "Failed to prepare/enable ipg clk, err=%d\n",
> > +   ret);
> > +   return ret;
> > +   }
> > +
> > +   ret = clk_prepare_enable(data->clk_ahb);
> > +   if (ret) {
> > +   dev_err(dev,
> > +   "Failed to prepare/enable ahb clk, err=%d\n",
> > +   ret);
> > +   clk_disable_unprepare(data->clk_ipg);
> > +   return ret;
> > +   }
> > +
> > +   ret = clk_prepare_enable(data->clk_per);
> > +   if (ret) {
> > +   dev_err(dev,
> > +   "Failed to prepare/enable per clk, err=%d\n",
> > +   ret);
> > +   clk_disable_unprepare(data->clk_ahb);
> > +   clk_disable_unprepare(data->clk_ipg);
> > +   return ret;
> > +   }
> > +   } else {
> > +   ret = clk_prepare_enable(data->clk);
> > +   if (ret) {
> > +   dev_err(dev,
> > +   "Failed to prepare/enable clk, err=%d\n",
> > +   ret);
> > +   return ret;
> > +   }
> > +   }
> > +
> > +   return ret;
> > +}
> > +
> > +static void imx_disable_unprepare_clks(struct device *dev)
> > +{
> > +   struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> > +
> > +   if (data->need_three_clks) {
> > +   clk_disable_unprepare(data->clk_per);
> > +   clk_disable_unprepare(data->clk_ipg);
> > +   clk_disable_unprepare(data->clk_ahb);
> 
> nit: you enable in this order: ipg, ahb, per. Disabling should be inverse.

Will change, 

Re: Enumerating the USB device more than the capability of Host in the Linux Platform

[Mathias Nyman]

Hi

Currently the Linux xhci driver will always try to enable a slot, if no slot is 
available xhci will return a
"No slot available" error command completion code.

If the command completion code is anything else than "Success" for the enable 
slot command, the driver
will print out the following error message:

"Error while assigning device slot ID"
"Max number of devices this xHCI host supports is %u."

This should be visible with dmesg

xhci_alloc_dev() returns 1 if slot was successfully enabled, 0 on failure.

xhci driver can not itself generate any popup messages, this is up to 
userspace. If the userspace you are using
is supposed to generate some popup message for this then it's possible that the failing 
"0" returned
by xhci_alloc_dev() is not properly propageated/translated through usb core, 
the used usb libarary
and whatever other userspace components on top.

Regards
-Mathias



On 13.10.2015 18:34, Binu Thanka Kumar Chinna Thankam wrote:

Hi Mathias,

Could you please take a look on the below  issue and provide your input on this.

Thanks and Regards
Binu

-Original Message-
From: Sarah Sharp [mailto:sarah.a.sh...@intel.com]
Sent: 13 October 2015 00:04
To: Binu Thanka Kumar Chinna Thankam 

Cc: Sunil Kumar G ; Badrinath Ramachandra 
; Nyman, Mathias 
Subject: Re: Enumerating the USB device more than the capability of Host in the 
Linux Platform

Please follow up with the new xHCI driver maintainer, Mathias Nyman 
, and Cc the public Linux USB mailing list 
.

Sarah Sharp

On Sun, Oct 11, 2015 at 02:50:31PM +, Binu Thanka Kumar Chinna Thankam 
wrote:

Hi Sara,

In one of our XHCI  Host DUT  configurations,  we have limited the max slots 
supported by our xHCI host as 8.

With this configuration, we tried to test our XHCI DUT in  FPGA with Linux 
driver  and we could see the interop tree with 8 devices is working fine.

We tried to do an experiment to check the behaviour of the System by connecting 
the 9th device to the tree.

During this time we saw the 9th device is not enumerated by the system and no 
message is displayed.

When we checked the PCI analyser  trace to view the XHCI  specific 
transactions, we noticed the following.

Software tried to enumerate the device by sending the enable slot command for 
the 9th slot which is not supported by our Host even though the Host indicated 
the max slot as 8 through the  HCSPARAMS1 register.

When that enable slot command for the 9 the device is processed by the Host, it 
sends out a command completion event with completion code as  Slot not 
available error.

I would expect the Software should send a popup message which should indicate 
that the device cannot be enumerated since it exceeds the capability of Host, 
during this condition.

Could you please provide your input if any.

Thanks and Regards
BInu


L Technology Services Ltd

www.LntTechservices.com

This Email may contain confidential or privileged information for the intended 
recipient (s). If you are not the intended recipient, please do not use or 
disseminate the information, notify the sender and delete it from your system.

L Technology Services Ltd

www.LntTechservices.com

This Email may contain confidential or privileged information for the intended 
recipient (s). If you are not the intended recipient, please do not use or 
disseminate the information, notify the sender and delete it from your system.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

Re: [PATCH v4 2/2] usb: chipidea: imx: refine clock operations to adapt for all platforms

[Marc Kleine-Budde]

On 10/14/2015 03:57 AM, Peter Chen wrote:
> Some i.mx platforms need three clocks to let controller work, but
> others only need one, refine clock operation to adapt for all
> platforms, it fixes a regression found at i.mx27.
> 
> Signed-off-by: Peter Chen 
> Tested-by: Fabio Estevam 
> Cc:  #v4.1+
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 132 
> -
>  1 file changed, 114 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 6ccbf60..82b1dfe 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -84,6 +84,12 @@ struct ci_hdrc_imx_data {
>   struct imx_usbmisc_data *usbmisc_data;
>   bool supports_runtime_pm;
>   bool in_lpm;
> + /* SoC before i.mx6 (except imx23/imx28) needs three clks */
> + bool need_three_clks;
> + struct clk *clk_ipg;
> + struct clk *clk_ahb;
> + struct clk *clk_per;
> + /* - */
>  };
>  
>  /* Common functions shared by usbmisc drivers */
> @@ -135,6 +141,103 @@ static struct imx_usbmisc_data 
> *usbmisc_get_init_data(struct device *dev)
>  }
>  
>  /* End of common functions shared by usbmisc drivers*/
> +static int imx_get_clks(struct device *dev)
> +{
> + struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + data->clk_ipg = devm_clk_get(dev, "ipg");
> + if (IS_ERR(data->clk_ipg)) {
> + /* If the platform only needs one clocks */
> + data->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(data->clk)) {
> + ret = PTR_ERR(data->clk);
> + dev_err(dev,
> + "Failed to get clock, err=%d\n", ret);
> + dev_err(dev,
> + "Failed to get ipg clock, err=%d\n", ret);

Nitpick, one error message should be enough.

> + return ret;
> + }
> + return ret;
> + }
> +
> + data->clk_ahb = devm_clk_get(dev, "ahb");
> + if (IS_ERR(data->clk_ahb)) {
> + ret = PTR_ERR(data->clk_ahb);
> + dev_err(dev,
> + "Failed to get ahb clock, err=%d\n", ret);
> + return ret;
> + }
> +
> + data->clk_per = devm_clk_get(dev, "per");
> + if (IS_ERR(data->clk_per)) {
> + ret = PTR_ERR(data->clk_per);
> + dev_err(dev,
> + "Failed to get per clock, err=%d\n", ret);
> + return ret;
> + }
> +
> + data->need_three_clks = true;
> + return ret;
> +}
> +
> +static int imx_prepare_enable_clks(struct device *dev)
> +{
> + struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + if (data->need_three_clks) {
> + ret = clk_prepare_enable(data->clk_ipg);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable ipg clk, err=%d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(data->clk_ahb);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable ahb clk, err=%d\n",
> + ret);
> + clk_disable_unprepare(data->clk_ipg);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(data->clk_per);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable per clk, err=%d\n",
> + ret);
> + clk_disable_unprepare(data->clk_ahb);
> + clk_disable_unprepare(data->clk_ipg);
> + return ret;
> + }
> + } else {
> + ret = clk_prepare_enable(data->clk);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable clk, err=%d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void imx_disable_unprepare_clks(struct device *dev)
> +{
> + struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> +
> + if (data->need_three_clks) {
> + clk_disable_unprepare(data->clk_per);
> + clk_disable_unprepare(data->clk_ipg);
> + clk_disable_unprepare(data->clk_ahb);

nit: you enable in this order: ipg, ahb, per. Disabling should be inverse.

> + } else {
> + clk_disable_unprepare(data->clk);
> + }
> +}

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung 

Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling

[Robert Baldyga]

Hi Felipe,

On 09/24/2015 06:49 PM, Felipe Balbi wrote:
> On Thu, Sep 24, 2015 at 05:23:09PM +0200, Robert Baldyga wrote:
>> Move function parameters to struct f_sourcesink to make them per instance
>> instead of having them as global variables. Since we can have multiple
>> instances of USB function we also want to have separate set of parameters
>> for each instance.
>>
>> Cc:  # 3.10+
>> Signed-off-by: Robert Baldyga 
> 
> why do you think this is stable material ? Looks like it's not
> fixing anything to me; just implementing a brand new requirement.

I will not insist on stable backporting of this patch, as it's actually
not very important fix (especially in case of sourcesink function).

Should I resend this patch without CC:stable?

Thanks,
Robert

> 
>> ---
>>  drivers/usb/gadget/function/f_sourcesink.c | 120 
>> +++--
>>  1 file changed, 62 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
>> b/drivers/usb/gadget/function/f_sourcesink.c
>> index 1353465..128abfb 100644
>> --- a/drivers/usb/gadget/function/f_sourcesink.c
>> +++ b/drivers/usb/gadget/function/f_sourcesink.c
>> @@ -50,6 +50,13 @@ struct f_sourcesink {
>>  struct usb_ep   *iso_in_ep;
>>  struct usb_ep   *iso_out_ep;
>>  int cur_alt;
>> +
>> +unsigned pattern;
>> +unsigned isoc_interval;
>> +unsigned isoc_maxpacket;
>> +unsigned isoc_mult;
>> +unsigned isoc_maxburst;
>> +unsigned buflen;
>>  };
>>  
>>  static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
>> @@ -57,13 +64,6 @@ static inline struct f_sourcesink *func_to_ss(struct 
>> usb_function *f)
>>  return container_of(f, struct f_sourcesink, function);
>>  }
>>  
>> -static unsigned pattern;
>> -static unsigned isoc_interval;
>> -static unsigned isoc_maxpacket;
>> -static unsigned isoc_mult;
>> -static unsigned isoc_maxburst;
>> -static unsigned buflen;
>> -
>>  
>> /*-*/
>>  
>>  static struct usb_interface_descriptor source_sink_intf_alt0 = {
>> @@ -298,7 +298,9 @@ static struct usb_gadget_strings *sourcesink_strings[] = 
>> {
>>  
>>  static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int 
>> len)
>>  {
>> -return alloc_ep_req(ep, len, buflen);
>> +struct f_sourcesink *ss = ep->driver_data;
>> +
>> +return alloc_ep_req(ep, len, ss->buflen);
>>  }
>>  
>>  void free_ep_req(struct usb_ep *ep, struct usb_request *req)
>> @@ -357,22 +359,22 @@ autoconf_fail:
>>  goto autoconf_fail;
>>  
>>  /* sanity check the isoc module parameters */
>> -if (isoc_interval < 1)
>> -isoc_interval = 1;
>> -if (isoc_interval > 16)
>> -isoc_interval = 16;
>> -if (isoc_mult > 2)
>> -isoc_mult = 2;
>> -if (isoc_maxburst > 15)
>> -isoc_maxburst = 15;
>> +if (ss->isoc_interval < 1)
>> +ss->isoc_interval = 1;
>> +if (ss->isoc_interval > 16)
>> +ss->isoc_interval = 16;
>> +if (ss->isoc_mult > 2)
>> +ss->isoc_mult = 2;
>> +if (ss->isoc_maxburst > 15)
>> +ss->isoc_maxburst = 15;
>>  
>>  /* fill in the FS isoc descriptors from the module parameters */
>> -fs_iso_source_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
>> -1023 : isoc_maxpacket;
>> -fs_iso_source_desc.bInterval = isoc_interval;
>> -fs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
>> -1023 : isoc_maxpacket;
>> -fs_iso_sink_desc.bInterval = isoc_interval;
>> +fs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
>> +1023 : ss->isoc_maxpacket;
>> +fs_iso_source_desc.bInterval = ss->isoc_interval;
>> +fs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
>> +1023 : ss->isoc_maxpacket;
>> +fs_iso_sink_desc.bInterval = ss->isoc_interval;
>>  
>>  /* allocate iso endpoints */
>>  ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, _iso_source_desc);
>> @@ -394,8 +396,8 @@ no_iso:
>>  ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
>>  }
>>  
>> -if (isoc_maxpacket > 1024)
>> -isoc_maxpacket = 1024;
>> +if (ss->isoc_maxpacket > 1024)
>> +ss->isoc_maxpacket = 1024;
>>  
>>  /* support high speed hardware */
>>  hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
>> @@ -406,15 +408,15 @@ no_iso:
>>   * We assume that the user knows what they are doing and won't
>>   * give parameters that their UDC doesn't support.
>>   */
>> -hs_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
>> -hs_iso_source_desc.wMaxPacketSize |= isoc_mult << 11;
>> -hs_iso_source_desc.bInterval 

Re: [GIT PULL] On-demand device probing

[Mark Brown]

On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote:

> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
> on-demand-probes-for-next

In don't think that's the URL you intended to use (also everything looks
word wrapped here)?


signature.asc
Description: PGP signature

[GIT PULL] On-demand device probing

[Tomeu Vizoso]

Hi Rob,

here is the pull request you asked for, with no changes from the version
that I posted last to the list.

The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f:

Linux 4.3-rc1 (2015-09-12 16:35:56 -0700)

are available in the git repository at:

git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git
on-demand-probes-for-next

for you to fetch changes up to 587402133fe433759d2d535e5d92ead87fd7f615:

of/platform: Defer probes of registered devices (2015-10-14 10:08:23 +0200)


Tomeu Vizoso (20):
driver core: handle -EPROBE_DEFER from bus_type.match()
ARM: amba: Move reading of periphid to amba_match()
of/platform: Point to struct device from device node
of: add function to allow probing a device from a OF node
gpio: Probe GPIO drivers on demand
pinctrl: Probe pinctrl devices on demand
regulator: core: Probe regulators on demand
drm: Probe panels on demand
drm/tegra: Probe dpaux devices on demand
i2c: core: Probe i2c adapters and devices on demand
pwm: Probe PWM chip devices on demand
backlight: Probe backlight devices on demand
usb: phy: Probe phy devices on demand
clk: Probe clk providers on demand
pinctrl: Probe pinctrl devices on demand
phy: core: Probe phy providers on demand
dma: of: Probe DMA controllers on demand
power-supply: Probe power supplies on demand
driver core: Allow deferring probes until late init
of/platform: Defer probes of registered devices

drivers/amba/bus.c | 88
++--
drivers/base/Kconfig | 18 ++
drivers/base/dd.c | 30 --
drivers/clk/clk.c | 3 +++
drivers/dma/of-dma.c | 3 +++
drivers/gpio/gpiolib-of.c | 5 +
drivers/gpu/drm/drm_panel.c | 3 +++
drivers/gpu/drm/tegra/dpaux.c | 3 +++
drivers/i2c/i2c-core.c | 4 
drivers/of/device.c | 61
+
drivers/of/platform.c | 30 ++
drivers/phy/phy-core.c | 3 +++
drivers/pinctrl/devicetree.c | 3 +++
drivers/power/power_supply_core.c | 3 +++
drivers/pwm/core.c | 3 +++
drivers/regulator/core.c | 2 ++
drivers/usb/phy/phy.c | 3 +++
drivers/video/backlight/backlight.c | 3 +++
include/linux/device.h | 4 +++-
include/linux/of.h | 1 +
include/linux/of_device.h | 3 +++
21 files changed, 219 insertions(+), 57 deletions(-)


Thanks,

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

Re: Crash in usb_f_mass_storage

[Paul Jones]

On 14 Oct 2015, at 14:57, Felipe Balbi  wrote:

> 
> Hi,
> 
> Paul Jones  writes:
>> Hi,
>> 
>> On 12 Oct 2015, at 14:16, Felipe Balbi  wrote:
>> 
>>> 
>>> Hi,
>>> 
>>> Paul Jones  writes:
 On 10 Oct 2015, at 16:32, Paul Jones  wrote:
 
> I came across the following kernel message on the latest 4.3-rc4 whilst 
> performance testing on a USB3380 device connected to a Mac (10.9.5):
> 
> [   51.613838] WARNING: CPU: 2 PID: 0 at 
> drivers/usb/gadget/function/f_mass_storage.c:346 fsg_setup+0x12a/0x170 
> [usb_f_mass_storage]()
> [   51.613838] Modules linked in: usb_f_mass_storage libcomposite 
> configfs drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 
> mac80211 snd_hda_codec_realtek snd_hda_codec_generic hid_generic 
> intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp 
> iwlwifi kvm_intel cfg80211 kvm drm_kms_helper drm snd_hda_intel 
> snd_hda_codec btusb btrtl crct10dif_pclmul crc32_pclmul btbcm 
> snd_hda_core ghash_clmulni_intel btintel bluetooth snd_hwdep e1000e 
> aesni_intel aes_x86_64 lrw snd_pcm gf128mul glue_helper ablk_helper 
> cryptd serio_raw alx mei_me lpc_ich usbhid mei snd_timer snd net2280 
> i2c_algo_bit ptp udc_core fb_sys_fops mdio syscopyarea pps_core 
> sysfillrect soundcore sysimgblt i2c_hid hid video dw_dmac sdhci_acpi 
> shpchp i2c_designware_platform dw_dmac_core spi_pxa2xx_platform sdhci 
> 8250_dw i2c_designware_core acpi_pad lp mac_hid parport
> [   51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW   
> 4.3.0-rc4+ #4
> [   51.613859] Hardware name: Gigabyte Technology Co., Ltd. 
> H97N-WIFI/H97N-WIFI, BIOS F7 04/21/2015
> [   51.613860]  a03e9e10 88021eb03d70 81393f5d 
> 
> [   51.613861]  88021eb03da8 81075856 880037be4400 
> 88020b3023c8
> [   51.613862]  880037be4400 ffa1  
> 88021eb03db8
> [   51.613863] Call Trace:
> [   51.613864][] dump_stack+0x44/0x57
> [   51.613867]  [] warn_slowpath_common+0x86/0xc0
> [   51.613868]  [] warn_slowpath_null+0x1a/0x20
> [   51.613870]  [] fsg_setup+0x12a/0x170 
> [usb_f_mass_storage]
> [   51.613872]  [] composite_setup+0x173/0x16b0 
> [libcomposite]
> [   51.613873]  [] ? ktime_get+0x3a/0x90
> [   51.613874]  [] ? lapic_next_deadline+0x29/0x30
> [   51.613875]  [] ? ktime_get+0x3a/0x90
> [   51.613877]  [] net2280_irq+0xba2/0x10db [net2280]
> [   51.613879]  [] handle_irq_event_percpu+0x39/0x180
> [   51.613880]  [] handle_irq_event+0x45/0x70
> [   51.613881]  [] handle_edge_irq+0x99/0x150
> [   51.613883]  [] handle_irq+0x1d/0x30
> [   51.613883]  [] do_IRQ+0x4d/0xd0
> [   51.613885]  [] common_interrupt+0x87/0x87
> [   51.613885][] ? 
> cpuidle_enter_state+0xb8/0x220
> [   51.613888]  [] cpuidle_enter+0x17/0x20
> [   51.613889]  [] call_cpuidle+0x32/0x60
> [   51.613890]  [] ? cpuidle_select+0x13/0x20
> [   51.613891]  [] cpu_startup_entry+0x21c/0x2e0
> [   51.613891]  [] start_secondary+0x104/0x130
> [   51.613892] ---[ end trace bda1c37ade46c57d ]—
> 
> I can also reliable reproduce this by connecting the USB3380 to a USB 
> port on the same Linux machine.
> In that case I also see an error:
> net2280 : net2280_enable: error=-22
> 
> Perhaps unrelated, but there is also a message:
> configfs-gadget gadget: common->fsg is NULL in fsg_setup at 511
 The same crash happens in 4.2 as well but not in 4.1.
>>> 
>>> care to run a git bisect and find offending commit ?
>> Unfortunately I encountered many kernels that hung my machine during a
>> git bisect, so I had to git bisect skip many a time.
>> 
>> Here’s the git bisect result (between v4.1 and v4.2):
>> There are only 'skip'ped commits left to test.
>> The first bad commit could be any of:
>> 4117a60c8e4c8d5f9fc05578e359d09d0fdf9d07
>> 4ae82e5d23961515796d76850499db3866c5e73b
>> We cannot bisect more!
>> 
>> Oddly neither of those commits seem very relevant to the problem. Not
>> sure if this helps…
> 
> Likely a problematic bisect, unfortunately this isn't helpful at all.
I’ll try again and see if I can find a way to avoid the hangs that I had before.

Paul.

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

Re: Crash in usb_f_mass_storage

[Alan Stern]

On Wed, 14 Oct 2015, Paul Jones wrote:

> On 12 Oct 2015, at 14:16, Felipe Balbi  wrote:
> 
> > 
> > Hi,
> > 
> > Paul Jones  writes:
> >> On 10 Oct 2015, at 16:32, Paul Jones  wrote:
> >> 
> >>> I came across the following kernel message on the latest 4.3-rc4 whilst 
> >>> performance testing on a USB3380 device connected to a Mac (10.9.5):
> >>> 
> >>> [   51.613838] WARNING: CPU: 2 PID: 0 at 
> >>> drivers/usb/gadget/function/f_mass_storage.c:346 fsg_setup+0x12a/0x170 
> >>> [usb_f_mass_storage]()
> >>> [   51.613838] Modules linked in: usb_f_mass_storage libcomposite 
> >>> configfs drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 
> >>> mac80211 snd_hda_codec_realtek snd_hda_codec_generic hid_generic 
> >>> intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp 
> >>> iwlwifi kvm_intel cfg80211 kvm drm_kms_helper drm snd_hda_intel 
> >>> snd_hda_codec btusb btrtl crct10dif_pclmul crc32_pclmul btbcm 
> >>> snd_hda_core ghash_clmulni_intel btintel bluetooth snd_hwdep e1000e 
> >>> aesni_intel aes_x86_64 lrw snd_pcm gf128mul glue_helper ablk_helper 
> >>> cryptd serio_raw alx mei_me lpc_ich usbhid mei snd_timer snd net2280 
> >>> i2c_algo_bit ptp udc_core fb_sys_fops mdio syscopyarea pps_core 
> >>> sysfillrect soundcore sysimgblt i2c_hid hid video dw_dmac sdhci_acpi 
> >>> shpchp i2c_designware_platform dw_dmac_core spi_pxa2xx_platform sdhci 
> >>> 8250_dw i2c_designware_core acpi_pad lp mac_hid parport
> >>> [   51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW   
> >>> 4.3.0-rc4+ #4
> >>> [   51.613859] Hardware name: Gigabyte Technology Co., Ltd. 
> >>> H97N-WIFI/H97N-WIFI, BIOS F7 04/21/2015
> >>> [   51.613860]  a03e9e10 88021eb03d70 81393f5d 
> >>> 
> >>> [   51.613861]  88021eb03da8 81075856 880037be4400 
> >>> 88020b3023c8
> >>> [   51.613862]  880037be4400 ffa1  
> >>> 88021eb03db8
> >>> [   51.613863] Call Trace:
> >>> [   51.613864][] dump_stack+0x44/0x57
> >>> [   51.613867]  [] warn_slowpath_common+0x86/0xc0
> >>> [   51.613868]  [] warn_slowpath_null+0x1a/0x20
> >>> [   51.613870]  [] fsg_setup+0x12a/0x170 
> >>> [usb_f_mass_storage]
> >>> [   51.613872]  [] composite_setup+0x173/0x16b0 
> >>> [libcomposite]
> >>> [   51.613873]  [] ? ktime_get+0x3a/0x90
> >>> [   51.613874]  [] ? lapic_next_deadline+0x29/0x30
> >>> [   51.613875]  [] ? ktime_get+0x3a/0x90
> >>> [   51.613877]  [] net2280_irq+0xba2/0x10db [net2280]
> >>> [   51.613879]  [] handle_irq_event_percpu+0x39/0x180
> >>> [   51.613880]  [] handle_irq_event+0x45/0x70
> >>> [   51.613881]  [] handle_edge_irq+0x99/0x150
> >>> [   51.613883]  [] handle_irq+0x1d/0x30
> >>> [   51.613883]  [] do_IRQ+0x4d/0xd0
> >>> [   51.613885]  [] common_interrupt+0x87/0x87
> >>> [   51.613885][] ? 
> >>> cpuidle_enter_state+0xb8/0x220
> >>> [   51.613888]  [] cpuidle_enter+0x17/0x20
> >>> [   51.613889]  [] call_cpuidle+0x32/0x60
> >>> [   51.613890]  [] ? cpuidle_select+0x13/0x20
> >>> [   51.613891]  [] cpu_startup_entry+0x21c/0x2e0
> >>> [   51.613891]  [] start_secondary+0x104/0x130
> >>> [   51.613892] ---[ end trace bda1c37ade46c57d ]�
> >>> 
> >>> I can also reliable reproduce this by connecting the USB3380 to a USB 
> >>> port on the same Linux machine.
> >>> In that case I also see an error:
> >>> net2280 : net2280_enable: error=-22
> >>> 
> >>> Perhaps unrelated, but there is also a message:
> >>> configfs-gadget gadget: common->fsg is NULL in fsg_setup at 511
> >> The same crash happens in 4.2 as well but not in 4.1.
> > 
> > care to run a git bisect and find offending commit ?
> Unfortunately I encountered many kernels that hung my machine during a git 
> bisect, so I had to git bisect skip many a time.
> Here�s the git bisect result (between v4.1 and v4.2):
>   There are only 'skip'ped commits left to test.
>   The first bad commit could be any of:
>   4117a60c8e4c8d5f9fc05578e359d09d0fdf9d07
>   4ae82e5d23961515796d76850499db3866c5e73b
>   We cannot bisect more!
> 
> Oddly neither of those commits seem very relevant to the problem. Not sure if 
> this helps�

Mian Yousaf Kaukab added a bunch of changes to the net2280 driver 
between 4.1 and 4.2.  Do:

git log v4.1..v4.2 -- drivers/usb/gadget/udc/net2280.c

One of them may be responsible.

Alan Stern

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

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Alan Stern]

On Wed, 14 Oct 2015, Felipe Balbi wrote:

> Hi,
> 
> Alan Stern  writes:
> > On Wed, 14 Oct 2015, Jassi Brar wrote:
> >
> >> BTW, should the gadget stack ever queue a Non-ZLP as reply to some
> >> setup request that has USB_DIR_IN not set?
> >
> > Yes.  If USB_DIR_IN is not set then the control transfer is OUT, so the
> > gadget needs to queue a request to receive some data from the host.  
> > That request will obviously need to be a non-ZLP.  In fact, it's hard
> > to think of a situation where a gadget would ever want to submit a
> > zero-length OUT request.  Isn't the UDC driver supposed to handle the
> > status stage of a control-IN transfer automatically?
> 
> yes and no. :-) If USB_GADGET_DELAYED_STATUS is returned, we need to
> wait for the gadget driver to queue a request.

USB_GADGET_DELAYED_STATUS is used with control-OUT transfers that don't
have a data stage.  I was speaking of control-IN, because the status
stage for a control-IN transfer is a zero-length OUT transaction.

But speaking of DELAYED_STATUS, there is a long-standing weakness in
the Gadget API.  When a gadget receives data from the host in a
control-OUT transfer, the API doesn't provide any way for the gadget to
delay the status stage until its processing is complete.  
USB_GADGET_DELAYED_STATUS only works as a return value from the setup
routine; once the data stage has started there's no way to delay the
status stage.  It's an unfortunate oversight in the original design.

Alan Stern

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

Re: Crash in usb_f_mass_storage

[Felipe Balbi]

Hi,

Paul Jones  writes:
> On 14 Oct 2015, at 14:57, Felipe Balbi  wrote:
>
>> 
>> Hi,
>> 
>> Paul Jones  writes:
>>> Hi,
>>> 
>>> On 12 Oct 2015, at 14:16, Felipe Balbi  wrote:
>>> 
 
 Hi,
 
 Paul Jones  writes:
> On 10 Oct 2015, at 16:32, Paul Jones  wrote:
> 
>> I came across the following kernel message on the latest 4.3-rc4 whilst 
>> performance testing on a USB3380 device connected to a Mac (10.9.5):
>> 
>> [   51.613838] WARNING: CPU: 2 PID: 0 at 
>> drivers/usb/gadget/function/f_mass_storage.c:346 fsg_setup+0x12a/0x170 
>> [usb_f_mass_storage]()
>> [   51.613838] Modules linked in: usb_f_mass_storage libcomposite 
>> configfs drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 
>> mac80211 snd_hda_codec_realtek snd_hda_codec_generic hid_generic 
>> intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp 
>> iwlwifi kvm_intel cfg80211 kvm drm_kms_helper drm snd_hda_intel 
>> snd_hda_codec btusb btrtl crct10dif_pclmul crc32_pclmul btbcm 
>> snd_hda_core ghash_clmulni_intel btintel bluetooth snd_hwdep e1000e 
>> aesni_intel aes_x86_64 lrw snd_pcm gf128mul glue_helper ablk_helper 
>> cryptd serio_raw alx mei_me lpc_ich usbhid mei snd_timer snd net2280 
>> i2c_algo_bit ptp udc_core fb_sys_fops mdio syscopyarea pps_core 
>> sysfillrect soundcore sysimgblt i2c_hid hid video dw_dmac sdhci_acpi 
>> shpchp i2c_designware_platform dw_dmac_core spi_pxa2xx_platform sdhci 
>> 8250_dw i2c_designware_core acpi_pad lp mac_hid parport
>> [   51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW   
>> 4.3.0-rc4+ #4
>> [   51.613859] Hardware name: Gigabyte Technology Co., Ltd. 
>> H97N-WIFI/H97N-WIFI, BIOS F7 04/21/2015
>> [   51.613860]  a03e9e10 88021eb03d70 81393f5d 
>> 
>> [   51.613861]  88021eb03da8 81075856 880037be4400 
>> 88020b3023c8
>> [   51.613862]  880037be4400 ffa1  
>> 88021eb03db8
>> [   51.613863] Call Trace:
>> [   51.613864][] dump_stack+0x44/0x57
>> [   51.613867]  [] warn_slowpath_common+0x86/0xc0
>> [   51.613868]  [] warn_slowpath_null+0x1a/0x20
>> [   51.613870]  [] fsg_setup+0x12a/0x170 
>> [usb_f_mass_storage]
>> [   51.613872]  [] composite_setup+0x173/0x16b0 
>> [libcomposite]
>> [   51.613873]  [] ? ktime_get+0x3a/0x90
>> [   51.613874]  [] ? lapic_next_deadline+0x29/0x30
>> [   51.613875]  [] ? ktime_get+0x3a/0x90
>> [   51.613877]  [] net2280_irq+0xba2/0x10db [net2280]
>> [   51.613879]  [] handle_irq_event_percpu+0x39/0x180
>> [   51.613880]  [] handle_irq_event+0x45/0x70
>> [   51.613881]  [] handle_edge_irq+0x99/0x150
>> [   51.613883]  [] handle_irq+0x1d/0x30
>> [   51.613883]  [] do_IRQ+0x4d/0xd0
>> [   51.613885]  [] common_interrupt+0x87/0x87
>> [   51.613885][] ? 
>> cpuidle_enter_state+0xb8/0x220
>> [   51.613888]  [] cpuidle_enter+0x17/0x20
>> [   51.613889]  [] call_cpuidle+0x32/0x60
>> [   51.613890]  [] ? cpuidle_select+0x13/0x20
>> [   51.613891]  [] cpu_startup_entry+0x21c/0x2e0
>> [   51.613891]  [] start_secondary+0x104/0x130
>> [   51.613892] ---[ end trace bda1c37ade46c57d ]—
>> 
>> I can also reliable reproduce this by connecting the USB3380 to a USB 
>> port on the same Linux machine.
>> In that case I also see an error:
>> net2280 : net2280_enable: error=-22
>> 
>> Perhaps unrelated, but there is also a message:
>> configfs-gadget gadget: common->fsg is NULL in fsg_setup at 511
> The same crash happens in 4.2 as well but not in 4.1.
 
 care to run a git bisect and find offending commit ?
>>> Unfortunately I encountered many kernels that hung my machine during a
>>> git bisect, so I had to git bisect skip many a time.
>>> 
>>> Here’s the git bisect result (between v4.1 and v4.2):
>>> There are only 'skip'ped commits left to test.
>>> The first bad commit could be any of:
>>> 4117a60c8e4c8d5f9fc05578e359d09d0fdf9d07
>>> 4ae82e5d23961515796d76850499db3866c5e73b
>>> We cannot bisect more!
>>> 
>>> Oddly neither of those commits seem very relevant to the problem. Not
>>> sure if this helps…
>> 
>> Likely a problematic bisect, unfortunately this isn't helpful at all.
> I’ll try again and see if I can find a way to avoid the hangs that I had 
> before.

okay, thanks.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: dwc2: host: Fix use after free w/ simultaneous irqs

[kbuild test robot]

Hi Douglas,

[auto build test WARNING on balbi-usb/next -- if it's inappropriate base, 
please suggest rules for selecting the more suitable base]

url:
https://github.com/0day-ci/linux/commits/Douglas-Anderson/usb-dwc2-host-Fix-use-after-free-w-simultaneous-irqs/20151015-024824
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/usb/dwc2/hcd_intr.c:1959:17: sparse: symbol 
>> 'dwc2_check_qtd_still_ok' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH] usb: dwc2: host: dwc2_check_qtd_still_ok() can be static

[kbuild test robot]

Signed-off-by: Fengguang Wu 
---
 hcd_intr.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index 57e71f9d..b31a1fb 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -1956,7 +1956,7 @@ static void dwc2_hc_chhltd_intr(struct dwc2_hsotg *hsotg,
  * the qtd from the top of the list, this will return NULL.  Otherwise
  * it will be passed back qtd.
  */
-struct dwc2_qtd *dwc2_check_qtd_still_ok(struct dwc2_qtd *qtd,
+static struct dwc2_qtd *dwc2_check_qtd_still_ok(struct dwc2_qtd *qtd,
struct list_head *qtd_list)
 {
struct dwc2_qtd *cur_head;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] usb: gadget: loopback: Fix looping back logic implementation

[Krzysztof Opasiak]

Since commit e0857ce58e8658657f5f12fe25272b93cfeb16aa
("usb: gadget: loopback: don't queue requests to bogus endpoints")

Loopback function is not realy working as that commit removed
all looping back logic. After that commit ep-out works like
/dev/null and ep-in works like /dev/zero.

This commit fix this issue by allocating set of out requests
and set of in requests but each out req shares buffer with
one in req:

out_req->buf ---> buf <--- in_req.buf
out_req->context <---> in_req.context

The completion routine simply  enqueue the suitable req in
an oposite direction.

Cc:  # 3.18+
Fixes: e0857ce58e8658657f5f12fe25272b93cfeb16aa
("usb: gadget: loopback: don't queue requests to bogus endpoints")

Signed-off-by: Krzysztof Opasiak 
---
Changes since v3:
- rebase onto balbi/fixes

Changes since v2:
- fix requests context assignment

Changes since v1:
- add missing usb_ep_free_request() in complete() callback
---
 drivers/usb/gadget/function/f_loopback.c |  131 +-
 1 file changed, 92 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 6e2fe63..f84de9b 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -245,22 +245,38 @@ static void loopback_complete(struct usb_ep *ep, struct 
usb_request *req)
int status = req->status;
 
switch (status) {
-
case 0: /* normal completion? */
if (ep == loop->out_ep) {
-   req->zero = (req->actual < req->length);
-   req->length = req->actual;
+   /*
+* We received some data from the host so let's
+* queue it so host can read the from our in ep
+*/
+   struct usb_request *in_req = req->context;
+
+   in_req->zero = (req->actual < req->length);
+   in_req->length = req->actual;
+   ep = loop->in_ep;
+   req = in_req;
+   } else {
+   /*
+* We have just looped back a bunch of data
+* to host. Now let's wait for some more data.
+*/
+   req = req->context;
+   ep = loop->out_ep;
}
 
-   /* queue the buffer for some later OUT packet */
-   req->length = buflen;
+   /* queue the buffer back to host or for next bunch of data */
status = usb_ep_queue(ep, req, GFP_ATOMIC);
-   if (status == 0)
+   if (status == 0) {
return;
+   } else {
+   ERROR(cdev, "Unable to loop back buffer to %s: %d\n",
+ ep->name, status);
+   goto free_req;
+   }
 
/* "should never get here" */
-   /* FALLTHROUGH */
-
default:
ERROR(cdev, "%s loop complete --> %d, %d/%d\n", ep->name,
status, req->actual, req->length);
@@ -274,6 +290,10 @@ static void loopback_complete(struct usb_ep *ep, struct 
usb_request *req)
case -ECONNABORTED: /* hardware forced ep reset */
case -ECONNRESET:   /* request dequeued */
case -ESHUTDOWN:/* disconnect from host */
+free_req:
+   usb_ep_free_request(ep == loop->in_ep ?
+   loop->out_ep : loop->in_ep,
+   req->context);
free_ep_req(ep, req);
return;
}
@@ -293,50 +313,72 @@ static inline struct usb_request *lb_alloc_ep_req(struct 
usb_ep *ep, int len)
return alloc_ep_req(ep, len, buflen);
 }
 
-static int enable_endpoint(struct usb_composite_dev *cdev, struct f_loopback 
*loop,
-   struct usb_ep *ep)
+static int alloc_requests(struct usb_composite_dev *cdev,
+ struct f_loopback *loop)
 {
-   struct usb_request  *req;
-   unsignedi;
-   int result;
-
-   /*
-* one endpoint writes data back IN to the host while another endpoint
-* just reads OUT packets
-*/
-   result = config_ep_by_speed(cdev->gadget, &(loop->function), ep);
-   if (result)
-   goto fail0;
-   result = usb_ep_enable(ep);
-   if (result < 0)
-   goto fail0;
-   ep->driver_data = loop;
+   struct usb_request *in_req, *out_req;
+   int i;
+   int result = 0;
 
/*
 * allocate a bunch of read buffers and queue them all at once.
-* we buffer at most 

Re: Crash in usb_f_mass_storage

[Felipe Balbi]

Hi,

Paul Jones  writes:
> Hi,
>
> On 12 Oct 2015, at 14:16, Felipe Balbi  wrote:
>
>> 
>> Hi,
>> 
>> Paul Jones  writes:
>>> On 10 Oct 2015, at 16:32, Paul Jones  wrote:
>>> 
 I came across the following kernel message on the latest 4.3-rc4 whilst 
 performance testing on a USB3380 device connected to a Mac (10.9.5):
 
 [   51.613838] WARNING: CPU: 2 PID: 0 at 
 drivers/usb/gadget/function/f_mass_storage.c:346 fsg_setup+0x12a/0x170 
 [usb_f_mass_storage]()
 [   51.613838] Modules linked in: usb_f_mass_storage libcomposite configfs 
 drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 mac80211 
 snd_hda_codec_realtek snd_hda_codec_generic hid_generic intel_rapl 
 iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp iwlwifi kvm_intel 
 cfg80211 kvm drm_kms_helper drm snd_hda_intel snd_hda_codec btusb btrtl 
 crct10dif_pclmul crc32_pclmul btbcm snd_hda_core ghash_clmulni_intel 
 btintel bluetooth snd_hwdep e1000e aesni_intel aes_x86_64 lrw snd_pcm 
 gf128mul glue_helper ablk_helper cryptd serio_raw alx mei_me lpc_ich 
 usbhid mei snd_timer snd net2280 i2c_algo_bit ptp udc_core fb_sys_fops 
 mdio syscopyarea pps_core sysfillrect soundcore sysimgblt i2c_hid hid 
 video dw_dmac sdhci_acpi shpchp i2c_designware_platform dw_dmac_core 
 spi_pxa2xx_platform sdhci 8250_dw i2c_designware_core acpi_pad lp mac_hid 
 parport
 [   51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW   
 4.3.0-rc4+ #4
 [   51.613859] Hardware name: Gigabyte Technology Co., Ltd. 
 H97N-WIFI/H97N-WIFI, BIOS F7 04/21/2015
 [   51.613860]  a03e9e10 88021eb03d70 81393f5d 
 
 [   51.613861]  88021eb03da8 81075856 880037be4400 
 88020b3023c8
 [   51.613862]  880037be4400 ffa1  
 88021eb03db8
 [   51.613863] Call Trace:
 [   51.613864][] dump_stack+0x44/0x57
 [   51.613867]  [] warn_slowpath_common+0x86/0xc0
 [   51.613868]  [] warn_slowpath_null+0x1a/0x20
 [   51.613870]  [] fsg_setup+0x12a/0x170 
 [usb_f_mass_storage]
 [   51.613872]  [] composite_setup+0x173/0x16b0 
 [libcomposite]
 [   51.613873]  [] ? ktime_get+0x3a/0x90
 [   51.613874]  [] ? lapic_next_deadline+0x29/0x30
 [   51.613875]  [] ? ktime_get+0x3a/0x90
 [   51.613877]  [] net2280_irq+0xba2/0x10db [net2280]
 [   51.613879]  [] handle_irq_event_percpu+0x39/0x180
 [   51.613880]  [] handle_irq_event+0x45/0x70
 [   51.613881]  [] handle_edge_irq+0x99/0x150
 [   51.613883]  [] handle_irq+0x1d/0x30
 [   51.613883]  [] do_IRQ+0x4d/0xd0
 [   51.613885]  [] common_interrupt+0x87/0x87
 [   51.613885][] ? 
 cpuidle_enter_state+0xb8/0x220
 [   51.613888]  [] cpuidle_enter+0x17/0x20
 [   51.613889]  [] call_cpuidle+0x32/0x60
 [   51.613890]  [] ? cpuidle_select+0x13/0x20
 [   51.613891]  [] cpu_startup_entry+0x21c/0x2e0
 [   51.613891]  [] start_secondary+0x104/0x130
 [   51.613892] ---[ end trace bda1c37ade46c57d ]—
 
 I can also reliable reproduce this by connecting the USB3380 to a USB port 
 on the same Linux machine.
 In that case I also see an error:
 net2280 : net2280_enable: error=-22
 
 Perhaps unrelated, but there is also a message:
 configfs-gadget gadget: common->fsg is NULL in fsg_setup at 511
>>> The same crash happens in 4.2 as well but not in 4.1.
>> 
>> care to run a git bisect and find offending commit ?
> Unfortunately I encountered many kernels that hung my machine during a
> git bisect, so I had to git bisect skip many a time.
>
> Here’s the git bisect result (between v4.1 and v4.2):
> There are only 'skip'ped commits left to test.
> The first bad commit could be any of:
> 4117a60c8e4c8d5f9fc05578e359d09d0fdf9d07
> 4ae82e5d23961515796d76850499db3866c5e73b
> We cannot bisect more!
>
> Oddly neither of those commits seem very relevant to the problem. Not
> sure if this helps…

Likely a problematic bisect, unfortunately this isn't helpful at all.

-- 
balbi


signature.asc
Description: PGP signature

Re: Crash in usb_f_mass_storage

[Paul Jones]

On 12 Oct 2015, at 14:16, Felipe Balbi  wrote:

> 
> Hi,
> 
> Paul Jones  writes:
>> On 10 Oct 2015, at 16:32, Paul Jones  wrote:
>> 
>>> I came across the following kernel message on the latest 4.3-rc4 whilst 
>>> performance testing on a USB3380 device connected to a Mac (10.9.5):
>>> 
>>> [   51.613838] WARNING: CPU: 2 PID: 0 at 
>>> drivers/usb/gadget/function/f_mass_storage.c:346 fsg_setup+0x12a/0x170 
>>> [usb_f_mass_storage]()
>>> [   51.613838] Modules linked in: usb_f_mass_storage libcomposite configfs 
>>> drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 mac80211 
>>> snd_hda_codec_realtek snd_hda_codec_generic hid_generic intel_rapl iosf_mbi 
>>> x86_pkg_temp_thermal intel_powerclamp coretemp iwlwifi kvm_intel cfg80211 
>>> kvm drm_kms_helper drm snd_hda_intel snd_hda_codec btusb btrtl 
>>> crct10dif_pclmul crc32_pclmul btbcm snd_hda_core ghash_clmulni_intel 
>>> btintel bluetooth snd_hwdep e1000e aesni_intel aes_x86_64 lrw snd_pcm 
>>> gf128mul glue_helper ablk_helper cryptd serio_raw alx mei_me lpc_ich usbhid 
>>> mei snd_timer snd net2280 i2c_algo_bit ptp udc_core fb_sys_fops mdio 
>>> syscopyarea pps_core sysfillrect soundcore sysimgblt i2c_hid hid video 
>>> dw_dmac sdhci_acpi shpchp i2c_designware_platform dw_dmac_core 
>>> spi_pxa2xx_platform sdhci 8250_dw i2c_designware_core acpi_pad lp mac_hid 
>>> parport
>>> [   51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW   
>>> 4.3.0-rc4+ #4
>>> [   51.613859] Hardware name: Gigabyte Technology Co., Ltd. 
>>> H97N-WIFI/H97N-WIFI, BIOS F7 04/21/2015
>>> [   51.613860]  a03e9e10 88021eb03d70 81393f5d 
>>> 
>>> [   51.613861]  88021eb03da8 81075856 880037be4400 
>>> 88020b3023c8
>>> [   51.613862]  880037be4400 ffa1  
>>> 88021eb03db8
>>> [   51.613863] Call Trace:
>>> [   51.613864][] dump_stack+0x44/0x57
>>> [   51.613867]  [] warn_slowpath_common+0x86/0xc0
>>> [   51.613868]  [] warn_slowpath_null+0x1a/0x20
>>> [   51.613870]  [] fsg_setup+0x12a/0x170 
>>> [usb_f_mass_storage]
>>> [   51.613872]  [] composite_setup+0x173/0x16b0 
>>> [libcomposite]
>>> [   51.613873]  [] ? ktime_get+0x3a/0x90
>>> [   51.613874]  [] ? lapic_next_deadline+0x29/0x30
>>> [   51.613875]  [] ? ktime_get+0x3a/0x90
>>> [   51.613877]  [] net2280_irq+0xba2/0x10db [net2280]
>>> [   51.613879]  [] handle_irq_event_percpu+0x39/0x180
>>> [   51.613880]  [] handle_irq_event+0x45/0x70
>>> [   51.613881]  [] handle_edge_irq+0x99/0x150
>>> [   51.613883]  [] handle_irq+0x1d/0x30
>>> [   51.613883]  [] do_IRQ+0x4d/0xd0
>>> [   51.613885]  [] common_interrupt+0x87/0x87
>>> [   51.613885][] ? cpuidle_enter_state+0xb8/0x220
>>> [   51.613888]  [] cpuidle_enter+0x17/0x20
>>> [   51.613889]  [] call_cpuidle+0x32/0x60
>>> [   51.613890]  [] ? cpuidle_select+0x13/0x20
>>> [   51.613891]  [] cpu_startup_entry+0x21c/0x2e0
>>> [   51.613891]  [] start_secondary+0x104/0x130
>>> [   51.613892] ---[ end trace bda1c37ade46c57d ]—
>>> 
>>> I can also reliable reproduce this by connecting the USB3380 to a USB port 
>>> on the same Linux machine.
>>> In that case I also see an error:
>>> net2280 : net2280_enable: error=-22
>>> 
>>> Perhaps unrelated, but there is also a message:
>>> configfs-gadget gadget: common->fsg is NULL in fsg_setup at 511
>> The same crash happens in 4.2 as well but not in 4.1.
> 
> care to run a git bisect and find offending commit ?
Unfortunately I encountered many kernels that hung my machine during a git 
bisect, so I had to git bisect skip many a time.
Here’s the git bisect result (between v4.1 and v4.2):
  There are only 'skip'ped commits left to test.
  The first bad commit could be any of:
  4117a60c8e4c8d5f9fc05578e359d09d0fdf9d07
  4ae82e5d23961515796d76850499db3866c5e73b
  We cannot bisect more!

Oddly neither of those commits seem very relevant to the problem. Not sure if 
this helps…

Paul.

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

Re: Crash in usb_f_mass_storage

[Paul Jones]

On 14 Oct 2015, at 15:37, Alan Stern  wrote:

> On Wed, 14 Oct 2015, Paul Jones wrote:
> 
>> On 12 Oct 2015, at 14:16, Felipe Balbi  wrote:
>> 
>>> 
>>> Hi,
>>> 
>>> Paul Jones  writes:
 On 10 Oct 2015, at 16:32, Paul Jones  wrote:
 
> I came across the following kernel message on the latest 4.3-rc4 whilst 
> performance testing on a USB3380 device connected to a Mac (10.9.5):
> 
> [   51.613838] WARNING: CPU: 2 PID: 0 at 
> drivers/usb/gadget/function/f_mass_storage.c:346 fsg_setup+0x12a/0x170 
> [usb_f_mass_storage]()
> [   51.613838] Modules linked in: usb_f_mass_storage libcomposite 
> configfs drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 
> mac80211 snd_hda_codec_realtek snd_hda_codec_generic hid_generic 
> intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp 
> iwlwifi kvm_intel cfg80211 kvm drm_kms_helper drm snd_hda_intel 
> snd_hda_codec btusb btrtl crct10dif_pclmul crc32_pclmul btbcm 
> snd_hda_core ghash_clmulni_intel btintel bluetooth snd_hwdep e1000e 
> aesni_intel aes_x86_64 lrw snd_pcm gf128mul glue_helper ablk_helper 
> cryptd serio_raw alx mei_me lpc_ich usbhid mei snd_timer snd net2280 
> i2c_algo_bit ptp udc_core fb_sys_fops mdio syscopyarea pps_core 
> sysfillrect soundcore sysimgblt i2c_hid hid video dw_dmac sdhci_acpi 
> shpchp i2c_designware_platform dw_dmac_core spi_pxa2xx_platform sdhci 
> 8250_dw i2c_designware_core acpi_pad lp mac_hid parport
> [   51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW   
> 4.3.0-rc4+ #4
> [   51.613859] Hardware name: Gigabyte Technology Co., Ltd. 
> H97N-WIFI/H97N-WIFI, BIOS F7 04/21/2015
> [   51.613860]  a03e9e10 88021eb03d70 81393f5d 
> 
> [   51.613861]  88021eb03da8 81075856 880037be4400 
> 88020b3023c8
> [   51.613862]  880037be4400 ffa1  
> 88021eb03db8
> [   51.613863] Call Trace:
> [   51.613864][] dump_stack+0x44/0x57
> [   51.613867]  [] warn_slowpath_common+0x86/0xc0
> [   51.613868]  [] warn_slowpath_null+0x1a/0x20
> [   51.613870]  [] fsg_setup+0x12a/0x170 
> [usb_f_mass_storage]
> [   51.613872]  [] composite_setup+0x173/0x16b0 
> [libcomposite]
> [   51.613873]  [] ? ktime_get+0x3a/0x90
> [   51.613874]  [] ? lapic_next_deadline+0x29/0x30
> [   51.613875]  [] ? ktime_get+0x3a/0x90
> [   51.613877]  [] net2280_irq+0xba2/0x10db [net2280]
> [   51.613879]  [] handle_irq_event_percpu+0x39/0x180
> [   51.613880]  [] handle_irq_event+0x45/0x70
> [   51.613881]  [] handle_edge_irq+0x99/0x150
> [   51.613883]  [] handle_irq+0x1d/0x30
> [   51.613883]  [] do_IRQ+0x4d/0xd0
> [   51.613885]  [] common_interrupt+0x87/0x87
> [   51.613885][] ? 
> cpuidle_enter_state+0xb8/0x220
> [   51.613888]  [] cpuidle_enter+0x17/0x20
> [   51.613889]  [] call_cpuidle+0x32/0x60
> [   51.613890]  [] ? cpuidle_select+0x13/0x20
> [   51.613891]  [] cpu_startup_entry+0x21c/0x2e0
> [   51.613891]  [] start_secondary+0x104/0x130
> [   51.613892] ---[ end trace bda1c37ade46c57d ]�
> 
> I can also reliable reproduce this by connecting the USB3380 to a USB 
> port on the same Linux machine.
> In that case I also see an error:
> net2280 : net2280_enable: error=-22
> 
> Perhaps unrelated, but there is also a message:
> configfs-gadget gadget: common->fsg is NULL in fsg_setup at 511
 The same crash happens in 4.2 as well but not in 4.1.
>>> 
>>> care to run a git bisect and find offending commit ?
>> Unfortunately I encountered many kernels that hung my machine during a git 
>> bisect, so I had to git bisect skip many a time.
>> Here�s the git bisect result (between v4.1 and v4.2):
>>  There are only 'skip'ped commits left to test.
>>  The first bad commit could be any of:
>>  4117a60c8e4c8d5f9fc05578e359d09d0fdf9d07
>>  4ae82e5d23961515796d76850499db3866c5e73b
>>  We cannot bisect more!
>> 
>> Oddly neither of those commits seem very relevant to the problem. Not sure 
>> if this helps�
> 
> Mian Yousaf Kaukab added a bunch of changes to the net2280 driver 
> between 4.1 and 4.2.  Do:
> 
>   git log v4.1..v4.2 -- drivers/usb/gadget/udc/net2280.c
> 
> One of them may be responsible.
I’ll try a manual bisect based on that.

Paul.

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

Re: Crash in usb_f_mass_storage

[Paul Jones]

On 14 Oct 2015, at 15:37, Alan Stern  wrote:

> On Wed, 14 Oct 2015, Paul Jones wrote:
> 
>> On 12 Oct 2015, at 14:16, Felipe Balbi  wrote:
>> 
>>> 
>>> Hi,
>>> 
>>> Paul Jones  writes:
 On 10 Oct 2015, at 16:32, Paul Jones  wrote:
 
> I came across the following kernel message on the latest 4.3-rc4 whilst 
> performance testing on a USB3380 device connected to a Mac (10.9.5):
> 
> [   51.613838] WARNING: CPU: 2 PID: 0 at 
> drivers/usb/gadget/function/f_mass_storage.c:346 fsg_setup+0x12a/0x170 
> [usb_f_mass_storage]()
> [   51.613838] Modules linked in: usb_f_mass_storage libcomposite 
> configfs drbg ansi_cprng ctr ccm arc4 snd_hda_codec_hdmi iwlmvm i915 
> mac80211 snd_hda_codec_realtek snd_hda_codec_generic hid_generic 
> intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp 
> iwlwifi kvm_intel cfg80211 kvm drm_kms_helper drm snd_hda_intel 
> snd_hda_codec btusb btrtl crct10dif_pclmul crc32_pclmul btbcm 
> snd_hda_core ghash_clmulni_intel btintel bluetooth snd_hwdep e1000e 
> aesni_intel aes_x86_64 lrw snd_pcm gf128mul glue_helper ablk_helper 
> cryptd serio_raw alx mei_me lpc_ich usbhid mei snd_timer snd net2280 
> i2c_algo_bit ptp udc_core fb_sys_fops mdio syscopyarea pps_core 
> sysfillrect soundcore sysimgblt i2c_hid hid video dw_dmac sdhci_acpi 
> shpchp i2c_designware_platform dw_dmac_core spi_pxa2xx_platform sdhci 
> 8250_dw i2c_designware_core acpi_pad lp mac_hid parport
> [   51.613858] CPU: 2 PID: 0 Comm: swapper/2 Tainted: GW   
> 4.3.0-rc4+ #4
> [   51.613859] Hardware name: Gigabyte Technology Co., Ltd. 
> H97N-WIFI/H97N-WIFI, BIOS F7 04/21/2015
> [   51.613860]  a03e9e10 88021eb03d70 81393f5d 
> 
> [   51.613861]  88021eb03da8 81075856 880037be4400 
> 88020b3023c8
> [   51.613862]  880037be4400 ffa1  
> 88021eb03db8
> [   51.613863] Call Trace:
> [   51.613864][] dump_stack+0x44/0x57
> [   51.613867]  [] warn_slowpath_common+0x86/0xc0
> [   51.613868]  [] warn_slowpath_null+0x1a/0x20
> [   51.613870]  [] fsg_setup+0x12a/0x170 
> [usb_f_mass_storage]
> [   51.613872]  [] composite_setup+0x173/0x16b0 
> [libcomposite]
> [   51.613873]  [] ? ktime_get+0x3a/0x90
> [   51.613874]  [] ? lapic_next_deadline+0x29/0x30
> [   51.613875]  [] ? ktime_get+0x3a/0x90
> [   51.613877]  [] net2280_irq+0xba2/0x10db [net2280]
> [   51.613879]  [] handle_irq_event_percpu+0x39/0x180
> [   51.613880]  [] handle_irq_event+0x45/0x70
> [   51.613881]  [] handle_edge_irq+0x99/0x150
> [   51.613883]  [] handle_irq+0x1d/0x30
> [   51.613883]  [] do_IRQ+0x4d/0xd0
> [   51.613885]  [] common_interrupt+0x87/0x87
> [   51.613885][] ? 
> cpuidle_enter_state+0xb8/0x220
> [   51.613888]  [] cpuidle_enter+0x17/0x20
> [   51.613889]  [] call_cpuidle+0x32/0x60
> [   51.613890]  [] ? cpuidle_select+0x13/0x20
> [   51.613891]  [] cpu_startup_entry+0x21c/0x2e0
> [   51.613891]  [] start_secondary+0x104/0x130
> [   51.613892] ---[ end trace bda1c37ade46c57d ]�
> 
> I can also reliable reproduce this by connecting the USB3380 to a USB 
> port on the same Linux machine.
> In that case I also see an error:
> net2280 : net2280_enable: error=-22
> 
> Perhaps unrelated, but there is also a message:
> configfs-gadget gadget: common->fsg is NULL in fsg_setup at 511
 The same crash happens in 4.2 as well but not in 4.1.
>>> 
>>> care to run a git bisect and find offending commit ?
>> Unfortunately I encountered many kernels that hung my machine during a git 
>> bisect, so I had to git bisect skip many a time.
>> Here�s the git bisect result (between v4.1 and v4.2):
>>  There are only 'skip'ped commits left to test.
>>  The first bad commit could be any of:
>>  4117a60c8e4c8d5f9fc05578e359d09d0fdf9d07
>>  4ae82e5d23961515796d76850499db3866c5e73b
>>  We cannot bisect more!
>> 
>> Oddly neither of those commits seem very relevant to the problem. Not sure 
>> if this helps�
> 
> Mian Yousaf Kaukab added a bunch of changes to the net2280 driver 
> between 4.1 and 4.2.  Do:
> 
>   git log v4.1..v4.2 -- drivers/usb/gadget/udc/net2280.c
> 
> One of them may be responsible.

Commit 25d40ee8 (the very first change in net2280) gives me the above error.
Commit af6e613bb (just before that change) still works.
I pretty much tried all versions with net2280 changes upto 11bece5e.
All of them exhibit the above error. 
For 11bece5e I couldn’t test as that revision hangs my kernel during boot up.

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

Re: Problems with printk logs and my driver

[Eric Curtin]

On 4 October 2015 at 16:28, Alan Stern  wrote:
> On Sun, 4 Oct 2015, Eric Curtin wrote:
>
>> Ok so for the fun of it, I changed the VENDOR_ID and DEVICE_ID of my
>> keyboard to use the driver for this samsung Wireless keyboard and
>> mouse, crazy I know since I have a different piece of hardware, but I
>> wanted to see what happens or at least does it change driver. When I
>> reboot to this kernel, my keyboard still uses the usbhid driver. Why
>> does this happen?
>
> Because the Samsung wireless keyboard and mouse also use the usbhid
> driver.  Besides, the choice of driver depends just as much on the
> bInterfaceClass, -Subclass, and -Protocol as on the vendor and product
> IDs.
>
> Basically, nothing needs to use usbkbd.  Anything that driver can
> handle will also work with usbhid.  The only reason for keeping usbkbd
> as a separate driver is because it is smaller than usbhid, so it's
> better suited to some embedded applications with limited memory.
>
> Alan Stern
>

After looking at this again today, I cannot reproduce the problem anymore.
I took out the battery on this keyboard and put it back in and the issue
is now not reproducible. Pity, I was hoping to get to the bottom of this
one.

It works in X but not in console mode. I tried another one and it had the
same result, it's a more standard usb keyboard.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: dwc2: host: Protect PCGCTL with lock in dwc2_port_resume()

[Douglas Anderson]

>From code inspection, it appears to be unsafe to do a read-modify-write
of PCGCTL in dwc2_port_resume().  Let's make sure the spinlock is held
around this operation.

Signed-off-by: Douglas Anderson 
---
 drivers/usb/dwc2/hcd.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index af4e4a2..e79baf7 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1500,6 +1500,8 @@ static void dwc2_port_resume(struct dwc2_hsotg *hsotg)
u32 hprt0;
u32 pcgctl;
 
+   spin_lock_irqsave(>lock, flags);
+
/*
 * If hibernation is supported, Phy clock is already resumed
 * after registers restore.
@@ -1508,10 +1510,11 @@ static void dwc2_port_resume(struct dwc2_hsotg *hsotg)
pcgctl = dwc2_readl(hsotg->regs + PCGCTL);
pcgctl &= ~PCGCTL_STOPPCLK;
dwc2_writel(pcgctl, hsotg->regs + PCGCTL);
+   spin_unlock_irqrestore(>lock, flags);
usleep_range(2, 4);
+   spin_lock_irqsave(>lock, flags);
}
 
-   spin_lock_irqsave(>lock, flags);
hprt0 = dwc2_read_hprt0(hsotg);
hprt0 |= HPRT0_RES;
hprt0 &= ~HPRT0_SUSP;
-- 
2.6.0.rc2.230.g3dd15c0

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

[PATCH v2] usb: dwc2: host: Fix use after free w/ simultaneous irqs

[Douglas Anderson]

While plugging / unplugging on a DWC2 host port with "slub_debug=FZPUA"
enabled, I found a crash that was quite obviously a use after free.

It appears that in some cases when we handle the various sub-cases of
HCINT we may end up freeing the QTD.  If there is more than one bit set
in HCINT we may then end up continuing to use the QTD, which is bad.
Let's be paranoid and check for this after each sub-case.  We only do
this if there are still further bits to process, so the overhead should
be small.  This should be safe since we officially have the
"hsotg->lock" (it was grabbed in dwc2_handle_hcd_intr).

The specific crash I found was:
 Unable to handle kernel paging request at virtual address 6b6b6b9f

At the time of the crash, the kernel reported:
 (dwc2_hc_nak_intr+0x5c/0x198)
 (dwc2_handle_hcd_intr+0xa84/0xbf8)
 (_dwc2_hcd_irq+0x1c/0x20)
 (usb_hcd_irq+0x34/0x48)

Popping into kgdb found that "*qtd" was filled with "0x6b", AKA qtd had
been freed and filled with slub_debug poison.

kgdb gave a little better stack crawl:
 0 dwc2_hc_nak_intr (hsotg=hsotg@entry=0xec42e058,
 chan=chan@entry=0xec546dc0, chnum=chnum@entry=4,
 qtd=qtd@entry=0xec679600) at drivers/usb/dwc2/hcd_intr.c:1237
 1 dwc2_hc_n_intr (chnum=4, hsotg=0xec42e058) at
 drivers/usb/dwc2/hcd_intr.c:2041
 2 dwc2_hc_intr (hsotg=0xec42e058) at drivers/usb/dwc2/hcd_intr.c:2078
 3 dwc2_handle_hcd_intr (hsotg=0xec42e058) at
 drivers/usb/dwc2/hcd_intr.c:2128
 4 _dwc2_hcd_irq (hcd=) at drivers/usb/dwc2/hcd.c:2837
 5 usb_hcd_irq (irq=, __hcd=) at
 drivers/usb/core/hcd.c:2353

Popping up to frame #1 (dwc2_hc_n_intr) found:
 (gdb) print /x hcint
 $12 = 0x12

AKA:
 #define HCINTMSK_CHHLTD  (1 << 1)
 #define HCINTMSK_NAK (1 << 4)

Further debugging found that by simulating receiving those two
interrupts at the same time it was trivial to replicate the
use-after-free.  See  for a patch and
instructions.  This lead to getting the following stack crawl of the
actual free:
 0  arch_kgdb_breakpoint () at arch/arm/include/asm/outercache.h:103
 1  kgdb_breakpoint () at kernel/debug/debug_core.c:1054
 2  dwc2_hcd_qtd_unlink_and_free (hsotg=, qh=, qtd=0xe4479a00) at drivers/usb/dwc2/hcd.h:488
 3  dwc2_deactivate_qh (free_qtd=, qh=0xe5efa280,
  hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:671
 4  dwc2_release_channel (hsotg=hsotg@entry=0xed424618,
  chan=chan@entry=0xed5be000, qtd=,
  halt_status=) at drivers/usb/dwc2/hcd_intr.c:742
 5  dwc2_halt_channel (hsotg=0xed424618, chan=0xed5be000, qtd=, halt_status=) at
  drivers/usb/dwc2/hcd_intr.c:804
 6  dwc2_complete_non_periodic_xfer (chnum=,
  halt_status=, qtd=, chan=, hsotg=) at drivers/usb/dwc2/hcd_intr.c:889
 7  dwc2_hc_xfercomp_intr (hsotg=hsotg@entry=0xed424618,
  chan=chan@entry=0xed5be000, chnum=chnum@entry=6,
  qtd=qtd@entry=0xe4479a00) at drivers/usb/dwc2/hcd_intr.c:1065
 8  dwc2_hc_chhltd_intr_dma (qtd=0xe4479a00, chnum=6, chan=0xed5be000,
  hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:1823
 9  dwc2_hc_chhltd_intr (qtd=0xe4479a00, chnum=6, chan=0xed5be000,
  hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:1944
 10 dwc2_hc_n_intr (chnum=6, hsotg=0xed424618) at
  drivers/usb/dwc2/hcd_intr.c:2052
 11 dwc2_hc_intr (hsotg=0xed424618) at drivers/usb/dwc2/hcd_intr.c:2097
 12 dwc2_handle_hcd_intr (hsotg=0xed424618) at
  drivers/usb/dwc2/hcd_intr.c:2147
 13 _dwc2_hcd_irq (hcd=) at drivers/usb/dwc2/hcd.c:2837
 14 usb_hcd_irq (irq=, __hcd=) at
  drivers/usb/core/hcd.c:2353

Though we could add specific code to handle this case, adding the
general purpose code to check for all cases where qtd might be freed
seemed safer.

Signed-off-by: Douglas Anderson 
---
Changes in v2:
- Add static as correctly pointed by kbuild test robot

 drivers/usb/dwc2/hcd_intr.c | 80 +++--
 1 file changed, 70 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index f70c970..1156e13 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -1949,6 +1949,25 @@ static void dwc2_hc_chhltd_intr(struct dwc2_hsotg *hsotg,
}
 }
 
+/*
+ * Check if the given qtd is still the top of the list (and thus valid).
+ *
+ * If dwc2_hcd_qtd_unlink_and_free() has been called since we grabbed
+ * the qtd from the top of the list, this will return NULL.  Otherwise
+ * it will be passed back qtd.
+ */
+static struct dwc2_qtd *dwc2_check_qtd_still_ok(struct dwc2_qtd *qtd,
+   struct list_head *qtd_list)
+{
+   struct dwc2_qtd *cur_head;
+
+   cur_head = list_first_entry(qtd_list, struct dwc2_qtd, qtd_list_entry);
+   if (cur_head == qtd)
+   return qtd;
+
+   return NULL;
+}
+
 /* Handles interrupt for a specific Host Channel */
 static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum)
 {
@@ -2031,26 +2050,67 @@ static void 

Re: [RFC PATCH] usb: dwc2: host: dwc2_check_qtd_still_ok() can be static

[Doug Anderson]

Hi,

On Wed, Oct 14, 2015 at 1:44 PM, kbuild test robot  wrote:
>
> Signed-off-by: Fengguang Wu 
> ---
>  hcd_intr.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
> index 57e71f9d..b31a1fb 100644
> --- a/drivers/usb/dwc2/hcd_intr.c
> +++ b/drivers/usb/dwc2/hcd_intr.c
> @@ -1956,7 +1956,7 @@ static void dwc2_hc_chhltd_intr(struct dwc2_hsotg 
> *hsotg,
>   * the qtd from the top of the list, this will return NULL.  Otherwise
>   * it will be passed back qtd.
>   */
> -struct dwc2_qtd *dwc2_check_qtd_still_ok(struct dwc2_qtd *qtd,
> +static struct dwc2_qtd *dwc2_check_qtd_still_ok(struct dwc2_qtd *qtd,
> struct list_head *qtd_list)
>  {
> struct dwc2_qtd *cur_head;

Fixed in v2.  https://patchwork.kernel.org/patch/7398911/

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

[PATCH v4] usb: gadget: loopback: fix: Don't share qlen and buflen between instances

[Krzysztof Opasiak]

Each instance of loopback function may have different qlen
and buflen attributes values. When linking function to
configuration those values had been assigned to global
variables. Linking other instance to config overwrites those
values.

This commit moves those values to f_loopback structure
to avoid overwriting. Now each function has its own instance
of those values.

Signed-off-by: Krzysztof Opasiak 
Reviewed-by: Robert Baldyga 
---
Changes since v3:
- rebase onto balbi/next
- remove cc to stable
---
 drivers/usb/gadget/function/f_loopback.c |   24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 6e2fe63..e4bfed4 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -34,6 +34,9 @@ struct f_loopback {
 
struct usb_ep   *in_ep;
struct usb_ep   *out_ep;
+
+   unsignedqlen;
+   unsignedbuflen;
 };
 
 static inline struct f_loopback *func_to_loop(struct usb_function *f)
@@ -41,13 +44,10 @@ static inline struct f_loopback *func_to_loop(struct 
usb_function *f)
return container_of(f, struct f_loopback, function);
 }
 
-static unsigned qlen;
-static unsigned buflen;
-
 /*-*/
 
 static struct usb_interface_descriptor loopback_intf = {
-   .bLength =  sizeof loopback_intf,
+   .bLength =  sizeof(loopback_intf),
.bDescriptorType =  USB_DT_INTERFACE,
 
.bNumEndpoints =2,
@@ -253,7 +253,7 @@ static void loopback_complete(struct usb_ep *ep, struct 
usb_request *req)
}
 
/* queue the buffer for some later OUT packet */
-   req->length = buflen;
+   req->length = loop->buflen;
status = usb_ep_queue(ep, req, GFP_ATOMIC);
if (status == 0)
return;
@@ -290,7 +290,9 @@ static void disable_loopback(struct f_loopback *loop)
 
 static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
 {
-   return alloc_ep_req(ep, len, buflen);
+   struct f_loopback   *loop = ep->driver_data;
+
+   return alloc_ep_req(ep, len, loop->buflen);
 }
 
 static int enable_endpoint(struct usb_composite_dev *cdev, struct f_loopback 
*loop,
@@ -317,7 +319,7 @@ static int enable_endpoint(struct usb_composite_dev *cdev, 
struct f_loopback *lo
 * we buffer at most 'qlen' transfers; fewer if any need more
 * than 'buflen' bytes each.
 */
-   for (i = 0; i < qlen && result == 0; i++) {
+   for (i = 0; i < loop->qlen && result == 0; i++) {
req = lb_alloc_ep_req(ep, 0);
if (!req)
goto fail1;
@@ -391,10 +393,10 @@ static struct usb_function *loopback_alloc(struct 
usb_function_instance *fi)
lb_opts->refcnt++;
mutex_unlock(_opts->lock);
 
-   buflen = lb_opts->bulk_buflen;
-   qlen = lb_opts->qlen;
-   if (!qlen)
-   qlen = 32;
+   loop->buflen = lb_opts->bulk_buflen;
+   loop->qlen = lb_opts->qlen;
+   if (!loop->qlen)
+   loop->qlen = 32;
 
loop->function.name = "loopback";
loop->function.bind = loopback_bind;
-- 
1.7.9.5

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

Re: [PATCH] USB: usbtmc: Add support for missing USBTMC-USB488 spec

[Oliver Neukum]

On Wed, 2015-10-14 at 15:13 +0200, dave penkler wrote:

Hi,

just a few remarks.

>  
> +static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
> + unsigned long arg)
> +{
> + u8 *buffer;
> + struct device *dev;
> + int rv;
> + u8 tag, stb;
> +
> + dev = >intf->dev;
> +
> + dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
> + data->iin_ep_present);
> +
> + buffer = kmalloc(8, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> +
> + atomic_set(>iin_data_valid, 0);
> +
> + /* must issue read_stb before using poll or select */
> + atomic_set(>srq_asserted, 0);
> +
> + rv = usb_control_msg(data->usb_dev,
> + usb_rcvctrlpipe(data->usb_dev, 0),
> + USBTMC488_REQUEST_READ_STATUS_BYTE,
> + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> + data->iin_bTag,
> + data->ifnum,
> + buffer, 0x03, USBTMC_TIMEOUT);
> +
> + if (rv < 0) {
> + dev_err(dev, "stb usb_control_msg returned %d\n", rv);
> + goto exit;
> + }
> +
> + if (buffer[0] != USBTMC_STATUS_SUCCESS) {
> + dev_err(dev, "control status returned %x\n",
> + buffer[0]);
> + rv = -EIO;
> + goto exit;
> + }
> +
> + if (data->iin_ep_present) {
> +
> + rv = wait_event_interruptible_timeout(
> + data->waitq,
> + (atomic_read(>iin_data_valid) != 0),
> + USBTMC_TIMEOUT
> + );
> +
> + if (rv < 0) {
> + dev_dbg(dev, "wait interrupted %d\n", rv);
> + goto exit;
> + }

If you do this, yet the transfer succeeds, how do you keep the tag
between host and device consistent?

> +
> + if (rv == 0) {
> + dev_dbg(dev, "wait timed out\n");
> + rv = -ETIME;
> + goto exit;
> + }
> +
> + tag = data->bNotify1 & 0x7f;
> +
> + if (tag != data->iin_bTag) {
> + dev_err(dev, "expected bTag %x got %x\n",
> + data->iin_bTag, tag);
> + }
> +
> + stb = data->bNotify2;
> + } else {
> + stb = buffer[2];
> + }
> +
> + /* bump interrupt bTag */
> + data->iin_bTag += 1;
> + if (data->iin_bTag > 127)
> + data->iin_bTag = 2;
> +
> + rv = copy_to_user((void *)arg, , sizeof(stb));
> + if (rv)
> + rv = -EFAULT;
> +
> + exit:
> + kfree(buffer);
> + return rv;
> +
> +}
> +
> +static unsigned int usbtmc_poll(struct file *file, poll_table *wait)
> +{
> + struct usbtmc_device_data *data = file->private_data;
> + unsigned int mask = 0;
> +
> + mutex_lock(>io_mutex);
> +
> + if (data->zombie)
> + goto no_poll;
> +
> + poll_wait(file, >waitq, wait);

Presumably the waiters should be woken when the device is disconnected.

> +
> + mask = data->zombie ? POLLHUP | POLLERR :
> + (atomic_read(>srq_asserted)) ? POLLIN | POLLRDNORM : 0;
> +
> +no_poll:
> + mutex_unlock(>io_mutex);
> + return mask;
> +}

Regards
Oliver


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

Re: [PATCH v2] usb: gadget: f_sourcesink: fix function params handling

[Felipe Balbi]

Hi,

Robert Baldyga  writes:
> Hi Felipe,
>
> On 09/24/2015 06:49 PM, Felipe Balbi wrote:
>> On Thu, Sep 24, 2015 at 05:23:09PM +0200, Robert Baldyga wrote:
>>> Move function parameters to struct f_sourcesink to make them per instance
>>> instead of having them as global variables. Since we can have multiple
>>> instances of USB function we also want to have separate set of parameters
>>> for each instance.
>>>
>>> Cc:  # 3.10+
>>> Signed-off-by: Robert Baldyga 
>> 
>> why do you think this is stable material ? Looks like it's not
>> fixing anything to me; just implementing a brand new requirement.
>
> I will not insist on stable backporting of this patch, as it's actually
> not very important fix (especially in case of sourcesink function).
>
> Should I resend this patch without CC:stable?

please, thank you.

-- 
balbi


signature.asc
Description: PGP signature

Killing USBNET as an independent Konfig option

[Oliver Neukum]

Hi,

would anybody mind me killing the separate config option
of usbnet? I can see no sense in having it as a separate
option, as it is just infrastructure.

Regards
Oliver


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

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Jassi Brar]

On Wed, Oct 14, 2015 at 11:18 PM, Alan Stern  wrote:
> On Wed, 14 Oct 2015, Jassi Brar wrote:
>
>> BTW, should the gadget stack ever queue a Non-ZLP as reply to some
>> setup request that has USB_DIR_IN not set?
>
> Yes.  If USB_DIR_IN is not set then the control transfer is OUT, so the
> gadget needs to queue a request to receive some data from the host.
> That request will obviously need to be a non-ZLP.
>
By 'reply' I meant after reading out and parsing the
setup(control-out) request. I am sure we need to send a ZLP.

> Could this cause the problem you're seeing?  The host tries to send
> more data than the gadget is ready to receive?  (Although then the
> error code on the gadget side should be -75, not -71.)
>
Thanks, but as you said my problem is not that (I get protocol error
-71).  My problem is my udc driver actually tries to send a 7 byte
response to a control-out command.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Jassi Brar]

On Wed, Oct 14, 2015 at 9:18 PM, Felipe Balbi  wrote:
>
> Hi,
>
> Jassi Brar  writes:
>> On Wed, Oct 14, 2015 at 8:42 PM, Felipe Balbi  wrote:
>>>
>>> Hi,
>>>
>>> jaswinder.si...@linaro.org writes:
 From: Jassi Brar 

 We must return 0 for any OUT setup request, otherwise
 protocol error may occur.
>>>
>>> have you seen any such errors ?
>>>
>> Yes. While testing my new udc driver.
>>
>>
>>> Care to describe what problems you have seen and which UDC you were using,
>>> what's the exact scenario. How have you tested this ?
>>>
>> The udc I am writing the driver for is not yet public. To test my
>> driver at lowest level possible, I use 'usbmon' to capture and analyze
>> traffic since I don't have any hardware probe.
>>
>> I see the following on gadget side
>> ---
>> configfs-gadget gadget: non-core control req21.20 v i0001 l7
>> configfs-gadget gadget: acm ttyGS0 req21.20 v i0001 l7
>> configfs-gadget gadget: acm ttyGS0 completion, err -71
>> ---
>>
>> and the following on host side usbmon capture
>> ---
>> 88041ed01f00 540296433 S Co:3:023:0 s 21 20  0001 0007 7 =
>> 8025 08
>> 88041ed01f00 540296790 C Co:3:023:0 -71 0
>> ---
>
> and you did you even consider this could be a bug in your new UDC driver
> at all ? This works fine with other UDCs.
>
I was happy too until I decided to look closely at the annoying print
on gadget side. This is a non-fatal error and visible only when
debugging is enabled, so every udc seems 'fine'

> How far are you in developing this new UDC driver ?
>
Its done and tested for fsg+acm composite and each alone.

> Did you run USBCV at all ?
>
No USBCV not yet. I borrowed Windows machine to test FSG but forgot
USBCV since it's been years I wrote last udc driver. Will give it a
try tomorrow but I don't think it could emulate the sequence I hit
with f_acm.

> Are you sure you're implementing EP0 handling correctly ? What
> sort of tests have you done with this UDC ? Is it working with testusb
> against a known good host (EHCI and XHCI should be good for that) ?
>
Well as I said I have been looking at low level transfers and
everything is good.

BTW, should the gadget stack ever queue a Non-ZLP as reply to some
setup request that has USB_DIR_IN not set?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: musb: communication issue with more than 12 FTDI ports

[Bin Liu]

Felipe,

On 10/14/2015 11:25 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

On 10/14/2015 10:56 AM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

Hi,

On 10/13/2015 01:22 PM, Felipe Balbi wrote:

Yegor Yefremov  writes:

On Mon, Oct 12, 2015 at 11:34 AM, Yegor Yefremov
 wrote:

We have a problem, when using more than 12 FTDI ports. Kernels tried:
3.18.1, 4.2.3 and 4.3-rc5. SoC am335x 600MHz

Below the USB topology:

# lsusb -t
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
   |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M
   |__ Port 1: Dev 9, If 0, Class=, Driver=hub/4p, 480M
   |__ Port 1: Dev 10, If 0, Class=, Driver=ftdi_sio, 12M
   |__ Port 2: Dev 11, If 0, Class=, Driver=ftdi_sio, 480M
   |__ Port 2: Dev 11, If 1, Class=, Driver=ftdi_sio, 480M
   |__ Port 2: Dev 11, If 2, Class=, Driver=ftdi_sio, 480M
   |__ Port 2: Dev 11, If 3, Class=, Driver=ftdi_sio, 480M
   |__ Port 3: Dev 12, If 0, Class=, Driver=ftdi_sio, 480M
   |__ Port 3: Dev 12, If 1, Class=, Driver=ftdi_sio, 480M
   |__ Port 3: Dev 12, If 2, Class=, Driver=ftdi_sio, 480M
   |__ Port 3: Dev 12, If 3, Class=, Driver=ftdi_sio, 480M
   |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M
   |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M
   |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M
   |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M
   |__ Port 3: Dev 7, If 0, Class=, Driver=ftdi_sio, 480M
   |__ Port 3: Dev 7, If 1, Class=, Driver=ftdi_sio, 480M
   |__ Port 3: Dev 7, If 2, Class=, Driver=ftdi_sio, 480M
   |__ Port 3: Dev 7, If 3, Class=, Driver=ftdi_sio, 480M


How many EPs does each FTDI device require? at least one INT EP, right?
If I read it right, the topology above has 2 hubs, and 16 high-speed
FTDI and 1 full-speed FTDI. So it requires at least 18 high-speed INT
EPs. MUSB driver only has 11 high-speed EPs for mode-4 which is the EP
configuration used by default. I am wondering how those devices got
enumerated properly.


dynamic EP allocation, but that has its own limitations.


MUSB does not support dynamic EP allocation for INT/ISOCH.


I remember isoc doesn't, not sure about int. Do you remember where that
part of the code is off the top of your head ?



The MUSB EP allocation is in musb_schedule() in musb_host.c.

It does not have specific policy for INT/ISOCH, but the issue is that 
for periodic EP, it got allocated during device enumeration but freed 
only when the device is disconnected. So practically there is no dynamic 
EP allocation for INT/ISOCH.


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

[PATCH] USB: usbtmc: Add support for missing USBTMC-USB488 spec

[dave penkler]

Implement support for the USB488 defined READ_STATUS_BYTE and SRQ
notifications with ioctl, fasync and poll/select in order to be able
to synchronize with variable duration instrument operations.

Add ioctls for other USB488 requests: REN_CONTROL, GOTO_LOCAL and
LOCAL_LOCKOUT.

Add convenience ioctl to return all device capabilities.

Signed-off-by: Dave Penkler 
---
 drivers/usb/class/usbtmc.c   | 359 +++
 include/uapi/linux/usb/tmc.h |  42 +++--
 2 files changed, 391 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 7a11a82..83c8416 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -87,6 +88,24 @@ struct usbtmc_device_data {
u8 bTag_last_write; /* needed for abort */
u8 bTag_last_read;  /* needed for abort */
 
+
+   /* data for interrupt in endpoint handling */
+   u8 bNotify1;
+   u8 bNotify2;
+   u16ifnum;
+   u8 iin_bTag;
+   u8*iin_buffer;
+   atomic_t   iin_data_valid;
+   unsigned int   iin_ep;
+   intiin_ep_present;
+   intiin_interval;
+   struct urb*iin_urb;
+   u16iin_wMaxPacketSize;
+   atomic_t   srq_asserted;
+
+   /* coalesced usb488_caps from usbtmc_dev_capabilities */
+   u8 usb488_caps;
+
u8 rigol_quirk;
 
/* attributes from the USB TMC spec for this device */
@@ -99,6 +118,8 @@ struct usbtmc_device_data {
struct usbtmc_dev_capabilities  capabilities;
struct kref kref;
struct mutex io_mutex;  /* only one i/o function running at a time */
+   wait_queue_head_t waitq;
+   struct fasync_struct *fasync;
 };
 #define to_usbtmc_data(d) container_of(d, struct usbtmc_device_data, kref)
 
@@ -373,6 +394,158 @@ exit:
return rv;
 }
 
+static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
+   unsigned long arg)
+{
+   u8 *buffer;
+   struct device *dev;
+   int rv;
+   u8 tag, stb;
+
+   dev = >intf->dev;
+
+   dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
+   data->iin_ep_present);
+
+   buffer = kmalloc(8, GFP_KERNEL);
+   if (!buffer)
+   return -ENOMEM;
+
+
+   atomic_set(>iin_data_valid, 0);
+
+   /* must issue read_stb before using poll or select */
+   atomic_set(>srq_asserted, 0);
+
+   rv = usb_control_msg(data->usb_dev,
+   usb_rcvctrlpipe(data->usb_dev, 0),
+   USBTMC488_REQUEST_READ_STATUS_BYTE,
+   USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+   data->iin_bTag,
+   data->ifnum,
+   buffer, 0x03, USBTMC_TIMEOUT);
+
+   if (rv < 0) {
+   dev_err(dev, "stb usb_control_msg returned %d\n", rv);
+   goto exit;
+   }
+
+   if (buffer[0] != USBTMC_STATUS_SUCCESS) {
+   dev_err(dev, "control status returned %x\n",
+   buffer[0]);
+   rv = -EIO;
+   goto exit;
+   }
+
+   if (data->iin_ep_present) {
+
+   rv = wait_event_interruptible_timeout(
+   data->waitq,
+   (atomic_read(>iin_data_valid) != 0),
+   USBTMC_TIMEOUT
+   );
+
+   if (rv < 0) {
+   dev_dbg(dev, "wait interrupted %d\n", rv);
+   goto exit;
+   }
+
+   if (rv == 0) {
+   dev_dbg(dev, "wait timed out\n");
+   rv = -ETIME;
+   goto exit;
+   }
+
+   tag = data->bNotify1 & 0x7f;
+
+   if (tag != data->iin_bTag) {
+   dev_err(dev, "expected bTag %x got %x\n",
+   data->iin_bTag, tag);
+   }
+
+   stb = data->bNotify2;
+   } else {
+   stb = buffer[2];
+   }
+
+   /* bump interrupt bTag */
+   data->iin_bTag += 1;
+   if (data->iin_bTag > 127)
+   data->iin_bTag = 2;
+
+   rv = copy_to_user((void *)arg, , sizeof(stb));
+   if (rv)
+   rv = -EFAULT;
+
+ exit:
+   kfree(buffer);
+   return rv;
+
+}
+
+static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
+   unsigned long arg,
+   unsigned int cmd)
+{
+   u8 *buffer;
+   struct device *dev;
+   int rv;
+   unsigned int val;
+   u16 wValue;
+
+   dev = >intf->dev;
+
+   if (0 == (data->usb488_caps & USBTMC488_CAPABILITY_SIMPLE))
+   return -EINVAL;
+
+ 

Re: [PATCH] usb: gadget: function: acm: return zlp for OUT setup

[Jassi Brar]

On Wed, Oct 14, 2015 at 10:40 PM, Felipe Balbi  wrote:
>
> Hi,
>
> Jassi Brar  writes:
>> On Wed, Oct 14, 2015 at 9:18 PM, Felipe Balbi  wrote:
>>>
>>> Hi,
>>>
>>> Jassi Brar  writes:
 On Wed, Oct 14, 2015 at 8:42 PM, Felipe Balbi  wrote:
>
> Hi,
>
> jaswinder.si...@linaro.org writes:
>> From: Jassi Brar 
>>
>> We must return 0 for any OUT setup request, otherwise
>> protocol error may occur.
>
> have you seen any such errors ?
>
 Yes. While testing my new udc driver.


> Care to describe what problems you have seen and which UDC you were using,
n> what's the exact scenario. How have you tested this ?
>
 The udc I am writing the driver for is not yet public. To test my
 driver at lowest level possible, I use 'usbmon' to capture and analyze
 traffic since I don't have any hardware probe.

 I see the following on gadget side
 ---
 configfs-gadget gadget: non-core control req21.20 v i0001 l7
 configfs-gadget gadget: acm ttyGS0 req21.20 v i0001 l7
 configfs-gadget gadget: acm ttyGS0 completion, err -71
 ---

 and the following on host side usbmon capture
 ---
 88041ed01f00 540296433 S Co:3:023:0 s 21 20  0001 0007 7 =
 8025 08
 88041ed01f00 540296790 C Co:3:023:0 -71 0
 ---
>>>
>>> and you did you even consider this could be a bug in your new UDC driver
>>> at all ? This works fine with other UDCs.
>>>
>> I was happy too until I decided to look closely at the annoying print
>> on gadget side. This is a non-fatal error and visible only when
>> debugging is enabled, so every udc seems 'fine'
>
> I tried with my sniffer and saw no stalls, nothing. My setup request to
> set line coding to 9600 baud worked just fine.
>
I am sure your udc driver will (need to) track stages of a transfer.
If you put some print in usb_ep_ops.queue() you will notice the stack
queues 7byte transfer but the udc driver silently drops it and send a
zlp here.

  I can move the change into my driver as well, but the point is
gadget should never try to _send_ non-zlp as reply to control-OUT
command.


>>> How far are you in developing this new UDC driver ?
>>>
>> Its done and tested for fsg+acm composite and each alone.
>
> stress tests ? Meaning, did you leave it running for a couple of weeks
> at least ?
>
I know I can't stress test enough, but this bug is 100% hit and
staring in the eye at protocol level.

>> BTW, should the gadget stack ever queue a Non-ZLP as reply to some
>> setup request that has USB_DIR_IN not set?
>
> What phase of the setup are we talking about ?
>
I said "_reply_ to setup request" so I meant status phase.

UDC driver receives the SETUP command as '21 20 00 00 01 00 07 00',
passes it onto composite, which hands it over to acm and which
pretends we need to return a 7byte packet to host (value = w_length =
7).
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] usb: gadget: f_sourcesink: fix function params handling

[Robert Baldyga]

Move function parameters to struct f_sourcesink to make them per instance
instead of having them as global variables. Since we can have multiple
instances of USB function we also want to have separate set of parameters
for each instance.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_sourcesink.c | 120 +++--
 1 file changed, 62 insertions(+), 58 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index 68efa01..d7646d3 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -50,6 +50,13 @@ struct f_sourcesink {
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
int cur_alt;
+
+   unsigned pattern;
+   unsigned isoc_interval;
+   unsigned isoc_maxpacket;
+   unsigned isoc_mult;
+   unsigned isoc_maxburst;
+   unsigned buflen;
 };
 
 static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
@@ -57,13 +64,6 @@ static inline struct f_sourcesink *func_to_ss(struct 
usb_function *f)
return container_of(f, struct f_sourcesink, function);
 }
 
-static unsigned pattern;
-static unsigned isoc_interval;
-static unsigned isoc_maxpacket;
-static unsigned isoc_mult;
-static unsigned isoc_maxburst;
-static unsigned buflen;
-
 /*-*/
 
 static struct usb_interface_descriptor source_sink_intf_alt0 = {
@@ -298,7 +298,9 @@ static struct usb_gadget_strings *sourcesink_strings[] = {
 
 static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int len)
 {
-   return alloc_ep_req(ep, len, buflen);
+   struct f_sourcesink *ss = ep->driver_data;
+
+   return alloc_ep_req(ep, len, ss->buflen);
 }
 
 void free_ep_req(struct usb_ep *ep, struct usb_request *req)
@@ -357,22 +359,22 @@ autoconf_fail:
goto autoconf_fail;
 
/* sanity check the isoc module parameters */
-   if (isoc_interval < 1)
-   isoc_interval = 1;
-   if (isoc_interval > 16)
-   isoc_interval = 16;
-   if (isoc_mult > 2)
-   isoc_mult = 2;
-   if (isoc_maxburst > 15)
-   isoc_maxburst = 15;
+   if (ss->isoc_interval < 1)
+   ss->isoc_interval = 1;
+   if (ss->isoc_interval > 16)
+   ss->isoc_interval = 16;
+   if (ss->isoc_mult > 2)
+   ss->isoc_mult = 2;
+   if (ss->isoc_maxburst > 15)
+   ss->isoc_maxburst = 15;
 
/* fill in the FS isoc descriptors from the module parameters */
-   fs_iso_source_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
-   1023 : isoc_maxpacket;
-   fs_iso_source_desc.bInterval = isoc_interval;
-   fs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket > 1023 ?
-   1023 : isoc_maxpacket;
-   fs_iso_sink_desc.bInterval = isoc_interval;
+   fs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
+   1023 : ss->isoc_maxpacket;
+   fs_iso_source_desc.bInterval = ss->isoc_interval;
+   fs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket > 1023 ?
+   1023 : ss->isoc_maxpacket;
+   fs_iso_sink_desc.bInterval = ss->isoc_interval;
 
/* allocate iso endpoints */
ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, _iso_source_desc);
@@ -394,8 +396,8 @@ no_iso:
ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
}
 
-   if (isoc_maxpacket > 1024)
-   isoc_maxpacket = 1024;
+   if (ss->isoc_maxpacket > 1024)
+   ss->isoc_maxpacket = 1024;
 
/* support high speed hardware */
hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
@@ -406,15 +408,15 @@ no_iso:
 * We assume that the user knows what they are doing and won't
 * give parameters that their UDC doesn't support.
 */
-   hs_iso_source_desc.wMaxPacketSize = isoc_maxpacket;
-   hs_iso_source_desc.wMaxPacketSize |= isoc_mult << 11;
-   hs_iso_source_desc.bInterval = isoc_interval;
+   hs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
+   hs_iso_source_desc.wMaxPacketSize |= ss->isoc_mult << 11;
+   hs_iso_source_desc.bInterval = ss->isoc_interval;
hs_iso_source_desc.bEndpointAddress =
fs_iso_source_desc.bEndpointAddress;
 
-   hs_iso_sink_desc.wMaxPacketSize = isoc_maxpacket;
-   hs_iso_sink_desc.wMaxPacketSize |= isoc_mult << 11;
-   hs_iso_sink_desc.bInterval = isoc_interval;
+   hs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
+   hs_iso_sink_desc.wMaxPacketSize |= ss->isoc_mult << 11;
+   hs_iso_sink_desc.bInterval = ss->isoc_interval;

Re: Killing USBNET as an independent Konfig option

[Greg KH]

On Wed, Oct 14, 2015 at 12:45:31PM +0200, Oliver Neukum wrote:
> Hi,
> 
> would anybody mind me killing the separate config option
> of usbnet? I can see no sense in having it as a separate
> option, as it is just infrastructure.

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