diff mbox series

[05/18] MIPS: DTS: jz4780: fix pinctrl as reported by dtbscheck

Message ID 1941bc4ed553b27f399ad00ea61ff2b0237d14e3.1649443080.git.hns@goldelico.com
State New
Headers show
Series MIPS: DTS: fix some findings by "make ci20_defconfig dt_binding_check dtbs_check" | expand

Commit Message

H. Nikolaus Schaller April 8, 2022, 6:37 p.m. UTC
arch/mips/boot/dts/ingenic/ci20.dtb: pin-controller@10010000: $nodename:0: 'pin-controller@10010000' does not match '^(pinctrl|pinmux)(@[0-9a-f]+)?$'
	From schema: Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Krzysztof Kozlowski April 9, 2022, 11:13 a.m. UTC | #1
On 08/04/2022 20:37, H. Nikolaus Schaller wrote:
> arch/mips/boot/dts/ingenic/ci20.dtb: pin-controller@10010000: $nodename:0: 'pin-controller@10010000' does not match '^(pinctrl|pinmux)(@[0-9a-f]+)?$'
> 	From schema: Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index 5f44cf004d473..b5299eaffb84a 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -155,7 +155,7 @@ rtc_dev: rtc@10003000 {
>  		clock-names = "rtc";
>  	};
>  
> -	pinctrl: pin-controller@10010000 {
> +	pinctrl: pinctrl@10010000 {

Do it once for all DTSes, not one file at a time. There are four more
places with this.

Best regards,
Krzysztof
Krzysztof Kozlowski April 9, 2022, 1:13 p.m. UTC | #2
On 09/04/2022 15:04, H. Nikolaus Schaller wrote:
> 
> 
>> Am 09.04.2022 um 13:13 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
>>
>> On 08/04/2022 20:37, H. Nikolaus Schaller wrote:
>>> arch/mips/boot/dts/ingenic/ci20.dtb: pin-controller@10010000: $nodename:0: 'pin-controller@10010000' does not match '^(pinctrl|pinmux)(@[0-9a-f]+)?$'
>>> 	From schema: Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml
>>>
>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>> ---
>>> arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> index 5f44cf004d473..b5299eaffb84a 100644
>>> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>> @@ -155,7 +155,7 @@ rtc_dev: rtc@10003000 {
>>> 		clock-names = "rtc";
>>> 	};
>>>
>>> -	pinctrl: pin-controller@10010000 {
>>> +	pinctrl: pinctrl@10010000 {
>>
>> Do it once for all DTSes, not one file at a time. There are four more
>> places with this.
> 
> Well, automation has no notion of "similarity" in this case to
> merge several patches.

What does that mean? One cannot create multiple patches and apply them?

> And they are not related. Every one is based on a different .yaml
> schema file.

Which does not matter, because the name of the node does not matter. We
enforce it in schema to makes things organized and easier in testing.
This does not fix any real problem, just the problem we created by
ourselves with schema.

> 
> That in all cases the result looks similar comes from similar
> requirements by the schemata and has no inherent connection.

All schemas will require it, won't they? The same for arm...

Best regards,
Krzysztof
H. Nikolaus Schaller April 9, 2022, 1:22 p.m. UTC | #3
> Am 09.04.2022 um 15:13 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> 
> On 09/04/2022 15:04, H. Nikolaus Schaller wrote:
>> 
>> 
>>> Am 09.04.2022 um 13:13 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
>>> 
>>> On 08/04/2022 20:37, H. Nikolaus Schaller wrote:
>>>> arch/mips/boot/dts/ingenic/ci20.dtb: pin-controller@10010000: $nodename:0: 'pin-controller@10010000' does not match '^(pinctrl|pinmux)(@[0-9a-f]+)?$'
>>>> 	From schema: Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml
>>>> 
>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>> ---
>>>> arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> 
>>>> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>> index 5f44cf004d473..b5299eaffb84a 100644
>>>> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
>>>> @@ -155,7 +155,7 @@ rtc_dev: rtc@10003000 {
>>>> 		clock-names = "rtc";
>>>> 	};
>>>> 
>>>> -	pinctrl: pin-controller@10010000 {
>>>> +	pinctrl: pinctrl@10010000 {
>>> 
>>> Do it once for all DTSes, not one file at a time. There are four more
>>> places with this.
>> 
>> Well, automation has no notion of "similarity" in this case to
>> merge several patches.
> 
> What does that mean? One cannot create multiple patches and apply them?

This patch set was created by some automatic scripts. And they produce one patch
per group of warnings.

But here you ask me to merge 4 unrelated topics into a single one.

Or do you mean something else?

> 
>> And they are not related. Every one is based on a different .yaml
>> schema file.
> 
> Which does not matter, because the name of the node does not matter. We
> enforce it in schema to makes things organized and easier in testing.
> This does not fix any real problem, just the problem we created by
> ourselves with schema.
> 
>> 
>> That in all cases the result looks similar comes from similar
>> requirements by the schemata and has no inherent connection.
> 
> All schemas will require it, won't they? The same for arm...

We may be talking about different things here.

My understanding:
you ask me to merge 5/18, 8/18, 9/18, 12/18 because they contain "controller" in the node-name.

Right? If not then we must clarify that first.

For pinctrl it is not allowed to have a -controller suffix while for the other it is mandatory
by the schema pattern.

BR and thanks,
Nikolaus
H. Nikolaus Schaller April 9, 2022, 1:41 p.m. UTC | #4
> Am 09.04.2022 um 15:24 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> 
> On 09/04/2022 15:22, H. Nikolaus Schaller wrote:
>>> 
>>> What does that mean? One cannot create multiple patches and apply them?
>> 
>> This patch set was created by some automatic scripts. And they produce one patch
>> per group of warnings.
>> 
>> But here you ask me to merge 4 unrelated topics into a single one.
>> 
>> Or do you mean something else?
> 
> You can edit a commit, right? git commit --amend? So where is the problem?

It is not about capabilites...

It is about understanding why you want it and what you expect.

> 
>> 
>>> 
>>>> And they are not related. Every one is based on a different .yaml
>>>> schema file.
>>> 
>>> Which does not matter, because the name of the node does not matter. We
>>> enforce it in schema to makes things organized and easier in testing.
>>> This does not fix any real problem, just the problem we created by
>>> ourselves with schema.
>>> 
>>>> 
>>>> That in all cases the result looks similar comes from similar
>>>> requirements by the schemata and has no inherent connection.
>>> 
>>> All schemas will require it, won't they? The same for arm...
>> 
>> We may be talking about different things here.
>> 
>> My understanding:
>> you ask me to merge 5/18, 8/18, 9/18, 12/18 because they contain "controller" in the node-name.
>> 
>> Right? If not then we must clarify that first.
> 
> No. I ask you to fix all pin-controller cases, for entire MIPS, not just
> one.

Oops. Nope. I am a volunteer and neither your employee nor slave.

> And in one month one more. And then again one more.

No. I work for the topics I choose to work on and share the results.
Open source lives from taking and giving...

If you want me to contribute, please be not demanding.

BR and thanks,
Nikolaus
Krzysztof Kozlowski April 9, 2022, 1:46 p.m. UTC | #5
On 09/04/2022 15:41, H. Nikolaus Schaller wrote:
>>
>> No. I ask you to fix all pin-controller cases, for entire MIPS, not just
>> one.
> 
> Oops. Nope. I am a volunteer and neither your employee nor slave.

No one thinks differently and I am sorry that you felt it. Please accept
my apologies, if you get different impression. You understand though the
meaning of word "ask for something" and "order something" (the latter
which I did not use).

I just asked.

Best regards,
Krzysztof
H. Nikolaus Schaller April 9, 2022, 1:57 p.m. UTC | #6
> Am 09.04.2022 um 15:46 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> 
> On 09/04/2022 15:41, H. Nikolaus Schaller wrote:
>>> 
>>> No. I ask you to fix all pin-controller cases, for entire MIPS, not just
>>> one.
>> 
>> Oops. Nope. I am a volunteer and neither your employee nor slave.
> 
> No one thinks differently and I am sorry that you felt it. Please accept
> my apologies, if you get different impression. You understand though the
> meaning of word "ask for something" and "order something" (the latter
> which I did not use).
> 
> I just asked.

Ok. Maybe english is not our mother language and we sometimes don't
get the nuances right. Sorry if I understood that wrongly.

At least I now understand what you did suggest.

Doing the same change for treewide MIPS is beyond my capabilities since
I can't easily test any compile setup. So far I only compile for CI20 and
as far as I know every machine still needs its own config for MIPS 
(haven't checked recently). So I am not even sure if dtbscheck tells me
all locations.

BR and thanks,
Nikolaus
H. Nikolaus Schaller April 9, 2022, 2:06 p.m. UTC | #7
> Am 09.04.2022 um 16:00 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
> 
> On 09/04/2022 15:57, H. Nikolaus Schaller wrote:
>> 
>> 
>>> Am 09.04.2022 um 15:46 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
>>> 
>>> On 09/04/2022 15:41, H. Nikolaus Schaller wrote:
>>>>> 
>>>>> No. I ask you to fix all pin-controller cases, for entire MIPS, not just
>>>>> one.
>>>> 
>>>> Oops. Nope. I am a volunteer and neither your employee nor slave.
>>> 
>>> No one thinks differently and I am sorry that you felt it. Please accept
>>> my apologies, if you get different impression. You understand though the
>>> meaning of word "ask for something" and "order something" (the latter
>>> which I did not use).
>>> 
>>> I just asked.
>> 
>> Ok. Maybe english is not our mother language and we sometimes don't
>> get the nuances right. Sorry if I understood that wrongly.
>> 
>> At least I now understand what you did suggest.
> 
> Yeah, probably I did not express my thoughts correctly.
> 
> I would like to state that I appreciate your work and I think it is
> important, even if I do not express it correctly. Please accept my
> apologies if I am bit harsh or impolite. That's not my intention.

I also was a little harsh in my response. Sorry again.

> 
>> 
>> Doing the same change for treewide MIPS is beyond my capabilities since
>> I can't easily test any compile setup. So far I only compile for CI20 and
>> as far as I know every machine still needs its own config for MIPS 
>> (haven't checked recently). So I am not even sure if dtbscheck tells me
>> all locations.
> 
> 
> OK, fair enough.
> 
> 
> Best regards,
> Krzysztof

Anyways thank you very much for your reviews and sharing comments.

As far as I see there are only the jz4780-nemc/simple-mfd and the snps,dwc2/jz4780-otg
issues really unsolved while the others are more cosmetics.

Let's see how these settle.

BR and thanks,
Nikolaus
diff mbox series

Patch

diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index 5f44cf004d473..b5299eaffb84a 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -155,7 +155,7 @@  rtc_dev: rtc@10003000 {
 		clock-names = "rtc";
 	};
 
-	pinctrl: pin-controller@10010000 {
+	pinctrl: pinctrl@10010000 {
 		compatible = "ingenic,jz4780-pinctrl";
 		reg = <0x10010000 0x600>;