cpufreq: qcom: fix compatibility issue with old binding

PATCH In review Ansuel Smith 3 participants 7 messages Apr 22, 2020 – Apr 27, 2020

Subsystems (5)

Device tree bindings Arm Qualcomm mailing list CPU frequency scaling framework Operating performance points (OPP) +1 more subsystems

lore

Share & tools

lore

Ansuel Smith

author

2020-04-22 20:12 UTC #0 lore raw

PATCH v2 1/2 cpufreq: qcom: fix compatibility issue with old binding

Binding has changed from operating-points-v2-kryo-cpu to operating-points-v2-qcom-cpu. Also check for old binding in driver probe.

Fixes a8811ec764f9 ("cpufreq: qcom: Add support for krait based socs")

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/cpufreq/qcom-cpufreq-nvmem.c +4 -0

index a1b8238872a2..9e9279162b5b 100644
@@ -278,6 +278,10 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
 		return -ENOENT;
 
 	ret = of_device_is_compatible(np, "operating-points-v2-qcom-cpu");
+	if (!ret)
+		ret = of_device_is_compatible(np,
+					      "operating-points-v2-kryo-cpu");
+
 	if (!ret) {
 		of_node_put(np);
 		return -ENOENT;

Signature · 1 line

2.25.1

Ansuel Smith

author

2020-04-22 20:12 UTC #1 lore raw

PATCH v2 2/2 dt-bindings: opp: Fix wrong binding in qcom-nvmem-cpufreq

Update binding to new generic name "operating-points-v2-qcom-cpu"

Fixes a8811ec764f9 ("cpufreq: qcom: Add support for krait based socs")

Signed-off-by Ansuel Smith <[email protected]>

---
 Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +1 -1

index 64f07417ecfb..537e1774f589 100644
@@ -19,7 +19,7 @@ In 'cpu' nodes:
 
 In 'operating-points-v2' table:
 - compatible: Should be
-	- 'operating-points-v2-kryo-cpu' for apq8096, msm8996, msm8974,
+	- 'operating-points-v2-qcom-cpu' for apq8096, msm8996, msm8974,
 					     apq8064, ipq8064, msm8960 and ipq8074.
 
 Optional properties:

Signature · 1 line

2.25.1

Rob Herring

2020-04-24 21:48 UTC #2 lore raw

Re: [PATCH v2 2/2] dt-bindings: opp: Fix wrong binding in qcom-nvmem-cpufreq

Quoted 14 lines · Ansuel Smith

Update binding to new generic name "operating-points-v2-qcom-cpu"
Fixes a8811ec764f9 ("cpufreq: qcom: Add support for krait based socs")
Signed-off-by Ansuel Smith <[email protected]>
---
 Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +0 -0
index 64f07417ecfb..537e1774f589 100644

Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt In 'cpu' nodes: · line 23

Quoted 6 lines

Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +1 -1

@@ -19,7 +19,7 @@ In 'cpu' nodes:
 
 In 'operating-points-v2' table:
 - compatible: Should be
-       - 'operating-points-v2-kryo-cpu' for apq8096, msm8996, msm8974,
+       - 'operating-points-v2-qcom-cpu' for apq8096, msm8996, msm8974,
                                             apq8064, ipq8064, msm8960 and ipq8074.

This is not how you fix the backwards compatibility issue pointed out on the Fixes reference.

Rob

[email protected]

author

2020-04-24 22:19 UTC #3 lore raw

R: [PATCH v2 2/2] dt-bindings: opp: Fix wrong binding in qcom-nvmem-cpufreq

Quoted 26 lines

Quoted 7 lines · Ansuel Smith
Update binding to new generic name "operating-points-v2-qcom-cpu"
Fixes a8811ec764f9 ("cpufreq: qcom: Add support for krait based socs")
Signed-off-by Ansuel Smith <[email protected]>
---
 Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 2
+-
>  1 file changed, 1 insertion(+), 1 deletion(-)
Review
Quoted 2 lines
(quoted diff) +0 -0
cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem- cpufreq.txt
Documentation/devicetree/bindings/opp/qcom-nvmem-
Quoted 3 lines
index 64f07417ecfb..537e1774f589 100644
Documentation/devicetree/bindings/opp/qcom-nvmem- renamed from Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +0 -0
cpufreq.txt
Documentation/devicetree/bindings/opp/qcom-nvmem- In 'cpu' nodes: · line 22
Quoted 6 lines
Documentation/devicetree/bindings/opp/qcom-nvmem- renamed from Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +1 -1
@@ -19,7 +19,7 @@ In 'cpu' nodes:
 
 In 'operating-points-v2' table:
 - compatible: Should be
-       - 'operating-points-v2-kryo-cpu' for apq8096, msm8996, msm8974,
+       - 'operating-points-v2-qcom-cpu' for apq8096, msm8996,
msm8974,

Quoted 6 lines

Documentation/devicetree/bindings/opp/qcom-nvmem- In 'cpu' nodes: · line 23
Quoted 1 line
Documentation/devicetree/bindings/opp/qcom-nvmem- renamed from Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +0 -0
                                             apq8064, ipq8064, msm8960 and ipq8074.
This is not how you fix the backwards compatibility issue pointed out on the Fixes reference.
Rob

Sorry but can you give some directive? Should I use the old binding and change the driver to use it instead of the new one (and drop it) ?

Viresh Kumar

maintainer

2020-04-27 03:49 UTC #4 lore raw

Re: R: [PATCH v2 2/2] dt-bindings: opp: Fix wrong binding in qcom-nvmem-cpufreq

Quoted 28 lines · On 25-04-20, 00:19, [email protected] wrote:

Quoted 28 lines
Quoted 7 lines · Ansuel Smith
Update binding to new generic name "operating-points-v2-qcom-cpu"
Fixes a8811ec764f9 ("cpufreq: qcom: Add support for krait based socs")
Signed-off-by Ansuel Smith <[email protected]>
---
 Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 2
+-
>  1 file changed, 1 insertion(+), 1 deletion(-)
Review
Quoted 2 lines
(quoted diff) +0 -0
cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem- cpufreq.txt
Documentation/devicetree/bindings/opp/qcom-nvmem-
Quoted 3 lines
index 64f07417ecfb..537e1774f589 100644
Documentation/devicetree/bindings/opp/qcom-nvmem- renamed from Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +0 -0
cpufreq.txt
Documentation/devicetree/bindings/opp/qcom-nvmem- In 'cpu' nodes: · line 22
Quoted 6 lines
Documentation/devicetree/bindings/opp/qcom-nvmem- renamed from Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +1 -1
@@ -19,7 +19,7 @@ In 'cpu' nodes:
 
 In 'operating-points-v2' table:
 - compatible: Should be
-       - 'operating-points-v2-kryo-cpu' for apq8096, msm8996, msm8974,
+       - 'operating-points-v2-qcom-cpu' for apq8096, msm8996,
msm8974,
Quoted 1 line
Documentation/devicetree/bindings/opp/qcom-nvmem- renamed from Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +0 -0
                                             apq8064, ipq8064, msm8960 and ipq8074.

Quoted 7 lines

Quoted 4 lines
This is not how you fix the backwards compatibility issue pointed out on the Fixes reference.
Rob
Sorry but can you give some directive? Should I use the old binding and change the driver to use it instead of the new one (and drop it) ?

It is not about the name of the binding, you can rename it to whatever you want. The kernel needs to keep supporting all the previous bindings, so we can keep on changing the kernel but keep the same bootloader (with earlier bindings).

Signature · 1 line

viresh

[email protected]

author

2020-04-27 10:43 UTC #5 lore raw

R: R: [PATCH v2 2/2] dt-bindings: opp: Fix wrong binding in qcom-nvmem-cpufreq

Quoted 22 lines · On 25-04-20, 00:19, [email protected] wrote:

Quoted 1 line
Quoted 1 line
On Wed, Apr 22, 2020 at 3:12 PM Ansuel Smith
<[email protected]>
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines · wrote:
Update binding to new generic name "operating-points-v2-qcom-cpu"
Fixes a8811ec764f9 ("cpufreq: qcom: Add support for krait based
socs")
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Signed-off-by Ansuel Smith <[email protected]>
---
Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt |
2
Quoted 8 lines
Quoted 8 lines
+-
>  1 file changed, 1 insertion(+), 1 deletion(-)
Review
Quoted 2 lines
(quoted diff) +0 -0
cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem- cpufreq.txt
Quoted 2 lines
index 64f07417ecfb..537e1774f589 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-
cpufreq.txt

Quoted 11 lines

Quoted 7 lines
Quoted 7 lines
Quoted 1 line
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-
cpufreq.txt
Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt In 'cpu' nodes: · line 22
Quoted 5 lines
Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +0 -1
@@ -19,7 +19,7 @@ In 'cpu' nodes:
 
 In 'operating-points-v2' table:
 - compatible: Should be
-       - 'operating-points-v2-kryo-cpu' for apq8096, msm8996,
msm8974,
Quoted 3 lines
Quoted 3 lines
Review In 'cpu' nodes: · line 22
Quoted 1 line
(quoted diff) +1 -0
+       - 'operating-points-v2-qcom-cpu' for apq8096, msm8996,
msm8974,
Quoted 1 line
(quoted diff) +0 -0
                                             apq8064, ipq8064,

msm8960 and ipq8074.

Quoted 10 lines

Quoted 7 lines
Quoted 5 lines
This is not how you fix the backwards compatibility issue pointed out on the Fixes reference.
Rob
Sorry but can you give some directive? Should I use the old binding and
change
Quoted 1 line
the driver to use it instead of the new one (and drop it) ?

Quoted 7 lines

It is not about the name of the binding, you can rename it to whatever you want. The kernel needs to keep supporting all the previous bindings, so we can keep on changing the kernel but keep the same bootloader (with earlier bindings).
--
viresh

Ok but still I can't understand why this is not right.
In 1/2 of this patchset I added the check for the old binding in the driver and
here I updated the Documentation with the new one. This way the kernel should support all the previous bindings and I can use the new better name.

Rob Herring

2020-04-27 17:46 UTC #6 lore raw

Re: R: [PATCH v2 2/2] dt-bindings: opp: Fix wrong binding in qcom-nvmem-cpufreq

Quoted 50 lines · On Mon, Apr 27, 2020 at 5:43 AM <[email protected]> wrote:

Quoted 23 lines · On 25-04-20, 00:19, [email protected] wrote:
Quoted 1 line
Quoted 1 line
On Wed, Apr 22, 2020 at 3:12 PM Ansuel Smith
<[email protected]>
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines · wrote:
Update binding to new generic name "operating-points-v2-qcom-cpu"
Fixes a8811ec764f9 ("cpufreq: qcom: Add support for krait based
socs")
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Signed-off-by Ansuel Smith <[email protected]>
---
Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt |
2
Quoted 8 lines
Quoted 8 lines
+-
>  1 file changed, 1 insertion(+), 1 deletion(-)
Review
Quoted 2 lines
(quoted diff) +0 -0
cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem- cpufreq.txt
Quoted 2 lines
index 64f07417ecfb..537e1774f589 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-
cpufreq.txt
Quoted 11 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-
cpufreq.txt
Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt In 'cpu' nodes: · line 22
Quoted 5 lines
Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +0 -1
@@ -19,7 +19,7 @@ In 'cpu' nodes:
 
 In 'operating-points-v2' table:
 - compatible: Should be
-       - 'operating-points-v2-kryo-cpu' for apq8096, msm8996,
msm8974,
Quoted 3 lines
Quoted 3 lines
Review In 'cpu' nodes: · line 22
Quoted 1 line
(quoted diff) +1 -0
+       - 'operating-points-v2-qcom-cpu' for apq8096, msm8996,
msm8974,
Quoted 1 line
(quoted diff) +0 -0
                                             apq8064, ipq8064,
msm8960 and ipq8074.
Quoted 15 lines
Quoted 7 lines
Quoted 5 lines
This is not how you fix the backwards compatibility issue pointed out on the Fixes reference.
Rob
Sorry but can you give some directive? Should I use the old binding and
change
Quoted 1 line
the driver to use it instead of the new one (and drop it) ?
It is not about the name of the binding, you can rename it to whatever you want. The kernel needs to keep supporting all the previous bindings, so we can keep on changing the kernel but keep the same bootloader (with earlier bindings).

Quoted 5 lines

Quoted 2 lines
--
viresh
Ok but still I can't understand why this is not right.
In 1/2 of this patchset I added the check for the old binding in the driver

I don't have patch 1 and this patch should stand on it's own.

Quoted 3 lines

and
here I updated the Documentation with the new one. This way the kernel should support all the previous bindings and I can use the new better name.

First, a compatible string is just an identifier. Maybe it wasn't the best name, but who cares really. Just use it even if it's not just kryo cpus. Otherwise, it's more complicated.

Are you changing every dts file? If not, then dts files now have an undocumented compatible string which is not okay. You'd need to keep the old compatible and mark it deprecated. If you are, then you are breaking compatibility between new dts and older kernels unless adding the new compatible string to the driver is backported to stable (which should be okay).

Rob