2020-04-22 20:13:49

by Christian Marangi

[permalink] [raw]
Subject: [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(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index a1b8238872a2..9e9279162b5b 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -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;
--
2.25.1


2020-04-22 20:14:11

by Christian Marangi

[permalink] [raw]
Subject: [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(-)

diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index 64f07417ecfb..537e1774f589 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -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:
--
2.25.1

2020-04-24 21:50:17

by Rob Herring

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

On Wed, Apr 22, 2020 at 3:12 PM Ansuel Smith <[email protected]> wrote:
>
> 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(-)
>
> diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> index 64f07417ecfb..537e1774f589 100644
> --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> @@ -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

2020-04-24 22:21:50

by Christian Marangi

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

> On Wed, Apr 22, 2020 at 3:12 PM Ansuel Smith <[email protected]>
> wrote:
> >
> > 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(-)
> >
> > diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-
> cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-
> cpufreq.txt
> > index 64f07417ecfb..537e1774f589 100644
> > --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> > +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-
> cpufreq.txt
> > @@ -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

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

2020-04-27 03:53:56

by Viresh Kumar

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

On 25-04-20, 00:19, [email protected] wrote:
> > On Wed, Apr 22, 2020 at 3:12 PM Ansuel Smith <[email protected]>
> > wrote:
> > >
> > > 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(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > cpufreq.txt
> > > index 64f07417ecfb..537e1774f589 100644
> > > --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
> > > +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > cpufreq.txt
> > > @@ -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
>
> 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).

--
viresh

2020-04-27 10:45:41

by Christian Marangi

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

> On 25-04-20, 00:19, [email protected] wrote:
> > > On Wed, Apr 22, 2020 at 3:12 PM Ansuel Smith
> <[email protected]>
> > > wrote:
> > > >
> > > > 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(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > > cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > > cpufreq.txt
> > > > index 64f07417ecfb..537e1774f589 100644
> > > > --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-
> cpufreq.txt
> > > > +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > > cpufreq.txt
> > > > @@ -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
> >
> > 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).
>
> --
> 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.

2020-04-27 17:47:57

by Rob Herring

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

On Mon, Apr 27, 2020 at 5:43 AM <[email protected]> wrote:
>
> > On 25-04-20, 00:19, [email protected] wrote:
> > > > On Wed, Apr 22, 2020 at 3:12 PM Ansuel Smith
> > <[email protected]>
> > > > wrote:
> > > > >
> > > > > 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(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > > > cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > > > cpufreq.txt
> > > > > index 64f07417ecfb..537e1774f589 100644
> > > > > --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > cpufreq.txt
> > > > > +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-
> > > > cpufreq.txt
> > > > > @@ -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
> > >
> > > 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).
> >
> > --
> > 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.

> 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