RE: [PATCH v2 1/2] usb: gadget: f_uac2: Stop endpoint before enabling it.
From: Felipe Balbi <balbi@kernel.org>
Date: 2021-04-28 10:33:43
Hi, Pawel Laszczak [off-list ref] writes:
quoted
Alan Stern [off-list ref] writes:quoted
On Mon, Apr 26, 2021 at 03:52:46PM +0300, Felipe Balbi wrote:quoted
yeah, this is a requirement by the spec, IIRC. A SetAlt to the same interface/altSetting means the host wants to reset that altSetting. From the peripheral point of view that means disabling the endpoints and reenabling them. I'm just not entirely sure if we should do this in u_audio or f_uac[12].c. Arguably, composite.c could detect this and disable the altSetting, but that would require a huge refactor on the framework. My gut feeling is that for the minimal bug fix, we should patch f_uac[12].c, but all audio function drivers have the same exact bug, so I don't know. If we follow the "standard" of patching the relevant set_alt functions in the function drivers, the moment we decide to go for a refactoring, it'll be easier to see common constructs.FWIW, f_mass_storage.c handles this in its do_set_interface() routine. That routine first deallocates any related request buffers and disables any enabled endpoints in the interface. It then goes on to enable endpoints for the new alternate setting and allocate request buffers. The audio function drivers could follow this approach.right, that's what all other drivers do, in fact. Only audio seems to be different. The question here is whether to patch every f_uac*.c (there are three of them) or patch it in the generic u_audio.cI agree that the best solution is to create common implementation in composite.c. Maybe usb_function->get_alt and usb-function->set_alt will be enougt to detect such case from composite.c. The problem can be with testing such change with all functions. For fast fix bug I don't have opinion which place is better u_audio.c or f_uac*.c. First version of this patch makes change only in f_uac2.c and the second Version moved fix to u_audio.c (as suggested Peter).
okay, I missed that Peter had already asked you to move to u_audio.c. I guess we should go with your patch, but it would be nice to get some Tested-bys. Peter, would you be willing to provide some testing for this patch?
Change in u_audio is simpler, and as wrote Felipe is common for all UAC drivers. Maybe for fast fix it's better. If you want, you can feel free to change and modify this patch.
heh, that's not how things work around here :-)
It is important for me to fix this issue because it was hard to debug.
yup, no question. -- balbi
Attachments
- signature.asc [application/pgp-signature] 857 bytes