[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b4eca95eaa0e6f27fc07479d5eab2131d20eb270.camel@foss.st.com>
Date: Wed, 15 Oct 2025 14:56:56 +0200
From: Antonio Borneo <antonio.borneo@...s.st.com>
To: Conor Dooley <conor@...nel.org>
CC: Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue
<alexandre.torgue@...s.st.com>,
Bartosz Golaszewski <brgl@...ev.pl>, <linux-gpio@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-stm32@...md-mailman.stormreply.com>,
<linux-arm-kernel@...ts.infradead.org>,
Christophe Roullier
<christophe.roullier@...s.st.com>,
Fabien Dessenne
<fabien.dessenne@...s.st.com>,
Valentin Caron <valentin.caron@...s.st.com>
Subject: Re: [PATCH v3 09/10] dt-bindings: pinctrl: stm32: Support I/O
synchronization parameters
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@...s.st.com>
> > Signed-off-by: Fabien Dessenne <fabien.dessenne@...s.st.com>
> > Signed-off-by: Antonio Borneo <antonio.borneo@...s.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@...40000 {
> > + 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
> >
Powered by blists - more mailing lists