Date:   Thu, 26 Aug 2021 09:30:21 +0200
From:   Ferry Toth <fntoth@...il.com>
To:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Pavel Hofman <pavel.hofman@...tera.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Felipe Balbi <balbi@...nel.org>,
        Wesley Cheng <wcheng@...eaurora.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "agross@...nel.org" <agross@...nel.org>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        "frowand.list@...il.com" <frowand.list@...il.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "jackp@...eaurora.org" <jackp@...eaurora.org>,
        "heikki.krogerus@...ux.intel.com" <heikki.krogerus@...ux.intel.com>,
        Ruslan Bilovol <ruslan.bilovol@...il.com>
Subject: Re: [PATCH v10 0/6] Re-introduce TX FIFO resize for larger EP
 bursting

Hi

Op 26-08-2021 om 03:18 schreef Thinh Nguyen:
> Ferry Toth wrote:
>> Hi
>>
>> Op 24-08-2021 om 07:39 schreef Pavel Hofman:
>>>
>>>
>>> Dne 24. 08. 21 v 0:50 Thinh Nguyen napsal(a):
>>>> Pavel Hofman wrote:
>>>>>
>>>>>
>>>>> Dne 23. 08. 21 v 17:21 Andy Shevchenko napsal(a):
>>>>>> On Mon, Aug 23, 2021 at 5:59 PM Pavel Hofman
>>>>>> <pavel.hofman@...tera.com> wrote:
>>>>>>> Dne 22. 08. 21 v 21:43 Ferry Toth napsal(a):
>>>>>>>> Op 19-08-2021 om 23:04 schreef Pavel Hofman:
>>>>>>>>> Dne 19. 08. 21 v 22:10 Ferry Toth napsal(a):
>>>>>>>>>> Op 19-08-2021 om 09:51 schreef Pavel Hofman:
>>>>>>>>>>> Dne 18. 08. 21 v 21:07 Ferry Toth napsal(a):
>>>>>>>>>>>> Op 18-08-2021 om 00:00 schreef Ferry Toth:
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>>>>>>> So, where do we go from here?
>>>>>>>>>>>
>>>>>>>>>>> I know the patches have been tested on dwc2 (by me and
>>>>>>>>>>> others).  I
>>>>>>>>>>> do not know if Ruslan or Jerome tested them on dwc3 but probably
>>>>>>>>>>> not. Ruslan has talked about RPi (my case too) and
>>>>>>>>>>> BeagleboneBlack,
>>>>>>>>>>> both with dwc2. Perhaps the dwc2 behaves a bit differently than
>>>>>>>>>>> dwc3?
>>>>>>>>>>>
>>>>>>>>>>> The patches add a new EP-IN for async feedback. I am sorry I have
>>>>>>>>>>> not followed your long thread (it started as unrelated to
>>>>>>>>>>> uac). Does
>>>>>>>>>>> the problem appear with f_uac1 or f_uac2? Please how have you
>>>>>>>>>>> reached the above problem?
>>>>>>>>>>
>>>>>>>>>> I'm sorry too. I first believed the issue was related to the patch
>>>>>>>>>> mentioned in the subject line.
>>>>>>>>>>
>>>>>>>>>> The problem appaers with f_uac2. I bost Edison_Arduino board in
>>>>>>>>>> host
>>>>>>>>>> mode (there is a switch allowing to select host/device mode). When
>>>>>>>>>> flipping the switch to device mode udev run a script:
>>>>>>>>>> But as I am using configfs (excerpt follows) and just disabling
>>>>>>>>>> the
>>>>>>>>>> last 2 line resolves the issue, I'm guessing uac2 is the issue. Or
>>>>>>>>>> exceeding the available resources.
>>>>>>>>>>
>>>>>>>>>> # Create directory structure
>>>>>>>>>> mkdir "${GADGET_BASE_DIR}"
>>>>>>>>>> cd "${GADGET_BASE_DIR}"
>>>>>>>>>> mkdir -p configs/c.1/strings/0x409
>>>>>>>>>> mkdir -p strings/0x409
>>>>>>>>>>
>>>>>>>>>> # Serial device
>>>>>>>>>> mkdir functions/gser.usb0
>>>>>>>>>> ln -s functions/gser.usb0 configs/c.1/
>>>>>>>>>> ###
>>>>>>>>>>
>>>>>>>>>> # Ethernet device
>>>>>>>>>> mkdir functions/eem.usb0
>>>>>>>>>> echo "${DEV_ETH_ADDR}" > functions/eem.usb0/dev_addr
>>>>>>>>>> echo "${HOST_ETH_ADDR}" > functions/eem.usb0/host_addr
>>>>>>>>>> ln -s functions/eem.usb0 configs/c.1/
>>>>>>>>>>
>>>>>>>>>> # Mass Storage device
>>>>>>>>>> mkdir functions/mass_storage.usb0
>>>>>>>>>> echo 1 > functions/mass_storage.usb0/stall
>>>>>>>>>> echo 0 > functions/mass_storage.usb0/lun.0/cdrom
>>>>>>>>>> echo 0 > functions/mass_storage.usb0/lun.0/ro
>>>>>>>>>> echo 0 > functions/mass_storage.usb0/lun.0/nofua
>>>>>>>>>> echo "${USBDISK}" > functions/mass_storage.usb0/lun.0/file
>>>>>>>>>> ln -s functions/mass_storage.usb0 configs/c.1/
>>>>>>>>>>
>>>>>>>>>> # UAC2 device
>>>>>>>>>> mkdir functions/uac2.usb0
>>>>>>>>>> ln -s functions/uac2.usb0 configs/c.1
>>>>>>>>>> ....
>>>>>>>>>
>>>>>>>>> As you say, could perhaps the reason be that the extra EP-IN
>>>>>>>>> added in
>>>>>>>>> those patches (previously 1, now 2 with the default config you use)
>>>>>>>>> exceeds your EP-IN max count or available fifos somehow?  You
>>>>>>>>> have a
>>>>>>>>> number of functions initialized. If you change the load order of
>>>>>>>>> the
>>>>>>>>> functions, do you get the error later with a different function?
>>>>>>>>> Just
>>>>>>>>> guessing...
>>>>>>>>>
>>>>>>>>> You should be able to switch the default async EP-OUT (which
>>>>>>>>> configures the new feedback EP-IN ) to adaptive EP-OUT (which
>>>>>>>>> requires
>>>>>>>>> no feedback EP) with c_sync=8 parameter of f_uac2.
>>>>>>>>>
>>>>>>>>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/usb/gadget/function/f_uac2.c*L47__;Iw!!A4F2R9G_pg!LBySrM_zgMGV0x-zZ7nSrs54yJw1GlnpUVUVxdQE8PeszSMZ6OkFX8lSoigwRbWQzLcU$
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/usb/gadget/function/f_uac2.c*L830__;Iw!!A4F2R9G_pg!LBySrM_zgMGV0x-zZ7nSrs54yJw1GlnpUVUVxdQE8PeszSMZ6OkFX8lSoigwRfP5TdZC$
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.14-rc6/source/include/uapi/linux/usb/ch9.h*L453__;Iw!!A4F2R9G_pg!LBySrM_zgMGV0x-zZ7nSrs54yJw1GlnpUVUVxdQE8PeszSMZ6OkFX8lSoigwRejzMbWO$
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Does that fix the problem?
>>>>>>>>
>>>>>>>> Not sure how to do that. Do you mean the module should have a
>>>>>>>> parameter
>>>>>>>> called c_sync? `modinfo` list no parameters for usb_f_uac2.
>>>>>>>
>>>>>>> Those are configfs params, not available in modinfo.
>>>>>>>
>>>>>>> I checked and the value is "adaptive"
>>>>>>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.14-rc7/source/drivers/usb/gadget/function/f_uac2.c*L1312__;Iw!!A4F2R9G_pg!LBySrM_zgMGV0x-zZ7nSrs54yJw1GlnpUVUVxdQE8PeszSMZ6OkFX8lSoigwRTETcbsN$
>>>>>>>
>>>>>>
>>>>>>
>>>>>>> In your configfs script:
>>>>>>
>>>>>> Kernel shouldn't crash with any available set of configuration
>>>>>> parameters, right? So, even if the parameter would fix the crash the
>>>>>> series is buggy and has to be reverted in my opinion.
>>>>>
>>>>> Sure, no problem with reverting. I am just trying to start up some
>>>>> troubleshooting. A resource exhaustion was mentioned here, that's why I
>>>>> suggested a way to test the patch with the original number of endpoints
>>>>> allocated. I do not get any crashes on my setup which uses fewer gadget
>>>>> functions. That's why I asked what happens if the functions load order
>>>>> is changed. I am afraid this thread is so complex that the actual
>>>>> problem has been burried in the history.
>>>>>
>>>>
>>>> As I pointed out previously, the crash is probably because of f_uac2
>>>> prematurely freeing feedback request before its completion.
>>>> usb_ep_dequeue() is asynchronous. dwc2() may treat it as a synchronous
>>>> call so you didn't get a crash.
>>>
>>> Thanks for your hint, it greatly helps.
>>>>>
>>>>
>>>> I'm not sure how easy it is for you to obtain/test a device with dwc3,
>>>> but it would be great to also have it as part of your testing suite. :)
>>>
>>> Can you recommend a reasonably priced device with viable kernel
>>> updates for the testing? Optionally with SS which you mentioned last
>>> time? Thanks.
>>>
>> Edison-Arduino kit 2nd hand with display on ebay ~$100 :-)
>>
> 
> Ferry can correct me if I'm wrong, but I think Edison-Arduino kit only
> supports up to highspeed. Regardless, Edison-Arduino seems to work well
> with the latest Linux kernel.

The usb phy TUSB1211 is built into the Edison module so the high speed 
limitation applies not only to Edison-Arduino kit. The kit does have a 
HW switch to switch between host / device mode which makes it perfect 
for testing. Other then that, Edison-Arduino currently receives acpi 
tables from U-Boot and configfs and works well in i686 and x86_64 since 
4.19. USB device mode works well since 5.13 with one patch to extcon or 
with a few patches to 5.10. The extcon patch landed in 5.14.

> I see that there are various development kits with rk3399 that supports
> up to SuperSpeed at reasonable price, but I think they all require some
> effort to bring up to the latest Linux kernel and in device mode.
> 
> Maybe Ferry/Felipe/anyone can provide more recommendations?
> 
> Thanks,
> Thinh
> 

Powered by blists - more mailing lists