regressions.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Marcelo Roberto Jimenez <marcelo.jimenez@gmail.com>
Cc: Cristian Birsan <Cristian.Birsan@microchip.com>,
	Jonas Bonn <jonas@norrbonn.se>,
	regressions@lists.linux.dev,
	Nicolas Ferre <Nicolas.Ferre@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Ludovic Desroches <Ludovic.Desroches@microchip.com>,
	linux-arm-kernel@lists.infradead.org, linux-usb@vger.kernel.org,
	Felipe Balbi <balbi@kernel.org>,
	Sergio Tanzilli <tanzilli@acmesystems.it>
Subject: Re: [PATCH] usb: gadget: atmel: Revert regression in USB Gadget (atmel_usba_udc)
Date: Mon, 20 Dec 2021 15:50:13 +0100	[thread overview]
Message-ID: <YcCYJeGTFIhxK62s@kroah.com> (raw)
In-Reply-To: <CACjc_5qZjXbE1CwQCpc4+vzbsobfn5YKpU=UCVJpMGG1ROEfTg@mail.gmail.com>

On Wed, Dec 15, 2021 at 12:54:49PM -0300, Marcelo Roberto Jimenez wrote:
> Hi Cristian,
> 
> On Wed, Dec 15, 2021 at 10:04 AM <Cristian.Birsan@microchip.com> wrote:
> >
> > Hi Marcelo, Jonas,
> >
> > On 12/13/21 4:31 PM, Marcelo Roberto Jimenez wrote:
> > >
> > > Some people who received this message don't often get email from marcelo.jimenez@gmail.com. Learn why this is important <http://aka.ms/LearnAboutSenderIdentification>
> 
> Hum, shame on me.
> 
> > > Hi Jonas,
> > >
> > > Thank you for the quick response, I really appreciate it.
> > >
> > > On Mon, Dec 13, 2021 at 7:02 AM Jonas Bonn <jonas@norrbonn.se <mailto:jonas@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.
> >
> > Is it possible to add a wire that connects VBUS to the right pin on the MPU ? Just to see if this has an impact or not ?
> 
> Yes. Maybe I was not clear about that issue, so let me try to clarify.
> The board _seems_ to have a provision for VBUS sensing, but it is not
> working. I was not involved in this board's project and I found no
> other documentation on the ACME Systems site, all I am saying here is
> from reading the circuit diagram, so it is all my personal
> interpretation. For hardware reasons, the aforementioned VBUS sensing
> pin does not reach zero volts when the USB connector is removed, which
> makes VBUS sensing ineffective. But I have tested it anyway and to
> make it work, the first step is to prepare the board as specified here
> https://www.acmesystems.it/arietta_power_supply in the section "Supply
> power at 3.3 volt". The key here is to remove the R36 resistor, so
> that the board can be fed by an external 3.3V voltage and become self
> powered. Then, you add a line "atmel,vbus-gpio = <&pioB 16 0>;" below
> the "usb2:" line in the Arietta DTD. After recompiling the kernel and
> installing, it still does not work by just unplugging the USB cable.
> You need to manually and carefully (!) short circuit the +5 USB line
> to the ground when the cable is not connected. Only then VBUS sensing
> will work and the device will accept enumeration again.
> 
> In short, the VBUS sensing code in the kernel is ok. But that is not
> my point. My point is that the kernel code implies that it is possible
> to do without a VBUS sensing pin. The file
> Documentation/devicetree/bindings/usb/atmel-usb.txt states that
> "atmel,vbus-gpio" is an optional property. So, it seems to me like the
> code should work without it, and indeed it worked. That is why I have
> called this a regression.

Yes, hardware that used to work, and now does not, is a regression.

So, should I revert the offending patch here?  Adding new features is
not a good reason to keep things that break systems.  Or is there a
potential fix in this thread that I missed?

thanks,

greg k-h

  reply	other threads:[~2021-12-20 15:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-11 18:36 Marcelo Roberto Jimenez
2021-12-12  9:48 ` Thorsten Leemhuis
2022-01-26 12:20   ` Greg Kroah-Hartman
2022-01-26 12:54     ` Thorsten Leemhuis
     [not found]     ` <CACjc_5o9eO5YTVt4jE7v1E9nirCwFMc1=Ub-jXSFCg1_8A2M-A@mail.gmail.com>
2022-01-26 13:43       ` Marcelo Roberto Jimenez
2021-12-13 10:02 ` Jonas Bonn
     [not found]   ` <CACjc_5pHbLStniQnOVLDW5iLbKn8ovePuQsFFqeEnQPSSYxJoQ@mail.gmail.com>
2021-12-15 13:04     ` Cristian.Birsan
2021-12-15 15:54       ` Marcelo Roberto Jimenez
2021-12-20 14:50         ` Greg Kroah-Hartman [this message]
2021-12-22 18:33           ` Cristian.Birsan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YcCYJeGTFIhxK62s@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Cristian.Birsan@microchip.com \
    --cc=Ludovic.Desroches@microchip.com \
    --cc=Nicolas.Ferre@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=balbi@kernel.org \
    --cc=jonas@norrbonn.se \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=marcelo.jimenez@gmail.com \
    --cc=regressions@lists.linux.dev \
    --cc=tanzilli@acmesystems.it \
    --subject='Re: [PATCH] usb: gadget: atmel: Revert regression in USB Gadget (atmel_usba_udc)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).