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.bez...@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.bez...@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 >