[U-Boot] [PATCH 3/5] dm: usb: Do not scan companion busses if no devices where handed over

[Simon Glass]

Hi Hans,

On 5 May 2015 at 16:14, Hans de Goede <hdegoede at redhat.com> wrote:
> Hi,
>
>
> On 05/05/2015 11:46 PM, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 5 May 2015 at 07:28, Hans de Goede <hdegoede at redhat.com> wrote:
>>>
>>> USB scanning is slow, and there is no need to scan the companion busses
>>> if no usb devices where handed over to the companinon controllers by any
>>> of the main controllers.
>>>
>>> This saves e.g. 2 seconds when booting a A10 OLinuxIno Lime with no USB-1
>>> devices plugged into the root usb ports.
>>>
>>> The use of a global variable for the companion handover counting is
>>> somewhat
>>> unfortunate, but we do not have the necessary info to link companions and
>>> main controllers together in devicetree, and since this is just an
>>> optimization adding a custom devicetree extenstion for this seems
>>> undesirable.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>> ---
>>>   common/usb.c                  |  2 ++
>>>   drivers/usb/host/usb-uclass.c | 12 +++++++-----
>>>   include/usb.h                 |  3 +++
>>>   3 files changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/common/usb.c b/common/usb.c
>>> index 1b26bfa..4a09583 100644
>>> --- a/common/usb.c
>>> +++ b/common/usb.c
>>> @@ -44,6 +44,8 @@
>>>
>>>   static int asynch_allowed;
>>>   char usb_started; /* flag for the started/stopped USB status */
>>> +/* Tracks how much devices were handed over to companion controllers */
>>> +int usb_companion_device_count;
>>>
>>>   #ifndef CONFIG_DM_USB
>>>   static struct usb_device usb_dev[USB_MAX_DEVICE];
>>> diff --git a/drivers/usb/host/usb-uclass.c
>>> b/drivers/usb/host/usb-uclass.c
>>> index d745c1c..877d307 100644
>>> --- a/drivers/usb/host/usb-uclass.c
>>> +++ b/drivers/usb/host/usb-uclass.c
>>> @@ -219,12 +219,14 @@ int usb_init(void)
>>>          /*
>>>           * Now that the primary controllers have been scanned and have
>>> handed
>>>           * over any devices they do not understand to their companions,
>>> scan
>>> -        * the companions.
>>> +        * the companions if necessary.
>>>           */
>>> -       uclass_foreach_dev(bus, uc) {
>>> -               priv = dev_get_uclass_priv(bus);
>>> -               if (priv->companion)
>>> -                       usb_scan_bus(bus, true);
>>> +       if (usb_companion_device_count) {
>>> +               uclass_foreach_dev(bus, uc) {
>>> +                       priv = dev_get_uclass_priv(bus);
>>> +                       if (priv->companion)
>>> +                               usb_scan_bus(bus, true);
>>> +               }
>>>          }
>>>
>>>          debug("scan end\n");
>>> diff --git a/include/usb.h b/include/usb.h
>>> index b81e796..d4c9f44 100644
>>> --- a/include/usb.h
>>> +++ b/include/usb.h
>>> @@ -171,6 +171,9 @@ enum usb_init_type {
>>>    * this is how the lowlevel part communicate with the outer world
>>>    */
>>>
>>> +/* Tracks how much devices were handed over to companion controllers */
>>> +extern int usb_companion_device_count;
>>
>>
>> Not keen on a global.
>>
>> Could this be a per-bus value, and go in the controller's private
>> data?
>
>
> No ideally it would be per main + companion-controller(s) cluster, but
> we do not know which controllers belong to each other, normal interrupt
> driven operation does not care, as the companions do not generate any
> events until devices are handed over.  Since we however do everything
> synchronous it is a worthwhile optimization to skip the companions scan,
> I believe that the best balance here is to just use a single var so that we
> at least get the boot time gain when no handover is done at all.
>
>> If not, uclass private data?
>
>
> That would be tricky, as the hand over is deep inside the ehci code,
> I can sure we can arrange a way to get to the uclass priv data there
> and make it #ifdef CONFIG_DM_USB, but that does add yet another #ifdef.
>
> And I've found another use for the global outside the uclass, see
> the patch I've just posted titled:
>
> "usb: Stop reset procedure when a dev is handed over to a companion hcd"
>
> At first I wanted to use a -ENXIO return value from usb_control_msg to
> indicate that a handover has happened, rather then do the compare
> usb_companion_device_count before and after thing I'm doing now, the
> problem is that currently usb_control_msg always returns 0 or -1 and
> not any other error.
>
> Oh wait I've just done a double check and I see that that is not true
> actually. So I could do something like propagate an error instead of
> the somewhat clunky usb_companion_device_count before and after thing,
> and then this can indeed become a uclass priv value, would that be
> acceptable ?

Let's try that, it seems better.

I do understand the challenge in moving this stuff over.

Regards,
Simon