[U-Boot] [RFC PATCH v2 3/6] dm: usb: Add UCLASS_USB_DEV_GENERIC shutdown

[Simon Glass]

Hi Jagan,

On 7 August 2018 at 01:03, Jagan Teki <jagan at amarulasolutions.com> wrote:
> On Tue, Jul 24, 2018 at 5:18 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Jagan,
>>
>> On 20 July 2018 at 01:13, Jagan Teki <jagan at amarulasolutions.com> wrote:
>>> Some OTG controllers which operates on Peripheral
>>> mode are registered as UCLASS_USB_DEV_GENERIC.
>>>
>>> So add support to shutdown them as well. shutdown
>>> happened during 'usb reset' and U-Boot handoff code
>>> for Linux boot.
>>>
>>> controller restarting in 'usb reset' like probing
>>> again is still missing for this type of UCLASS.
>>>
>>> Cc: Simon Glass <sjg at chromium.org>
>>> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
>>> ---
>>> Note:
>>> I tried of traversing for multiple UCLASS in removal
>>> code in usb_stop, but couldn't come with proper solutions.
>>> I don't think any other area of code need a requirement like
>>> this, or may be I'm missing. any help would appreciate.
>>>
>>> Changes for v2:
>>> - none
>>>
>>>  drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++
>>>  1 file changed, 43 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>>> index 611ea97a72..99cf3d2b49 100644
>>> --- a/drivers/usb/host/usb-uclass.c
>>> +++ b/drivers/usb/host/usb-uclass.c
>>> @@ -158,6 +158,45 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t *size)
>>>         return ops->get_max_xfer_size(bus, size);
>>>  }
>>>
>>> +int __usb_stop(void)
>>
>> Why the __ ? I think it should be something like usb_remove_and_unbind_all()
>>
>>> +{
>>> +       struct udevice *bus;
>>> +       struct udevice *rh;
>>> +       struct uclass *uc;
>>> +       struct usb_uclass_priv *uc_priv;
>>> +       int err = 0, ret;
>>> +
>>> +       /* De-activate any devices that have been activated */
>>> +       ret = uclass_get(UCLASS_USB_DEV_GENERIC, &uc);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       uc_priv = uc->priv;
>>> +
>>> +       uclass_foreach_dev(bus, uc) {
>>> +               ret = device_remove(bus, DM_REMOVE_NORMAL);
>>> +               if (ret && !err)
>>> +                       err = ret;
>>> +
>>> +               /* Locate root hub device */
>>> +               device_find_first_child(bus, &rh);
>>> +               if (rh) {
>>> +                       /*
>>> +                        * All USB devices are children of root hub.
>>> +                        * Unbinding root hub will unbind all of its children.
>>> +                        */
>>> +                       ret = device_unbind(rh);
>>> +                       if (ret && !err)
>>> +                               err = ret;
>>> +               }
>>> +       }
>>> +
>>> +       uc_priv->companion_device_count = 0;
>>> +       usb_started = 0;
>>> +
>>> +       return err;
>>> +}
>>> +
>>>  int usb_stop(void)
>>>  {
>>>         struct udevice *bus;
>>> @@ -166,6 +205,10 @@ int usb_stop(void)
>>>         struct usb_uclass_priv *uc_priv;
>>>         int err = 0, ret;
>>>
>>> +       ret = __usb_stop();
>>> +       if (ret)
>>> +               return ret;
>>> +
>>
>> This looks like the same code that appears below here, or very
>> similar. Why is this needed?
>
> Here the usage-case it to remove/unbind UCLASS_USB and
> UCLASS_USB_DEV_GENERIC and same code will use to do that, any
> suggestions?

I'm just wondering why you appear to be duplicating the exact code
that is already there? Maybe I am missing something, iwc please can
you explain it?

Thanks,
Simon