On Mon, Aug 23, 2021 at 5:59 PM Pavel Hofman [off-list ref] wrote:

Dne 22. 08. 21 v 21:43 Ferry Toth napsal(a):

quoted

Op 19-08-2021 om 23:04 schreef Pavel Hofman:

quoted

Dne 19. 08. 21 v 22:10 Ferry Toth napsal(a):

quoted

Op 19-08-2021 om 09:51 schreef Pavel Hofman:

quoted

Dne 18. 08. 21 v 21:07 Ferry Toth napsal(a):

quoted

Op 18-08-2021 om 00:00 schreef Ferry Toth:

...

quoted

quoted

quoted

quoted

quoted

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://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/usb/gadget/function/f_uac2.c#L47

https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/usb/gadget/function/f_uac2.c#L830

https://elixir.bootlin.com/linux/v5.14-rc6/source/include/uapi/linux/usb/ch9.h#L453

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://elixir.bootlin.com/linux/v5.14-rc7/source/drivers/usb/gadget/function/f_uac2.c#L1312

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.

# UAC2 device
mkdir functions/uac2.usb0
ln -s functions/uac2.usb0 configs/c.1


On the USB Host:
cat /proc/asound/Gadget/stream:
Playback:
   Status: Stop
   Interface 1
     Altset 1
     Format: S16_LE
     Channels: 2
     Endpoint: 0x01 (1 OUT) (ASYNC)
     Rates: 64000
     Data packet interval: 125 us
     Bits: 16
     Channel map: FL FR
     Sync Endpoint: 0x82 (2 IN)
     Sync EP Interface: 1
     Sync EP Altset: 1
     Implicit Feedback Mode: No

lsusb -v -d 1d6b:0104 | | grep EP.*IN
         bEndpointAddress     0x81  EP 1 IN
         bEndpointAddress     0x82  EP 2 IN
         bEndpointAddress     0x83  EP 3 IN

I have additional patches applied which define controls via EP IN
interrupt mode, not part of that patchset.

Switching to the adaptive mode:
# UAC2 device
mkdir functions/uac2.usb0
echo "adaptive" > functions/uac2.usb0/c_sync
ln -s functions/uac2.usb0 configs/c.1

On the USB Host:
cat /proc/asound/Gadget/stream:
Playback:
   Status: Stop
   Interface 1
     Altset 1
     Format: S16_LE
     Channels: 2
     Endpoint: 0x01 (1 OUT) (ADAPTIVE)
     Rates: 64000
     Data packet interval: 125 us
     Bits: 16
     Channel map: FL FR

lsusb -v -d 1d6b:0104 | grep EP.*IN
         bEndpointAddress     0x81  EP 1 IN
         bEndpointAddress     0x82  EP 2 IN

The feedback EP-IN is gone because the mode is adaptive now.

If you think the new input endpoints should not be created by default
for resource-compatibility reasons, the adaptive mode can be set by
default in a fixed patch.

Would it be possible to change the mode? If so, then the user may
configure it and crash again.

Also the patches defining the new interrupt endpoints can have the
controls disabled by default (and not allocate the EP-IN). These patches
on top of the async patches are already accepted by Greg for usb-next
branch
https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/usb/+/eaf6cbe0992052a46d93047dc122fad5126aa3bd
. That's why I was trying to sort out the async patches without
reverting them as it will call for more reverts.

-- 
With Best Regards,
Andy Shevchenko