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

Message

Patrick Delaunay April 25, 2024, 7:48 a.m. UTC

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

Comments

Rob Herring (Arm) April 25, 2024, 4:30 p.m. UTC | #1

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

Patrick Delaunay April 26, 2024, 11:41 a.m. UTC | #2

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/[e-mail]/


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

Rob Herring (Arm) April 26, 2024, 12:51 p.m. UTC | #3

On Fri, Apr 26, 2024 at 6:42 AM Patrick DELAUNAY
<[e-mail]> 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/[e-mail]/
>

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

Patrick Delaunay April 26, 2024, 2:28 p.m. UTC | #4

Hi,

On 4/26/24 14:51, Rob Herring wrote:
> On Fri, Apr 26, 2024 at 6:42 AM Patrick DELAUNAY
> <[e-mail]> 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/[e-mail]/
>>
> 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