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

[Conor Dooley]

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.

Attachment: signature.asc
Description: PGP signature