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
> 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
>> ---
>> 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, >parent->child_head,
>> +sibling_node) {
>> + if (pos->driver != dev->driver)
>> + continue;
>> +
>> + list_del(>sibling_node);
>> + list_add_tail(>sibling_node, _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
>
