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.