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

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

[Patrick Delaunay]

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

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

[Patrick Delaunay]

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 <patrick.delaunay@foss.st.com>
---

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

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

[Krzysztof Kozlowski]

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 <patrick.delaunay@foss.st.com>
> ---
> 
>  .../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

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

[Patrick DELAUNAY]

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 <patrick.delaunay@foss.st.com>
>> ---
>>
>>   .../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

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

[Patrick Delaunay]

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 <patrick.delaunay@foss.st.com>
---

 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

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

[Krzysztof Kozlowski]

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 <patrick.delaunay@foss.st.com>
> ---
> 
>  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

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

[Patrick DELAUNAY]

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 <patrick.delaunay@foss.st.com>
>> ---
>>
>>   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

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

[Rob Herring]

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

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

[Patrick DELAUNAY]

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/20240319024534.103299-1-marex@denx.de/


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

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

[Rob Herring]

On Fri, Apr 26, 2024 at 6:42 AM Patrick DELAUNAY
<patrick.delaunay@foss.st.com> 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/20240319024534.103299-1-marex@denx.de/
>

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

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

[Patrick DELAUNAY]

Hi,

On 4/26/24 14:51, Rob Herring wrote:
> On Fri, Apr 26, 2024 at 6:42 AM Patrick DELAUNAY
> <patrick.delaunay@foss.st.com> 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/20240319024534.103299-1-marex@denx.de/
>>
> 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