Re: [PATCH 11/14] dt-bindings: pinctrl: stm32: support for stm32mp215 and additional packages

[Antonio Borneo <antonio.borneo@foss.st.com>]

On Wed, 2024-10-23 at 10:51 +0200, Krzysztof Kozlowski wrote:
> On Tue, Oct 22, 2024 at 05:56:55PM +0200, Antonio Borneo wrote:
> > From: Amelie Delaunay <amelie.delaunay@foss.st.com>
> > 
> > Add support for st,stm32mp215-pinctrl and st,stm32mp215-z-pinctrl.
> 
> So all previous patches are for this? Then they are supposed to be here.

Hi Krzysztof,

I'm not sure I fully get your point here.
The previous patches in this series add the new features to the already upstreamed STM32MP257.
The same features are also needed here by STM32MP215 and in next patches 12/14 and 13/14 by STM32MP235.

> 
> > Add packages AM, AN and AO (values : 0x1000, 0x2000 and 0x8000)
> > 
> > Signed-off-by: Amelie Delaunay <amelie.delaunay@foss.st.com>
> > Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
> > ---
> >  .../devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml         | 4 +++-
> >  include/dt-bindings/pinctrl/stm32-pinfunc.h                   | 3 +++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > index 9a7ecfea6eb5b..0a2d644dbece3 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > @@ -27,6 +27,8 @@ properties:
> >        - st,stm32mp135-pinctrl
> >        - st,stm32mp157-pinctrl
> >        - st,stm32mp157-z-pinctrl
> > +      - st,stm32mp215-pinctrl
> > +      - st,stm32mp215-z-pinctrl
> >        - st,stm32mp257-pinctrl
> >        - st,stm32mp257-z-pinctrl
> >  
> > @@ -59,7 +61,7 @@ properties:
> >        Indicates the SOC package used.
> >        More details in include/dt-bindings/pinctrl/stm32-pinfunc.h
> >      $ref: /schemas/types.yaml#/definitions/uint32
> > -    enum: [0x1, 0x2, 0x4, 0x8, 0x100, 0x400, 0x800]
> > +    enum: [0x1, 0x2, 0x4, 0x8, 0x100, 0x400, 0x800, 0x1000, 0x2000, 0x4000]
> >  
> >  patternProperties:
> >    '^gpio@[0-9a-f]*$':
> > diff --git a/include/dt-bindings/pinctrl/stm32-pinfunc.h b/include/dt-bindings/pinctrl/stm32-pinfunc.h
> > index af3fd388329a0..01bc8be78ef72 100644
> > --- a/include/dt-bindings/pinctrl/stm32-pinfunc.h
> > +++ b/include/dt-bindings/pinctrl/stm32-pinfunc.h
> > @@ -41,6 +41,9 @@
> >  #define STM32MP_PKG_AI 0x100
> >  #define STM32MP_PKG_AK 0x400
> >  #define STM32MP_PKG_AL 0x800
> > +#define STM32MP_PKG_AM 0x1000
> > +#define STM32MP_PKG_AN 0x2000
> > +#define STM32MP_PKG_AO 0x4000
> 
> Why these are some random hex values but not for example 0x801, 0x802
> and 0x803? That's an enum, so bitmask does not make sense here.

The are bitmask. You can check in patch 14/14 that adds a new package to the existing code of STM32MP257.
Do you prefer I rewrite them all as, e.g.
#define STM32MP_PKG_AO (1 << 14)
?

Thanks for the review!
Regards,
Antonio