Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O synchronization parameters

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

On Tue, 2025-10-14 at 19:10 +0100, Conor Dooley wrote:
> On Tue, Oct 14, 2025 at 04:04:50PM +0200, Antonio Borneo wrote:
> > Document the support of the I/O synchronization parameters:
> > - skew-delay-input;
> > - skew-delay-output;
> > - st,io-sync.
> > 
> > Forbid 'skew-delay-input' and 'skew-delay-output' to be both
> > present on the same pin.
> > Allow the new properties only with compatibles that support them.
> > Add an example that uses the new properties.
> > 
> > Co-developed-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
> > Signed-off-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
> > Signed-off-by: Antonio Borneo <antonio.borneo@foss.st.com>
> > ---
> >  .../bindings/pinctrl/st,stm32-pinctrl.yaml    | 98 +++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > index 2df141ed7222d..0010762127c05 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml
> > @@ -220,12 +220,89 @@ patternProperties:
> >              minimum: 0
> >              maximum: 3
> >  
> > +          skew-delay-input:
> > +            description: |
> > +              IO synchronization skew rate applied to the input path
> > +              0: No delay
> > +              1: Delay 0.30 ns
> > +              2: Delay 0.50 ns
> > +              3: Delay 0.75 ns
> > +              4: Delay 1.00 ns
> > +              5: Delay 1.25 ns
> > +              6: Delay 1.50 ns
> > +              7: Delay 1.75 ns
> > +              8: Delay 2.00 ns
> > +              9: Delay 2.25 ns
> > +              10: Delay 2.50 ns
> > +              11: Delay 2.75 ns
> > +              12: Delay 3.00 ns
> > +              13: Delay 3.25 ns
> > +            minimum: 0
> > +            maximum: 13
> > +
> > +          skew-delay-output:
> > +            description: |
> > +              IO synchronization latch delay applied to the output path
> > +              0: No delay
> > +              1: Delay 0.30 ns
> > +              2: Delay 0.50 ns
> > +              3: Delay 0.75 ns
> > +              4: Delay 1.00 ns
> > +              5: Delay 1.25 ns
> > +              6: Delay 1.50 ns
> > +              7: Delay 1.75 ns
> > +              8: Delay 2.00 ns
> > +              9: Delay 2.25 ns
> > +              10: Delay 2.50 ns
> > +              11: Delay 2.75 ns
> > +              12: Delay 3.00 ns
> > +              13: Delay 3.25 ns
> > +            minimum: 0
> > +            maximum: 13
> 
> Same comments here as on the earlier patch. I would like to see times
> used natively.

Yes, I replied to the earlier patch.

> pw-bot: changes-requested
> 
> > +
> > +          st,io-sync:
> > +            description: |
> > +              IO synchronization through re-sampling or inversion
> > +              0: data or clock GPIO pass-through
> > +              1: clock GPIO inverted
> > +              2: data GPIO re-sampled on clock rising edge
> > +              3: data GPIO re-sampled on clock falling edge
> > +              4: data GPIO re-sampled on both clock edges
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +            enum: [0, 1, 2, 3, 4]
> 
> I really don't like this kinds of properties that lead to "random"
> numbers in devicetree. I'd much rather see a string list here.

Agree!
I just need to figure out some reasonably short but still meaningful
string for them.

Thanks for the review!
Antonio

> 
> > +
> >          required:
> >            - pinmux
> >  
> > +        # Not allowed both skew-delay-input and skew-delay-output
> > +        if:
> > +          required:
> > +            - skew-delay-input
> > +        then:
> > +          properties:
> > +            skew-delay-output: false
> > +
> >  allOf:
> >    - $ref: pinctrl.yaml#
> >  
> > +  - if:
> > +      not:
> > +        properties:
> > +          compatible:
> > +            contains:
> > +              enum:
> > +                - st,stm32mp257-pinctrl
> > +                - st,stm32mp257-z-pinctrl
> > +    then:
> > +      patternProperties:
> > +        '-[0-9]*$':
> > +          patternProperties:
> > +            '^pins':
> > +              properties:
> > +                skew-delay-input: false
> > +                skew-delay-output: false
> > +                st,io-sync: false
> > +
> >  required:
> >    - compatible
> >    - '#address-cells'
> > @@ -306,4 +383,25 @@ examples:
> >                  pinctrl-names = "default";
> >      };
> >  
> > +  - |
> > +    #include <dt-bindings/pinctrl/stm32-pinfunc.h>
> > +    //Example 4 skew-delay and st,io-sync
> > +      pinctrl: pinctrl@44240000 {
> > +              compatible = "st,stm32mp257-pinctrl";
> > +              #address-cells = <1>;
> > +              #size-cells = <1>;
> > +              ranges = <0 0x44240000 0xa0400>;
> > +
> > +              eth3_rgmii_pins_a: eth3-rgmii-0 {
> > +                      pins1 {
> > +                              pinmux = <STM32_PINMUX('A', 6, AF14)>;
> > +                              st,io-sync = <4>;
> > +                      };
> > +                      pins2 {
> > +                              pinmux = <STM32_PINMUX('H', 2, AF14)>;
> > +                              skew-delay-output = <2>;
> > +                      };
> > +              };
> > +      };
> > +
> >  ...
> > -- 
> > 2.34.1
> >