Re: [PATCH v10 0/6] Re-introduce TX FIFO resize for larger EP bursting

[Ferry Toth]

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@xxxxxxxxxxx> 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 :-)

> Best regards,
>
> Pavel.