Linux-Amlogic Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk
@ 2019-08-09 23:09 Kevin Hilman
  2019-08-12  7:18 ` Neil Armstrong
  2019-08-13 22:47 ` Kevin Hilman
  0 siblings, 2 replies; 5+ messages in thread
From: Kevin Hilman @ 2019-08-09 23:09 UTC (permalink / raw)
  To: Neil Armstrong, Martin Blumenstingl; +Cc: linux-amlogic

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);
 
-- 
2.22.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk
  2019-08-09 23:09 [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk Kevin Hilman
@ 2019-08-12  7:18 ` Neil Armstrong
  2019-08-13 22:47 ` Kevin Hilman
  1 sibling, 0 replies; 5+ messages in thread
From: Neil Armstrong @ 2019-08-12  7:18 UTC (permalink / raw)
  To: Kevin Hilman, Martin Blumenstingl; +Cc: linux-amlogic

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>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk
  2019-08-09 23:09 [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk Kevin Hilman
  2019-08-12  7:18 ` Neil Armstrong
@ 2019-08-13 22:47 ` Kevin Hilman
  2019-08-14  7:45   ` Neil Armstrong
  1 sibling, 1 reply; 5+ messages in thread
From: Kevin Hilman @ 2019-08-13 22:47 UTC (permalink / raw)
  To: Neil Armstrong, Martin Blumenstingl; +Cc: linux-amlogic

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk
  2019-08-13 22:47 ` Kevin Hilman
@ 2019-08-14  7:45   ` Neil Armstrong
  2019-08-14 14:12     ` Kevin Hilman
  0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2019-08-14  7:45 UTC (permalink / raw)
  To: Kevin Hilman, Martin Blumenstingl; +Cc: linux-amlogic

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
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk
  2019-08-14  7:45   ` Neil Armstrong
@ 2019-08-14 14:12     ` Kevin Hilman
  0 siblings, 0 replies; 5+ messages in thread
From: Kevin Hilman @ 2019-08-14 14:12 UTC (permalink / raw)
  To: Neil Armstrong, Martin Blumenstingl; +Cc: linux-amlogic

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-09 23:09 [PATCH/RFT] soc: amlogic: meson-gx-pwrc-vpu: switch to clk_bulk Kevin Hilman
2019-08-12  7:18 ` Neil Armstrong
2019-08-13 22:47 ` Kevin Hilman
2019-08-14  7:45   ` Neil Armstrong
2019-08-14 14:12     ` Kevin Hilman

Linux-Amlogic Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-amlogic/0 linux-amlogic/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-amlogic linux-amlogic/ https://lore.kernel.org/linux-amlogic \
		linux-amlogic@lists.infradead.org linux-amlogic@archiver.kernel.org
	public-inbox-index linux-amlogic


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-amlogic


AGPL code for this site: git clone https://public-inbox.org/ public-inbox