dt-bindings: opp: Allow multi-worded node names

Not all OPP table names and OPP entries consist of a single word. In particular NVIDIA Tegra OPP tables use multi-word names. Allow OPP node and OPP entry name to have multi-worded names to silence DT checker warnings about the multi-word names separated by hyphen.

Reviewed-by David Heidelberg <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +0 -0

index ae3ae4d39843..298cf24af270 100644

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +1 -1

   opp-shared:
     description:

@@ -33,7 +33,7 @@ properties:
     type: boolean
 
 patternProperties:
-  '^opp-?[0-9]+$':
+  '^opp(-[0-9]+)*$':
     type: object
     description:
       One or more OPP nodes describing voltage-current-frequency combinations.

Signature · 1 line

2.32.0

Quoted 16 lines

Not all OPP table names and OPP entries consist of a single word. In particular NVIDIA Tegra OPP tables use multi-word names. Allow OPP node and OPP entry name to have multi-worded names to silence DT checker warnings about the multi-word names separated by hyphen.

Reviewed-by David Heidelberg <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +0 -0

index ae3ae4d39843..298cf24af270 100644

Documentation/devicetree/bindings/opp/opp-v2-base.yaml select: false · line 25

Quoted 5 lines

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +1 -1

@@ -22,7 +22,7 @@ select: false
 
 properties:
   $nodename:
-    pattern: '^opp-table(-[a-z0-9]+)?$'
+    pattern: '^opp-table(-[a-z0-9]+)*$'

I don't see how this helps you. What I see needed upstream is a prefix:

'-?opp-table(-[0-9]+)?$'

Though really what I'd like to see is the OPP nodes moved into the device nodes they belong to when appropriate (i.e. when not shared between multiple devices).

20.10.2021 17:46, Rob Herring пишет:

Quoted 25 lines · Dmitry Osipenko

Quoted 16 lines

Not all OPP table names and OPP entries consist of a single word. In particular NVIDIA Tegra OPP tables use multi-word names. Allow OPP node and OPP entry name to have multi-worded names to silence DT checker warnings about the multi-word names separated by hyphen.

Reviewed-by David Heidelberg <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +0 -0

index ae3ae4d39843..298cf24af270 100644

Documentation/devicetree/bindings/opp/opp-v2-base.yaml select: false · line 25

Quoted 5 lines

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +1 -1

@@ -22,7 +22,7 @@ select: false
 
 properties:
   $nodename:
-    pattern: '^opp-table(-[a-z0-9]+)?$'
+    pattern: '^opp-table(-[a-z0-9]+)*$'

I don't see how this helps you. What I see needed upstream is a prefix:

Quoted 5 lines

'-?opp-table(-[0-9]+)?$'

Though really what I'd like to see is the OPP nodes moved into the device nodes they belong to when appropriate (i.e. when not shared between multiple devices).

I already prepared patches to rename Tegra OPP tables in accordance to the new naming scheme where opp-table- is the prefix.

Quoted 17 lines · Dmitry Osipenko

Not all OPP table names and OPP entries consist of a single word. In particular NVIDIA Tegra OPP tables use multi-word names. Allow OPP node and OPP entry name to have multi-worded names to silence DT checker warnings about the multi-word names separated by hyphen.

Reviewed-by David Heidelberg <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +0 -0

index ae3ae4d39843..298cf24af270 100644
@@ -22,7 +22,7 @@ select: false
 

Documentation/devicetree/bindings/opp/opp-v2-base.yaml select: false · line 25

Quoted 4 lines

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +1 -1

 properties:
   $nodename:
-    pattern: '^opp-table(-[a-z0-9]+)?$'
+    pattern: '^opp-table(-[a-z0-9]+)*$'

I don't see how this helps you. What I see needed upstream is a prefix:

'-?opp-table(-[0-9]+)?$'

Though really what I'd like to see is the OPP nodes moved into the device nodes they belong to when appropriate (i.e. when not shared between multiple devices).

Quoted 18 lines · Dmitry Osipenko

Quoted 17 lines

Not all OPP table names and OPP entries consist of a single word. In particular NVIDIA Tegra OPP tables use multi-word names. Allow OPP node and OPP entry name to have multi-worded names to silence DT checker warnings about the multi-word names separated by hyphen.

Reviewed-by David Heidelberg <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +0 -0

index ae3ae4d39843..298cf24af270 100644
@@ -22,7 +22,7 @@ select: false
 

Quoted 8 lines

Documentation/devicetree/bindings/opp/opp-v2-base.yaml select: false · line 25

Quoted 4 lines

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +1 -1

 properties:
   $nodename:
-    pattern: '^opp-table(-[a-z0-9]+)?$'
+    pattern: '^opp-table(-[a-z0-9]+)*$'

I don't see how this helps you. What I see needed upstream is a prefix:

'-?opp-table(-[0-9]+)?$'

I wonder if we should disallow that to keep naming more consistent.

Quoted 3 lines

Though really what I'd like to see is the OPP nodes moved into the device nodes they belong to when appropriate (i.e. when not shared between multiple devices).

+1

The only reason for keeping it outside was because CPUs were sharing them.

Quoted 25 lines · Dmitry Osipenko

Quoted 16 lines

Not all OPP table names and OPP entries consist of a single word. In particular NVIDIA Tegra OPP tables use multi-word names. Allow OPP node and OPP entry name to have multi-worded names to silence DT checker warnings about the multi-word names separated by hyphen.

Reviewed-by David Heidelberg <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +0 -0

index ae3ae4d39843..298cf24af270 100644

Documentation/devicetree/bindings/opp/opp-v2-base.yaml select: false · line 25

Quoted 5 lines

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +1 -1

@@ -22,7 +22,7 @@ select: false
 
 properties:
   $nodename:
-    pattern: '^opp-table(-[a-z0-9]+)?$'
+    pattern: '^opp-table(-[a-z0-9]+)*$'

I don't see how this helps you. What I see needed upstream is a prefix:

Quoted 1 line

'-?opp-table(-[0-9]+)?$'

I wonder if we should disallow that to keep naming more

22.10.2021 07:43, Viresh Kumar пишет:

Quoted 28 lines · Rob Herring

Quoted 26 lines · Dmitry Osipenko

Quoted 16 lines

Not all OPP table names and OPP entries consist of a single word. In particular NVIDIA Tegra OPP tables use multi-word names. Allow OPP node and OPP entry name to have multi-worded names to silence DT checker warnings about the multi-word names separated by hyphen.

Reviewed-by David Heidelberg <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +0 -0

index ae3ae4d39843..298cf24af270 100644

Documentation/devicetree/bindings/opp/opp-v2-base.yaml select: false · line 25

Quoted 5 lines

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +1 -1

@@ -22,7 +22,7 @@ select: false
 
 properties:
   $nodename:
-    pattern: '^opp-table(-[a-z0-9]+)?$'
+    pattern: '^opp-table(-[a-z0-9]+)*$'

I don't see how this helps you. What I see needed upstream is a prefix:

'-?opp-table(-[0-9]+)?$'

Quoted 1 line

I wonder if we should disallow that to keep naming more

I also think that postfix variant should be disallowed for consistency. I sent out patches for both variants.

V2 uses pattern that supports both naming schemes. If V2 is less appropriate, then please take this V1.

Quoted 34 lines

22.10.2021 07:43, Viresh Kumar пишет:

Quoted 26 lines · Rob Herring

Quoted 25 lines · Dmitry Osipenko

Quoted 16 lines

Not all OPP table names and OPP entries consist of a single word. In particular NVIDIA Tegra OPP tables use multi-word names. Allow OPP node and OPP entry name to have multi-worded names to silence DT checker warnings about the multi-word names separated by hyphen.

Reviewed-by David Heidelberg <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---
 Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +0 -0

index ae3ae4d39843..298cf24af270 100644

Documentation/devicetree/bindings/opp/opp-v2-base.yaml select: false · line 25

Quoted 5 lines

Documentation/devicetree/bindings/opp/opp-v2-base.yaml +1 -1

@@ -22,7 +22,7 @@ select: false
 
 properties:
   $nodename:
-    pattern: '^opp-table(-[a-z0-9]+)?$'
+    pattern: '^opp-table(-[a-z0-9]+)*$'

I don't see how this helps you. What I see needed upstream is a prefix:

Quoted 3 lines

Quoted 1 line

'-?opp-table(-[0-9]+)?$'

I wonder if we should disallow that to keep naming more

I also think that postfix variant should be disallowed for consistency. I sent out patches for both variants.

Quoted 2 lines

V2 uses pattern that supports both naming schemes. If V2 is less appropriate, then please take this V1.

I didn't like V2, we can mandate to keep it like opp-* and opp-table-*.

This patch looked okay to me, lets see what Rob has to say.

What we currently have for Tegra is a tegra-opps.dtsi and tegra.dtsi which includes the OPP's dtsi.

the tegra-opps.dtsi has this structure:

table: devname-opp-table {
	opp: ...
};

and tegra.dtsi:

#include "tegra-opps.dtsi"

device@0000 {
	operating-points-v2 = <&table>;
};

It just occurred to me that there is no need to move all tables to tegra.dtsi, but change structure of tegra-opps.dtsi to:

device@0000 {
	operating-points-v2 = <&table>;

	table: opp-table {
		opp: ...
	};
};

Then there no need to change current naming scheme. Let me try to implement it and see how it goes.

Quoted 20 lines

What we currently have for Tegra is a tegra-opps.dtsi and tegra.dtsi which includes the OPP's dtsi.

the tegra-opps.dtsi has this structure:

table: devname-opp-table {
	opp: ...
};

and tegra.dtsi:

#include "tegra-opps.dtsi"

device@0000 {
	operating-points-v2 = <&table>;
};

It just occurred to me that there is no need to move all tables to tegra.dtsi, but change structure of tegra-opps.dtsi to:

Quoted 7 lines

device@0000 {
	operating-points-v2 = <&table>;

	table: opp-table {
		opp: ...
	};
};

I thought you would have already thought about that and I was surprised when you saw the tables are big enough to be moved. I was wondering what does it really mean :)

Quoted 2 lines

Then there no need to change current naming scheme. Let me try to implement it and see how it goes.

That's good then.

22.10.2021 10:45, Viresh Kumar пишет:

Quoted 29 lines · Dmitry Osipenko

Quoted 20 lines

What we currently have for Tegra is a tegra-opps.dtsi and tegra.dtsi which includes the OPP's dtsi.

the tegra-opps.dtsi has this structure:

table: devname-opp-table {
	opp: ...
};

and tegra.dtsi:

#include "tegra-opps.dtsi"

device@0000 {
	operating-points-v2 = <&table>;
};

It just occurred to me that there is no need to move all tables to tegra.dtsi, but change structure of tegra-opps.dtsi to:

Quoted 7 lines

device@0000 {
	operating-points-v2 = <&table>;

	table: opp-table {
		opp: ...
	};
};

Quoted 9 lines

I thought you would have already thought about that and I was surprised when you saw the tables are big enough to be moved. I was wondering what does it really mean :)

Quoted 2 lines

Then there no need to change current naming scheme. Let me try to implement it and see how it goes.

That's good then.

I implemented that approach and it works, but there are two problems:

1. I had to factor out OPP tables from SPI device-tree nodes because DTC doesn't allow to have them within SPI nodes [1] and dtb fails to compile.

[1] https://elixir.bootlin.com/linux/v5.15-rc6/source/scripts/dtc/checks.c#L1141

2. dtbs_check now warns about every opp-table sub-node, like this:

/home/runner/work/linux/linux/arch/arm/boot/dts/tegra30-asus-nexus7-grouper-E1565.dt.yaml: memory-controller@7000f400: 'opp-table' does not match any of the regexes: '^emc-timings-[0-9]+$', 'pinctrl-[0-9]+'
From schema: /home/runner/work/linux/linux/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml

23.10.2021 16:00, Dmitry Osipenko пишет:

Quoted 42 lines

22.10.2021 10:45, Viresh Kumar пишет:

Quoted 29 lines · Dmitry Osipenko

Quoted 20 lines

What we currently have for Tegra is a tegra-opps.dtsi and tegra.dtsi which includes the OPP's dtsi.

the tegra-opps.dtsi has this structure:

table: devname-opp-table {
	opp: ...
};

and tegra.dtsi:

#include "tegra-opps.dtsi"

device@0000 {
	operating-points-v2 = <&table>;
};

It just occurred to me that there is no need to move all tables to tegra.dtsi, but change structure of tegra-opps.dtsi to:

Quoted 7 lines

device@0000 {
	operating-points-v2 = <&table>;

	table: opp-table {
		opp: ...
	};
};

Quoted 9 lines

I thought you would have already thought about that and I was surprised when you saw the tables are big enough to be moved. I was wondering what does it really mean :)

Quoted 2 lines

Then there no need to change current naming scheme. Let me try to implement it and see how it goes.

That's good then.

I implemented that approach and it works, but there are two problems:

Quoted 9 lines

1. I had to factor out OPP tables from SPI device-tree nodes because DTC doesn't allow to have them within SPI nodes [1] and dtb fails to compile.

[1] https://elixir.bootlin.com/linux/v5.15-rc6/source/scripts/dtc/checks.c#L1141

2. dtbs_check now warns about every opp-table sub-node, like this:

/home/runner/work/linux/linux/arch/arm/boot/dts/tegra30-asus-nexus7-grouper-E1565.dt.yaml: memory-controller@7000f400: 'opp-table' does not match any of the regexes: '^emc-timings-[0-9]+$', 'pinctrl-[0-9]+'
From schema: /home/runner/work/linux/linux/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml

Thinking more about this, it's not a good idea to add new sub-node to nodes which already have sub-nodes because existing driver code that parses sub-nodes may not be ready to face the new opp-table sub-node.

I'll revert back to the previous variant with distinct opp-table nodes.