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

[Jagan Teki]

Hi Simon,

On Fri, Aug 17, 2018 at 6:18 PM, Simon Glass <sjg at chromium.org> wrote:
> 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?

Yes, the code is duplicate. Here, I'm looking for suggestion to
unbind/remove multiple UCLASS's like UCLASS_USB and
ULASS_USB_DEV_GENERIC in this use-case. do you have any? or am I
missing something?

Jagan.