[U-Boot] "usb storage" command issues

[Stefan Roese]

Hi Simon,

On 29.11.2017 14:08, Simon Glass wrote:
> Hi Stefan,
> 
> On 28 November 2017 at 02:40, Stefan Roese <sr at denx.de> wrote:
>> Hi Bin, Hi Simon,
>>
>> On 20.11.2017 16:38, Simon Glass wrote:
>>>
>>> On 20 November 2017 at 00:07, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>
>>>>
>>>> Hi Stefan,
>>>>
>>>> On Wed, Sep 27, 2017 at 1:03 AM, Stefan Roese <sr at denx.de> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I'm currently testing USB on my x86 platform. And noticed, that
>>>>> the "usb storage" command does not work as expected:
>>>>>
>>>>> => usb reset
>>>>> resetting USB...
>>>>> USB0:   Register 7000820 NbrPorts 7
>>>>> Starting the controller
>>>>> USB XHCI 1.00
>>>>> scanning bus 0 for devices... 5 USB Device(s) found
>>>>>          scanning usb for storage devices... 1 Storage Device(s) found
>>>>> => usb tree
>>>>> USB device tree:
>>>>>     1  Hub (5 Gb/s, 0mA)
>>>>>     |  U-Boot XHCI Host Controller
>>>>>     |
>>>>>     +-2  Mass Storage (480 Mb/s, 224mA)
>>>>>     |    SanDisk Ultra 4C530001010620110505
>>>>>     |
>>>>>     +-3  Hub (480 Mb/s, 0mA)
>>>>>       |
>>>>>       +-4  Hub (480 Mb/s, 100mA)
>>>>>         |
>>>>>         +-5  Hub (12 Mb/s, 100mA)
>>>>>
>>>>> => usb storage
>>>>> Card did not respond to voltage select!
>>>>> mmc_init: -95, time 28
>>>>> No storage devices, perhaps not 'usb start'ed..?
>>>>>
>>>>>
>>>>> While debugging I found, that usb_stor_info() calls
>>>>> blk_first_device(IF_TYPE_USB, &dev)
>>>>> which calls uclass_first_device(UCLASS_BLK, devp). With my current DM
>>>>> tree:
>>>>>
>>>>> => dm tree
>>>>>    Class       Probed   Name
>>>>> ----------------------------------------
>>>>>    root        [ + ]    root_driver
>>>>> ...
>>>>>    mmc         [ + ]    |   |-- pci_mmc
>>>>>    blk         [   ]    |   |   `-- pci_mmc.blk
>>>>>    mmc         [ + ]    |   |-- pci_mmc
>>>>>    blk         [   ]    |   |   `-- pci_mmc.blk
>>>>>
>>>>>
>>>>> the first BLK device is a MMC device. With uclass_first_device() its
>>>>> probe function is called here, which fails in this case. Resulting in
>>>>> an abort for the loop over all BLK devices.
>>>>>
>>>>> How should this be handled for the "usb storage" command. Probing
>>>>> all BLK devices while running this command seems a bit too much.
>>>>>
>>>>> Any suggestions on how to fix this?
>>>>>
>>>>
>>>> Sorry I missed this before. Is it still broken in current mainline?
>>
>>
>> Finally I'm getting back to this issue. And yes, its still broken in
>> mainline.
>>
>>> Me too.
>>>
>>> Probing the block device should call mmc_blk_probe() which should call
>>> mmc_init(), etc. I'm not sure what is going wrong there?
>>
>>
>> The problem is, that *all* block devices are probed with the "usb
>> storage" command, including MMC devices, which come first in my DM tree.
>> As the first MMC device is not available, this command stops here with
>> the error listed above.
>>
>> IMHO, "usb storage" should only list the USB block devices. Not sure,
>> if this ever worked with DM USB enabled.
> 
> I think there is a problem in blk_first_device() and
> blk_next_device(). They use uclass_first/next_device() to iterate
> through the block devices, but those functions probe each device they
> find, even if not a USB device.
> 
> The fix is to adjust blk_first/next_device() to use
> uclass_find_first/next_device() instead, and then only probe the ones
> that need to be returned.

Thanks. I've posted a patch to change this behavior, fixing the
"usb storage" issue, I've been experiencing:

http://patchwork.ozlabs.org/patch/842638/

Thanks,
Stefan