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

Jean-Jacques Hiblot jjhiblot at ti.com
Fri Oct 5 11:42:27 UTC 2018



On 05/10/2018 10:50, Jean-Jacques Hiblot wrote:
> 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.
OK. So I think I understand a bit more what is happening here.
The omap3_logic does not actually take advantage of SPL_OF_PLATDATA nor 
SPL_OF_CONTROL. Devices are instantiated by the SPL the old way, using 
U_BOOT_DEVICE() (board/logicpd/omap3logic.c:45-70). In the SPL, the I2C 
instantiation is missing, but I guess that it is ok because nobody 
checks the return value of the calls to twl4030 functions.

You probably can remove SPL_OF_PLATDATA and SPL_OF_CONTROL and still be 
able to boot.

JJ

>
>>
>>> 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
>



More information about the U-Boot mailing list