Skip to contentSkip to conversation

MIPS: DTS: fix some findings by "make ci20_defconfig dt_binding_check dtbs_check"

PATCH In review H. Nikolaus Schaller 5 participants 83 messages Apr 8, 2022 – Apr 12, 2022

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover MIPS: DTS: fix some findings by "make ci20_defconfig dt_binding_check dtbs_check"

PATCH V1 2022-04-08 20:38:00:
This series fixes many (not all) warnings from dt_binding_check/dtbs_check for the jz4780 based Imagination CI20 board.

H. Nikolaus Schaller (18):
MIPS: DTS: jz4780: remove cpu clock-names as reported by dtbscheck
MIPS: DTS: jz4780: fix cgu as reported by dtbscheck
MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck
MIPS: DTS: jz4780: fix ost timer as reported by dtbscheck
MIPS: DTS: jz4780: fix pinctrl as reported by dtbscheck
MIPS: DTS: jz4780: fix rtc node as reported by dtbscheck
MIPS: DTS: jz4780: fix otg node as reported by dtbscheck
MIPS: DTS: jz4780: fix lcd controllers as reported by dtbscheck
MIPS: DTS: jz4780: fix dma-controller as reported by dtbscheck
MIPS: DTS: jz4780: fix uart dmas as reported by dtbscheck
MIPS: DTS: jz4780: fix i2c dmas as reported by dtbscheck
MIPS: DTS: jz4780: fix nemc memory controller as reported by dtbscheck
dt-bindings: fix jz4780-nemc issue as reported by dtbscheck
MIPS: DTS: CI20: add missing model as reported by dtbscheck
MIPS: DTS: CI20: fix fixed regulators as reported by dtbscheck
MIPS: DTS: CI20: fix /memory as reported by dtbscheck
MIPS: DTS: CI20: fix wifi as reported by dtbscheck
MIPS: DTS: CI20: fix bluetooth as reported by dtbscheck

 .../memory-controllers/ingenic,nemc.yaml      |  2 +-
 arch/mips/boot/dts/ingenic/ci20.dts           | 14 ++--
 arch/mips/boot/dts/ingenic/jz4780.dtsi        | 71 ++++++++++++++-----
 3 files changed, 64 insertions(+), 23 deletions(-)
Signature · 1 line
2.33.0
#1 lore raw

PATCH 1/18 MIPS: DTS: jz4780: remove cpu clock-names as reported by dtbscheck

arch/mips/boot/dts/ingenic/ci20.dtb: cpu@0: clock-names does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: cpu@1: clock-names does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 --
 1 file changed, 2 deletions(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -2
index b998301f179ce..710605df40ed3 100644
@@ -18,7 +18,6 @@ cpu0: cpu@0 {
 			reg = <0>;
 
 			clocks = <&cgu JZ4780_CLK_CPU>;
-			clock-names = "cpu";
 		};
 
 		cpu1: cpu@1 {
@@ -27,7 +26,6 @@ cpu1: cpu@1 {
 			reg = <1>;
 
 			clocks = <&cgu JZ4780_CLK_CORE1>;
-			clock-names = "cpu";
 		};
 	};
 
Signature · 1 line
2.33.0
Reviewed-by on v1 1/18
#2 lore raw

Re: [PATCH 01/18] MIPS: DTS: jz4780: remove cpu clock-names as reported by dtbscheck

Quoted 4 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: cpu@0: clock-names does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: cpu@1: clock-names does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml

No need to put the same warning twice (cpu index really does not matter). One warning is enough.

Fixing warnings is good, but what if the property should be there and the bindings are not correct? If you know the answer, please add it to the commit msg.

This applies to all your patches. Blind fixing of DTS warnings my produce incorrect results. :(

Best regards,
Krzysztof

#3 lore raw

Re: [PATCH 01/18] MIPS: DTS: jz4780: remove cpu clock-names as reported by dtbscheck

Hi Krzysztof,

Quoted 10 lines

Am 09.04.2022 um 13:07 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 4 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: cpu@0: clock-names does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: cpu@1: clock-names does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml

No need to put the same warning twice (cpu index really does not matter). One warning is enough.

Well, how can you as a reviewer see immediately that dtbscheck warns for both instances and that they are both really fixed by the complete patch?

Assume there were only the first one mentioned and the second one forgotten to fix...

If it is not included here (completely), you have to run dtbscheck yourself to see that there are two instances involved. Or deduce it from looking into the source file.

In both cases much more work for you or other reviewers.

This is why I have kept this redundancy. One commit hunk for every warning line.

And it may only look redundant in the commit message, not in code where it would really hurt to have duplication.

Quoted 4 lines

Fixing warnings is good, but what if the property should be there and the bindings are not correct? If you know the answer, please add it to the commit msg.

It has of course been tested on real hardware.
There was only one case where the bindings seem to be wrong (patch 13/18).

And usually I simply assume such generic bindings are tested on many other platforms and therefore the reference. What are bindings and dtbscheck good for if we doubt them as DTS developers?

Quoted 2 lines

This applies to all your patches. Blind fixing of DTS warnings my produce incorrect results. :(

Every patch of this series has been tested on real hardware to have no negative side-effects.

Quoted 5 lines

Best regards,
Krzysztof

BR and thanks,
Nikolaus

#4 lore raw

Re: [PATCH 01/18] MIPS: DTS: jz4780: remove cpu clock-names as reported by dtbscheck

Quoted 24 lines · H. Nikolaus Schaller

Hi Krzysztof,

Quoted 10 lines

Am 09.04.2022 um 13:07 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 4 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: cpu@0: clock-names does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: cpu@1: clock-names does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml

No need to put the same warning twice (cpu index really does not matter). One warning is enough.

Well, how can you as a reviewer see immediately that dtbscheck warns for both instances and that they are both really fixed by the complete patch?

Assume there were only the first one mentioned and the second one forgotten to fix...

If it is not included here (completely), you have to run dtbscheck yourself to see that there are two instances involved. Or deduce it from looking into the source file.

Quoted 7 lines

In both cases much more work for you or other reviewers.

This is why I have kept this redundancy. One commit hunk for every warning line.

And it may only look redundant in the commit message, not in code where it would really hurt to have duplication.

Sorry, but the second warning is obvious. It really does not bring any information and better to keep things simple and concise. Concise helps to read/understand the commit.

Quoted 7 lines
Quoted 5 lines

Fixing warnings is good, but what if the property should be there and the bindings are not correct? If you know the answer, please add it to the commit msg.

It has of course been tested on real hardware.

Good, but it's not everything. DTS is a description of hardware and we might want to have the extended description, e.g. with clock-names.

Quoted 5 lines

There was only one case where the bindings seem to be wrong (patch 13/18).

And usually I simply assume such generic bindings are tested on many other platforms and therefore the reference. What are bindings and dtbscheck good for if we doubt them as DTS developers?

You should always doubt, knowing how incompletely or poorly bindings were written or converted (including the ones I was doing myself).

Quoted 6 lines
Quoted 3 lines

This applies to all your patches. Blind fixing of DTS warnings my produce incorrect results. :(

Every patch of this series has been tested on real hardware to have no negative side-effects.

Which is not everything. DTS might be used in other projects - did you test them as well?

Best regards,
Krzysztof

#5 lore raw

PATCH 2/18 MIPS: DTS: jz4780: fix cgu as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: jz4780-cgu@10000000: $nodename:0: 'jz4780-cgu@10000000' does not match '^clock-controller@[0-9a-f]+$'
From schema: Documentation/devicetree/bindings/clock/ingenic,cgu.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
index 710605df40ed3..6fb6229c3186b 100644
@@ -58,7 +58,7 @@ rtc: rtc {
 		clock-frequency = <32768>;
 	};
 
-	cgu: jz4780-cgu@10000000 {
+	cgu: clock-controller@10000000 {
 		compatible = "ingenic,jz4780-cgu", "simple-mfd";
 		reg = <0x10000000 0x100>;
 		#address-cells = <1>;
Signature · 1 line
2.33.0
Reviewed-by on v1 2/18
#6 lore raw

Re: [PATCH 02/18] MIPS: DTS: jz4780: fix cgu as reported by dtbscheck

Quoted 8 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: jz4780-cgu@10000000: $nodename:0: 'jz4780-cgu@10000000' does not match '^clock-controller@[0-9a-f]+$'
From schema: Documentation/devicetree/bindings/clock/ingenic,cgu.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Best regards,
Krzysztof

#7 lore raw

PATCH 3/18 MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-tcu', 'ingenic,jz4770-tcu', 'simple-mfd'] is too long
'ingenic,jz4780-tcu' is not one of ['ingenic,jz4740-tcu', 'ingenic,jz4725b-tcu', 'ingenic,jz4760-tcu', 'ingenic,x1000-tcu'] 'simple-mfd' was expected
'ingenic,jz4760-tcu' was expected
From schema: Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
index 6fb6229c3186b..6027f14c393e3 100644
@@ -91,7 +91,7 @@ rng: rng@d8 {
 
 	tcu: timer@10002000 {
 		compatible = "ingenic,jz4780-tcu",
-			     "ingenic,jz4770-tcu",
+			     "ingenic,jz4760-tcu",
 			     "simple-mfd";
 		reg = <0x10002000 0x1000>;
 		#address-cells = <1>;
Signature · 1 line
2.33.0
#8 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

Quoted 5 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: compatible: 'oneOf' conditional failed, one must be fixed:

	['ingenic,jz4780-tcu', 'ingenic,jz4770-tcu', 'simple-mfd'] is too long
	'ingenic,jz4780-tcu' is not one of ['ingenic,jz4740-tcu', 'ingenic,jz4725b-tcu', 'ingenic,jz4760-tcu', 'ingenic,x1000-tcu']
	'simple-mfd' was expected
	'ingenic,jz4760-tcu' was expected

Trim it a bit...

Quoted 1 line

From schema: Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

You need to explain this. You're changing the effective compatible of the device and doing so based only on schema warning does not look enough. Please write real reason instead of this fat warning, e.g. that both devices are actually compatible and this has no real effect except schema checks.

Best regards,
Krzysztof

maintainer
#9 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

Hi Krzysztof,

Quoted 12 lines · Krzysztof Kozlowski

Review

Quoted 8 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: compatible:
'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-tcu', 'ingenic,jz4770-tcu', 'simple-mfd'] is too
long
'ingenic,jz4780-tcu' is not one of ['ingenic,jz4740-tcu',
'ingenic,jz4725b-tcu', 'ingenic,jz4760-tcu', 'ingenic,x1000-tcu']

(quoted diff) +0 -0
 	'simple-mfd' was expected
 	'ingenic,jz4760-tcu' was expected

Trim it a bit...

Quoted 8 lines
Quoted 2 lines

From schema:
Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

You need to explain this. You're changing the effective compatible of the device and doing so based only on schema warning does not look enough. Please write real reason instead of this fat warning, e.g. that both devices are actually compatible and this has no real effect except schema checks.

Well, if the schema says that it should use a particular fallback string, then that's what the DTS should use, right?

If making the DTS schema-compliant causes breakages, then that means the schema is wrong and should be fixed.

Cheers,
-Paul

#10 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

Quoted 27 lines · Paul Cercueil

Hi Krzysztof,

Quoted 15 lines · Krzysztof Kozlowski

Review

Quoted 8 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: compatible:
'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-tcu', 'ingenic,jz4770-tcu', 'simple-mfd'] is too
long
'ingenic,jz4780-tcu' is not one of ['ingenic,jz4740-tcu',
'ingenic,jz4725b-tcu', 'ingenic,jz4760-tcu', 'ingenic,x1000-tcu']

(quoted diff) +0 -0
 	'simple-mfd' was expected
 	'ingenic,jz4760-tcu' was expected

Trim it a bit...

Quoted 2 lines

From schema:
Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

Quoted 7 lines

You need to explain this. You're changing the effective compatible of the device and doing so based only on schema warning does not look enough. Please write real reason instead of this fat warning, e.g. that
both devices are actually compatible and this has no real effect except
schema checks.

Quoted 2 lines

Well, if the schema says that it should use a particular fallback string, then that's what the DTS should use, right?

Or the schema is wrong. :)

Quoted 2 lines

If making the DTS schema-compliant causes breakages, then that means the schema is wrong and should be fixed.

Exactly, so the commit needs a bit of explanation why one solution was chosen over the other. BTW, I am not saying that schema or DTS is wrong, just that commit is not explained enough.

Best regards,
Krzysztof

#11 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

Quoted 32 lines

Am 09.04.2022 um 14:38 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 27 lines · Paul Cercueil

Hi Krzysztof,

Quoted 22 lines · Krzysztof Kozlowski
Quoted 8 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: compatible:
'oneOf' conditional failed, one must be fixed:

	['ingenic,jz4780-tcu', 'ingenic,jz4770-tcu', 'simple-mfd'] is too 
long
	'ingenic,jz4780-tcu' is not one of ['ingenic,jz4740-tcu', 

'ingenic,jz4725b-tcu', 'ingenic,jz4760-tcu', 'ingenic,x1000-tcu']
'simple-mfd' was expected
'ingenic,jz4760-tcu' was expected

Trim it a bit...

Quoted 2 lines

From schema:
Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

You need to explain this. You're changing the effective compatible of the device and doing so based only on schema warning does not look enough. Please write real reason instead of this fat warning, e.g. that
both devices are actually compatible and this has no real effect except
schema checks.

Quoted 7 lines
Quoted 2 lines

Well, if the schema says that it should use a particular fallback string, then that's what the DTS should use, right?

Or the schema is wrong. :)

Quoted 2 lines

If making the DTS schema-compliant causes breakages, then that means the schema is wrong and should be fixed.

Well, in this case making the DTS fit the schema does not break. So why think or explain why the schema is right?

Quoted 7 lines

Exactly, so the commit needs a bit of explanation why one solution was chosen over the other. BTW, I am not saying that schema or DTS is wrong, just that commit is not explained enough.

Best regards,
Krzysztof

#12 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

In reply to Krzysztof Kozlowski
Quoted 13 lines

Am 09.04.2022 um 13:11 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 5 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: compatible: 'oneOf' conditional failed, one must be fixed:

	['ingenic,jz4780-tcu', 'ingenic,jz4770-tcu', 'simple-mfd'] is too long
	'ingenic,jz4780-tcu' is not one of ['ingenic,jz4740-tcu', 'ingenic,jz4725b-tcu', 'ingenic,jz4760-tcu', 'ingenic,x1000-tcu']
	'simple-mfd' was expected
	'ingenic,jz4760-tcu' was expected

Trim it a bit...

Quoted 7 lines
Quoted 1 line

From schema: Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

You need to explain this. You're changing the effective compatible of the device and doing so based only on schema warning does not look enough. Please write real reason instead of this fat warning, e.g. that both devices are actually compatible and this has no real effect except schema checks.

both use jz4740_soc_info / jz4770_soc_info and there is no ingenic,jz4780-tcu... So it doesn't change function, just makes it fit to the bindings.

We could solve it differently add ingenic,jz4780-tcu to bindings and the driver compatible table.

#13 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

Quoted 21 lines · H. Nikolaus Schaller
Quoted 13 lines

Am 09.04.2022 um 13:11 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 5 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: compatible: 'oneOf' conditional failed, one must be fixed:

	['ingenic,jz4780-tcu', 'ingenic,jz4770-tcu', 'simple-mfd'] is too long
	'ingenic,jz4780-tcu' is not one of ['ingenic,jz4740-tcu', 'ingenic,jz4725b-tcu', 'ingenic,jz4760-tcu', 'ingenic,x1000-tcu']
	'simple-mfd' was expected
	'ingenic,jz4760-tcu' was expected

Trim it a bit...

Quoted 7 lines
Quoted 1 line

From schema: Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

You need to explain this. You're changing the effective compatible of the device and doing so based only on schema warning does not look enough. Please write real reason instead of this fat warning, e.g. that both devices are actually compatible and this has no real effect except schema checks.

Quoted 5 lines

both use jz4740_soc_info / jz4770_soc_info and there is no ingenic,jz4780-tcu... So it doesn't change function, just makes it fit to the bindings.

We could solve it differently add ingenic,jz4780-tcu to bindings and the driver compatible table.

Just please use it in commit msg instead of or next to the warning. Don't focus on the bindings check but focus on actual hardware and its description.

Best regards,
Krzysztof

#14 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

Quoted 29 lines

Am 09.04.2022 um 15:11 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 24 lines · H. Nikolaus Schaller
Quoted 13 lines

Am 09.04.2022 um 13:11 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 5 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: compatible: 'oneOf' conditional failed, one must be fixed:

	['ingenic,jz4780-tcu', 'ingenic,jz4770-tcu', 'simple-mfd'] is too long
	'ingenic,jz4780-tcu' is not one of ['ingenic,jz4740-tcu', 'ingenic,jz4725b-tcu', 'ingenic,jz4760-tcu', 'ingenic,x1000-tcu']
	'simple-mfd' was expected
	'ingenic,jz4760-tcu' was expected

Trim it a bit...

Quoted 7 lines
Quoted 1 line

From schema: Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

You need to explain this. You're changing the effective compatible of the device and doing so based only on schema warning does not look enough. Please write real reason instead of this fat warning, e.g. that both devices are actually compatible and this has no real effect except schema checks.

both use jz4740_soc_info / jz4770_soc_info and there is no ingenic,jz4780-tcu... So it doesn't change function, just makes it fit to the bindings.

Quoted 6 lines
Quoted 2 lines

We could solve it differently add ingenic,jz4780-tcu to bindings and the driver compatible table.

Just please use it in commit msg instead of or next to the warning. Don't focus on the bindings check but focus on actual hardware and its description.

Well, again, my assumption is that bindings and .yaml files formally describe the actual hardware components. And they have been reviewed.

So they have a higher level of authority than any current driver or .dts implementation. Unless there is evidence that the bindings are wrong.

I.e. if the bindings feel right why is there a need to argue for that?

It is like test-driven development model. There you have to write code that passes the tests. Not argue against the tests.

BR and thanks,
Nikolaus

#15 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

Quoted 3 lines · H. Nikolaus Schaller

Well, again, my assumption is that bindings and .yaml files formally describe the actual hardware components. And they have been reviewed.

The bindings try to describe it. They are pretty often incomplete or might have mistakes. The true reason of doing a change is not that some tool tells you "do like this". The true reason is because the change properly describes hardware.

Quoted 3 lines

So they have a higher level of authority than any current driver or .dts implementation. Unless there is evidence that the bindings are wrong.

This is just a tool, not an authority.

Quoted 1 line

I.e. if the bindings feel right why is there a need to argue for that?

Because doing things "just because bindings told me" hides the true explanation and makes the code review, code management more difficult. Later person will look at this and wonder why this was done like this. If you write "because some tool me" this is not a good help. But if you write "because hardware is like this exactly" this is proper comment.

Quoted 3 lines

It is like test-driven development model. There you have to write code that passes the tests. Not argue against the tests.

Again, don't focus on the tool... Tool is just a tool...

Best regards,
Krzysztof

#16 lore raw

Re: [PATCH 03/18] MIPS: DTS: jz4780: fix tcu timer as reported by dtbscheck

Quoted 11 lines

Am 09.04.2022 um 15:22 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller

Well, again, my assumption is that bindings and .yaml files formally describe the actual hardware components. And they have been reviewed.

The bindings try to describe it. They are pretty often incomplete or might have mistakes.

Indeed they have. But what If I have found that they are right. Why should I comment on that? It should at least be the default assumption.

Quoted 8 lines

The true reason of doing a change is not that some
tool tells you "do like this". The true reason is because the change properly describes hardware.

Quoted 3 lines

So they have a higher level of authority than any current driver or .dts implementation. Unless there is evidence that the bindings are wrong.

Quoted 6 lines

This is just a tool, not an authority.

Quoted 1 line

I.e. if the bindings feel right why is there a need to argue for that?

Because doing things "just because bindings told me" hides the true explanation and makes the code review, code management more difficult.

Well, I always wonder why schemas were done that way they were done since their introduction. If I would write down commetns every time nobody would be happy...

Quoted 9 lines

Later person will look at this and wonder why this was done like this. If you write "because some tool me" this is not a good help. But if you write "because hardware is like this exactly" this is proper comment.

Quoted 3 lines

It is like test-driven development model. There you have to write code that passes the tests. Not argue against the tests.

Again, don't focus on the tool... Tool is just a tool...

A tool I can't ignore because Rob's robot tells me it is "the truth"...

BR and thanks,
Nikolaus

#17 lore raw

PATCH 4/18 MIPS: DTS: jz4780: fix ost timer as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: timer@e0:compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-ost', 'ingenic,jz4770-ost'] is too long
'ingenic,jz4780-ost' is not one of ['ingenic,jz4725b-ost', 'ingenic,jz4760b-ost'] 'ingenic,jz4760-ost' was expected
'ingenic,jz4725b-ost' was expected
'ingenic,jz4760b-ost' was expected
From schema: Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
index 6027f14c393e3..5f44cf004d473 100644
@@ -134,7 +134,7 @@ pwm: pwm@40 {
 		};
 
 		ost: timer@e0 {
-			compatible = "ingenic,jz4780-ost", "ingenic,jz4770-ost";
+			compatible = "ingenic,jz4780-ost", "ingenic,jz4760b-ost";
 			reg = <0xe0 0x20>;
 
 			clocks = <&tcu TCU_CLK_OST>;
Signature · 1 line
2.33.0
#18 lore raw

Re: [PATCH 04/18] MIPS: DTS: jz4780: fix ost timer as reported by dtbscheck

Quoted 8 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: timer@10002000: timer@e0:compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-ost', 'ingenic,jz4770-ost'] is too long
'ingenic,jz4780-ost' is not one of ['ingenic,jz4725b-ost', 'ingenic,jz4760b-ost'] 'ingenic,jz4760-ost' was expected
'ingenic,jz4725b-ost' was expected
'ingenic,jz4760b-ost' was expected
From schema: Documentation/devicetree/bindings/timer/ingenic,tcu.yaml

Quoted 5 lines
---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

The same as patch 3 - needs explanation.

Best regards,
Krzysztof

#19 lore raw

PATCH 5/18 MIPS: DTS: jz4780: fix pinctrl as reported by dtbscheck

In reply to H. Nikolaus Schaller

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

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
index 5f44cf004d473..b5299eaffb84a 100644
@@ -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>;
 
Signature · 1 line
2.33.0
#20 lore raw

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

Quoted 13 lines · H. Nikolaus Schaller

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

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index 5f44cf004d473..b5299eaffb84a 100644

arch/mips/boot/dts/ingenic/jz4780.dtsi rtc_dev: rtc@10003000 { · line 158

Quoted 5 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
@@ -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

#21 lore raw

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

Quoted 21 lines

Am 09.04.2022 um 13:13 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 16 lines · H. Nikolaus Schaller

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

---
arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index 5f44cf004d473..b5299eaffb84a 100644
@@ -155,7 +155,7 @@ rtc_dev: rtc@10003000 {
 		clock-names = "rtc";
 	};
 
Quoted 5 lines

arch/mips/boot/dts/ingenic/jz4780.dtsi rtc_dev: rtc@10003000 { · line 158

Quoted 2 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
-	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.

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

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

#22 lore raw

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

Quoted 21 lines · H. Nikolaus Schaller
Quoted 21 lines

Am 09.04.2022 um 13:13 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 16 lines · H. Nikolaus Schaller

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

---
arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index 5f44cf004d473..b5299eaffb84a 100644
@@ -155,7 +155,7 @@ rtc_dev: rtc@10003000 {
 		clock-names = "rtc";
 	};
 
Quoted 8 lines
Quoted 5 lines

arch/mips/boot/dts/ingenic/jz4780.dtsi rtc_dev: rtc@10003000 { · line 158

Quoted 2 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
-	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?

Quoted 2 lines

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.

Quoted 3 lines

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

#23 lore raw

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

Quoted 32 lines

Am 09.04.2022 um 15:13 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 27 lines · H. Nikolaus Schaller
Quoted 26 lines

Am 09.04.2022 um 13:13 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 13 lines · H. Nikolaus Schaller

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

---
arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index 5f44cf004d473..b5299eaffb84a 100644

arch/mips/boot/dts/ingenic/jz4780.dtsi rtc_dev: rtc@10003000 { · line 158

Quoted 5 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
@@ -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.

Quoted 4 lines
Quoted 2 lines

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?

Quoted 10 lines
Quoted 3 lines

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.

Quoted 1 line
Quoted 4 lines
Quoted 2 lines

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

#24 lore raw

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

Quoted 9 lines · H. Nikolaus Schaller
Quoted 2 lines

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?

Quoted 14 lines
Quoted 14 lines
Quoted 4 lines

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.

Quoted 3 lines

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

Quoted 8 lines
Quoted 1 line

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.

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

Best regards,
Krzysztof

#25 lore raw

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

Quoted 8 lines

Am 09.04.2022 um 15:24 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 3 lines · H. Nikolaus Schaller
Quoted 2 lines

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

Quoted 8 lines
Quoted 6 lines

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.

Quoted 19 lines
Quoted 19 lines
Quoted 12 lines
Quoted 5 lines

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.

Quoted 1 line
Quoted 4 lines
Quoted 2 lines

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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 1 line

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

#26 lore raw

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

Quoted 5 lines · H. Nikolaus Schaller
Quoted 3 lines

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

#27 lore raw

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

Quoted 9 lines

Am 09.04.2022 um 15:46 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 4 lines · H. Nikolaus Schaller
Quoted 3 lines

No. I ask you to fix all pin-controller cases, for entire MIPS, not just one.

Quoted 8 lines
Quoted 1 line

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

#28 lore raw

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

Quoted 16 lines · H. Nikolaus Schaller
Quoted 16 lines

Am 09.04.2022 um 15:46 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 5 lines · H. Nikolaus Schaller
Quoted 3 lines

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).

Quoted 6 lines
Quoted 1 line

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.

Quoted 6 lines

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

#29 lore raw

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

Quoted 26 lines

Am 09.04.2022 um 16:00 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 21 lines · H. Nikolaus Schaller
Quoted 9 lines

Am 09.04.2022 um 15:46 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 4 lines · H. Nikolaus Schaller
Quoted 3 lines

No. I ask you to fix all pin-controller cases, for entire MIPS, not just one.

Quoted 8 lines
Quoted 1 line

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.

Quoted 7 lines
Quoted 1 line

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.

Quoted 9 lines
Quoted 7 lines

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.

Quoted 5 lines

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

#30 lore raw

PATCH 6/18 MIPS: DTS: jz4780: fix rtc node as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: rtc@10003000: compatible: 'oneOf' conditional failed, one must be fixed:

	['ingenic,jz4780-rtc'] is too short
	'ingenic,jz4780-rtc' is not one of ['ingenic,jz4740-rtc', 'ingenic,jz4760-rtc']
	'ingenic,jz4725b-rtc' was expected
	From schema: Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +2 -1
index b5299eaffb84a..2021836983c96 100644
@@ -145,7 +145,8 @@ ost: timer@e0 {
 	};
 
 	rtc_dev: rtc@10003000 {
-		compatible = "ingenic,jz4780-rtc";
+		compatible = "ingenic,jz4780-rtc",
+			     "ingenic,jz4760-rtc";
 		reg = <0x10003000 0x4c>;
 
 		interrupt-parent = <&intc>;
Signature · 1 line
2.33.0
#31 lore raw

Re: [PATCH 06/18] MIPS: DTS: jz4780: fix rtc node as reported by dtbscheck

Quoted 5 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: rtc@10003000: compatible: 'oneOf' conditional failed, one must be fixed:

	['ingenic,jz4780-rtc'] is too short
	'ingenic,jz4780-rtc' is not one of ['ingenic,jz4740-rtc', 'ingenic,jz4760-rtc']
	'ingenic,jz4725b-rtc' was expected
	From schema: Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml

Why? Maybe the schema is wrong. These devices might not be compatible.

Best regards,
Krzysztof

#32 lore raw

Re: [PATCH 06/18] MIPS: DTS: jz4780: fix rtc node as reported by dtbscheck

Quoted 11 lines

Am 09.04.2022 um 13:14 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 5 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: rtc@10003000: compatible: 'oneOf' conditional failed, one must be fixed:

	['ingenic,jz4780-rtc'] is too short
	'ingenic,jz4780-rtc' is not one of ['ingenic,jz4740-rtc', 'ingenic,jz4760-rtc']
	'ingenic,jz4725b-rtc' was expected
	From schema: Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml

Why? Maybe the schema is wrong. These devices might not be compatible.

Here you may be right that the .yaml is wrong. Paul?

#33 lore raw

PATCH 7/18 MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-otg', 'snps,dwc2'] is too long
['ingenic,jz4780-otg', 'snps,dwc2'] is too short
'brcm,bcm2835-usb' was expected
'hisilicon,hi6220-usb' was expected
'rockchip,rk3066-usb' was expected
'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] 'lantiq,arx100-usb' was expected
'lantiq,xrx200-usb' was expected
'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg']
'amcc,dwc-otg' was expected
'apm,apm82181-dwc-otg' was expected
'snps,dwc2' was expected
'st,stm32f4x9-fsotg' was expected
'st,stm32f4x9-hsotg' was expected
'st,stm32f7-hsotg' was expected
'st,stm32mp15-fsotg' was expected
'st,stm32mp15-hsotg' was expected
'samsung,s3c6400-hsotg' was expected
'intel,socfpga-agilex-hsotg' was expected
From schema: Documentation/devicetree/bindings/usb/dwc2.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
index 2021836983c96..c5124459678b7 100644
@@ -576,7 +576,7 @@ bch: bch@134d0000 {
 	};
 
 	otg: usb@13500000 {
-		compatible = "ingenic,jz4780-otg", "snps,dwc2";
+		compatible = "snps,dwc2";
 		reg = <0x13500000 0x40000>;
 
 		interrupt-parent = <&intc>;
Signature · 1 line
2.33.0
#34 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Quoted 12 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-otg', 'snps,dwc2'] is too long
['ingenic,jz4780-otg', 'snps,dwc2'] is too short
'brcm,bcm2835-usb' was expected
'hisilicon,hi6220-usb' was expected
'rockchip,rk3066-usb' was expected
'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] 'lantiq,arx100-usb' was expected
'lantiq,xrx200-usb' was expected
'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg']
'amcc,dwc-otg' was expected
'apm,apm82181-dwc-otg' was expected

Quoted 8 lines

'snps,dwc2' was expected
'st,stm32f4x9-fsotg' was expected
'st,stm32f4x9-hsotg' was expected
'st,stm32f7-hsotg' was expected
'st,stm32mp15-fsotg' was expected
'st,stm32mp15-hsotg' was expected
'samsung,s3c6400-hsotg' was expected
'intel,socfpga-agilex-hsotg' was expected

You really don't need to paste entire warning.

Quoted 12 lines

From schema: Documentation/devicetree/bindings/usb/dwc2.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index 2021836983c96..c5124459678b7 100644

arch/mips/boot/dts/ingenic/jz4780.dtsi bch: bch@134d0000 { · line 579

Quoted 5 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
@@ -576,7 +576,7 @@ bch: bch@134d0000 {
 	};
 
 	otg: usb@13500000 {
-		compatible = "ingenic,jz4780-otg", "snps,dwc2";
+		compatible = "snps,dwc2";

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Best regards,
Best regards,
Krzysztof

maintainer
#35 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Hi,

Quoted 43 lines · Krzysztof Kozlowski
Quoted 13 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible:
'oneOf' conditional failed, one must be fixed:

(quoted diff) +0 -0
 	['ingenic,jz4780-otg', 'snps,dwc2'] is too long
 	['ingenic,jz4780-otg', 'snps,dwc2'] is too short
 	'brcm,bcm2835-usb' was expected
 	'hisilicon,hi6220-usb' was expected
 	'rockchip,rk3066-usb' was expected
 	'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 

'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb']

(quoted diff) +0 -0
 	'lantiq,arx100-usb' was expected
 	'lantiq,xrx200-usb' was expected
 	'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 

'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg']

Review

Quoted 10 lines
(quoted diff) +0 -0
 	'amcc,dwc-otg' was expected
 	'apm,apm82181-dwc-otg' was expected
 	'snps,dwc2' was expected
 	'st,stm32f4x9-fsotg' was expected
 	'st,stm32f4x9-hsotg' was expected
 	'st,stm32f7-hsotg' was expected
 	'st,stm32mp15-fsotg' was expected
 	'st,stm32mp15-hsotg' was expected
 	'samsung,s3c6400-hsotg' was expected
 	'intel,socfpga-agilex-hsotg' was expected

You really don't need to paste entire warning.

Quoted 15 lines

From schema: Documentation/devicetree/bindings/usb/dwc2.yaml

---

  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 2021836983c96..c5124459678b7 100644 --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi @@ -576,7 +576,7 @@ bch: bch@134d0000 {
};

Quoted 6 lines
Quoted 3 lines

otg: usb@13500000 {
- compatible = "ingenic,jz4780-otg", "snps,dwc2";
+ compatible = "snps,dwc2";

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Agreed. The "snps,dwc2" should be a fallback string, otherwise there is no way to uniquely identify the JZ4780 implementation of the IP.

Cheers,
-Paul

#36 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Hi,

Quoted 44 lines

Am 09.04.2022 um 14:27 schrieb Paul Cercueil <[email protected]>:

Hi,

Quoted 22 lines · Krzysztof Kozlowski
Quoted 12 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-otg', 'snps,dwc2'] is too long
['ingenic,jz4780-otg', 'snps,dwc2'] is too short
'brcm,bcm2835-usb' was expected
'hisilicon,hi6220-usb' was expected
'rockchip,rk3066-usb' was expected
'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] 'lantiq,arx100-usb' was expected
'lantiq,xrx200-usb' was expected
'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg']
'amcc,dwc-otg' was expected
'apm,apm82181-dwc-otg' was expected

Quoted 8 lines

'snps,dwc2' was expected
'st,stm32f4x9-fsotg' was expected
'st,stm32f4x9-hsotg' was expected
'st,stm32f7-hsotg' was expected
'st,stm32mp15-fsotg' was expected
'st,stm32mp15-hsotg' was expected
'samsung,s3c6400-hsotg' was expected
'intel,socfpga-agilex-hsotg' was expected

You really don't need to paste entire warning.

Quoted 16 lines
Quoted 10 lines

From schema: Documentation/devicetree/bindings/usb/dwc2.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index 2021836983c96..c5124459678b7 100644

arch/mips/boot/dts/ingenic/jz4780.dtsi bch: bch@134d0000 { · line 578

Quoted 4 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
@@ -576,7 +576,7 @@ bch: bch@134d0000 {
 	};
 	otg: usb@13500000 {
-		compatible = "ingenic,jz4780-otg", "snps,dwc2";
+		compatible = "snps,dwc2";

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Quoted 1 line

Agreed. The "snps,dwc2" should be a fallback string, otherwise there is no way to uniquely identify the JZ4780 implementation of the IP.

Well, there is no specifc implementation and driver for it. So no need to uniquely identify it.

BR and thanks,
Nikolaus

#37 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

On 09/04/2022 15:18, H. Nikolaus Schaller wrote:
hould have a specific compatible.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Please mention why it does not.

Agreed. The "snps,dwc2" should be a fallback string, otherwise there is no way to uniquely identify the JZ4780 implementation of the IP.

Well, there is no specifc implementation and driver for it. So no need to uniquely identify it.

Specific implementation and driver are not arguments here. This does not matter. It's really unrelated argument.

Bindings are not about implementation in Linux. Implementation can change, so bindings should also?

Best regards,
Krzysztof

#38 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Quoted 6 lines

Am 09.04.2022 um 15:23 schrieb Krzysztof Kozlowski <[email protected]>:

On 09/04/2022 15:18, H. Nikolaus Schaller wrote:
hould have a specific compatible.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Please mention why it does not.

Agreed. The "snps,dwc2" should be a fallback string, otherwise there is no way to uniquely identify the JZ4780 implementation of the IP.

Well, there is no specifc implementation and driver for it. So no need to uniquely identify it.

Specific implementation and driver are not arguments here. This does not matter. It's really unrelated argument.

The argumentation is in reverse: if there is no need for a specialized driver or implementation, why is there is a need to define a specialization.

Your argument was:
"there is no way to uniquely identify the JZ4780 implementation of the IP"

My question is:
"what do we need that for?"

Quoted 2 lines

Bindings are not about implementation in Linux. Implementation can change, so bindings should also?

No. Implementations should be agnostic.

maintainer
#39 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Quoted 19 lines · H. Nikolaus Schaller
Quoted 12 lines

Am 09.04.2022 um 15:23 schrieb Krzysztof Kozlowski
<[email protected]>:

On 09/04/2022 15:18, H. Nikolaus Schaller wrote:
hould have a specific compatible.

Quoted 5 lines
Quoted 4 lines
Quoted 1 line

Please mention why it does not.

Agreed. The "snps,dwc2" should be a fallback string, otherwise
there is no way to uniquely identify the JZ4780 implementation of the IP.

Quoted 6 lines
Quoted 2 lines

Well, there is no specifc implementation and driver for it. So no
need to uniquely identify it.

Specific implementation and driver are not arguments here. This
does not
matter. It's really unrelated argument.

Quoted 8 lines

The argumentation is in reverse: if there is no need for a specialized driver or implementation, why is there is a need to define a specialization.

Your argument was:
"there is no way to uniquely identify the JZ4780 implementation of the IP"

My question is:
"what do we need that for?"

You may not need the differenciation now, but if you need it in the future and you had only the "snps,dwc2" compatible previously, then you're screwed, since your driver must support older device tree blobs.

-Paul

Quoted 5 lines
Quoted 2 lines

Bindings are not about implementation in Linux. Implementation can change, so bindings should also?

No. Implementations should be agnostic.

#40 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

In reply to Krzysztof Kozlowski
Quoted 43 lines

Am 09.04.2022 um 13:15 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 12 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: usb@13500000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-otg', 'snps,dwc2'] is too long
['ingenic,jz4780-otg', 'snps,dwc2'] is too short
'brcm,bcm2835-usb' was expected
'hisilicon,hi6220-usb' was expected
'rockchip,rk3066-usb' was expected
'ingenic,jz4780-otg' is not one of ['rockchip,px30-usb', 'rockchip,rk3036-usb', 'rockchip,rk3188-usb', 'rockchip,rk3228-usb', 'rockchip,rk3288-usb', 'rockchip,rk3308-usb', 'rockchip,rk3328-usb', 'rockchip,rk3368-usb', 'rockchip,rv1108-usb'] 'lantiq,arx100-usb' was expected
'lantiq,xrx200-usb' was expected
'ingenic,jz4780-otg' is not one of ['amlogic,meson8-usb', 'amlogic,meson8b-usb', 'amlogic,meson-gxbb-usb', 'amlogic,meson-g12a-usb', 'intel,socfpga-agilex-hsotg']
'amcc,dwc-otg' was expected
'apm,apm82181-dwc-otg' was expected

Quoted 8 lines

'snps,dwc2' was expected
'st,stm32f4x9-fsotg' was expected
'st,stm32f4x9-hsotg' was expected
'st,stm32f7-hsotg' was expected
'st,stm32mp15-fsotg' was expected
'st,stm32mp15-hsotg' was expected
'samsung,s3c6400-hsotg' was expected
'intel,socfpga-agilex-hsotg' was expected

You really don't need to paste entire warning.

Quoted 14 lines

From schema: Documentation/devicetree/bindings/usb/dwc2.yaml

---
arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index 2021836983c96..c5124459678b7 100644
@@ -576,7 +576,7 @@ bch: bch@134d0000 {
 	};
 
Quoted 6 lines

arch/mips/boot/dts/ingenic/jz4780.dtsi bch: bch@134d0000 { · line 579

Quoted 3 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
 	otg: usb@13500000 {
-		compatible = "ingenic,jz4780-otg", "snps,dwc2";
+		compatible = "snps,dwc2";

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Well, I did not even have that idea that it could need an explanation.

There is no "ingenic,jz4780-otg" and none is needed here to make it work.

Therefore the generic "snps,dwc2" is sufficient.

#41 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Quoted 7 lines · H. Nikolaus Schaller
Quoted 3 lines

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Well, I did not even have that idea that it could need an explanation.

There is no "ingenic,jz4780-otg" and none is needed here to make it work.

Make it work in what terms? We talk about hardware description, right?

Quoted 2 lines

Therefore the generic "snps,dwc2" is sufficient.

No, you are mixing now driver behavior (is sufficient) with hardware description. Most of licensed blocks require the specific compatible to differentiate it.

Best regards,
Krzysztof

#42 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Quoted 6 lines

Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 1 line · H. Nikolaus Schaller
Quoted 1 line
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Well, I did not even have that idea that it could need an explanation.

There is no "ingenic,jz4780-otg" and none is needed here to make it work.

Make it work in what terms? We talk about hardware description, right?

Yes.

Quoted 6 lines
Quoted 3 lines

Therefore the generic "snps,dwc2" is sufficient.

No, you are mixing now driver behavior (is sufficient) with hardware description.

No. "snps,dwc2" is a hardware description for a licensed block. Not a driver behavior.

Quoted 2 lines

Most of licensed blocks require the specific compatible to differentiate it.

If there is a need to differentiate.

#43 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Quoted 19 lines · H. Nikolaus Schaller
Quoted 6 lines

Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 1 line · H. Nikolaus Schaller
Quoted 1 line
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Well, I did not even have that idea that it could need an explanation.

There is no "ingenic,jz4780-otg" and none is needed here to make it work.

Make it work in what terms? We talk about hardware description, right?

Yes.

Quoted 2 lines
Quoted 2 lines
Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Therefore the generic "snps,dwc2" is sufficient.

No, you are mixing now driver behavior (is sufficient) with hardware description.

No. "snps,dwc2" is a hardware description for a licensed block. Not a driver behavior.

snps,dwc2 matches the original block, not necessarily this implementation. Unless you are sure?

Quoted 5 lines
Quoted 3 lines

Most of licensed blocks require the specific compatible to differentiate it.

If there is a need to differentiate.

No, regardless whether there is a need currently, most of them have specific compatibles, because there are some minor differences. Even if difference is not visible from programming model or wiring, it might justify it's own specific compatible. For example because maybe once that tiny difference will require some changes.

Someone added the ingenic compatible, so why do you assume that one tool (bindings) is correct but other piece of code (using specific compatible) is not? You use the argument "bindings warning" which is not enough. Argument that blocks are 100% same, is good enough, if you are sure. Just use it in commit msg. But are you sure that these are the same? Same pins, same programming model (entire model, not used by Linux)?

Best regards,
Krzysztof

#44 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Quoted 26 lines

Am 09.04.2022 um 15:44 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 21 lines · H. Nikolaus Schaller
Quoted 6 lines

Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 1 line · H. Nikolaus Schaller
Quoted 1 line
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Well, I did not even have that idea that it could need an explanation.

There is no "ingenic,jz4780-otg" and none is needed here to make it work.

Make it work in what terms? We talk about hardware description, right?

Yes.

Quoted 4 lines
Quoted 3 lines

Therefore the generic "snps,dwc2" is sufficient.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

No, you are mixing now driver behavior (is sufficient) with hardware description.

No. "snps,dwc2" is a hardware description for a licensed block. Not a driver behavior.

snps,dwc2 matches the original block, not necessarily this implementation. Unless you are sure?

I assume. Nobody has reported an issue without having any specific jz4780 driver in place. Well, that is only evidence, not bullet proof.

Quoted 13 lines
Quoted 6 lines
Quoted 4 lines

Most of licensed blocks require the specific compatible to differentiate it.

If there is a need to differentiate.

No, regardless whether there is a need currently, most of them have specific compatibles, because there are some minor differences. Even if difference is not visible from programming model or wiring, it might justify it's own specific compatible. For example because maybe once that tiny difference will require some changes.

Quoted 6 lines

Someone added the ingenic compatible, so why do you assume that one tool (bindings) is correct but other piece of code (using specific compatible) is not? You use the argument "bindings warning" which is not enough. Argument that blocks are 100% same, is good enough, if you are sure. Just use it in commit msg. But are you sure that these are the same? Same pins, same programming model (entire model, not used by Linux)?

The compatible ingenic,jz4780-otg was introduced in 158c774d3c64859e84dd20e04d5fb18c8d3d318e. Hence I have added Yanjie for clarification why he added it in the .dts and not in the bindings.

BR and thanks,
Nikolaus

#45 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Hi folks,

Quoted 35 lines · H. Nikolaus Schaller
Quoted 16 lines

Am 09.04.2022 um 15:44 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 12 lines · H. Nikolaus Schaller
Quoted 10 lines

Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 5 lines · H. Nikolaus Schaller
Quoted 2 lines

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Well, I did not even have that idea that it could need an explanation.

There is no "ingenic,jz4780-otg" and none is needed here to make it work.

Make it work in what terms? We talk about hardware description, right?

Yes.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Therefore the generic "snps,dwc2" is sufficient.

No, you are mixing now driver behavior (is sufficient) with hardware description.

No. "snps,dwc2" is a hardware description for a licensed block. Not a driver behavior.

snps,dwc2 matches the original block, not necessarily this implementation. Unless you are sure?

I assume. Nobody has reported an issue without having any specific jz4780 driver in place. Well, that is only evidence, not bullet proof.

Quoted 9 lines
Quoted 3 lines
Quoted 2 lines

Most of licensed blocks require the specific compatible to differentiate it.

If there is a need to differentiate.

No, regardless whether there is a need currently, most of them have specific compatibles, because there are some minor differences. Even if difference is not visible from programming model or wiring, it might justify it's own specific compatible. For example because maybe once that tiny difference will require some changes.

Quoted 8 lines
Quoted 6 lines

Someone added the ingenic compatible, so why do you assume that one tool (bindings) is correct but other piece of code (using specific compatible) is not? You use the argument "bindings warning" which is not enough. Argument that blocks are 100% same, is good enough, if you are sure. Just use it in commit msg. But are you sure that these are the same? Same pins, same programming model (entire model, not used by Linux)?

The compatible ingenic,jz4780-otg was introduced in 158c774d3c64859e84dd20e04d5fb18c8d3d318e. Hence I have added Yanjie for clarification why he added it in the .dts and not in the bindings.

It's my fault, last year I made an OTG driver for Ingenic SoCs and sent it to the mailing list, and then I received some revision comments, but for some personal reasons I didn't continue to improve it.

I'll finish these modifications as soon as possible and send them out. Then after they merge into the mainline, this problem will be solved.

Thanks and best regards!

Quoted 4 lines

BR and thanks,
Nikolaus

#46 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Hi,

Quoted 50 lines

Am 10.04.2022 um 18:32 schrieb Zhou Yanjie <[email protected]>:

Hi folks,

Quoted 35 lines · H. Nikolaus Schaller
Quoted 16 lines

Am 09.04.2022 um 15:44 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 12 lines · H. Nikolaus Schaller
Quoted 10 lines

Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 5 lines · H. Nikolaus Schaller
Quoted 2 lines

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Well, I did not even have that idea that it could need an explanation.

There is no "ingenic,jz4780-otg" and none is needed here to make it work.

Make it work in what terms? We talk about hardware description, right?

Yes.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Therefore the generic "snps,dwc2" is sufficient.

No, you are mixing now driver behavior (is sufficient) with hardware description.

No. "snps,dwc2" is a hardware description for a licensed block. Not a driver behavior.

snps,dwc2 matches the original block, not necessarily this implementation. Unless you are sure?

I assume. Nobody has reported an issue without having any specific jz4780 driver in place. Well, that is only evidence, not bullet proof.

Quoted 9 lines
Quoted 3 lines
Quoted 2 lines

Most of licensed blocks require the specific compatible to differentiate it.

If there is a need to differentiate.

No, regardless whether there is a need currently, most of them have specific compatibles, because there are some minor differences. Even if difference is not visible from programming model or wiring, it might justify it's own specific compatible. For example because maybe once that tiny difference will require some changes.

Quoted 8 lines
Quoted 6 lines

Someone added the ingenic compatible, so why do you assume that one tool (bindings) is correct but other piece of code (using specific compatible) is not? You use the argument "bindings warning" which is not enough. Argument that blocks are 100% same, is good enough, if you are sure. Just use it in commit msg. But are you sure that these are the same? Same pins, same programming model (entire model, not used by Linux)?

The compatible ingenic,jz4780-otg was introduced in 158c774d3c64859e84dd20e04d5fb18c8d3d318e. Hence I have added Yanjie for clarification why he added it in the .dts and not in the bindings.

Quoted 6 lines

It's my fault, last year I made an OTG driver for Ingenic SoCs and sent it to the mailing list, and then I received some revision comments, but for some personal reasons I didn't continue to improve it.

I'll finish these modifications as soon as possible and send them out. Then after they merge into the mainline, this problem will be solved.

No need to apologize.

If you agree I can add "ingenic,jz4780-otg" to the schema file and keep the .dts in the v2 of my series.

And I'll add you to the list of reviewers, so you can please comment v2 if it is correct or if we are still missing something.

Best regards and thanks,
Nikolaus

#47 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Hi Nikolaus,

Quoted 53 lines · H. Nikolaus Schaller

Hi,

Quoted 51 lines

Am 10.04.2022 um 18:32 schrieb Zhou Yanjie <[email protected]>:

Hi folks,

Quoted 33 lines · H. Nikolaus Schaller
Quoted 14 lines

Am 09.04.2022 um 15:44 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 11 lines · H. Nikolaus Schaller
Quoted 9 lines

Am 09.04.2022 um 15:15 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 5 lines · H. Nikolaus Schaller
Quoted 2 lines

This looks wrong, the block usually should have a specific compatible. Please mention why it does not.

Well, I did not even have that idea that it could need an explanation.

There is no "ingenic,jz4780-otg" and none is needed here to make it work.

Make it work in what terms? We talk about hardware description, right?

Yes.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Therefore the generic "snps,dwc2" is sufficient.

No, you are mixing now driver behavior (is sufficient) with hardware description.

No. "snps,dwc2" is a hardware description for a licensed block. Not a driver behavior.

snps,dwc2 matches the original block, not necessarily this implementation. Unless you are sure?

I assume. Nobody has reported an issue without having any specific jz4780 driver in place. Well, that is only evidence, not bullet proof.

Quoted 9 lines
Quoted 3 lines
Quoted 2 lines

Most of licensed blocks require the specific compatible to differentiate it.

If there is a need to differentiate.

No, regardless whether there is a need currently, most of them have specific compatibles, because there are some minor differences. Even if difference is not visible from programming model or wiring, it might justify it's own specific compatible. For example because maybe once that tiny difference will require some changes.

Quoted 8 lines
Quoted 6 lines

Someone added the ingenic compatible, so why do you assume that one tool (bindings) is correct but other piece of code (using specific compatible) is not? You use the argument "bindings warning" which is not enough. Argument that blocks are 100% same, is good enough, if you are sure. Just use it in commit msg. But are you sure that these are the same? Same pins, same programming model (entire model, not used by Linux)?

The compatible ingenic,jz4780-otg was introduced in 158c774d3c64859e84dd20e04d5fb18c8d3d318e. Hence I have added Yanjie for clarification why he added it in the .dts and not in the bindings.

It's my fault, last year I made an OTG driver for Ingenic SoCs and sent it to the mailing list, and then I received some revision comments, but for some personal reasons I didn't continue to improve it.

Quoted 6 lines
Quoted 2 lines

I'll finish these modifications as soon as possible and send them out. Then after they merge into the mainline, this problem will be solved.

No need to apologize.

If you agree I can add "ingenic,jz4780-otg" to the schema file and keep the .dts in the v2 of my series.

Sure.

Or you can wait a bit, I plan to send out new patches later today, it contains "ingenic,jz4780-otg".

Quoted 2 lines

And I'll add you to the list of reviewers, so you can please comment v2 if it is correct or if we are still missing something.

Okay, thanks!

Quoted 3 lines

Best regards and thanks,
Nikolaus

#48 lore raw

Re: [PATCH 07/18] MIPS: DTS: jz4780: fix otg node as reported by dtbscheck

Hi Yanjie,

Quoted 8 lines

Am 12.04.2022 um 11:49 schrieb Zhou Yanjie <[email protected]>:

Hi Nikolaus,

Quoted 3 lines · H. Nikolaus Schaller

Hi,

Quoted 7 lines
Quoted 2 lines

If you agree I can add "ingenic,jz4780-otg" to the schema file and keep the .dts in the v2 of my series.

Sure.

Or you can wait a bit, I plan to send out new patches later today, it contains "ingenic,jz4780-otg".

Would be fine! I think you will be faster than me :)

So I'll check with your patch and drop mine from my v2 series.

BR and thanks,
Nikolaus

#49 lore raw

PATCH 8/18 MIPS: DTS: jz4780: fix lcd controllers as reported by dtbscheck

In reply to H. Nikolaus Schaller

/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc0@13050000: $nodename:0: 'lcdc0@13050000' does not match '^lcd-controller@[0-9a-f]+$'
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc0@13050000: clock-names:0: 'lcd_pclk' was expected
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc0@13050000: clock-names:1: 'lcd' was expected
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc1@130a0000: $nodename:0: 'lcdc1@130a0000' does not match '^lcd-controller@[0-9a-f]+$'
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc1@130a0000: clock-names:0: 'lcd_pclk' was expected
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc1@130a0000: clock-names:1: 'lcd' was expected
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +6 -6
index c5124459678b7..8b95486c8afa7 100644
@@ -457,12 +457,12 @@ hdmi: hdmi@10180000 {
 		status = "disabled";
 	};
 
-	lcdc0: lcdc0@13050000 {
+	lcdc0: lcd-controller@13050000 {
 		compatible = "ingenic,jz4780-lcd";
 		reg = <0x13050000 0x1800>;
 
-		clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD0PIXCLK>;
-		clock-names = "lcd", "lcd_pclk";
+		clocks = <&cgu JZ4780_CLK_LCD0PIXCLK>, <&cgu JZ4780_CLK_TVE>;
+		clock-names = "lcd_pclk", "lcd";
 
 		interrupt-parent = <&intc>;
 		interrupts = <31>;
@@ -470,12 +470,12 @@ lcdc0: lcdc0@13050000 {
 		status = "disabled";
 	};
 
-	lcdc1: lcdc1@130a0000 {
+	lcdc1: lcd-controller@130a0000 {
 		compatible = "ingenic,jz4780-lcd";
 		reg = <0x130a0000 0x1800>;
 
-		clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD1PIXCLK>;
-		clock-names = "lcd", "lcd_pclk";
+		clocks = <&cgu JZ4780_CLK_LCD1PIXCLK>, <&cgu JZ4780_CLK_TVE>;
+		clock-names = "lcd_pclk", "lcd";
 
 		interrupt-parent = <&intc>;
 		interrupts = <23>;
Signature · 1 line
2.33.0
Reviewed-by on v1 8/18
#50 lore raw

Re: [PATCH 08/18] MIPS: DTS: jz4780: fix lcd controllers as reported by dtbscheck

Quoted 6 lines · H. Nikolaus Schaller

/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc0@13050000: $nodename:0: 'lcdc0@13050000' does not match '^lcd-controller@[0-9a-f]+$'
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc0@13050000: clock-names:0: 'lcd_pclk' was expected
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: lcdc0@13050000: clock-names:1: 'lcd' was expected
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/display/ingenic,lcd.yaml

It's enough, no need to have two same warnings, so all duplicated messages below can be removed.

With that change:

Best regards,
Krzysztof

#51 lore raw

PATCH 9/18 MIPS: DTS: jz4780: fix dma-controller as reported by dtbscheck

In reply to H. Nikolaus Schaller

/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: dma@13420000: $nodename:0: 'dma@13420000' does not match '^dma-controller(@.*)?$'
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/dma/ingenic,dma.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
index 8b95486c8afa7..dc88f9e813453 100644
@@ -515,7 +515,7 @@ eth0_addr: eth-mac-addr@22 {
 		};
 	};
 
-	dma: dma@13420000 {
+	dma: dma-controller@13420000 {
 		compatible = "ingenic,jz4780-dma";
 		reg = <0x13420000 0x400>, <0x13421000 0x40>;
 		#dma-cells = <2>;
Signature · 1 line
2.33.0
#52 lore raw

Re: [PATCH 09/18] MIPS: DTS: jz4780: fix dma-controller as reported by dtbscheck

Quoted 13 lines · H. Nikolaus Schaller

/Volumes/CaseSensitive/master/arch/mips/boot/dts/ingenic/ci20.dtb: dma@13420000: $nodename:0: 'dma@13420000' does not match '^dma-controller(@.*)?$'
From schema: /Volumes/CaseSensitive/master/Documentation/devicetree/bindings/dma/ingenic,dma.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +0 -0
index 8b95486c8afa7..dc88f9e813453 100644

arch/mips/boot/dts/ingenic/jz4780.dtsi eth0_addr: eth-mac-addr@22 { · line 518

Quoted 5 lines
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
@@ -515,7 +515,7 @@ eth0_addr: eth-mac-addr@22 {
 		};
 	};
 
-	dma: dma@13420000 {
+	dma: dma-controller@13420000 {

Fix all the files.

Best regards,
Krzysztof

#53 lore raw

PATCH 10/18 MIPS: DTS: jz4780: fix uart dmas as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10050000: 'dmas' is a required property

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/mips/boot/dts/ingenic/jz4780.dtsi +20 -0
index dc88f9e813453..73cd05cf26472 100644
@@ -283,6 +283,10 @@ uart0: serial@10030000 {
 		clocks = <&ext>, <&cgu JZ4780_CLK_UART0>;
 		clock-names = "baud", "module";
 
+		dmas = <&dma JZ4780_DMA_UART0_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_UART0_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
@@ -296,6 +300,10 @@ uart1: serial@10031000 {
 		clocks = <&ext>, <&cgu JZ4780_CLK_UART1>;
 		clock-names = "baud", "module";
 
+		dmas = <&dma JZ4780_DMA_UART1_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_UART1_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
@@ -309,6 +317,10 @@ uart2: serial@10032000 {
 		clocks = <&ext>, <&cgu JZ4780_CLK_UART2>;
 		clock-names = "baud", "module";
 
+		dmas = <&dma JZ4780_DMA_UART2_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_UART2_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
@@ -322,6 +334,10 @@ uart3: serial@10033000 {
 		clocks = <&ext>, <&cgu JZ4780_CLK_UART3>;
 		clock-names = "baud", "module";
 
+		dmas = <&dma JZ4780_DMA_UART3_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_UART3_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
@@ -335,6 +351,10 @@ uart4: serial@10034000 {
 		clocks = <&ext>, <&cgu JZ4780_CLK_UART4>;
 		clock-names = "baud", "module";
 
+		dmas = <&dma JZ4780_DMA_UART4_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_UART4_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
Signature · 1 line
2.33.0
#54 lore raw

Re: [PATCH 10/18] MIPS: DTS: jz4780: fix uart dmas as reported by dtbscheck

Quoted 13 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dmas' is a required property

Quoted 8 lines

From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10050000: 'dmas' is a required property

All these warnings are the same two warnings...

Best regards,
Krzysztof

#55 lore raw

Re: [PATCH 10/18] MIPS: DTS: jz4780: fix uart dmas as reported by dtbscheck

Quoted 27 lines

Am 09.04.2022 um 13:18 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 13 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dmas' is a required property

Quoted 8 lines

From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10050000: 'dmas' is a required property

Quoted 1 line

All these warnings are the same two warnings...

See my earlier explanation that without them you can't verify by just reading commit message and diff that all existing warnings have been addressed.

#56 lore raw

Re: [PATCH 10/18] MIPS: DTS: jz4780: fix uart dmas as reported by dtbscheck

Quoted 27 lines · H. Nikolaus Schaller
Quoted 27 lines

Am 09.04.2022 um 13:18 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 21 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10050000: 'dmas' is a required property

Quoted 4 lines
Quoted 1 line

All these warnings are the same two warnings...

See my earlier explanation that without them you can't verify by just reading commit message and diff that all existing warnings have been addressed.

Which does not make sense and there is no need... Automation does it (see Rob's tools). Don't make human life more difficult...

Best regards,
Krzysztof

#57 lore raw

Re: [PATCH 10/18] MIPS: DTS: jz4780: fix uart dmas as reported by dtbscheck

Quoted 32 lines

Am 09.04.2022 um 15:16 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 27 lines · H. Nikolaus Schaller
Quoted 27 lines

Am 09.04.2022 um 13:18 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 21 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10030000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10031000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10032000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10033000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: serial@10034000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/serial/ingenic,uart.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10050000: 'dmas' is a required property

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

All these warnings are the same two warnings...

See my earlier explanation that without them you can't verify by just reading commit message and diff that all existing warnings have been addressed.

Which does not make sense and there is no need... Automation does it (see Rob's tools). Don't make human life more difficult...

Ok, you are right. If you apply this patch and then run dtbscheck again, there would be a warning left over.

But may I honestly ask why you review the commits and read the commit message at all? You could simply ignore it... And it would be easier for both of us to leave it completely to Rob's tools :)

BR and thanks,
Nikolaus

#58 lore raw

Re: [PATCH 10/18] MIPS: DTS: jz4780: fix uart dmas as reported by dtbscheck

Quoted 11 lines · H. Nikolaus Schaller
Quoted 3 lines

Which does not make sense and there is no need... Automation does it (see Rob's tools). Don't make human life more difficult...

Ok, you are right. If you apply this patch and then run dtbscheck again, there would be a warning left over.

But may I honestly ask why you review the commits and read the commit message at all? You could simply ignore it... And it would be easier for both of us to leave it completely to Rob's tools :)

I am not reading it. :) It takes more effort to scroll to the actual contents.

Best regards,
Krzysztof

#59 lore raw

Re: [PATCH 10/18] MIPS: DTS: jz4780: fix uart dmas as reported by dtbscheck

Quoted 12 lines

Am 09.04.2022 um 15:28 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 7 lines · H. Nikolaus Schaller
Quoted 3 lines

Which does not make sense and there is no need... Automation does it (see Rob's tools). Don't make human life more difficult...

Ok, you are right. If you apply this patch and then run dtbscheck again, there would be a warning left over.

Quoted 7 lines
Quoted 4 lines

But may I honestly ask why you review the commits and read the commit message at all? You could simply ignore it... And it would be easier for both of us to leave it completely to Rob's tools :)

I am not reading it. :) It takes more effort to scroll to the actual contents.

Ok, now I got it...

Maybe I have a larger screen so that it doesn't even need scrolling and therefore don't notice this difference. So may I leave it as it is since you don't read it anyways :)

Quoted 3 lines

Best regards,
Krzysztof

BR and thanks,
Nikolaus

#60 lore raw

PATCH 11/18 MIPS: DTS: jz4780: fix i2c dmas as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10050000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10051000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10051000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10052000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10052000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10053000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10053000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10054000: 'dmas' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: i2c@10054000: 'dma-names' is a required property
From schema: Documentation/devicetree/bindings/i2c/ingenic,i2c.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/mips/boot/dts/ingenic/jz4780.dtsi +20 -0
index 73cd05cf26472..dcb93d0158856 100644
@@ -392,6 +392,10 @@ i2c0: i2c@10050000 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pins_i2c0_data>;
 
+		dmas = <&dma JZ4780_DMA_SMB0_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_SMB0_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
@@ -409,6 +413,10 @@ i2c1: i2c@10051000 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pins_i2c1_data>;
 
+		dmas = <&dma JZ4780_DMA_SMB1_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_SMB1_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
@@ -426,6 +434,10 @@ i2c2: i2c@10052000 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pins_i2c2_data>;
 
+		dmas = <&dma JZ4780_DMA_SMB2_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_SMB2_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
@@ -443,6 +455,10 @@ i2c3: i2c@10053000 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pins_i2c3_data>;
 
+		dmas = <&dma JZ4780_DMA_SMB3_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_SMB3_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
@@ -460,6 +476,10 @@ i2c4: i2c@10054000 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pins_i2c4_data>;
 
+		dmas = <&dma JZ4780_DMA_SMB4_RX 0xffffffff>,
+		       <&dma JZ4780_DMA_SMB4_TX 0xffffffff>;
+		dma-names = "rx", "tx";
+
 		status = "disabled";
 	};
 
Signature · 1 line
2.33.0
#61 lore raw

PATCH 12/18 MIPS: DTS: jz4780: fix nemc memory controller as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: nemc@13410000: $nodename:0: 'nemc@13410000' does not match '^memory-controller@[0-9a-f]+$'
From schema: Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +1 -1
index dcb93d0158856..cf259dac519ad 100644
@@ -523,7 +523,7 @@ lcdc1: lcd-controller@130a0000 {
 		status = "disabled";
 	};
 
-	nemc: nemc@13410000 {
+	nemc: memory-controller@13410000 {
 		compatible = "ingenic,jz4780-nemc", "simple-mfd";
 		reg = <0x13410000 0x10000>;
 		#address-cells = <2>;
Signature · 1 line
2.33.0
Reviewed-by on v1 12/18
#62 lore raw

Re: [PATCH 12/18] MIPS: DTS: jz4780: fix nemc memory controller as reported by dtbscheck

Quoted 8 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: nemc@13410000: $nodename:0: 'nemc@13410000' does not match '^memory-controller@[0-9a-f]+$'
From schema: Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Best regards,
Krzysztof

#63 lore raw

PATCH 13/18 dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

In reply to H. Nikolaus Schaller

jz4780-nemc needs to be compatible to simple-mfd as well or we get

arch/mips/boot/dts/ingenic/ci20.dtb: memory-controller@13410000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-nemc', 'simple-mfd'] is too long
'ingenic,jz4725b-nemc' was expected
'ingenic,jz4740-nemc' was expected
From schema: Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml

---
 .../devicetree/bindings/memory-controllers/ingenic,nemc.yaml    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml +1 -1
index 24f9e19820282..3b1116588de3d 100644
@@ -17,7 +17,7 @@ properties:
     oneOf:
       - enum:
           - ingenic,jz4740-nemc
-          - ingenic,jz4780-nemc
+          - [ ingenic,jz4780-nemc, simple-mfd ]
       - items:
           - const: ingenic,jz4725b-nemc
           - const: ingenic,jz4740-nemc
Signature · 1 line
2.33.0
#64 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

Quoted 18 lines · H. Nikolaus Schaller

jz4780-nemc needs to be compatible to simple-mfd as well or we get

arch/mips/boot/dts/ingenic/ci20.dtb: memory-controller@13410000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-nemc', 'simple-mfd'] is too long
'ingenic,jz4725b-nemc' was expected
'ingenic,jz4740-nemc' was expected
From schema: Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml

---
 .../devicetree/bindings/memory-controllers/ingenic,nemc.yaml    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml +0 -0
index 24f9e19820282..3b1116588de3d 100644

Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml properties: · line 20

Quoted 5 lines
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml +1 -1
@@ -17,7 +17,7 @@ properties:
     oneOf:
       - enum:
           - ingenic,jz4740-nemc
-          - ingenic,jz4780-nemc
+          - [ ingenic,jz4780-nemc, simple-mfd ]

This is not correct representation. If you really need simple-mfd, then this should be a separate item below oneOf.

The true question is whether you need simple-mfd. Isn't the binding (and the driver) expected to instantiate its children?

Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml properties: · line 23

Quoted 3 lines
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml +0 -0
       - items:
           - const: ingenic,jz4725b-nemc
           - const: ingenic,jz4740-nemc

Best regards,
Krzysztof

maintainer
#65 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

Hi Krzysztof,

Quoted 31 lines · Krzysztof Kozlowski
Quoted 16 lines · H. Nikolaus Schaller

jz4780-nemc needs to be compatible to simple-mfd as well or we get

arch/mips/boot/dts/ingenic/ci20.dtb: memory-controller@13410000:
compatible: 'oneOf' conditional failed, one must be fixed:

(quoted diff) +0 -0
 	['ingenic,jz4780-nemc', 'simple-mfd'] is too long
 	'ingenic,jz4725b-nemc' was expected
 	'ingenic,jz4740-nemc' was expected
 	From schema: 

Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml

---
.../devicetree/bindings/memory-controllers/ingenic,nemc.yaml |
2 +-

  1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines

diff --git
a/Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml b/Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml
index 24f9e19820282..3b1116588de3d 100644
---
a/Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml
+++
b/Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml
@@ -17,7 +17,7 @@ properties:
oneOf:
- enum:
- ingenic,jz4740-nemc
- - ingenic,jz4780-nemc
+ - [ ingenic,jz4780-nemc, simple-mfd ]

Quoted 2 lines

This is not correct representation. If you really need simple-mfd, then this should be a separate item below oneOf.

Correct.

Quoted 2 lines

The true question is whether you need simple-mfd. Isn't the binding (and the driver) expected to instantiate its children?

I can explain that one. There is the EFUSE controller located inside the nemc's memory area, and the two are pretty much unrelated, hence the "simple-mfd" compatible string.

Cheers,
-Paul

#66 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

Quoted 7 lines · Paul Cercueil
Quoted 3 lines

The true question is whether you need simple-mfd. Isn't the binding (and
the driver) expected to instantiate its children?

I can explain that one. There is the EFUSE controller located inside the nemc's memory area, and the two are pretty much unrelated, hence the "simple-mfd" compatible string.

I saw the efuse children and that's why I asked who is expected to populate them. You said that simple-mfd is required for this, I say no. It should work without simple-mfd...

I am kind of repeating myself but I really do not see the need of simple-mfd in the bindings.

Best regards,
Krzysztof

maintainer
#67 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

Quoted 9 lines · Krzysztof Kozlowski
Quoted 7 lines · Paul Cercueil
Quoted 3 lines

The true question is whether you need simple-mfd. Isn't the binding (and
the driver) expected to instantiate its children?

I can explain that one. There is the EFUSE controller located inside the nemc's memory area, and the two are pretty much unrelated, hence the "simple-mfd" compatible string.

Quoted 6 lines

I saw the efuse children and that's why I asked who is expected to populate them. You said that simple-mfd is required for this, I say no. It should work without simple-mfd...

I am kind of repeating myself but I really do not see the need of simple-mfd in the bindings.

Well, it is a "simple MFD", so I don't see why we can't use the "simple-mfd" compatible. Why would we not want to use it?

Besides, if the nemc driver is responsible for populating the efuse device, that means the nemc driver must be enabled for the efuse to work, which is nonsense, the two IP blocks being unrelated.

-Paul

#68 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

Quoted 10 lines · Paul Cercueil
Quoted 8 lines

I saw the efuse children and that's why I asked who is expected to populate them. You said that simple-mfd is required for this, I say no.
It should work without simple-mfd...

I am kind of repeating myself but I really do not see the need of simple-mfd in the bindings.

Well, it is a "simple MFD",

It's not a simple MFD, it is a memory controller. MFD is a purely Linux/software term, so there are no devices which are MFD. Everything which we model as MFD is actually something else in real life (e.g. PMIC, memory controller, system controller).

Quoted 2 lines

so I don't see why we can't use the
"simple-mfd" compatible. Why would we not want to use it?

No one said that you cannot. You just might not need...

Quoted 4 lines

Besides, if the nemc driver is responsible for populating the efuse device, that means the nemc driver must be enabled for the efuse to work, which is nonsense, the two IP blocks being unrelated.

That's actually the explanation I was looking for. It would be nice to use it in commit msg instead of the dtbs_check warning.

Best regards,
Krzysztof

#69 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

In reply to Krzysztof Kozlowski
Quoted 18 lines

Am 09.04.2022 um 13:26 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 13 lines · H. Nikolaus Schaller

jz4780-nemc needs to be compatible to simple-mfd as well or we get

arch/mips/boot/dts/ingenic/ci20.dtb: memory-controller@13410000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-nemc', 'simple-mfd'] is too long
'ingenic,jz4725b-nemc' was expected
'ingenic,jz4740-nemc' was expected
From schema: Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml

---
.../devicetree/bindings/memory-controllers/ingenic,nemc.yaml | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines

Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml properties: · line 20

Quoted 10 lines
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml +1 -1
index 24f9e19820282..3b1116588de3d 100644
@@ -17,7 +17,7 @@ properties:
    oneOf:
      - enum:
          - 
-          - ingenic,jz4780-nemc
+          - [ , simple-mfd ]

This is not correct representation. If you really need simple-mfd, then this should be a separate item below oneOf.

Well, it is valid YAML syntax and seems to be accepted by dtbscheck.

Quoted 2 lines

The true question is whether you need simple-mfd. Isn't the binding (and the driver) expected to instantiate its children?

I had expected that but current ingenic,jz4780-nemc code doesn't.

Everything is explained in 190607f2d59e174bcf8415efb1bb390737f8d428

But if someone writes a fix for the ingenic,jz4740-nemc driever we can of course live without this patch. And partially revert 190607f2d59e174bcf8415efb1bb390737f8d428

#70 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

Quoted 32 lines · H. Nikolaus Schaller
Quoted 18 lines

Am 09.04.2022 um 13:26 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 13 lines · H. Nikolaus Schaller

jz4780-nemc needs to be compatible to simple-mfd as well or we get

arch/mips/boot/dts/ingenic/ci20.dtb: memory-controller@13410000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-nemc', 'simple-mfd'] is too long
'ingenic,jz4725b-nemc' was expected
'ingenic,jz4740-nemc' was expected
From schema: Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml

---
.../devicetree/bindings/memory-controllers/ingenic,nemc.yaml | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 13 lines

Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml properties: · line 20

Quoted 10 lines
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml +1 -1
index 24f9e19820282..3b1116588de3d 100644
@@ -17,7 +17,7 @@ properties:
    oneOf:
      - enum:
          - 
-          - ingenic,jz4780-nemc
+          - [ , simple-mfd ]

This is not correct representation. If you really need simple-mfd, then this should be a separate item below oneOf.

Quoted 1 line

Well, it is valid YAML syntax and seems to be accepted by dtbscheck.

It's not how we code it. Please do not introduce inconsistent - even if valid - blocks.

Quoted 5 lines
Quoted 3 lines

The true question is whether you need simple-mfd. Isn't the binding (and the driver) expected to instantiate its children?

I had expected that but current ingenic,jz4780-nemc code doesn't.

Paul provided good reason for the simple-mfd. Use this one instead of dt check warning. DT check warning means nothing, does not bring the actual answer to "why", because it is artificial tool. The answer to "why" is in what Paul wrote.

Best regards,
Krzysztof

#71 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

Quoted 1 line · Krzysztof Kozlowski

On 09/04/2022 15:09, H. Nikolaus Schaller wrote:

(...)

Quoted 11 lines
Quoted 11 lines
Quoted 9 lines

Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml properties: · line 20

Quoted 6 lines
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml +1 -1
@@ -17,7 +17,7 @@ properties:
    oneOf:
      - enum:
          - 
-          - ingenic,jz4780-nemc
+          - [ , simple-mfd ]

This is not correct representation. If you really need simple-mfd, then this should be a separate item below oneOf.

Well, it is valid YAML syntax and seems to be accepted by dtbscheck.

Minor update:
Well, it is not a valid schema. Rob's checker now confirmed. If you run dt_bindings_check by yourself you will see the error:

properties:compatible:oneOf:0:enum:1: ['ingenic', 'jz4780-nemc',
'simple-mfd'] is not of type 'string'

Probably because enum expects string, not another enum (so enum inside enum is not correct).

If you do not see the error, you might be missing some packages (mentioned in writing-schema + yamllint for a different issue) or your dtschema is old.

Quoted 3 lines

It's not how we code it. Please do not introduce inconsistent - even if valid - blocks.

Best regards,
Krzysztof

#72 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

Hi,

Quoted 17 lines

Am 10.04.2022 um 17:35 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 1 line · Krzysztof Kozlowski

On 09/04/2022 15:09, H. Nikolaus Schaller wrote:

(...)

Quoted 10 lines
Quoted 10 lines
Quoted 9 lines

Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml properties: · line 20

Quoted 6 lines
Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml +1 -1
@@ -17,7 +17,7 @@ properties:
   oneOf:
     - enum:
         - 
-          - ingenic,jz4780-nemc
+          - [ , simple-mfd ]

This is not correct representation. If you really need simple-mfd, then this should be a separate item below oneOf.

Quoted 5 lines
Quoted 1 line
Quoted 1 line

Well, it is valid YAML syntax and seems to be accepted by dtbscheck.

Minor update:
Well, it is not a valid schema. Rob's checker now confirmed. If you run dt_bindings_check by yourself you will see the error:

I did run dt_bindings_check. Otherwise I wouldn't have seen that there is a problem at all, before suggesting these changes...

Quoted 10 lines

properties:compatible:oneOf:0:enum:1: ['ingenic', 'jz4780-nemc',
'simple-mfd'] is not of type 'string'

Probably because enum expects string, not another enum (so enum inside enum is not correct).

If you do not see the error, you might be missing some packages (mentioned in writing-schema + yamllint for a different issue) or your dtschema is old.

Neither.

yamllint etc. are all the most recent versions. Updated just two days ago.

But my filter looking for ci20.dtb did not catch it because the report doesn't mention that file.

So Rob's robot is more mature than mine...

BR,
Nikolaus

maintainer
#73 lore raw

Re: [PATCH 13/18] dt-bindings: fix jz4780-nemc issue as reported by dtbscheck

In reply to H. Nikolaus Schaller
Quoted 8 lines · H. Nikolaus Schaller

jz4780-nemc needs to be compatible to simple-mfd as well or we get

arch/mips/boot/dts/ingenic/ci20.dtb: memory-controller@13410000: compatible: 'oneOf' conditional failed, one must be fixed:
['ingenic,jz4780-nemc', 'simple-mfd'] is too long
'ingenic,jz4725b-nemc' was expected
'ingenic,jz4740-nemc' was expected
From schema: Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml

Quoted 5 lines
---
 .../devicetree/bindings/memory-controllers/ingenic,nemc.yaml    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml:20:23: [warning] too few spaces after comma (commas)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml: properties:compatible:oneOf:0:enum:1: ['ingenic', 'jz4780-nemc', 'simple-mfd'] is not of type 'string'
from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.yaml: ignoring, error in schema: properties: compatible: oneOf: 0: enum: 1 Documentation/devicetree/bindings/mtd/ingenic,nand.example.dtb:0:0: /example-0/memory-controller@13410000: failed to match any schema with compatible: ['ingenic,jz4780-nemc'] Documentation/devicetree/bindings/memory-controllers/ingenic,nemc.example.dtb:0:0: /example-0/memory-controller@13410000: failed to match any schema with compatible: ['ingenic,jz4780-nemc']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.

#74 lore raw

PATCH 14/18 MIPS: DTS: CI20: add missing model as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: /: 'model' is a required property
From schema: dtschema/schemas/root-node.yaml

---
 arch/mips/boot/dts/ingenic/ci20.dts | 2 ++
 1 file changed, 2 insertions(+)
arch/mips/boot/dts/ingenic/ci20.dts +2 -0
index ab6e3dc0bc1d0..cb5257d32b55e 100644
@@ -11,6 +11,8 @@
 / {
 	compatible = "img,ci20", "ingenic,jz4780";
 
+	model = "Imagination creator CI20";
+
 	aliases {
 		serial0 = &uart0;
 		serial1 = &uart1;
Signature · 1 line
2.33.0
#75 lore raw

PATCH 15/18 MIPS: DTS: CI20: fix fixed regulators as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: /: fixedregulator@0: 'anyOf' conditional failed, one must be fixed:
'reg' is a required property
'ranges' is a required property
From schema: python/site-packages/dtschema/schemas/root-node.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: /: fixedregulator@1: 'anyOf' conditional failed, one must be fixed:
'reg' is a required property
'ranges' is a required property
From schema: python/site-packages/dtschema/schemas/root-node.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: /: fixedregulator@2: 'anyOf' conditional failed, one must be fixed:
'reg' is a required property
'ranges' is a required property
From schema: python/site-packages/dtschema/schemas/root-node.yaml

---
 arch/mips/boot/dts/ingenic/ci20.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/mips/boot/dts/ingenic/ci20.dts +3 -3
index cb5257d32b55e..c045c4f75895e 100644
@@ -69,7 +69,7 @@ led3 {
 		};
 	};
 
-	eth0_power: fixedregulator@0 {
+	eth0_power: eth3v3 {
 		compatible = "regulator-fixed";
 
 		regulator-name = "eth0_power";
@@ -99,7 +99,7 @@ ir: ir {
 		gpios = <&gpe 3 GPIO_ACTIVE_LOW>;
 	};
 
-	wlan0_power: fixedregulator@1 {
+	wlan0_power: wlan_power {
 		compatible = "regulator-fixed";
 
 		regulator-name = "wlan0_power";
@@ -108,7 +108,7 @@ wlan0_power: fixedregulator@1 {
 		enable-active-high;
 	};
 
-	otg_power: fixedregulator@2 {
+	otg_power: otg5v0 {
 		compatible = "regulator-fixed";
 
 		regulator-name = "otg_power";
Signature · 1 line
2.33.0
#76 lore raw

Re: [PATCH 15/18] MIPS: DTS: CI20: fix fixed regulators as reported by dtbscheck

Quoted 23 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: /: fixedregulator@0: 'anyOf' conditional failed, one must be fixed:
'reg' is a required property
'ranges' is a required property
From schema: python/site-packages/dtschema/schemas/root-node.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: /: fixedregulator@1: 'anyOf' conditional failed, one must be fixed:
'reg' is a required property
'ranges' is a required property
From schema: python/site-packages/dtschema/schemas/root-node.yaml
arch/mips/boot/dts/ingenic/ci20.dtb: /: fixedregulator@2: 'anyOf' conditional failed, one must be fixed:
'reg' is a required property
'ranges' is a required property
From schema: python/site-packages/dtschema/schemas/root-node.yaml

---
 arch/mips/boot/dts/ingenic/ci20.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/mips/boot/dts/ingenic/ci20.dts +0 -0
index cb5257d32b55e..c045c4f75895e 100644

arch/mips/boot/dts/ingenic/ci20.dts led3 { · line 72

Quoted 5 lines
arch/mips/boot/dts/ingenic/ci20.dts +1 -1
@@ -69,7 +69,7 @@ led3 {
 		};
 	};
 
-	eth0_power: fixedregulator@0 {
+	eth0_power: eth3v3 {

No, Devicetree spec requires generic node name, so "regulator-0" is appropriate. See also examples of device tree node names in the spec. "eth3v3" is a specific one, not generic.

Best regards,
Krzysztof

#77 lore raw

PATCH 16/18 MIPS: DTS: CI20: fix /memory as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: /: memory: False schema does not allow {device_type: [memory], reg: [[0, 268435456], [805306368, 805306368]]}
From schema: python/site-packages/dtschema/schemas/root-node.yaml

---
 arch/mips/boot/dts/ingenic/ci20.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/ci20.dts +1 -1
index c045c4f75895e..98f0a5ccbb4da 100644
@@ -24,7 +24,7 @@ chosen {
 		stdout-path = &uart4;
 	};
 
-	memory {
+	memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x10000000
 		       0x30000000 0x30000000>;
Signature · 1 line
2.33.0
#78 lore raw

PATCH 17/18 MIPS: DTS: CI20: fix wifi as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: wifi@1: compatible: oneOf conditional failed, one must be fixed:
[brcm,bcm4330-fmac] is too short
brcm,bcm4329-fmac was expected
From schema: Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml

---
 arch/mips/boot/dts/ingenic/ci20.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/ci20.dts +1 -1
index 98f0a5ccbb4da..dc587b4b36009 100644
@@ -176,7 +176,7 @@ &mmc1 {
 
 	brcmf: wifi@1 {
 /*		reg = <4>;*/
-		compatible = "brcm,bcm4330-fmac";
+		compatible = "brcm,bcm4330-fmac", "brcm,bcm4329-fmac";
 		vcc-supply = <&wlan0_power>;
 		device-wakeup-gpios = <&gpd 9 GPIO_ACTIVE_HIGH>;
 		shutdown-gpios = <&gpf 7 GPIO_ACTIVE_LOW>;
Signature · 1 line
2.33.0
Reviewed-by on v1 17/18
#79 lore raw

Re: [PATCH 17/18] MIPS: DTS: CI20: fix wifi as reported by dtbscheck

Quoted 10 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: wifi@1: compatible: oneOf conditional failed, one must be fixed:
[brcm,bcm4330-fmac] is too short
brcm,bcm4329-fmac was expected
From schema: Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml

---
 arch/mips/boot/dts/ingenic/ci20.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Best regards,
Krzysztof

#80 lore raw

PATCH 18/18 MIPS: DTS: CI20: fix bluetooth as reported by dtbscheck

In reply to H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: bluetooth: vcc-supply does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml

---
 arch/mips/boot/dts/ingenic/ci20.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/ci20.dts +1 -1
index dc587b4b36009..8a120f9374331 100644
@@ -207,7 +207,7 @@ &uart2 {
 	bluetooth {
 		compatible = "brcm,bcm4330-bt";
 		reset-gpios = <&gpf 8 GPIO_ACTIVE_HIGH>;
-		vcc-supply = <&wlan0_power>;
+		vbat-supply = <&wlan0_power>;
 		device-wakeup-gpios = <&gpf 5 GPIO_ACTIVE_HIGH>;
 		host-wakeup-gpios = <&gpf 6 GPIO_ACTIVE_HIGH>;
 		shutdown-gpios = <&gpf 4 GPIO_ACTIVE_LOW>;
Signature · 1 line
2.33.0
Acked-by on v1 18/18
#81 lore raw

Re: [PATCH 18/18] MIPS: DTS: CI20: fix bluetooth as reported by dtbscheck

Quoted 13 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: bluetooth: vcc-supply does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml

---
 arch/mips/boot/dts/ingenic/ci20.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/ci20.dts +0 -0
index dc587b4b36009..8a120f9374331 100644

arch/mips/boot/dts/ingenic/ci20.dts &uart2 { · line 210

Quoted 5 lines
arch/mips/boot/dts/ingenic/ci20.dts +1 -1
@@ -207,7 +207,7 @@ &uart2 {
 	bluetooth {
 		compatible = "brcm,bcm4330-bt";
 		reset-gpios = <&gpf 8 GPIO_ACTIVE_HIGH>;
-		vcc-supply = <&wlan0_power>;
+		vbat-supply = <&wlan0_power>;

Could be also vddio...

Best regards,
Krzysztof

#82 lore raw

Re: [PATCH 18/18] MIPS: DTS: CI20: fix bluetooth as reported by dtbscheck

Quoted 13 lines

Am 09.04.2022 um 13:22 schrieb Krzysztof Kozlowski <[email protected]>:

Quoted 8 lines · H. Nikolaus Schaller

arch/mips/boot/dts/ingenic/ci20.dtb: bluetooth: vcc-supply does not match any of the regexes: pinctrl-[0-9]+
From schema: Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml

---
arch/mips/boot/dts/ingenic/ci20.dts | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 12 lines

arch/mips/boot/dts/ingenic/ci20.dts &uart2 { · line 210

Quoted 10 lines
arch/mips/boot/dts/ingenic/ci20.dts +1 -1
index dc587b4b36009..8a120f9374331 100644
@@ -207,7 +207,7 @@ &uart2 {
 	bluetooth {
 		compatible = "brcm,bcm4330-bt";
 		reset-gpios = <&gpf 8 GPIO_ACTIVE_HIGH>;
-		vcc-supply = <&wlan0_power>;
+		vbat-supply = <&wlan0_power>;

Could be also vddio...

Good hint.

Schematics has both vbat AND vddio connected.

But with that finding, there is also a wifi_io regulator for 2.5V. It is always-on and NOT yet controlled.

So this is worth a functional addition. But not related to dtbscheck cleanup because it even is optional. Hence a separate topic.

Quoted 7 lines

Best regards,
Krzysztof

In summary for reviewing all patches: many thanks!

BR,
Nikolaus