Re: [PATCH v3 1/2] dt-bindings: regulator: st,stm32mp1-pwr-reg: add compatible for STM32MP13

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 5/14/24 22:08, Conor Dooley wrote:
On Mon, May 13, 2024 at 08:02:21PM +0200, Patrick DELAUNAY wrote:
Hi,

On 5/13/24 17:16, Conor Dooley wrote:
On Mon, May 13, 2024 at 04:34:20PM +0200, Marek Vasut wrote:
On 5/13/24 11:56 AM, Patrick Delaunay wrote:
Add new compatible "st,stm32mp13-pwr-reg" for STM32MP13 SoC family.

Signed-off-by: Patrick Delaunay <patrick.delaunay@xxxxxxxxxxx>
---

Changes in v3:
- Replace oneOf/const by enum; solve the V2 issues for dt_binding_check

Changes in v2:
- update for Rob review, only add compatible for STM32MP13 family

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

diff --git a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
index c9586d277f41..c766f0a15a31 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,stm32mp13-pwr-reg
Should the st,stm32mp1,pwr-reg be treated as fallback compatible for
st,stm32mp13-pwr-reg or not ?

In other words, should the DT contain:
compatible = "st,stm32mp13-pwr-reg", "st,stm32mp1,pwr-reg";
or
compatible = "st,stm32mp13-pwr-reg";
? Which one is preferable ?

I think the former one, since the MP13 PWR block could also be operated by
older MP1(5) PWR block driver(s) without any adverse effects, except the SD
IO domain configuration won't be available, right ?
Aye, the fallback sounds like what should be being used here, especially
if another user of the DT might not need to implement the extra domain.

Yes it is the the only difference but I think that type of fallback is no
more recommended for different device and

the PWR device on STM32MP13 and on STM32MP15 are different.


The other user of the non-secure device tree don't use the yet the PWR
driver for STM32MP13,

so for me the fallback is not needed for non secure world (Linux/U-Boot).


So I prefer to introduce a new compatible in Linux kernel before the
STM32MP13 PWR node is really used  to avoid ABI break in futur.
How is it going to break an ABI? If the mp13 implements a functional
subset of what the mp1 does, then that's what fallback compatibles
are intended for. Marek's mail suggests that this is the case, and
therefore a fallback should be used.

If code written for the mp1 would not work on the mp13, then a fallback
should not be used.

Neither I nor Marek are saying that a new compatible should not be
created, in case you misunderstood that. This only affects the binding
patch, and your driver etc are free to use the mp13 compatible.

Thanks,
Conor.


The IP PWR change with STM32MP13 and it is not just a subset

but for the range used by the compatible <0x50001000 0x10>

I agreed that feature are just added.


Other feature are also removed but for next registers (the associated

registers or bits becomes reserved, for PWR_MCUCR for example).


So I can change in binding to support the DT:


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

or for STM32MP13:

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

with

  compatible:
    oneOf:
        - items:
           - const: st,stm32mp1,pwr-reg
        - items:
           - const: st,stm32mp13-pwr-reg
           - const: st,stm32mp1,pwr-reg


But for me this fallback in device tree is a little confusing for PWR.


Patrick





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux