All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Tom Rini <trini@konsulko.com>
Cc: Marek Vasut <marex@denx.de>,
	u-boot@lists.denx.de, Kevin Hilman <khilman@baylibre.com>,
	Lukasz Majewski <lukma@denx.de>, Simon Glass <sjg@chromium.org>
Subject: Re: [PATCH v4 1/4] cmd: bind: Add unbind command with driver filter
Date: Fri, 4 Aug 2023 19:04:31 +0200	[thread overview]
Message-ID: <20230804190431.6f21efd8@xps-13> (raw)
In-Reply-To: <20230804163757.GS3630934@bill-the-cat>

Hi Tom,

> > > >>>>> Cannot find a device with path /ocp/usb@47400000/usb@47401000    
> > > >>>>> => unbind /ocp/usb@47400000/usb@47401000
> > > >>>>> => dm tree    
> > > >>>>>     misc          0  [ + ]   ti-musb-wrapper       |   |-- usb@47400000
> > > >>>>>     usb           0  [   ]   ti-musb-host          |   |   `-- usb@47401800    
> > > >>>>> => fastboot usb 0
> > > >>>>> => bind /ocp/usb@47400000/usb@47401000 ti-musb-peripheral
> > > >>>>> => dm tree    
> > > >>>>>     misc          0  [ + ]   ti-musb-wrapper       |   |-- usb@47400000
> > > >>>>>     usb           0  [   ]   ti-musb-host          |   |   |-- usb@47401800
> > > >>>>>     usb           0  [   ]   ti-musb-peripheral    |   |   `-- usb@47401000    
> > > >>>>> => fastboot usb 0    
> > > >>>>> musb-hdrc: peripheral reset irq lost!
> > > >>>>> # works! (the irq-related line above as always been there)
> > > >>>>>
> > > >>>>> So now, how do we make this process easy/understandable?    
> > > >>>>
> > > >>>> What would be your proposal ?    
> > > > 
> > > > At least I would appreciate:
> > > > - to select CMD_BIND "by default" when relevant
> > > > - to make the fastboot error more readable for the regular user    
> > > 
> > > Since with this 'unbind ethernet 0' this is orthogonal to this series, send separate patches, thanks.  
> > 
> > This is not orthogonal, I am sorry.
> > 
> > version X:
> > - tftp works "out of the box"
> > - fastboot works "out of the box"
> > version X+1:
> > - tftp works "out of the box"
> > - fastboot returns an obscure error
> > 
> > 1/ If we now *need* the bind/unbind commands, the series must take care
> >    of it.
> > 2/ Without proper error message you just break fastboot for most
> >    regular users (basically everyone but few U-Boot devs).  
> 
> You're missing the class of users that will be impacted here.  In order
> for there to be a change here, you have to already be in the case where
> you have CONFIG_USB_ETHER=y and gadget ethernet device isn't just
> enabled but also initialized by default by calling usb_ether_init().
> That's a very small list.  It's basically am33xx, two mediatek reference
> platforms and xilinx_zynqmp_virt.  Given that am33xx defconfigs also
> setup DFU, I'm not really sure just how many people use gadget ethernet.
> The normal flow on modern devices is to be calling bind/unbind here
> already.

Can we make this behavior explicit to the user? I am sorry, maybe it is
the normal flow for you, but I am a regular U-Boot user and I totally
missed that requirement.

Typical situation: one needs to use <whatever-gadget> but none is bound
to the UDC (or another is bound), could we make the error messages more
explicit if we decide not to unbind/bind the right one automatically
because it is too "costly"?

Thanks,
Miquèl

  reply	other threads:[~2023-08-04 17:04 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-02 12:46 [PATCH v4 1/4] cmd: bind: Add unbind command with driver filter Marek Vasut
2023-08-02 12:46 ` [PATCH v4 2/4] usb: gadget: ether: Inline functions used once Marek Vasut
2023-08-02 12:46 ` [PATCH v4 3/4] usb: gadget: ether: Move probe function above driver structure Marek Vasut
2023-08-02 12:46 ` [PATCH v4 4/4] usb: gadget: ether: Handle gadget driver registration in probe and remove Marek Vasut
2023-08-02 21:31 ` [PATCH v4 1/4] cmd: bind: Add unbind command with driver filter Simon Glass
2023-08-02 22:04   ` Marek Vasut
2023-08-04  7:00 ` Miquel Raynal
2023-08-04 14:42   ` Marek Vasut
2023-08-04 15:01     ` Tom Rini
2023-08-04 15:05       ` Marek Vasut
2023-08-04 15:12         ` Miquel Raynal
2023-08-04 15:40           ` Marek Vasut
2023-08-04 16:00             ` Miquel Raynal
2023-08-04 16:15               ` Tom Rini
2023-08-04 17:01                 ` Miquel Raynal
2023-08-04 17:18                   ` Marek Vasut
2023-08-04 17:20                   ` Tom Rini
2023-08-04 18:01                     ` Miquel Raynal
2023-08-04 18:51                       ` Tom Rini
2023-08-04 19:38                         ` Miquel Raynal
2023-08-04 16:37               ` Tom Rini
2023-08-04 17:04                 ` Miquel Raynal [this message]
2023-08-04 17:19                   ` Marek Vasut
2023-08-04 17:23                   ` Tom Rini
2023-08-04 17:24             ` Miquel Raynal
2023-08-04 17:31               ` Marek Vasut
2023-08-04 17:46                 ` Miquel Raynal
2023-08-04 17:54                   ` Marek Vasut

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=20230804190431.6f21efd8@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=khilman@baylibre.com \
    --cc=lukma@denx.de \
    --cc=marex@denx.de \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.