[U-Boot] [PATCH v1 03/19] dm: device: Allow using uclass_find_device_by_seq() without OF_CONTROL

[Jean-Jacques Hiblot]

Hi Adam,



On 05/10/2018 00:42, Adam Ford wrote:
> On Thu, Oct 4, 2018 at 8:48 AM Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>> If OF_CONTROL is not enabled and DM_SEQ_ALIAS is enabled, we must
>> assign an alias (requested sequence number) to devices that belongs to a
>> class with the DM_UC_FLAG_SEQ_ALIAS flag. Otherwise
> What about conditions where both OF_CONTROL and PLATDATA are set?  I
> am not sure what it all does, so maybe it's OK, but I know PLATDATA is
> a reduced library which I know the omap3_logic uses to keep SPL small.
Thanks for the review and for trying the patches.

if OF_PLATDATA is used, dts is not parsed at all, so alias can't be read 
from dt => the seq_alias should be computed as if OF_CONTROL is not set.

Now I'm curious how enabling OF_PLATDATA even works on omap3_logic. Most 
TI drivers (all ?) do not support it.
I don't have any omap3 board, but I'll give it a try on beaglebone.

>
>> uclass_find_device_by_seq() cannot be used to get/probe a device. In
>> particular i2c_get_chip_for_busnum() cannot be used.
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
>> ---
>>
>>   drivers/core/device.c        | 10 ++++++----
>>   drivers/core/uclass.c        | 24 ++++++++++++++++++++++++
>>   include/dm/uclass-internal.h | 13 +++++++++++++
>>   3 files changed, 43 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>> index feed43c..e441021 100644
>> --- a/drivers/core/device.c
>> +++ b/drivers/core/device.c
>> @@ -70,7 +70,8 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
>>
>>          dev->seq = -1;
>>          dev->req_seq = -1;
>> -       if (CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DM_SEQ_ALIAS)) {
>> +       if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) &&
> Is there are reason these cannot be #if statements (opposed to just
> 'if') to let the pre-compiler enable/disable code?  I am assuming
> these values won't change, but I could be wrong.
The solution woulb de to use
if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA))
instead of
if (CONFIG_IS_ENABLED(OF_CONTROL)
>
>> +           (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) {
>>                  /*
>>                   * Some devices, such as a SPI bus, I2C bus and serial ports
>>                   * are numbered using aliases.
>> @@ -78,10 +79,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
>>                   * This is just a 'requested' sequence, and will be
>>                   * resolved (and ->seq updated) when the device is probed.
>>                   */
>> -               if (uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS) {
>> -                       if (uc->uc_drv->name && ofnode_valid(node)) {
>> +               if (CONFIG_IS_ENABLED(OF_CONTROL)) {
>> +                       if (uc->uc_drv->name && ofnode_valid(node))
>>                                  dev_read_alias_seq(dev, &dev->req_seq);
>> -                       }
>> +               } else {
>> +                       dev->req_seq = uclass_find_next_free_req_seq(drv->id);
>>                  }
>>          }
>>
>> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
>> index 3113d6a..376f882 100644
>> --- a/drivers/core/uclass.c
>> +++ b/drivers/core/uclass.c
>> @@ -269,6 +269,30 @@ int uclass_find_device_by_name(enum uclass_id id, const char *name,
>>          return -ENODEV;
>>   }
>>
>> +#if !CONFIG_IS_ENABLED(OF_CONTROL)
ditto
>> +int uclass_find_next_free_req_seq(enum uclass_id id)
>> +{
>> +       struct uclass *uc;
>> +       struct udevice *dev;
>> +       int ret;
>> +       int max = -1;
>> +
>> +       ret = uclass_get(id, &uc);
>> +       if (ret)
>> +               return ret;
>> +
>> +       list_for_each_entry(dev, &uc->dev_head, uclass_node) {
>> +               if ((dev->req_seq != -1) && (dev->req_seq > max))
>> +                       max = dev->req_seq;
>> +       }
>> +
>> +       if (max == -1)
>> +               return 0;
>> +
>> +       return max + 1;
>> +}
>> +#endif
>> +
>>   int uclass_find_device_by_seq(enum uclass_id id, int seq_or_req_seq,
>>                                bool find_req_seq, struct udevice **devp)
>>   {
>> diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h
>> index 30d5a4f..18a838c 100644
>> --- a/include/dm/uclass-internal.h
>> +++ b/include/dm/uclass-internal.h
>> @@ -12,6 +12,19 @@
>>   #include <dm/ofnode.h>
>>
>>   /**
>> + * uclass_find_next_free_req_seq() - Get the next free req_seq number
>> + *
>> + * This returns the next free req_seq number. This is useful only if
>> + * OF_CONTROL is not used. The next free req_seq number is simply the
>> + * maximum req_seq of the uclass + 1.
>> + * This allows assiging req_seq number in the binding order.
>> + *
>> + * @id:                Id number of the uclass
>> + * @return     The next free req_seq number
>> + */
>> +int uclass_find_next_free_req_seq(enum uclass_id id);
>> +
>> +/**
>>    * uclass_get_device_tail() - handle the end of a get_device call
>>    *
>>    * This handles returning an error or probing a device as needed.
>> --
>> 2.7.4
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot