| Message ID | 1596640325-20014-4-git-send-email-claudiu.beznea@microchip.com |
|---|---|
| State | Superseded |
| Delegated to: | Eugen Hristev |
| Headers | show |
| Series | clk: at91: add sama7g5 support | expand |
Hi Simon, On 05.08.2020 18:11, Claudiu Beznea wrote: > Clock re-parenting is not binding the clock's device to its new > parent device, it only calls the clock's ops->set_parent() API. The > changes in this commit re-parent the clock device to its new parent > so that subsequent operations like clk_get_parent() to point to the > proper parent. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > Reviewed-by: Simon Glass <sjg@chromium.org> I added your Reviewed-by here but at the same time I also added the sandbox test in this patch. Let me know if you want the sandbox test in a separate patch. Same thing for patch 4/21 in this series. Thank you, Claudiu Beznea > --- > drivers/clk/clk-uclass.c | 11 ++++++++++- > test/dm/clk_ccf.c | 27 +++++++++++++++++++++++++++ > 2 files changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c > index 934cd5787a5c..5e0c8419d65b 100644 > --- a/drivers/clk/clk-uclass.c > +++ b/drivers/clk/clk-uclass.c > @@ -14,6 +14,7 @@ > #include <errno.h> > #include <log.h> > #include <malloc.h> > +#include <dm/device-internal.h> > #include <dm/devres.h> > #include <dm/read.h> > #include <linux/bug.h> > @@ -512,6 +513,7 @@ ulong clk_set_rate(struct clk *clk, ulong rate) > int clk_set_parent(struct clk *clk, struct clk *parent) > { > const struct clk_ops *ops; > + int ret; > > debug("%s(clk=%p, parent=%p)\n", __func__, clk, parent); > if (!clk_valid(clk)) > @@ -521,7 +523,14 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > if (!ops->set_parent) > return -ENOSYS; > > - return ops->set_parent(clk, parent); > + ret = ops->set_parent(clk, parent); > + if (ret) > + return ret; > + > + if (CONFIG_IS_ENABLED(CLK_CCF)) > + ret = device_reparent(clk->dev, parent->dev); > + > + return ret; > } > > int clk_enable(struct clk *clk) > diff --git a/test/dm/clk_ccf.c b/test/dm/clk_ccf.c > index da2292a51a95..b039cc5f8d8f 100644 > --- a/test/dm/clk_ccf.c > +++ b/test/dm/clk_ccf.c > @@ -22,6 +22,10 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) > struct udevice *dev; > long long rate; > int ret; > +#if CONFIG_IS_ENABLED(CLK_CCF) > + const char *clkname; > + int clkid; > +#endif > > /* Get the device using the clk device */ > ut_assertok(uclass_get_device_by_name(UCLASS_CLK, "clk-ccf", &dev)); > @@ -91,6 +95,29 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) > > ret = sandbox_clk_enable_count(pclk); > ut_asserteq(ret, 0); > + > + /* Test clock re-parenting. */ > + ret = clk_get_by_id(SANDBOX_CLK_USDHC1_SEL, &clk); > + ut_assertok(ret); > + ut_asserteq_str("usdhc1_sel", clk->dev->name); > + > + pclk = clk_get_parent(clk); > + ut_assertok_ptr(pclk); > + if (!strcmp(pclk->dev->name, "pll3_60m")) { > + clkname = "pll3_80m"; > + clkid = SANDBOX_CLK_PLL3_80M; > + } else { > + clkname = "pll3_60m"; > + clkid = SANDBOX_CLK_PLL3_60M; > + } > + > + ret = clk_get_by_id(clkid, &pclk); > + ut_assertok(ret); > + ret = clk_set_parent(clk, pclk); > + ut_assertok(ret); > + pclk = clk_get_parent(clk); > + ut_assertok_ptr(pclk); > + ut_asserteq_str(clkname, pclk->dev->name); > #endif > > return 1; >
Hi Claudiu, On Thu, 6 Aug 2020 at 02:24, <Claudiu.Beznea@microchip.com> wrote: > > Hi Simon, > > On 05.08.2020 18:11, Claudiu Beznea wrote: > > Clock re-parenting is not binding the clock's device to its new > > parent device, it only calls the clock's ops->set_parent() API. The > > changes in this commit re-parent the clock device to its new parent > > so that subsequent operations like clk_get_parent() to point to the > > proper parent. > > > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > I added your Reviewed-by here but at the same time I also added the sandbox > test in this patch. Let me know if you want the sandbox test in a separate > patch. Same thing for patch 4/21 in this series. I think it is nice to have the test in the same patch in many cases, since it clearly shows that there is a test for the change / new functionality. Regards, Simon
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 934cd5787a5c..5e0c8419d65b 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -14,6 +14,7 @@ #include <errno.h> #include <log.h> #include <malloc.h> +#include <dm/device-internal.h> #include <dm/devres.h> #include <dm/read.h> #include <linux/bug.h> @@ -512,6 +513,7 @@ ulong clk_set_rate(struct clk *clk, ulong rate) int clk_set_parent(struct clk *clk, struct clk *parent) { const struct clk_ops *ops; + int ret; debug("%s(clk=%p, parent=%p)\n", __func__, clk, parent); if (!clk_valid(clk)) @@ -521,7 +523,14 @@ int clk_set_parent(struct clk *clk, struct clk *parent) if (!ops->set_parent) return -ENOSYS; - return ops->set_parent(clk, parent); + ret = ops->set_parent(clk, parent); + if (ret) + return ret; + + if (CONFIG_IS_ENABLED(CLK_CCF)) + ret = device_reparent(clk->dev, parent->dev); + + return ret; } int clk_enable(struct clk *clk) diff --git a/test/dm/clk_ccf.c b/test/dm/clk_ccf.c index da2292a51a95..b039cc5f8d8f 100644 --- a/test/dm/clk_ccf.c +++ b/test/dm/clk_ccf.c @@ -22,6 +22,10 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) struct udevice *dev; long long rate; int ret; +#if CONFIG_IS_ENABLED(CLK_CCF) + const char *clkname; + int clkid; +#endif /* Get the device using the clk device */ ut_assertok(uclass_get_device_by_name(UCLASS_CLK, "clk-ccf", &dev)); @@ -91,6 +95,29 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) ret = sandbox_clk_enable_count(pclk); ut_asserteq(ret, 0); + + /* Test clock re-parenting. */ + ret = clk_get_by_id(SANDBOX_CLK_USDHC1_SEL, &clk); + ut_assertok(ret); + ut_asserteq_str("usdhc1_sel", clk->dev->name); + + pclk = clk_get_parent(clk); + ut_assertok_ptr(pclk); + if (!strcmp(pclk->dev->name, "pll3_60m")) { + clkname = "pll3_80m"; + clkid = SANDBOX_CLK_PLL3_80M; + } else { + clkname = "pll3_60m"; + clkid = SANDBOX_CLK_PLL3_60M; + } + + ret = clk_get_by_id(clkid, &pclk); + ut_assertok(ret); + ret = clk_set_parent(clk, pclk); + ut_assertok(ret); + pclk = clk_get_parent(clk); + ut_assertok_ptr(pclk); + ut_asserteq_str(clkname, pclk->dev->name); #endif return 1;