| Message ID | 20190809230904.28747-1-khilman@baylibre.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk | expand |
On 10/08/2019 01:09, Kevin Hilman wrote: > Instead of expecting a specific number of clocks with specific clock > names, switch to using the bulk clock API. > > This is a first step towards generalizing this driver to work with > other domains. > > Cc: Neil Armstrong <narmstrong@baylibre.com> > Signed-off-by: Kevin Hilman <khilman@baylibre.com> > --- > Boot tested on meson-g12a-sei510 and verified that framebuffer console > comes up and still works. > > drivers/soc/amlogic/meson-gx-pwrc-vpu.c | 41 ++++++------------------- > 1 file changed, 10 insertions(+), 31 deletions(-) > > diff --git a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c > index 511b6856225d..5f6519f43a31 100644 > --- a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c > +++ b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c > @@ -34,8 +34,8 @@ struct meson_gx_pwrc_vpu { > struct regmap *regmap_ao; > struct regmap *regmap_hhi; > struct reset_control *rstc; > - struct clk *vpu_clk; > - struct clk *vapb_clk; > + struct clk_bulk_data *clks; > + int num_clks; > }; > > static inline > @@ -76,8 +76,7 @@ static int meson_gx_pwrc_vpu_power_off(struct generic_pm_domain *genpd) > > msleep(20); > > - clk_disable_unprepare(pd->vpu_clk); > - clk_disable_unprepare(pd->vapb_clk); > + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); > > return 0; > } > @@ -119,25 +118,14 @@ static int meson_g12a_pwrc_vpu_power_off(struct generic_pm_domain *genpd) > > msleep(20); > > - clk_disable_unprepare(pd->vpu_clk); > - clk_disable_unprepare(pd->vapb_clk); > + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); > > return 0; > } > > static int meson_gx_pwrc_vpu_setup_clk(struct meson_gx_pwrc_vpu *pd) > { > - int ret; > - > - ret = clk_prepare_enable(pd->vpu_clk); > - if (ret) > - return ret; > - > - ret = clk_prepare_enable(pd->vapb_clk); > - if (ret) > - clk_disable_unprepare(pd->vpu_clk); > - > - return ret; > + return clk_bulk_prepare_enable(pd->num_clks, pd->clks); > } > > static int meson_gx_pwrc_vpu_power_on(struct generic_pm_domain *genpd) > @@ -273,8 +261,6 @@ static int meson_gx_pwrc_vpu_probe(struct platform_device *pdev) > struct regmap *regmap_ao, *regmap_hhi; > struct meson_gx_pwrc_vpu *vpu_pd; > struct reset_control *rstc; > - struct clk *vpu_clk; > - struct clk *vapb_clk; > bool powered_off; > int ret; > > @@ -310,23 +296,16 @@ static int meson_gx_pwrc_vpu_probe(struct platform_device *pdev) > return PTR_ERR(rstc); > } > > - vpu_clk = devm_clk_get(&pdev->dev, "vpu"); > - if (IS_ERR(vpu_clk)) { > - dev_err(&pdev->dev, "vpu clock request failed\n"); > - return PTR_ERR(vpu_clk); > - } > - > - vapb_clk = devm_clk_get(&pdev->dev, "vapb"); > - if (IS_ERR(vapb_clk)) { > - dev_err(&pdev->dev, "vapb clock request failed\n"); > - return PTR_ERR(vapb_clk); > + ret = devm_clk_bulk_get_all(&pdev->dev, &vpu_pd->clks); > + if (ret < 0) { > + dev_err(&pdev->dev, "bulk clock request failed\n"); > + return ret; > } > + vpu_pd->num_clks = ret; > > vpu_pd->regmap_ao = regmap_ao; > vpu_pd->regmap_hhi = regmap_hhi; > vpu_pd->rstc = rstc; > - vpu_pd->vpu_clk = vpu_clk; > - vpu_pd->vapb_clk = vapb_clk; > > platform_set_drvdata(pdev, vpu_pd); > > Acked-by: Neil Armstrong <narmstrong@baylibre.com>
Hi Neil, Kevin Hilman <khilman@baylibre.com> writes: > Instead of expecting a specific number of clocks with specific clock > names, switch to using the bulk clock API. > > This is a first step towards generalizing this driver to work with > other domains. > > Cc: Neil Armstrong <narmstrong@baylibre.com> > Signed-off-by: Kevin Hilman <khilman@baylibre.com> > --- > Boot tested on meson-g12a-sei510 and verified that framebuffer console > comes up and still works. > > drivers/soc/amlogic/meson-gx-pwrc-vpu.c | 41 ++++++------------------- > 1 file changed, 10 insertions(+), 31 deletions(-) > > diff --git a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c > index 511b6856225d..5f6519f43a31 100644 > --- a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c > +++ b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c > @@ -34,8 +34,8 @@ struct meson_gx_pwrc_vpu { > struct regmap *regmap_ao; > struct regmap *regmap_hhi; > struct reset_control *rstc; > - struct clk *vpu_clk; > - struct clk *vapb_clk; > + struct clk_bulk_data *clks; > + int num_clks; > }; > > static inline > @@ -76,8 +76,7 @@ static int meson_gx_pwrc_vpu_power_off(struct generic_pm_domain *genpd) > > msleep(20); > > - clk_disable_unprepare(pd->vpu_clk); > - clk_disable_unprepare(pd->vapb_clk); > + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); Note the original turn-off order here is VPU then VAPB... > return 0; > } > @@ -119,25 +118,14 @@ static int meson_g12a_pwrc_vpu_power_off(struct generic_pm_domain *genpd) > > msleep(20); > > - clk_disable_unprepare(pd->vpu_clk); > - clk_disable_unprepare(pd->vapb_clk); > + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); ... and the origianl turn-on ordr is also VPU then VAPB. Using the clock bulk API, the new turn-on order will be the order they clocks appear in DT. The turn-off order will be the reverse of that. That seems right to me, but it is a change in behavior from the current code. Did you set the enable and disable ordering the same for any specific reason? Any reason to thing reversing the disable order is going to cause any issues? Thanks, Kevin
Hi, On 14/08/2019 00:47, Kevin Hilman wrote: > Hi Neil, > > Kevin Hilman <khilman@baylibre.com> writes: > >> Instead of expecting a specific number of clocks with specific clock >> names, switch to using the bulk clock API. >> >> This is a first step towards generalizing this driver to work with >> other domains. >> >> Cc: Neil Armstrong <narmstrong@baylibre.com> >> Signed-off-by: Kevin Hilman <khilman@baylibre.com> >> --- >> Boot tested on meson-g12a-sei510 and verified that framebuffer console >> comes up and still works. >> >> drivers/soc/amlogic/meson-gx-pwrc-vpu.c | 41 ++++++------------------- >> 1 file changed, 10 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c >> index 511b6856225d..5f6519f43a31 100644 >> --- a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c >> +++ b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c >> @@ -34,8 +34,8 @@ struct meson_gx_pwrc_vpu { >> struct regmap *regmap_ao; >> struct regmap *regmap_hhi; >> struct reset_control *rstc; >> - struct clk *vpu_clk; >> - struct clk *vapb_clk; >> + struct clk_bulk_data *clks; >> + int num_clks; >> }; >> >> static inline >> @@ -76,8 +76,7 @@ static int meson_gx_pwrc_vpu_power_off(struct generic_pm_domain *genpd) >> >> msleep(20); >> >> - clk_disable_unprepare(pd->vpu_clk); >> - clk_disable_unprepare(pd->vapb_clk); >> + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); > > Note the original turn-off order here is VPU then VAPB... > >> return 0; >> } >> @@ -119,25 +118,14 @@ static int meson_g12a_pwrc_vpu_power_off(struct generic_pm_domain *genpd) >> >> msleep(20); >> >> - clk_disable_unprepare(pd->vpu_clk); >> - clk_disable_unprepare(pd->vapb_clk); >> + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); > > ... and the origianl turn-on ordr is also VPU then VAPB. > > Using the clock bulk API, the new turn-on order will be the order they > clocks appear in DT. The turn-off order will be the reverse of that. > > That seems right to me, but it is a change in behavior from the current > code. > > Did you set the enable and disable ordering the same for any specific > reason? Any reason to thing reversing the disable order is going to > cause any issues? No the order is not an issue here, the 2 clocks feeds 2 different parts of the VPU, one is the APB register bridge (vapb) and the other feeds the vpu video pipeline, so the order is not an issue. Neil > > Thanks, > > Kevin >
Neil Armstrong <narmstrong@baylibre.com> writes: > Hi, > > On 14/08/2019 00:47, Kevin Hilman wrote: >> Hi Neil, >> >> Kevin Hilman <khilman@baylibre.com> writes: >> >>> Instead of expecting a specific number of clocks with specific clock >>> names, switch to using the bulk clock API. >>> >>> This is a first step towards generalizing this driver to work with >>> other domains. >>> >>> Cc: Neil Armstrong <narmstrong@baylibre.com> >>> Signed-off-by: Kevin Hilman <khilman@baylibre.com> >>> --- >>> Boot tested on meson-g12a-sei510 and verified that framebuffer console >>> comes up and still works. >>> >>> drivers/soc/amlogic/meson-gx-pwrc-vpu.c | 41 ++++++------------------- >>> 1 file changed, 10 insertions(+), 31 deletions(-) >>> >>> diff --git a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c >>> index 511b6856225d..5f6519f43a31 100644 >>> --- a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c >>> +++ b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c >>> @@ -34,8 +34,8 @@ struct meson_gx_pwrc_vpu { >>> struct regmap *regmap_ao; >>> struct regmap *regmap_hhi; >>> struct reset_control *rstc; >>> - struct clk *vpu_clk; >>> - struct clk *vapb_clk; >>> + struct clk_bulk_data *clks; >>> + int num_clks; >>> }; >>> >>> static inline >>> @@ -76,8 +76,7 @@ static int meson_gx_pwrc_vpu_power_off(struct generic_pm_domain *genpd) >>> >>> msleep(20); >>> >>> - clk_disable_unprepare(pd->vpu_clk); >>> - clk_disable_unprepare(pd->vapb_clk); >>> + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); >> >> Note the original turn-off order here is VPU then VAPB... >> >>> return 0; >>> } >>> @@ -119,25 +118,14 @@ static int meson_g12a_pwrc_vpu_power_off(struct generic_pm_domain *genpd) >>> >>> msleep(20); >>> >>> - clk_disable_unprepare(pd->vpu_clk); >>> - clk_disable_unprepare(pd->vapb_clk); >>> + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); >> >> ... and the origianl turn-on ordr is also VPU then VAPB. >> >> Using the clock bulk API, the new turn-on order will be the order they >> clocks appear in DT. The turn-off order will be the reverse of that. >> >> That seems right to me, but it is a change in behavior from the current >> code. >> >> Did you set the enable and disable ordering the same for any specific >> reason? Any reason to thing reversing the disable order is going to >> cause any issues? > > No the order is not an issue here, the 2 clocks feeds 2 different parts of the VPU, > one is the APB register bridge (vapb) and the other feeds the vpu video pipeline, > so the order is not an issue. OK, thanks for confirming. Kevin
diff --git a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c index 511b6856225d..5f6519f43a31 100644 --- a/drivers/soc/amlogic/meson-gx-pwrc-vpu.c +++ b/drivers/soc/amlogic/meson-gx-pwrc-vpu.c @@ -34,8 +34,8 @@ struct meson_gx_pwrc_vpu { struct regmap *regmap_ao; struct regmap *regmap_hhi; struct reset_control *rstc; - struct clk *vpu_clk; - struct clk *vapb_clk; + struct clk_bulk_data *clks; + int num_clks; }; static inline @@ -76,8 +76,7 @@ static int meson_gx_pwrc_vpu_power_off(struct generic_pm_domain *genpd) msleep(20); - clk_disable_unprepare(pd->vpu_clk); - clk_disable_unprepare(pd->vapb_clk); + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); return 0; } @@ -119,25 +118,14 @@ static int meson_g12a_pwrc_vpu_power_off(struct generic_pm_domain *genpd) msleep(20); - clk_disable_unprepare(pd->vpu_clk); - clk_disable_unprepare(pd->vapb_clk); + clk_bulk_disable_unprepare(pd->num_clks, pd->clks); return 0; } static int meson_gx_pwrc_vpu_setup_clk(struct meson_gx_pwrc_vpu *pd) { - int ret; - - ret = clk_prepare_enable(pd->vpu_clk); - if (ret) - return ret; - - ret = clk_prepare_enable(pd->vapb_clk); - if (ret) - clk_disable_unprepare(pd->vpu_clk); - - return ret; + return clk_bulk_prepare_enable(pd->num_clks, pd->clks); } static int meson_gx_pwrc_vpu_power_on(struct generic_pm_domain *genpd) @@ -273,8 +261,6 @@ static int meson_gx_pwrc_vpu_probe(struct platform_device *pdev) struct regmap *regmap_ao, *regmap_hhi; struct meson_gx_pwrc_vpu *vpu_pd; struct reset_control *rstc; - struct clk *vpu_clk; - struct clk *vapb_clk; bool powered_off; int ret; @@ -310,23 +296,16 @@ static int meson_gx_pwrc_vpu_probe(struct platform_device *pdev) return PTR_ERR(rstc); } - vpu_clk = devm_clk_get(&pdev->dev, "vpu"); - if (IS_ERR(vpu_clk)) { - dev_err(&pdev->dev, "vpu clock request failed\n"); - return PTR_ERR(vpu_clk); - } - - vapb_clk = devm_clk_get(&pdev->dev, "vapb"); - if (IS_ERR(vapb_clk)) { - dev_err(&pdev->dev, "vapb clock request failed\n"); - return PTR_ERR(vapb_clk); + ret = devm_clk_bulk_get_all(&pdev->dev, &vpu_pd->clks); + if (ret < 0) { + dev_err(&pdev->dev, "bulk clock request failed\n"); + return ret; } + vpu_pd->num_clks = ret; vpu_pd->regmap_ao = regmap_ao; vpu_pd->regmap_hhi = regmap_hhi; vpu_pd->rstc = rstc; - vpu_pd->vpu_clk = vpu_clk; - vpu_pd->vapb_clk = vapb_clk; platform_set_drvdata(pdev, vpu_pd);
Instead of expecting a specific number of clocks with specific clock names, switch to using the bulk clock API. This is a first step towards generalizing this driver to work with other domains. Cc: Neil Armstrong <narmstrong@baylibre.com> Signed-off-by: Kevin Hilman <khilman@baylibre.com> --- Boot tested on meson-g12a-sei510 and verified that framebuffer console comes up and still works. drivers/soc/amlogic/meson-gx-pwrc-vpu.c | 41 ++++++------------------- 1 file changed, 10 insertions(+), 31 deletions(-)