[PATCH 03/22] dm: core: add support for device re-parenting

[Claudiu.Beznea at microchip.com]

On 04.08.2020 05:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Claudiu,
> 
> On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea
> <claudiu.beznea at microchip.com> wrote:
>>
>> In common clock framework the relation b/w parent and child clocks is
>> determined based on the udevice parent/child information. A clock
>> parent could be changed based on devices needs. In case this is happen
>> the functionalities for clock who's parent is changed are broken. Add
>> a function that reparent a device. This will be used in clk-uclass.c
>> to reparent a clock device.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea at microchip.com>
>> ---
>>  drivers/core/device.c        | 26 ++++++++++++++++++++++++++
>>  include/dm/device-internal.h |  9 +++++++++
>>  2 files changed, 35 insertions(+)
> 
> Please add a sandbox test for this function.

OK.

> 
>>
>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>> index a7408d9c76c6..f149d55ac1e1 100644
>> --- a/drivers/core/device.c
>> +++ b/drivers/core/device.c
>> @@ -267,6 +267,32 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
>>                         devp);
>>  }
>>
>> +int device_reparent(struct udevice *dev, struct udevice *new_parent)
>> +{
>> +       struct udevice *cparent;
>> +       struct udevice *pos, *n;
>> +
>> +       if (!dev || !new_parent)
>> +               return -EINVAL;
>> +
> 
> This is an error by the caller and would not be present in production
> code. Perhaps use assert()?

OK, I'll use assert().

> 
>> +       if (!dev->parent)
>> +               return -ENODEV;
> 
> This can't happen. Every device except for the root one has a parent.

Sure, I'll remove it.

> 
>> +
>> +       list_for_each_entry_safe(pos, n, &dev->parent->child_head,
>> +                                sibling_node) {
>> +               if (pos->driver != dev->driver)
>> +                       continue;
>> +
>> +               list_del(&dev->sibling_node);
>> +               list_add_tail(&dev->sibling_node, &new_parent->child_head);
>> +               dev->parent = new_parent;
>> +
>> +               return 0;
>> +       }
>> +
>> +       return -ENODEV;
> 
> What does this error mean?

That the device who needs re-parenting has no parent. But you already
pointed that this should not happen. This means that the above loop will
always have a match and here we should return success code.

> 
>> +}
>> +
>>  static void *alloc_priv(int size, uint flags)
>>  {
>>         void *priv;
>> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
>> index 294d6c18105a..c5d7ec0650f9 100644
>> --- a/include/dm/device-internal.h
>> +++ b/include/dm/device-internal.h
>> @@ -84,6 +84,15 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
>>                         const struct driver_info *info, struct udevice **devp);
>>
>>  /**
>> + * device_reparent: reparent the device to a new parent
>> + *
>> + * @dev: pointer to device to be reparented
>> + * @new_parent: pointer to new parent device
>> + * @return 0 if OK, -ve on error
>> + */
>> +int device_reparent(struct udevice *dev, struct udevice *new_parent);
>> +
>> +/**
>>   * device_ofdata_to_platdata() - Read platform data for a device
>>   *
>>   * Read platform data for a device (typically from the device tree) so that
>> --
>> 2.7.4
>>
> 
> Regards,
> Simon
>