[U-Boot] [UBOOT] [PATCH] cmd: usb: ignore block devices under mass storage device
Simon Glass
sjg at chromium.org
Thu Aug 31 12:52:48 UTC 2017
Hi Suneel,
On 15 August 2017 at 11:06, Suneel Garapati <suneelglinux at gmail.com> wrote:
> Hi Simon,
>
>
> On Sun, Aug 13, 2017 at 2:37 PM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Suneel,
>>
>> On 10 August 2017 at 23:53, Suneel Garapati <suneelglinux at gmail.com> wrote:
>>> usb tree and info commands may cause crash otherwise
>>>
>>> Signed-off-by: Suneel Garapati <suneelglinux at gmail.com>
>>> ---
>>> cmd/usb.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>
>> Thank you for the patch - it certainly looks like a bug. Can you
>> please expand the commit message a little? E.g. you have
>> UCLASS_USB_EMUL below.
>
> I will change the description
>
>>
>>> diff --git a/cmd/usb.c b/cmd/usb.c
>>> index 992d414..81e1a7b 100644
>>> --- a/cmd/usb.c
>>> +++ b/cmd/usb.c
>>> @@ -415,7 +415,8 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
>>> udev = dev_get_parent_priv(child);
>>>
>>> /* Ignore emulators, we only want real devices */
>>> - if (device_get_uclass_id(child) != UCLASS_USB_EMUL) {
>>> + if (device_get_uclass_id(child) !=
>>> + (UCLASS_USB_EMUL | UCLASS_BLK)) {
>>
>> This seems odd to me. Do you mean to check that the child uclass is
>> neither USB_EMUL nor BLK?
>>
>> Would it be possible to check that the parent is UCLASS_USB? That
>> seems like a better condition to determine whether the child has USB
>> parent data.
>
> It is possible to check parent uclass but would that ever fail?
How would it fail? The only device that does not have a parent is the
root device, and we know it cannot be that since it will be
UCLASS_ROOT.
> I assume, block device under usb storage device will always have
> parent as usb class.
Yes that sounds right.
> Also, tree is called on only usb class devices. Maybe I am missing something.
Maybe, but I think it is more robust to check for the thing you want
than the things you don't. If someone adds a new thing that can be a
child then this code will need updating.
>
> Regards,
> Suneel
>
>>
>>> usb_show_tree_graph(udev, pre);
>>> pre[index] = 0;
>>> }
>>> @@ -605,7 +606,8 @@ static void usb_show_info(struct usb_device *udev)
>>> for (device_find_first_child(udev->dev, &child);
>>> child;
>>> device_find_next_child(&child)) {
>>> - if (device_active(child)) {
>>> + if (device_active(child) &&
>>> + (device_get_uclass_id(child) != UCLASS_BLK)) {
>>> udev = dev_get_parent_priv(child);
>>> usb_show_info(udev);
>>> }
>>> --
>>> 2.7.4
>>>
Regards,
Simon
More information about the U-Boot
mailing list