On 25.07.2022 11:25, AKASHI Takahiro wrote: > On Sat, Jul 23, 2022 at 05:03:39PM +0200, Jan Palus wrote: > > On 23.07.2022 16:43, Jan Palus wrote: > > > On 23.07.2022 16:19, Jan Palus wrote: > > > > On 22.07.2022 02:59, Simon Glass wrote: > > > > > Hi Jan, > > > > > > > > > > On Mon, 18 Jul 2022 at 11:48, Jan Palus <[email protected]> wrote: > > > > > > > > > > > > u-boot 2022.07 boots fine without any USB devices attached to > > > > > > RaspberryPi 2 however it hangs early on if external USB drive is > > > > > > connected, right after: > > > > > > > > > > > > Request Sense returned 02 04 01 > > > > > > > > > > > > git bisect indicates first commit to cause regression is: > > > > > > > > > > > > 8c9812a5d557c4eacf164147d7380b3af1b222ec is the first bad commit > > > > > > commit 8c9812a5d557c4eacf164147d7380b3af1b222ec > > > > > > Author: AKASHI Takahiro <[email protected]> > > > > > > Date: Tue Mar 8 20:36:40 2022 +0900 > > > > > > > > > > > > usb: storage: call device_probe() after scanning > > > > > > > > > > > > Every time a usb bus/port is scanned and a new device is > > > > > > detected, > > > > > > we want to call device_probe() as it will give us a chance to > > > > > > run > > > > > > additional post-processings for some purposes. > > > > > > > > > > > > In particular, support for creating partitions on a device > > > > > > will be added. > > > > > > > > > > > > Signed-off-by: AKASHI Takahiro <[email protected]> > > > > > > Reviewed-by: Simon Glass <[email protected]> > > > > > > > > > > > > Reverting this commit fixes the issue. > > > > > > > > > > > > Note that USB drive is TOSHIBA MQ04UBD200 and it's not used for > > > > > > booting. > > > > > > Also note that without this change 0 storage devices are detected > > > > > > even > > > > > > when drive is attached. > > > > > > > > > > I am not sure what is going on here. Can you provide the full console > > > > > trace of the boot? Any idea where it is hanging? > > > > > > > ... > > > > > > > > Now the place where it hangs is: > > > > > > > > part_efi.c: > > > > > > > > static int part_test_efi(struct blk_desc *dev_desc) > > > > { > > > > ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, legacymbr, 1, > > > > dev_desc->blksz); > > > > > > > > where dev_desc->blksz is 3782209548. > > > > > > > > > > So it appears block size is read incorrectly? Should be 512 but not sure > > > where this value 3782209548 is coming from, it's not block capacity > > > either. After plugging in Linux reports: > > > > > > sd 0:0:0:0: [sda] 3907029168 512-byte logical blocks: (2.00 TB/1.82 TiB) > > > > Since this was logged: > > > > Device NOT ready > > Request Sense returned 02 04 01 > > > > then it seems capacity/block size were never determined (happens right > > after these log messages if they *don't* occur) so I guess they are > > random values and this usb storage should never have been probed? I'll > > stop here due to my cluelessness. > > The code looks like: > usb_stor_probe_device() > ... > blk_create_devicef(); > ret = usb_stor_get_info(udev, data, blkdev); > if (ret == 1) { > usb_max_devs++; > debug("%s: Found device %p\n", __func__, udev); > } else { > debug("usb_stor_get_info: Invalid device\n"); > ret = device_unbind(dev); > if (ret) > return ret; > <== (A) > } > > blk_probe_or_unbind(dev); > ... > > > usb_stor_get_info() returns 0 when it generates "Device NOT ready" message. > Then blk_probe_or_unbind(), hence part_test_efi(), is accidentally called > although blkdev is not fully initialised/populated. > > I think we should skip the subsequent processing by adding "continue" at (A).
I see there is a change proposed in ml in the same place: https://lists.denx.de/pipermail/u-boot/2022-July/489531.html Though I'm also wondering "Request Sense returned 02 04 01" from a quick search appears to indeed mean "Device NOT ready", but "it's becoming ready" so in my case that's likely about spinning up HDD plates. I wonder if it shouldn't be treated like -EAGAIN with some timeout as in second or two the drive would likely report it is ready.
