All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
@ 2021-12-07  7:27 ` Baruch Siach
  0 siblings, 0 replies; 12+ messages in thread
From: Baruch Siach @ 2021-12-07  7:27 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson
  Cc: linux-arm-msm, linux-arm-kernel, devicetree, Florian Fainelli,
	Baruch Siach, Sricharan R

There must be three parameters in gpio-ranges property. Fixes this not
very helpful error message:

  OF: /soc/pinctrl@1000000: (null) = 3 found 3

Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
Cc: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---

The error message was improved in commit af3be70a321 ("of: Improve
of_phandle_iterator_next() error message"), but there is evidently some
more room for improvement. As I don't really understand the code, I
added the commit author and the DT list to Cc.
---
 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 6a22bb5f42f4..a717fc17523d 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			gpio-ranges = <&tlmm 0 80>;
+			gpio-ranges = <&tlmm 0 0 80>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 
-- 
2.33.0


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

* [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
@ 2021-12-07  7:27 ` Baruch Siach
  0 siblings, 0 replies; 12+ messages in thread
From: Baruch Siach @ 2021-12-07  7:27 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson
  Cc: linux-arm-msm, linux-arm-kernel, devicetree, Florian Fainelli,
	Baruch Siach, Sricharan R

There must be three parameters in gpio-ranges property. Fixes this not
very helpful error message:

  OF: /soc/pinctrl@1000000: (null) = 3 found 3

Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
Cc: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---

The error message was improved in commit af3be70a321 ("of: Improve
of_phandle_iterator_next() error message"), but there is evidently some
more room for improvement. As I don't really understand the code, I
added the commit author and the DT list to Cc.
---
 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 6a22bb5f42f4..a717fc17523d 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			gpio-ranges = <&tlmm 0 80>;
+			gpio-ranges = <&tlmm 0 0 80>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 
-- 
2.33.0


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

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

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
  2021-12-07  7:27 ` Baruch Siach
@ 2021-12-12 15:44   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2021-12-12 15:44 UTC (permalink / raw)
  To: Baruch Siach, Andy Gross, Bjorn Andersson
  Cc: linux-arm-msm, linux-arm-kernel, devicetree, Florian Fainelli,
	Sricharan R

On 07/12/2021 07:27, Baruch Siach wrote:
> There must be three parameters in gpio-ranges property. Fixes this not
> very helpful error message:
> 
>    OF: /soc/pinctrl@1000000: (null) = 3 found 3
> 
> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
> Cc: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 
> The error message was improved in commit af3be70a321 ("of: Improve
> of_phandle_iterator_next() error message"), but there is evidently some
> more room for improvement. As I don't really understand the code, I
> added the commit author and the DT list to Cc.
> ---
>   arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 6a22bb5f42f4..a717fc17523d 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>   			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>   			gpio-controller;
>   			#gpio-cells = <2>;
> -			gpio-ranges = <&tlmm 0 80>;
> +			gpio-ranges = <&tlmm 0 0 80>;
>   			interrupt-controller;
>   			#interrupt-cells = <2>;
>   
> 
Works for me

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
@ 2021-12-12 15:44   ` Bryan O'Donoghue
  0 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2021-12-12 15:44 UTC (permalink / raw)
  To: Baruch Siach, Andy Gross, Bjorn Andersson
  Cc: linux-arm-msm, linux-arm-kernel, devicetree, Florian Fainelli,
	Sricharan R

On 07/12/2021 07:27, Baruch Siach wrote:
> There must be three parameters in gpio-ranges property. Fixes this not
> very helpful error message:
> 
>    OF: /soc/pinctrl@1000000: (null) = 3 found 3
> 
> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
> Cc: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 
> The error message was improved in commit af3be70a321 ("of: Improve
> of_phandle_iterator_next() error message"), but there is evidently some
> more room for improvement. As I don't really understand the code, I
> added the commit author and the DT list to Cc.
> ---
>   arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 6a22bb5f42f4..a717fc17523d 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>   			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>   			gpio-controller;
>   			#gpio-cells = <2>;
> -			gpio-ranges = <&tlmm 0 80>;
> +			gpio-ranges = <&tlmm 0 0 80>;
>   			interrupt-controller;
>   			#interrupt-cells = <2>;
>   
> 
Works for me

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

_______________________________________________
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] 12+ messages in thread

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
  2021-12-07  7:27 ` Baruch Siach
@ 2021-12-12 15:45   ` Bryan O'Donoghue
  -1 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2021-12-12 15:45 UTC (permalink / raw)
  To: Baruch Siach, Andy Gross, Bjorn Andersson
  Cc: linux-arm-msm, linux-arm-kernel, devicetree, Florian Fainelli,
	Sricharan R

On 07/12/2021 07:27, Baruch Siach wrote:
> There must be three parameters in gpio-ranges property. Fixes this not
> very helpful error message:
> 
>    OF: /soc/pinctrl@1000000: (null) = 3 found 3
> 
> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
> Cc: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 
> The error message was improved in commit af3be70a321 ("of: Improve
> of_phandle_iterator_next() error message"), but there is evidently some
> more room for improvement. As I don't really understand the code, I
> added the commit author and the DT list to Cc.
> ---
>   arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 6a22bb5f42f4..a717fc17523d 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>   			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>   			gpio-controller;
>   			#gpio-cells = <2>;
> -			gpio-ranges = <&tlmm 0 80>;
> +			gpio-ranges = <&tlmm 0 0 80>;
>   			interrupt-controller;
>   			#interrupt-cells = <2>;
>   
> 

Works for me

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
@ 2021-12-12 15:45   ` Bryan O'Donoghue
  0 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2021-12-12 15:45 UTC (permalink / raw)
  To: Baruch Siach, Andy Gross, Bjorn Andersson
  Cc: linux-arm-msm, linux-arm-kernel, devicetree, Florian Fainelli,
	Sricharan R

On 07/12/2021 07:27, Baruch Siach wrote:
> There must be three parameters in gpio-ranges property. Fixes this not
> very helpful error message:
> 
>    OF: /soc/pinctrl@1000000: (null) = 3 found 3
> 
> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
> Cc: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 
> The error message was improved in commit af3be70a321 ("of: Improve
> of_phandle_iterator_next() error message"), but there is evidently some
> more room for improvement. As I don't really understand the code, I
> added the commit author and the DT list to Cc.
> ---
>   arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 6a22bb5f42f4..a717fc17523d 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>   			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>   			gpio-controller;
>   			#gpio-cells = <2>;
> -			gpio-ranges = <&tlmm 0 80>;
> +			gpio-ranges = <&tlmm 0 0 80>;
>   			interrupt-controller;
>   			#interrupt-cells = <2>;
>   
> 

Works for me

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

_______________________________________________
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] 12+ messages in thread

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
  2021-12-07  7:27 ` Baruch Siach
@ 2021-12-15 22:27   ` Bjorn Andersson
  -1 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2021-12-15 22:27 UTC (permalink / raw)
  To: Andy Gross, Baruch Siach
  Cc: linux-arm-msm, linux-arm-kernel, Florian Fainelli, Sricharan R,
	devicetree

On Tue, 7 Dec 2021 09:27:10 +0200, Baruch Siach wrote:
> There must be three parameters in gpio-ranges property. Fixes this not
> very helpful error message:
> 
>   OF: /soc/pinctrl@1000000: (null) = 3 found 3
> 
> 

Applied, thanks!

[1/1] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
      commit: 72cb4c48a46a7cfa58eb5842c0d3672ddd5bd9ad

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

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

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
@ 2021-12-15 22:27   ` Bjorn Andersson
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2021-12-15 22:27 UTC (permalink / raw)
  To: Andy Gross, Baruch Siach
  Cc: linux-arm-msm, linux-arm-kernel, Florian Fainelli, Sricharan R,
	devicetree

On Tue, 7 Dec 2021 09:27:10 +0200, Baruch Siach wrote:
> There must be three parameters in gpio-ranges property. Fixes this not
> very helpful error message:
> 
>   OF: /soc/pinctrl@1000000: (null) = 3 found 3
> 
> 

Applied, thanks!

[1/1] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
      commit: 72cb4c48a46a7cfa58eb5842c0d3672ddd5bd9ad

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

_______________________________________________
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] 12+ messages in thread

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
  2021-12-07  7:27 ` Baruch Siach
@ 2021-12-15 22:44   ` Florian Fainelli
  -1 siblings, 0 replies; 12+ messages in thread
From: Florian Fainelli @ 2021-12-15 22:44 UTC (permalink / raw)
  To: Baruch Siach, Andy Gross, Bjorn Andersson
  Cc: linux-arm-msm, linux-arm-kernel, devicetree, Sricharan R

On 12/6/21 11:27 PM, Baruch Siach wrote:
> There must be three parameters in gpio-ranges property. Fixes this not
> very helpful error message:
> 
>   OF: /soc/pinctrl@1000000: (null) = 3 found 3
> 
> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
> Cc: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 
> The error message was improved in commit af3be70a321 ("of: Improve
> of_phandle_iterator_next() error message"), but there is evidently some
> more room for improvement. As I don't really understand the code, I
> added the commit author and the DT list to Cc.

In the commit message you find what the error message was and then
became which tells you which property was used to determine the cell
size, and what count was obtained instead.

In your particular case here however, since the proper was with the
'gpio-ranges' property and there is no property that indicates the cell
size AFAICT, it might been more helpful to revert back to the old
message maybe? So maybe something like which is hopefully the best of
both worlds:

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 372a177f3b80..63bff37bf035 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1378,9 +1378,13 @@ int of_phandle_iterator_next(struct
of_phandle_iterator *it)
                 * property data length
                 */
                if (it->cur + count > it->list_end) {
-                       pr_err("%pOF: %s = %d found %d\n",
-                              it->parent, it->cells_name,
-                              count, it->cell_count);
+                       if (it->cells_name)
+                               pr_err("%pOF: %s = %d found %d\n",
+                                       it->parent, it->cells_name,
+                                       count, it->cell_count);
+                       else
+                               pr_err("%pOF: arguments longer than
property\n",
+                                      it->parent);
                        goto err;
                }
        }


> ---
>  arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 6a22bb5f42f4..a717fc17523d 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>  			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
> -			gpio-ranges = <&tlmm 0 80>;
> +			gpio-ranges = <&tlmm 0 0 80>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
>  
> 


-- 
Florian

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

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
@ 2021-12-15 22:44   ` Florian Fainelli
  0 siblings, 0 replies; 12+ messages in thread
From: Florian Fainelli @ 2021-12-15 22:44 UTC (permalink / raw)
  To: Baruch Siach, Andy Gross, Bjorn Andersson
  Cc: linux-arm-msm, linux-arm-kernel, devicetree, Sricharan R

On 12/6/21 11:27 PM, Baruch Siach wrote:
> There must be three parameters in gpio-ranges property. Fixes this not
> very helpful error message:
> 
>   OF: /soc/pinctrl@1000000: (null) = 3 found 3
> 
> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
> Cc: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 
> The error message was improved in commit af3be70a321 ("of: Improve
> of_phandle_iterator_next() error message"), but there is evidently some
> more room for improvement. As I don't really understand the code, I
> added the commit author and the DT list to Cc.

In the commit message you find what the error message was and then
became which tells you which property was used to determine the cell
size, and what count was obtained instead.

In your particular case here however, since the proper was with the
'gpio-ranges' property and there is no property that indicates the cell
size AFAICT, it might been more helpful to revert back to the old
message maybe? So maybe something like which is hopefully the best of
both worlds:

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 372a177f3b80..63bff37bf035 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1378,9 +1378,13 @@ int of_phandle_iterator_next(struct
of_phandle_iterator *it)
                 * property data length
                 */
                if (it->cur + count > it->list_end) {
-                       pr_err("%pOF: %s = %d found %d\n",
-                              it->parent, it->cells_name,
-                              count, it->cell_count);
+                       if (it->cells_name)
+                               pr_err("%pOF: %s = %d found %d\n",
+                                       it->parent, it->cells_name,
+                                       count, it->cell_count);
+                       else
+                               pr_err("%pOF: arguments longer than
property\n",
+                                      it->parent);
                        goto err;
                }
        }


> ---
>  arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 6a22bb5f42f4..a717fc17523d 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>  			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>  			gpio-controller;
>  			#gpio-cells = <2>;
> -			gpio-ranges = <&tlmm 0 80>;
> +			gpio-ranges = <&tlmm 0 0 80>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
>  
> 


-- 
Florian

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

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

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
  2021-12-15 22:44   ` Florian Fainelli
@ 2021-12-16 12:11     ` Baruch Siach
  -1 siblings, 0 replies; 12+ messages in thread
From: Baruch Siach @ 2021-12-16 12:11 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andy Gross, Bjorn Andersson, linux-arm-msm, linux-arm-kernel,
	devicetree, Sricharan R

Hi Florian,

On Wed, Dec 15 2021, Florian Fainelli wrote:
> On 12/6/21 11:27 PM, Baruch Siach wrote:
>> There must be three parameters in gpio-ranges property. Fixes this not
>> very helpful error message:
>> 
>>   OF: /soc/pinctrl@1000000: (null) = 3 found 3
>> 
>> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
>> Cc: Sricharan R <sricharan@codeaurora.org>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>> 
>> The error message was improved in commit af3be70a321 ("of: Improve
>> of_phandle_iterator_next() error message"), but there is evidently some
>> more room for improvement. As I don't really understand the code, I
>> added the commit author and the DT list to Cc.
>
> In the commit message you find what the error message was and then
> became which tells you which property was used to determine the cell
> size, and what count was obtained instead.
>
> In your particular case here however, since the proper was with the
> 'gpio-ranges' property and there is no property that indicates the cell
> size AFAICT, it might been more helpful to revert back to the old
> message maybe?

Thanks for taking a look. A few comment below.

> So maybe something like which is hopefully the best of
> both worlds:
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 372a177f3b80..63bff37bf035 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1378,9 +1378,13 @@ int of_phandle_iterator_next(struct
> of_phandle_iterator *it)
>                  * property data length
>                  */
>                 if (it->cur + count > it->list_end) {
> -                       pr_err("%pOF: %s = %d found %d\n",
> -                              it->parent, it->cells_name,
> -                              count, it->cell_count);
> +                       if (it->cells_name)
> +                               pr_err("%pOF: %s = %d found %d\n",
> +                                       it->parent, it->cells_name,
> +                                       count, it->cell_count);

Thinking about it, shouldn't we have 'it->list_end - it->cur' instead of
it->cell_count here? This change produces:

  OF: /soc/pinctrl@1000000: (null) = 3 found 2

which looks more correct. This is not related to my original issue,
though it might have helped me back then.

> +                       else
> +                               pr_err("%pOF: arguments longer than
> property\n",
> +                                      it->parent);
>                         goto err;
>                 }
>         }

I think that the best aid would have been the property name. But as far
as I can see struct of_phandle_iterator does not preserve the list_name
parameter of of_phandle_iterator_init().

Second best would probably be the phandle name, that is
of_node_full_name(it->node). It is a little confusing in this case
because it->node is the same as it->parent.

What do you think?

baruch

>>  arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> index 6a22bb5f42f4..a717fc17523d 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>>  			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>>  			gpio-controller;
>>  			#gpio-cells = <2>;
>> -			gpio-ranges = <&tlmm 0 80>;
>> +			gpio-ranges = <&tlmm 0 0 80>;
>>  			interrupt-controller;
>>  			#interrupt-cells = <2>;
>>  
>> 


-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

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

* Re: [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property
@ 2021-12-16 12:11     ` Baruch Siach
  0 siblings, 0 replies; 12+ messages in thread
From: Baruch Siach @ 2021-12-16 12:11 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andy Gross, Bjorn Andersson, linux-arm-msm, linux-arm-kernel,
	devicetree, Sricharan R

Hi Florian,

On Wed, Dec 15 2021, Florian Fainelli wrote:
> On 12/6/21 11:27 PM, Baruch Siach wrote:
>> There must be three parameters in gpio-ranges property. Fixes this not
>> very helpful error message:
>> 
>>   OF: /soc/pinctrl@1000000: (null) = 3 found 3
>> 
>> Fixes: 1e8277854b49 ("arm64: dts: Add ipq6018 SoC and CP01 board support")
>> Cc: Sricharan R <sricharan@codeaurora.org>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>> 
>> The error message was improved in commit af3be70a321 ("of: Improve
>> of_phandle_iterator_next() error message"), but there is evidently some
>> more room for improvement. As I don't really understand the code, I
>> added the commit author and the DT list to Cc.
>
> In the commit message you find what the error message was and then
> became which tells you which property was used to determine the cell
> size, and what count was obtained instead.
>
> In your particular case here however, since the proper was with the
> 'gpio-ranges' property and there is no property that indicates the cell
> size AFAICT, it might been more helpful to revert back to the old
> message maybe?

Thanks for taking a look. A few comment below.

> So maybe something like which is hopefully the best of
> both worlds:
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 372a177f3b80..63bff37bf035 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1378,9 +1378,13 @@ int of_phandle_iterator_next(struct
> of_phandle_iterator *it)
>                  * property data length
>                  */
>                 if (it->cur + count > it->list_end) {
> -                       pr_err("%pOF: %s = %d found %d\n",
> -                              it->parent, it->cells_name,
> -                              count, it->cell_count);
> +                       if (it->cells_name)
> +                               pr_err("%pOF: %s = %d found %d\n",
> +                                       it->parent, it->cells_name,
> +                                       count, it->cell_count);

Thinking about it, shouldn't we have 'it->list_end - it->cur' instead of
it->cell_count here? This change produces:

  OF: /soc/pinctrl@1000000: (null) = 3 found 2

which looks more correct. This is not related to my original issue,
though it might have helped me back then.

> +                       else
> +                               pr_err("%pOF: arguments longer than
> property\n",
> +                                      it->parent);
>                         goto err;
>                 }
>         }

I think that the best aid would have been the property name. But as far
as I can see struct of_phandle_iterator does not preserve the list_name
parameter of of_phandle_iterator_init().

Second best would probably be the phandle name, that is
of_node_full_name(it->node). It is a little confusing in this case
because it->node is the same as it->parent.

What do you think?

baruch

>>  arch/arm64/boot/dts/qcom/ipq6018.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> index 6a22bb5f42f4..a717fc17523d 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
>> @@ -220,7 +220,7 @@ tlmm: pinctrl@1000000 {
>>  			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
>>  			gpio-controller;
>>  			#gpio-cells = <2>;
>> -			gpio-ranges = <&tlmm 0 80>;
>> +			gpio-ranges = <&tlmm 0 0 80>;
>>  			interrupt-controller;
>>  			#interrupt-cells = <2>;
>>  
>> 


-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

_______________________________________________
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] 12+ messages in thread

end of thread, other threads:[~2021-12-16 13:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-07  7:27 [PATCH] arm64: dts: qcom: ipq6018: Fix gpio-ranges property Baruch Siach
2021-12-07  7:27 ` Baruch Siach
2021-12-12 15:44 ` Bryan O'Donoghue
2021-12-12 15:44   ` Bryan O'Donoghue
2021-12-12 15:45 ` Bryan O'Donoghue
2021-12-12 15:45   ` Bryan O'Donoghue
2021-12-15 22:27 ` Bjorn Andersson
2021-12-15 22:27   ` Bjorn Andersson
2021-12-15 22:44 ` Florian Fainelli
2021-12-15 22:44   ` Florian Fainelli
2021-12-16 12:11   ` Baruch Siach
2021-12-16 12:11     ` Baruch Siach

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.