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

[Santhosh Kumar K <s-k6@ti.com>]

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@ti.com>
>> ---
>>   .../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
>>