All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pinctrl: sunxi: always look for apb block
@ 2018-03-01 11:37 ` Andre Przywara
  0 siblings, 0 replies; 6+ messages in thread
From: Andre Przywara @ 2018-03-01 11:37 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai
  Cc: Linus Walleij, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

The Allwinner pinctrl device tree binding suggests that a clock named
"apb" would drive the pin controller IP. However (for legacy reasons) we
rely on this clock actually being the first clock defined.
Since named clocks can be in any order, let's explicitly check for a
clock called "apb" first, then revert to using the first (and only!)
clock to keep compatibility with older DTs.

Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 4b6cb25bc796..897ba13e6a03 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -1360,10 +1360,21 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
 			goto gpiochip_error;
 	}
 
-	clk = devm_clk_get(&pdev->dev, NULL);
+	clk = devm_clk_get(&pdev->dev, "apb");
 	if (IS_ERR(clk)) {
-		ret = PTR_ERR(clk);
-		goto gpiochip_error;
+		/*
+		 * If no "apb" clock can be found, try the first one, which
+		 * must be the only one defined.
+		 */
+		if (PTR_ERR(clk) == -ENOENT &&
+		    of_count_phandle_with_args(node, "clocks",
+					       "#clock-cells") == 1)
+			clk = devm_clk_get(&pdev->dev, NULL);
+
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+			goto gpiochip_error;
+		}
 	}
 
 	ret = clk_prepare_enable(clk);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] pinctrl: sunxi: always look for apb block
@ 2018-03-01 11:37 ` Andre Przywara
  0 siblings, 0 replies; 6+ messages in thread
From: Andre Przywara @ 2018-03-01 11:37 UTC (permalink / raw)
  To: linux-arm-kernel

The Allwinner pinctrl device tree binding suggests that a clock named
"apb" would drive the pin controller IP. However (for legacy reasons) we
rely on this clock actually being the first clock defined.
Since named clocks can be in any order, let's explicitly check for a
clock called "apb" first, then revert to using the first (and only!)
clock to keep compatibility with older DTs.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index 4b6cb25bc796..897ba13e6a03 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -1360,10 +1360,21 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
 			goto gpiochip_error;
 	}
 
-	clk = devm_clk_get(&pdev->dev, NULL);
+	clk = devm_clk_get(&pdev->dev, "apb");
 	if (IS_ERR(clk)) {
-		ret = PTR_ERR(clk);
-		goto gpiochip_error;
+		/*
+		 * If no "apb" clock can be found, try the first one, which
+		 * must be the only one defined.
+		 */
+		if (PTR_ERR(clk) == -ENOENT &&
+		    of_count_phandle_with_args(node, "clocks",
+					       "#clock-cells") == 1)
+			clk = devm_clk_get(&pdev->dev, NULL);
+
+		if (IS_ERR(clk)) {
+			ret = PTR_ERR(clk);
+			goto gpiochip_error;
+		}
 	}
 
 	ret = clk_prepare_enable(clk);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] pinctrl: sunxi: always look for apb block
  2018-03-01 11:37 ` Andre Przywara
@ 2018-03-01 11:48   ` Maxime Ripard
  -1 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2018-03-01 11:48 UTC (permalink / raw)
  To: Andre Przywara
  Cc: linux-gpio, Chen-Yu Tsai, linux-sunxi, Linus Walleij, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1911 bytes --]

Hi,

On Thu, Mar 01, 2018 at 11:37:01AM +0000, Andre Przywara wrote:
> The Allwinner pinctrl device tree binding suggests that a clock named
> "apb" would drive the pin controller IP. However (for legacy reasons) we
> rely on this clock actually being the first clock defined.
> Since named clocks can be in any order, let's explicitly check for a
> clock called "apb" first, then revert to using the first (and only!)
> clock to keep compatibility with older DTs.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 4b6cb25bc796..897ba13e6a03 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -1360,10 +1360,21 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>  			goto gpiochip_error;
>  	}
>  
> -	clk = devm_clk_get(&pdev->dev, NULL);
> +	clk = devm_clk_get(&pdev->dev, "apb");
>  	if (IS_ERR(clk)) {
> -		ret = PTR_ERR(clk);
> -		goto gpiochip_error;
> +		/*
> +		 * If no "apb" clock can be found, try the first one, which
> +		 * must be the only one defined.
> +		 */
> +		if (PTR_ERR(clk) == -ENOENT &&
> +		    of_count_phandle_with_args(node, "clocks",
> +					       "#clock-cells") == 1)
> +			clk = devm_clk_get(&pdev->dev, NULL);
> +
> +		if (IS_ERR(clk)) {
> +			ret = PTR_ERR(clk);
> +			goto gpiochip_error;
> +		}

I guess this can be made slightly less convoluted using something like

ret = of_count_phandle_with_args(node, "clocks", "#clock-cells");
clk = devm_clk_get(&pdev->dev, ret == 1 ? NULL : "apb");

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] pinctrl: sunxi: always look for apb block
@ 2018-03-01 11:48   ` Maxime Ripard
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2018-03-01 11:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, Mar 01, 2018 at 11:37:01AM +0000, Andre Przywara wrote:
> The Allwinner pinctrl device tree binding suggests that a clock named
> "apb" would drive the pin controller IP. However (for legacy reasons) we
> rely on this clock actually being the first clock defined.
> Since named clocks can be in any order, let's explicitly check for a
> clock called "apb" first, then revert to using the first (and only!)
> clock to keep compatibility with older DTs.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 4b6cb25bc796..897ba13e6a03 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -1360,10 +1360,21 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>  			goto gpiochip_error;
>  	}
>  
> -	clk = devm_clk_get(&pdev->dev, NULL);
> +	clk = devm_clk_get(&pdev->dev, "apb");
>  	if (IS_ERR(clk)) {
> -		ret = PTR_ERR(clk);
> -		goto gpiochip_error;
> +		/*
> +		 * If no "apb" clock can be found, try the first one, which
> +		 * must be the only one defined.
> +		 */
> +		if (PTR_ERR(clk) == -ENOENT &&
> +		    of_count_phandle_with_args(node, "clocks",
> +					       "#clock-cells") == 1)
> +			clk = devm_clk_get(&pdev->dev, NULL);
> +
> +		if (IS_ERR(clk)) {
> +			ret = PTR_ERR(clk);
> +			goto gpiochip_error;
> +		}

I guess this can be made slightly less convoluted using something like

ret = of_count_phandle_with_args(node, "clocks", "#clock-cells");
clk = devm_clk_get(&pdev->dev, ret == 1 ? NULL : "apb");

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180301/bd3a1390/attachment.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] pinctrl: sunxi: always look for apb block
  2018-03-01 11:48   ` Maxime Ripard
@ 2018-03-02 10:07     ` Andre Przywara
  -1 siblings, 0 replies; 6+ messages in thread
From: Andre Przywara @ 2018-03-02 10:07 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, Linus Walleij,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

Hi Maxime,

On 01/03/18 11:48, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Mar 01, 2018 at 11:37:01AM +0000, Andre Przywara wrote:
>> The Allwinner pinctrl device tree binding suggests that a clock named
>> "apb" would drive the pin controller IP. However (for legacy reasons) we
>> rely on this clock actually being the first clock defined.
>> Since named clocks can be in any order, let's explicitly check for a
>> clock called "apb" first, then revert to using the first (and only!)
>> clock to keep compatibility with older DTs.
>>
>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>> ---
>>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> index 4b6cb25bc796..897ba13e6a03 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> @@ -1360,10 +1360,21 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>>  			goto gpiochip_error;
>>  	}
>>  
>> -	clk = devm_clk_get(&pdev->dev, NULL);
>> +	clk = devm_clk_get(&pdev->dev, "apb");
>>  	if (IS_ERR(clk)) {
>> -		ret = PTR_ERR(clk);
>> -		goto gpiochip_error;
>> +		/*
>> +		 * If no "apb" clock can be found, try the first one, which
>> +		 * must be the only one defined.
>> +		 */
>> +		if (PTR_ERR(clk) == -ENOENT &&
>> +		    of_count_phandle_with_args(node, "clocks",
>> +					       "#clock-cells") == 1)
>> +			clk = devm_clk_get(&pdev->dev, NULL);
>> +
>> +		if (IS_ERR(clk)) {
>> +			ret = PTR_ERR(clk);
>> +			goto gpiochip_error;
>> +		}
> 
> I guess this can be made slightly less convoluted using something like

                           ^^^^^^^^ that's a slight understatement

> 
> ret = of_count_phandle_with_args(node, "clocks", "#clock-cells");
> clk = devm_clk_get(&pdev->dev, ret == 1 ? NULL : "apb");

Pah, that's just embarrassing.
I guess I will need to back your Kickstarter now to buy me out of this.

Will send v2 after testing tonight.

Thanks a lot for having a look!

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] pinctrl: sunxi: always look for apb block
@ 2018-03-02 10:07     ` Andre Przywara
  0 siblings, 0 replies; 6+ messages in thread
From: Andre Przywara @ 2018-03-02 10:07 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Maxime,

On 01/03/18 11:48, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Mar 01, 2018 at 11:37:01AM +0000, Andre Przywara wrote:
>> The Allwinner pinctrl device tree binding suggests that a clock named
>> "apb" would drive the pin controller IP. However (for legacy reasons) we
>> rely on this clock actually being the first clock defined.
>> Since named clocks can be in any order, let's explicitly check for a
>> clock called "apb" first, then revert to using the first (and only!)
>> clock to keep compatibility with older DTs.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> index 4b6cb25bc796..897ba13e6a03 100644
>> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
>> @@ -1360,10 +1360,21 @@ int sunxi_pinctrl_init_with_variant(struct platform_device *pdev,
>>  			goto gpiochip_error;
>>  	}
>>  
>> -	clk = devm_clk_get(&pdev->dev, NULL);
>> +	clk = devm_clk_get(&pdev->dev, "apb");
>>  	if (IS_ERR(clk)) {
>> -		ret = PTR_ERR(clk);
>> -		goto gpiochip_error;
>> +		/*
>> +		 * If no "apb" clock can be found, try the first one, which
>> +		 * must be the only one defined.
>> +		 */
>> +		if (PTR_ERR(clk) == -ENOENT &&
>> +		    of_count_phandle_with_args(node, "clocks",
>> +					       "#clock-cells") == 1)
>> +			clk = devm_clk_get(&pdev->dev, NULL);
>> +
>> +		if (IS_ERR(clk)) {
>> +			ret = PTR_ERR(clk);
>> +			goto gpiochip_error;
>> +		}
> 
> I guess this can be made slightly less convoluted using something like

                           ^^^^^^^^ that's a slight understatement

> 
> ret = of_count_phandle_with_args(node, "clocks", "#clock-cells");
> clk = devm_clk_get(&pdev->dev, ret == 1 ? NULL : "apb");

Pah, that's just embarrassing.
I guess I will need to back your Kickstarter now to buy me out of this.

Will send v2 after testing tonight.

Thanks a lot for having a look!

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-02 10:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-01 11:37 [PATCH] pinctrl: sunxi: always look for apb block Andre Przywara
2018-03-01 11:37 ` Andre Przywara
2018-03-01 11:48 ` Maxime Ripard
2018-03-01 11:48   ` Maxime Ripard
2018-03-02 10:07   ` Andre Przywara
2018-03-02 10:07     ` Andre Przywara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.