| Message ID | 1596034301-5428-4-git-send-email-claudiu.beznea@microchip.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Eugen Hristev |
| Headers | show |
| Series | clk: at91: add sama7g5 support | expand |
Hi Claudiu, On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea <claudiu.beznea@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@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. > > 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()? > + if (!dev->parent) > + return -ENODEV; This can't happen. Every device except for the root one has a parent. > + > + 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? > +} > + > 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
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@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@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 >
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; + + if (!dev->parent) + return -ENODEV; + + 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; +} + 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
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@microchip.com> --- drivers/core/device.c | 26 ++++++++++++++++++++++++++ include/dm/device-internal.h | 9 +++++++++ 2 files changed, 35 insertions(+)