Fwd: [PATCH] usb: gadget: atmel: Revert regression in USB Gadget (atmel_usba_udc)

[Marcelo Roberto Jimenez]

Resending to the list, due to gmail html rejection, sorry for that.

---------- Forwarded message ---------
From: Marcelo Roberto Jimenez <marcelo.jimenez at gmail.com>
Date: Mon, Dec 13, 2021 at 11:31 AM
Subject: Re: [PATCH] usb: gadget: atmel: Revert regression in USB
Gadget (atmel_usba_udc)
To: Jonas Bonn <jonas at norrbonn.se>
Cc: <regressions at lists.linux.dev>, Nicolas Ferre
<nicolas.ferre at microchip.com>, Alexandre Belloni
<alexandre.belloni at bootlin.com>, Ludovic Desroches
<ludovic.desroches at microchip.com>,
<linux-arm-kernel at lists.infradead.org>, Cristian Birsan
<cristian.birsan at microchip.com>, <linux-usb at vger.kernel.org>, Greg
Kroah-Hartman <gregkh at linuxfoundation.org>, Felipe Balbi
<balbi at kernel.org>, Sergio Tanzilli <tanzilli at acmesystems.it>


Hi Jonas,

Thank you for the quick response, I really appreciate it.

On Mon, Dec 13, 2021 at 7:02 AM Jonas Bonn <jonas at norrbonn.se> wrote:
>
>
> Given that the patch that you want to revert is almost 3 years old, it's
> a bit of a stretch to call this a regression.  I suspect that a cleaner
> way forward is to introduce some kind of quirk for your board.


Well, the board is basically the MPU, so if there is a hardware
problem it would mean that it is a problem with the on chip
peripheral.

>
> I have a self-powered board where the USB suspend sequence works and
> device add/remove works fine.  So fundamentally, I suspect that the
> driver is ok.


Maybe you have VBUS sensing enabled in your board. As I reported
before, the VBUS sensing is not an option in this board. Nonetheless,
the code in the kernel suggests that VBUS sensing is optional, since
the presence of a VBUS sensing pin is explicitly tested in it.

I have not read the full USB spec, but if this is a fundamentally not
resolvable problem, maybe we could have a configuration option,
runtime or compile time, to enable or disable SUSPEND state, assuming
that there is no problem with the original patch.

In my particular application, it is more important to detect the
disconnection, so that we can reconnect, than to enter SUSPEND.
Whenever USB is not necessary, it will not be connected, so the energy
saved will be very little in my case.

My intention with this patch is also to provide a quick fix for anyone
facing this problem, reverting is not necessarily the best long term
solution, especially if the code is found to be correct. But assuming
the chip USB controller has no design flaws, and assuming it should be
possible to do without VBUS sensing, then the present code should be
missing some detail.

>
> With all that said, I'm not immediately in a position to run tests on my
> SAMA5D2 board right now and the kernel on that board is 5.2.  I'm not
> sure when we we would notice that USB suspend stopped working because
> there is no kernel maintenance planned for that board, as far as I know;
> someday, however, that work might happen and the lack of working USB
> suspend will be a regression in and of itself.


I can test it with the AT91SAM9G25 processor and I can also get a
SAMA5D27 board to test with.

>
> Thanks,
> Jonas


Best regards,
Marcelo.