| Message ID | 20210826185739.3868-1-ftoth@exalondelft.nl |
|---|---|
| Headers | show |
| Series | Revert "usb: gadget: u_audio: add real feedback implementation" | expand |
On Thu 26 Aug 2021 at 20:57, Ferry Toth <ftoth@exalondelft.nl> wrote: > Although there is a patch resolving this issue (see > https://lore.kernel.org/linux-usb/1jilzsy8g7.fsf@starbuckisacylon.baylibre.com/T/#u) > it needs a little work and will not be ready for v5.14.0 release. Until then > revert the series. Seems like a quite messy solution when the fix available :/ Change with the updated commit description is avaialable BTW [0] [0]: https://lore.kernel.org/20210827075853.266912-1-jbrunet@baylibre.com > > v2: > - Added SoB (Balbi) > > > Ferry Toth (3): > Revert "usb: gadget: u_audio: add real feedback implementation" > Revert "usb: gadget: f_uac2: add adaptive sync support for capture" > Revert "usb: gadget: f_uac2/u_audio: add feedback endpoint support" > > .../ABI/testing/configfs-usb-gadget-uac2 | 2 - > Documentation/usb/gadget-testing.rst | 2 - > drivers/usb/gadget/function/f_uac2.c | 144 +---------- > drivers/usb/gadget/function/u_audio.c | 225 +----------------- > drivers/usb/gadget/function/u_audio.h | 12 - > drivers/usb/gadget/function/u_uac2.h | 4 - > 6 files changed, 6 insertions(+), 383 deletions(-)
On Thu, Aug 26, 2021 at 08:57:36PM +0200, Ferry Toth wrote: > Although there is a patch resolving this issue (see > https://lore.kernel.org/linux-usb/1jilzsy8g7.fsf@starbuckisacylon.baylibre.com/T/#u) > it needs a little work and will not be ready for v5.14.0 release. Until then > revert the series. revert the series for what? 5.14-final needs these reverts? Or are these for 5.15-rc1? confused, greg k-h
On Thu, Aug 26, 2021 at 08:57:37PM +0200, Ferry Toth wrote: > This reverts commit e89bb4288378b85c82212b60dc98ecda6b3d3a70. > > The commit is part of a series with commit > 24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3 > hardware, at least on Intel Merrifield platform when configured > through configfs: > BUG: kernel NULL pointer dereference, address: 0000000000000008 > ... > RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0 > ... > Call Trace: > dwc3_remove_requests.constprop.0+0x12f/0x170 > __dwc3_gadget_ep_disable+0x7a/0x160 > dwc3_gadget_ep_disable+0x3d/0xd0 > usb_ep_disable+0x1c/0x70 > u_audio_stop_capture+0x79/0x120 [u_audio] > afunc_set_alt+0x73/0x80 [usb_f_uac2] > composite_setup+0x224/0x1b90 [libcomposite] > > Pavel's suggestion to add > `echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script > resolves the issue. > Thinh suggests "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." > > Revert as this is a regression and the kernel shouldn't crash depending > on configuration parameters. Are these normal configuration options in the wild, or is this just something that you "can do"? > Signed-off-by: Ferry Toth <ftoth@exalondelft.nl> I need an ack from the original authors to revert all this... thanks, greg k-h
On Fri, Aug 27, 2021 at 11:05 AM Jerome Brunet <jbrunet@baylibre.com> wrote: > On Thu 26 Aug 2021 at 20:57, Ferry Toth <ftoth@exalondelft.nl> wrote: > > > Although there is a patch resolving this issue (see > > https://lore.kernel.org/linux-usb/1jilzsy8g7.fsf@starbuckisacylon.baylibre.com/T/#u) > > it needs a little work and will not be ready for v5.14.0 release. Until then > > revert the series. > > Seems like a quite messy solution when the fix available :/ Is it? AFAIR Thing pointed out to some problems with the proposed solution. Have those been addressed? > Change with the updated commit description is avaialable BTW [0] > [0]: https://lore.kernel.org/20210827075853.266912-1-jbrunet@baylibre.com Dunno, but this gives me 404. -- With Best Regards, Andy Shevchenko
Op 27-08-2021 om 10:29 schreef Greg Kroah-Hartman: > On Thu, Aug 26, 2021 at 08:57:37PM +0200, Ferry Toth wrote: >> This reverts commit e89bb4288378b85c82212b60dc98ecda6b3d3a70. >> >> The commit is part of a series with commit >> 24f779dac8f3efb9629adc0e486914d93dc45517 causing a BUG on dwc3 >> hardware, at least on Intel Merrifield platform when configured >> through configfs: >> BUG: kernel NULL pointer dereference, address: 0000000000000008 >> ... >> RIP: 0010:dwc3_gadget_del_and_unmap_request+0x19/0xe0 >> ... >> Call Trace: >> dwc3_remove_requests.constprop.0+0x12f/0x170 >> __dwc3_gadget_ep_disable+0x7a/0x160 >> dwc3_gadget_ep_disable+0x3d/0xd0 >> usb_ep_disable+0x1c/0x70 >> u_audio_stop_capture+0x79/0x120 [u_audio] >> afunc_set_alt+0x73/0x80 [usb_f_uac2] >> composite_setup+0x224/0x1b90 [libcomposite] >> >> Pavel's suggestion to add >> `echo "adaptive" > functions/uac2.usb0/c_sync` to the configfs script >> resolves the issue. >> Thinh suggests "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." >> >> Revert as this is a regression and the kernel shouldn't crash depending >> on configuration parameters. > > Are these normal configuration options in the wild, or is this just > something that you "can do"? It's a work around suggested by Pavel, a line that can be added to configfs script. >> Signed-off-by: Ferry Toth <ftoth@exalondelft.nl> > > I need an ack from the original authors to revert all this... A fix has appeared: https://lore.kernel.org/linux-usb/1jfsuvw817.fsf@starbuckisacylon.baylibre.com/T/#m922149b7e74204c8ed1bed2c624910ab4eafd43c This has been acked by Felipe. If we can get that in 5.14.0 it would be preferable to reverting this series. > thanks, > > greg k-h >
Op 27-08-2021 om 11:15 schreef Andy Shevchenko: > On Fri, Aug 27, 2021 at 11:05 AM Jerome Brunet <jbrunet@baylibre.com> wrote: >> On Thu 26 Aug 2021 at 20:57, Ferry Toth <ftoth@exalondelft.nl> wrote: >> >>> Although there is a patch resolving this issue (see >>> https://lore.kernel.org/linux-usb/1jilzsy8g7.fsf@starbuckisacylon.baylibre.com/T/#u) >>> it needs a little work and will not be ready for v5.14.0 release. Until then >>> revert the series. >> >> Seems like a quite messy solution when the fix available :/ > > Is it? AFAIR Thing pointed out to some problems with the proposed > solution. Have those been addressed? > >> Change with the updated commit description is avaialable BTW [0] > >> [0]: https://lore.kernel.org/20210827075853.266912-1-jbrunet@baylibre.com > > Dunno, but this gives me 404. > I think: https://lore.kernel.org/linux-usb/1jfsuvw817.fsf@starbuckisacylon.baylibre.com/T/#m922149b7e74204c8ed1bed2c624910ab4eafd43c
Hi Op 27-08-2021 om 10:27 schreef Greg Kroah-Hartman: > On Thu, Aug 26, 2021 at 08:57:36PM +0200, Ferry Toth wrote: >> Although there is a patch resolving this issue (see >> https://lore.kernel.org/linux-usb/1jilzsy8g7.fsf@starbuckisacylon.baylibre.com/T/#u) >> it needs a little work and will not be ready for v5.14.0 release. Until then >> revert the series. > > revert the series for what? 5.14-final needs these reverts? Or are > these for 5.15-rc1? > > confused, Apologies. Yes, the idea was to revert for 5.14-final as the series creates a regression. However, an updated patch "usb: gadget: f_uac2: fixup feedback endpoint stop" https://lore.kernel.org/linux-usb/1jfsuvw817.fsf@starbuckisacylon.baylibre.com/T/#m922149b7e74204c8ed1bed2c624910ab4eafd43c now has been acked by Felipe. If we can get that it for 5.14-final it would be less messy. Can we? > greg k-h >