2024-04-25 07:50:11

by Patrick Delaunay

[permalink] [raw]
Subject: [PATCH 0/3] ARM: st: use a correct pwr compatible for stm32mp15


This patchset removes the unexpected comma in the PWR compatible
"st,stm32mp1,pwr-reg" and uses a new compatible "st,stm32mp1-pwr-reg"
in STM3MP15 device trees.

The support of old compatible is keep to avoid ABI break.



Patrick Delaunay (3):
dt-bindings: regulator: st,stm32mp1-pwr-reg: add correct compatible
regulator: stm32-pwr: add support of correct compatible
ARM: dts: st: update the pwr compatible for stm32mp15

.../devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml | 6 ++++--
arch/arm/boot/dts/st/stm32mp151.dtsi | 2 +-
drivers/regulator/stm32-pwr.c | 1 +
3 files changed, 6 insertions(+), 3 deletions(-)

--
2.25.1



2024-04-25 07:50:17

by Patrick Delaunay

[permalink] [raw]
Subject: [PATCH 3/3] ARM: dts: st: update the pwr compatible for stm32mp15

Remove the unexpected comma in the compatible "st,stm32mp1,pwr-reg",
and use the new supported compatible "st,stm32mp1-pwr-reg" in STM3MP15
SoC dtsi.

Signed-off-by: Patrick Delaunay <[email protected]>
---

arch/arm/boot/dts/st/stm32mp151.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/st/stm32mp151.dtsi b/arch/arm/boot/dts/st/stm32mp151.dtsi
index 16bd6eee32b4..702cdaa1343a 100644
--- a/arch/arm/boot/dts/st/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp151.dtsi
@@ -144,7 +144,7 @@ rcc: rcc@50000000 {
};

pwr_regulators: pwr@50001000 {
- compatible = "st,stm32mp1,pwr-reg";
+ compatible = "st,stm32mp1-pwr-reg";
reg = <0x50001000 0x10>;

reg11: reg11 {
--
2.25.1


2024-04-25 07:50:18

by Patrick Delaunay

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: regulator: st,stm32mp1-pwr-reg: add correct compatible

Remove the unexpected comma in the compatible "st,stm32mp1,pwr-reg"
and define the new compatible "st,stm32mp1-pwr-reg".
The old compatible is only keep for compatibility with old device trees.

Signed-off-by: Patrick Delaunay <[email protected]>
---

.../devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
index c9586d277f41..2a52f9e769c2 100644
--- a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
+++ b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
@@ -11,7 +11,9 @@ maintainers:

properties:
compatible:
- const: st,stm32mp1,pwr-reg
+ enum:
+ - st,stm32mp1-pwr-reg
+ - st,stm32mp1,pwr-reg

reg:
maxItems: 1
@@ -37,7 +39,7 @@ additionalProperties: false
examples:
- |
pwr@50001000 {
- compatible = "st,stm32mp1,pwr-reg";
+ compatible = "st,stm32mp1-pwr-reg";
reg = <0x50001000 0x10>;
vdd-supply = <&vdd>;
vdd_3v3_usbfs-supply = <&vdd_usb>;
--
2.25.1


2024-04-25 08:52:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: regulator: st,stm32mp1-pwr-reg: add correct compatible

On 25/04/2024 09:48, Patrick Delaunay wrote:
> Remove the unexpected comma in the compatible "st,stm32mp1,pwr-reg"
> and define the new compatible "st,stm32mp1-pwr-reg".
> The old compatible is only keep for compatibility with old device trees.
>
> Signed-off-by: Patrick Delaunay <[email protected]>
> ---
>
> .../devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
> index c9586d277f41..2a52f9e769c2 100644
> --- a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
> +++ b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
> @@ -11,7 +11,9 @@ maintainers:
>
> properties:
> compatible:
> - const: st,stm32mp1,pwr-reg
> + enum:
> + - st,stm32mp1-pwr-reg
> + - st,stm32mp1,pwr-reg

Please make it oneOf:
- const: new one
- const: old one
deprecated: true

Best regards,
Krzysztof


2024-04-25 08:58:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/3] ARM: dts: st: update the pwr compatible for stm32mp15

On 25/04/2024 09:48, Patrick Delaunay wrote:
> Remove the unexpected comma in the compatible "st,stm32mp1,pwr-reg",
> and use the new supported compatible "st,stm32mp1-pwr-reg" in STM3MP15
> SoC dtsi.
>
> Signed-off-by: Patrick Delaunay <[email protected]>
> ---
>
> arch/arm/boot/dts/st/stm32mp151.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

This will break the users and is not bisectable, so patch should wait at
least one cycle. This will preserve bisectability, although users will
be affected anyway.

Best regards,
Krzysztof


2024-04-25 13:55:25

by Patrick Delaunay

[permalink] [raw]
Subject: Re: [PATCH 3/3] ARM: dts: st: update the pwr compatible for stm32mp15

Hi,

On 4/25/24 10:57, Krzysztof Kozlowski wrote:
> On 25/04/2024 09:48, Patrick Delaunay wrote:
>> Remove the unexpected comma in the compatible "st,stm32mp1,pwr-reg",
>> and use the new supported compatible "st,stm32mp1-pwr-reg" in STM3MP15
>> SoC dtsi.
>>
>> Signed-off-by: Patrick Delaunay <[email protected]>
>> ---
>>
>> arch/arm/boot/dts/st/stm32mp151.dtsi | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
> This will break the users and is not bisectable, so patch should wait at
> least one cycle. This will preserve bisectability, although users will
> be affected anyway.


Sorry, I didn't know this constraint

But Ok, I remove this patch in serie V2 and push it later.


>
> Best regards,
> Krzysztof

Regards,

Patrick


2024-04-25 13:57:27

by Patrick Delaunay

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: regulator: st,stm32mp1-pwr-reg: add correct compatible

Hi,

On 4/25/24 10:52, Krzysztof Kozlowski wrote:
> On 25/04/2024 09:48, Patrick Delaunay wrote:
>> Remove the unexpected comma in the compatible "st,stm32mp1,pwr-reg"
>> and define the new compatible "st,stm32mp1-pwr-reg".
>> The old compatible is only keep for compatibility with old device trees.
>>
>> Signed-off-by: Patrick Delaunay <[email protected]>
>> ---
>>
>> .../devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
>> index c9586d277f41..2a52f9e769c2 100644
>> --- a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
>> +++ b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
>> @@ -11,7 +11,9 @@ maintainers:
>>
>> properties:
>> compatible:
>> - const: st,stm32mp1,pwr-reg
>> + enum:
>> + - st,stm32mp1-pwr-reg
>> + - st,stm32mp1,pwr-reg
> Please make it oneOf:
> - const: new one
> - const: old one
> deprecated: true

ok, I push a V2 soon.


>
> Best regards,
> Krzysztof

Regards

Patrick


2024-04-25 16:31:23

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 0/3] ARM: st: use a correct pwr compatible for stm32mp15

On Thu, Apr 25, 2024 at 09:48:31AM +0200, Patrick Delaunay wrote:
>
> This patchset removes the unexpected comma in the PWR compatible
> "st,stm32mp1,pwr-reg" and uses a new compatible "st,stm32mp1-pwr-reg"
> in STM3MP15 device trees.

Why? I don't see any warnings from this. Yes, we wouldn't new cases
following this pattern, but I don't think it is worth maintaining
support for both strings. We're stuck with it. And the only way to
maintain forward compatibility is:

compatible = "st,stm32mp1-pwr-reg", "st,stm32mp1,pwr-reg";

Rob

2024-04-26 11:42:29

by Patrick Delaunay

[permalink] [raw]
Subject: Re: [PATCH 0/3] ARM: st: use a correct pwr compatible for stm32mp15

Hi,

On 4/25/24 18:30, Rob Herring wrote:
> On Thu, Apr 25, 2024 at 09:48:31AM +0200, Patrick Delaunay wrote:
>> This patchset removes the unexpected comma in the PWR compatible
>> "st,stm32mp1,pwr-reg" and uses a new compatible "st,stm32mp1-pwr-reg"
>> in STM3MP15 device trees.
> Why? I don't see any warnings from this. Yes, we wouldn't new cases
> following this pattern, but I don't think it is worth maintaining
> support for both strings. We're stuck with it. And the only way to
> maintain forward compatibility is:


Yes, no warning because the compatible string are not yet checked by tools.


I propose this patch to avoid the usage of this compatible for other SoC
in STM32MP1 family.


I see the invalid compatible string when I reviewed the U-Boot patch to
add the PWR node for STM32MP13 family:

https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/


So I prefer change the PWR binding before to have the same patch applied
on Linux device tree

> compatible = "st,stm32mp1-pwr-reg", "st,stm32mp1,pwr-reg";


Yes, I will update the SoC patch with you proposal.

it is the only way to have compatibility of OLD Linux kernel (with old
driver) with NEW device tree.

I miss this compatibility issue.


>
> Rob
>
>
>
Regards

Patrick


2024-04-26 12:52:04

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 0/3] ARM: st: use a correct pwr compatible for stm32mp15

On Fri, Apr 26, 2024 at 6:42 AM Patrick DELAUNAY
<[email protected]> wrote:
>
> Hi,
>
> On 4/25/24 18:30, Rob Herring wrote:
> > On Thu, Apr 25, 2024 at 09:48:31AM +0200, Patrick Delaunay wrote:
> >> This patchset removes the unexpected comma in the PWR compatible
> >> "st,stm32mp1,pwr-reg" and uses a new compatible "st,stm32mp1-pwr-reg"
> >> in STM3MP15 device trees.
> > Why? I don't see any warnings from this. Yes, we wouldn't new cases
> > following this pattern, but I don't think it is worth maintaining
> > support for both strings. We're stuck with it. And the only way to
> > maintain forward compatibility is:
>
>
> Yes, no warning because the compatible string are not yet checked by tools.

What do you mean? There's a schema for it, so it is checked. I ran the
tools and there's no warning. If there was a warning, I'd fix the
tools in this case.

> I propose this patch to avoid the usage of this compatible for other SoC
> in STM32MP1 family.
>
>
> I see the invalid compatible string when I reviewed the U-Boot patch to
> add the PWR node for STM32MP13 family:
>
> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
>

Perhaps you should add SoC specific compatible string instead.

> So I prefer change the PWR binding before to have the same patch applied
> on Linux device tree
>
> > compatible = "st,stm32mp1-pwr-reg", "st,stm32mp1,pwr-reg";
>
>
> Yes, I will update the SoC patch with you proposal.

NO! We don't want to support that.

We have *tons* of examples in DT which don't follow recommended
patterns and we're stuck with them. This is no different. We can get
away with changing node names, but that's about it.

Rob

2024-04-26 14:29:45

by Patrick Delaunay

[permalink] [raw]
Subject: Re: [PATCH 0/3] ARM: st: use a correct pwr compatible for stm32mp15

Hi,

On 4/26/24 14:51, Rob Herring wrote:
> On Fri, Apr 26, 2024 at 6:42 AM Patrick DELAUNAY
> <[email protected]> wrote:
>> Hi,
>>
>> On 4/25/24 18:30, Rob Herring wrote:
>>> On Thu, Apr 25, 2024 at 09:48:31AM +0200, Patrick Delaunay wrote:
>>>> This patchset removes the unexpected comma in the PWR compatible
>>>> "st,stm32mp1,pwr-reg" and uses a new compatible "st,stm32mp1-pwr-reg"
>>>> in STM3MP15 device trees.
>>> Why? I don't see any warnings from this. Yes, we wouldn't new cases
>>> following this pattern, but I don't think it is worth maintaining
>>> support for both strings. We're stuck with it. And the only way to
>>> maintain forward compatibility is:
>>
>> Yes, no warning because the compatible string are not yet checked by tools.
> What do you mean? There's a schema for it, so it is checked. I ran the
> tools and there's no warning. If there was a warning, I'd fix the
> tools in this case.


Sorry, I am  no clear


the tools (dts check or check patch) don't check the recommendation for
compatible name:

    vendor specific string in the form|<vendor>,[<device>-]<usage>|

|   => for me: compatible should have only one comma,
              used as separator between vendor prefix end the device
identifier.|


But it is normal because existing device tree have a already lot a
strange compatible


>> I propose this patch to avoid the usage of this compatible for other SoC
>> in STM32MP1 family.
>>
>>
>> I see the invalid compatible string when I reviewed the U-Boot patch to
>> add the PWR node for STM32MP13 family:
>>
>> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
>>
> Perhaps you should add SoC specific compatible string instead.


yes it is a solution.


>
>> So I prefer change the PWR binding before to have the same patch applied
>> on Linux device tree
>>
>>> compatible = "st,stm32mp1-pwr-reg", "st,stm32mp1,pwr-reg";
>>
>> Yes, I will update the SoC patch with you proposal.
> NO! We don't want to support that.


Even mark the old binding deprecated is not acceptable:

 properties:
   compatible:
-    const: st,stm32mp1,pwr-reg
+    oneOf:
+    - const: st,stm32mp1-pwr-reg
+    - items:
+      - const: st,stm32mp1-pwr-reg
+      - const: st,stm32mp1,pwr-reg
+      deprecated: true


I understood.


>
> We have *tons* of examples in DT which don't follow recommended
> patterns and we're stuck with them. This is no different. We can get
> away with changing node names, but that's about it.


Ok,  I am stucked with this compatible for STM32MP15 = "st,stm32mp1,pwr-reg"

and I have no elegant solution to solved it.

So I will change my serie to add a new compatible for STM32MP13

"st,stm32mp13-pwr-reg"


>
> Rob


Regards

Patrick