Messages in this thread | | | Subject | Re: [PATCH] of/platform: Support dynamic device tree on AMBA bus | From | Jaewon Kim <> | Date | Thu, 1 Nov 2018 00:32:31 +0900 |
| |
Hi Frank,
Thanks to review my patch.
On 18. 10. 31. 오전 8:04, Frank Rowand wrote: > Hi Jaewon, > > On 10/25/18 9:39 AM, Jaewon Kim wrote: >> This patch supports dynamic device-tree for AMBA device. > Add AMBA devices and buses to of_platform_notify() so that dynamic device-tree > will support AMBA.
What is meaning of this comment?
I already adds AMBA to of_platform_notify().
> >> The AMBA device must be registered on the AMBA bus, not the platform bus. >> >> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> >> --- >> drivers/of/platform.c | 93 ++++++++++++++++++++++++++++++++++++++++----------- >> 1 file changed, 73 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/of/platform.c b/drivers/of/platform.c >> index 04ad312..b9ac105 100644 >> --- a/drivers/of/platform.c >> +++ b/drivers/of/platform.c >> @@ -286,6 +286,22 @@ static struct amba_device *of_amba_device_create(struct device_node *node, >> of_node_clear_flag(node, OF_POPULATED); >> return NULL; >> } >> + >> +/** >> + * of_find_amba_device_by_node - Find the amba_device associated with a node >> + * @np: Pointer to device tree node >> + * >> + * Returns amba_device pointer, or NULL if not found >> + */ >> +struct amba_device *of_find_amba_device_by_node(struct device_node *np) >> +{ >> + struct device *dev; >> + >> + dev = bus_find_device(&amba_bustype, NULL, np, of_dev_node_match); >> + return dev ? to_amba_device(dev) : NULL; >> +} >> +EXPORT_SYMBOL(of_find_amba_device_by_node); >> + >> #else /* CONFIG_ARM_AMBA */ >> static struct amba_device *of_amba_device_create(struct device_node *node, >> const char *bus_id, >> @@ -294,6 +310,12 @@ static struct amba_device *of_amba_device_create(struct device_node *node, >> { >> return NULL; >> } >> + >> +struct amba_device *of_find_amba_device_by_node(struct device_node *np) >> +{ >> + return NULL; >> +} >> +EXPORT_SYMBOL(of_find_amba_device_by_node); >> #endif /* CONFIG_ARM_AMBA */ >> >> /** >> @@ -665,6 +687,7 @@ static int of_platform_notify(struct notifier_block *nb, >> { >> struct of_reconfig_data *rd = arg; >> struct platform_device *pdev_parent, *pdev; >> + struct amba_device *adev_p, *adev; >> bool children_left; >> >> switch (of_reconfig_get_state_change(action, rd)) { >> @@ -677,17 +700,34 @@ static int of_platform_notify(struct notifier_block *nb, >> if (of_node_check_flag(rd->dn, OF_POPULATED)) >> return NOTIFY_OK; >> >> - /* pdev_parent may be NULL when no bus platform device */ >> - pdev_parent = of_find_device_by_node(rd->dn->parent); >> - pdev = of_platform_device_create(rd->dn, NULL, >> - pdev_parent ? &pdev_parent->dev : NULL); >> - of_dev_put(pdev_parent); >> - >> - if (pdev == NULL) { >> - pr_err("%s: failed to create for '%pOF'\n", >> - __func__, rd->dn); >> - /* of_platform_device_create tosses the error code */ >> - return notifier_from_errno(-EINVAL); >> + if (of_device_is_compatible(rd->dn, "arm,primecell")) { >> + /* adev_p may be NULL when no bus amba device */ >> + adev_p = of_find_amba_device_by_node(rd->dn->parent); >> + adev = of_amba_device_create(rd->dn, NULL, NULL, >> + adev_p ? &adev_p->dev : NULL); >> + >> + if (adev_p) >> + put_device(&adev_p->dev); > Please follow the same model as of_dev_put() here, create a new function > of_amba_dev_put() to implement these two lines. Okay, I will create of_amba_dev_put() function. > > >> + >> + if (adev == NULL) { >> + pr_err("%s: failed to create for '%s'\n", >> + __func__, rd->dn->full_name); >> + /* of_amba_device_create tosses the error */ >> + return notifier_from_errno(-EINVAL); >> + } >> + } else { >> + /* pdev_parent may be NULL when no bus platform device*/ >> + pdev_parent = of_find_device_by_node(rd->dn->parent); >> + pdev = of_platform_device_create(rd->dn, NULL, >> + pdev_parent ? &pdev_parent->dev : NULL); >> + of_dev_put(pdev_parent); >> + >> + if (pdev == NULL) { >> + pr_err("%s: failed to create for '%pOF'\n", >> + __func__, rd->dn); >> + /* of_platform_device_create tosses the error */ >> + return notifier_from_errno(-EINVAL); >> + } >> } >> break; >> >> @@ -698,15 +738,28 @@ static int of_platform_notify(struct notifier_block *nb, >> return NOTIFY_OK; >> >> /* find our device by node */ >> - pdev = of_find_device_by_node(rd->dn); >> - if (pdev == NULL) >> - return NOTIFY_OK; /* no? not meant for us */ >> - >> - /* unregister takes one ref away */ >> - of_platform_device_destroy(&pdev->dev, &children_left); >> - >> - /* and put the reference of the find */ >> - of_dev_put(pdev); >> + if (of_device_is_compatible(rd->dn, "arm,primecell")) { >> + adev = of_find_amba_device_by_node(rd->dn); >> + if (adev == NULL) >> + return NOTIFY_OK; /* no? not meant for us */ >> + >> + /* unregister takes one ref away */ >> + of_platform_device_destroy(&adev->dev, &children_left); >> + >> + /* and put the reference of the find */ >> + if (adev) >> + put_device(&adev->dev); > of_amba_dev_put() here also > > -Frank Okay, I will create it. > >> + } else { >> + pdev = of_find_device_by_node(rd->dn); >> + if (pdev == NULL) >> + return NOTIFY_OK; /* no? not meant for us */ >> + >> + /* unregister takes one ref away */ >> + of_platform_device_destroy(&pdev->dev, &children_left); >> + >> + /* and put the reference of the find */ >> + of_dev_put(pdev); >> + } >> break; >> } >> >> Thanks
Jaewon Kim
| |