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

[Hans de Goede]

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 ?

Regards,

Hans


>
>> +
>>   #if defined(CONFIG_USB_UHCI) || defined(CONFIG_USB_OHCI) || \
>>          defined(CONFIG_USB_EHCI) || defined(CONFIG_USB_OHCI_NEW) || \
>>          defined(CONFIG_USB_SL811HS) || defined(CONFIG_USB_ISP116X_HCD) || \
>> --
>> 2.3.6
>>
>
> Regards,
> Simon
>