From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claudiu.Beznea at microchip.com Date: Tue, 4 Aug 2020 07:24:42 +0000 Subject: [PATCH 03/22] dm: core: add support for device re-parenting In-Reply-To: References: <1596034301-5428-1-git-send-email-claudiu.beznea@microchip.com> <1596034301-5428-4-git-send-email-claudiu.beznea@microchip.com> Message-ID: <1fadabeb-b2e9-88c4-1752-6db2f7f4fce2@microchip.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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, &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 >