Re: [SPAM] Re: [PATCH] cmd: usb: Prevent reset in usb tree/info command

[Xavier Drudis Ferran <xdrudis@tinet.cat>]

El Tue, Jun 20, 2023 at 11:49:36AM +0200, Marek Vasut deia:
> 
> Default, see:
> $ git grep CONFIG_BOOTCOMMAND configs/
>

I'm lost.

I called default what Kconfig used as default.
You seem to call default what's in board specific config files.
Whatever. Fix the wording in the commit message if you like.


> 
> So, what is the minimal test case ?
> I have been asking for that for a while.
>

I sent a minimal test case last week.

https://lists.denx.de/pipermail/u-boot/2023-June/520109.html

You build for a Rock Pi 4 board, boot with usb stick and no boot media
and run usb info and you get a reset.

I won't send it again because I can't guess what you consider minimal.

> 
> I would really like a minimal test case. Empty your env and figure out the
> commands which need to be executed to trigger this. Without any interference
> from other commands/scripting/...
>

I'm sorry but if what I sent isn't enough I don't think I'll have time
to help you any further. Find your minimal test case yourself or
ignore my patch.

> > If it's just that you can't reproduce it, can you try to ?:
> > 
> > - set up a board with no boot media (I tested like this but it might
> >    not be needed),
> > 
> > - put usb in boot_targets (if you put only usb there you may not need
> >    the previous step):
> >    setenv boot_targets usb
> 
> Here you assume distro bootcommand or some such . Can we remove that
> assumption ? (I think yes, and we should)
>

I don't think I'm assuming anything about bootcommand. That's
precisely why I wrote these steps instead of the "just boot a Rock Pi
4" scenario last week.

The commit message talks about bootcmd because it justifies
that the bootflow scan will be called automatically in some cases,
so the bug has more impact that it would otherwise have.

But the bug should appear whether or not you have bootcmd.
The bug should be an interaction between what bootflow scan does
and what usb info or usb tree do.

I'm assuming bootflow reads the boot_targets environment variable to
know where it searches for boot devices, and therefore to which
devices it will attach a UCLASS_BOOTDEV child to some devices, in
particular to usb mass storage devices if any is present, that will
later break usb info/usb tree. Whether bootflow is called from bootcmd
or not should be irrelevant.

If you follow the code from the bootflow command you may find
yourself that the boot_targets variable is involved. I did it
last week or sometime and won't do it again now for you, sorry.
I know I may have misunderstood something, and I'm sorry for the
noise if so.

> > - plug a non-booting usb mass storage device to an usb port,
> > 
> > - run usb reset in case you already had usb initialized at boot, or
> >    skip this if usb is not initialized yet. If in doubt, run usb reset.
> > 
> > - run bootflow scan
> > 
> > - run usb info
> > 
> > It should list some devices, but give you a reset just after listing the
> > usb mass storage device without my patch, and it should just list all
> > usb devices and go back to the prompt with my patch.
> 
> Does it crash if you empty your env and run simply
> 
> => usb reset ; bootflow scan ; usb info
> 
> ?

I guess it won't crash if environment var boot_targets is absent or
empty.  Or even if it's full but has no "usb" in it. But I'm not sure
anymore at what time the variable is read, so it might be that
emptying it when bootflow structures are already set up wouldn't
change things, I don't know, I don't remember.

But I won't try it, sorry.

I found a bug, I sent a 4 line patch. Since I didn't justify it
enough I sent followup mails on how to reproduce.
This week I sent a second version, with redundant measures to seek
consensus. It's a 6 line patch or 8 or whatever.
I didn't write bootflow, and I didn't write cmd/usb.c

And I don't have time to keep writing long emails. I'm sorry.  Not
even to count how many lines of text I wrote compared to the 8 lines
of code or whatever. If someone has the bureaucratic skills and
patience to pursue this further, they can do what they want with my
patch (under GPL2 as implied by the sign off). If not, you all can
keep your bugs, I won't try anymore to steal them.

Bye and sorry for any disturbances.