Re: [PATCH v3 01/13] spi: dt-bindings: allow spi-max-frequency to specify a frequency pair

[Santhosh Kumar K]

Hello Conor,

On 28/05/26 23:06, Conor Dooley wrote:
> On Wed, May 27, 2026 at 11:25:15PM +0530, Santhosh Kumar K wrote:
> > Some SPI controllers support high-speed operating modes that require
> > controller-side configuration before the device can be driven at its
> > rated maximum frequency. In these cases two frequencies are relevant:
> > a conservative speed usable without any such configuration, and the
> > maximum speed achievable once the controller is set up accordingly.
> >
> > The existing spi-max-frequency property accepts only a single u32,
> > which cannot express this distinction. Extend it to accept either a
> > single value (retaining full backward compatibility) or a two-element
> > array [base-frequency, max-frequency], where base-frequency is the
> > conservative operating speed and max-frequency is the highest speed
> > the device supports after controller-side configuration.
> >
> > Signed-off-by: Santhosh Kumar K <s-k6@xxxxxx>
> > ---
> >   .../devicetree/bindings/spi/spi-peripheral-props.yaml  | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
>
> Pretty sure this hasn't been tested, dt_binding_check cannot even build
> processed-schema.json with this applied because there are multiple
> definitions of spi-max-frequency with it applied.
>
> The sashiko makes the point that this breaks every binding that uses
> minimum/maximum to set constraints too, because these properties do not
> apply to arrays unless applied per item.

Thanks for catching this. I missed dtbs_check before posting, my bad!

I understand the issue here, once the property type is changed to an
array, the minimum and maximum constraints are meaningless as they're
item-level constraints.

> I also don't get the point of this property, why can't you just set the
> max that the device can do and if the controller can configure itself to
> be fast enough it will do so, and if it can't then it'll pick whatever
> the fastest it can actually do instead?
> Seems like you're abusing a peripheral property to encode information
> about the controller.

The controller-side approach you mentioned is similar to what I had in
v2, where a compatible-specific base_freq is used for non-PHY ops.

Miquel,

I think we should revert to the v2 approach.

The non-PHY frequency is a controller limitation/capability rather than
a flash characteristic, so it seems more appropriate to keep it in the
controller driver as Conor suggested.

Regards,
Santhosh.

> pw-bot: changes-requested
>
> Thanks,
> Conor.
>
> > diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > index 880a9f624566..c88f6f3a1801 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > @@ -41,9 +41,15 @@ properties:
> >         The device requires the LSB first mode.
> >   
> >     spi-max-frequency:
> > -    $ref: /schemas/types.yaml#/definitions/uint32
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    minItems: 1
> > +    maxItems: 2
> >       description:
> > -      Maximum SPI clocking speed of the device in Hz.
> > +      SPI clocking speed of the device in Hz. Either a single maximum
> > +      frequency, or two values [base-frequency, max-frequency] where
> > +      base-frequency is the conservative speed and max-frequency is the
> > +      highest speed the device supports after controller-side configurations
> > +      such as data training.
> >   
> >     spi-cs-setup-delay-ns:
> >       description:
> > --
> > 2.34.1