On 12/10/2023 15:59, Hugo Villeneuve wrote:
>>> + clock-frequency:
>>> + description:
>>> + When there is no clock provider visible to the platform, this
>>> + is the source crystal frequency for the IC in Hz.
>>> + minimum: 1000000
>>> + maximum: 4000000
>>
>> This wasn't in original binding. Explain this in the commit msg.
>
> I will add the corresponding explanation in V2.
>
> The 'clock-frequency' property is already supported
> by the driver but was not documented in the original txt binding.
>
> This is related to the commit d4d6f03c4fb3: serial: max310x: Try to
> get crystal clock rate from property (Author: Andy Shevchenko):
>
> In some configurations, mainly ACPI-based, the clock frequency of
> the device is supplied by very well established 'clock-frequency'
> property. Hence, try to get it from the property at last if no other
> providers are available.
This does not justify adding it to bindings. ACPI stuff stays with ACPI.
Drop it.
>
>
>>
>>> +
>>> + clock-names:
>>> + enum:
>>> + - xtal # External crystal
>>> + - osc # External clock source
>>
>> clock-names follow immediately clocks.
>
> Will fix for V2.
>
>
>>
>>> +
>>> + gpio-controller: true
>>> +
>>> + "#gpio-cells":
>>> + const: 2
>>> +
>>> + gpio-line-names:
>>> + minItems: 1
>>> + maxItems: 16
>>> +
>>> +allOf:
>>
>> allOf: block goes after required: block.
>
> Will fix for V2.
>
>
>>
>>> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>> + - $ref: /schemas/serial/serial.yaml#
>>> + - $ref: /schemas/serial/rs485.yaml#
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - interrupts
>>> +
>>> +oneOf:
>>> + - required:
>>> + - clocks
>>> + - clock-names
>>> + - required:
>>> + - clock-frequency
>>
>> That's also something new as well. The original binding required clocks.
>> Why are you changing this?
>
> See explanation above about clock-frequency.
>
> If clocks is not provided, than at least 'clock-frequency' must be
> provided.
>
>
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>> + i2c {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + serial@2c {
>>> + compatible = "maxim,max3107";
>>> + reg = <0x2c>;
>>> + clocks = <&xtal4m>;
>>> + clock-names = "xtal";
>>> + interrupt-parent = <&gpio3>;
>>> + interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
>>> + gpio-controller;
>>> + #gpio-cells = <2>;
>>> + };
>>> +
>>
>> One example is enuogh. All other are the same.
>
> Not really, clock-names is different for example 2 (osc), and example 3
> shows that 'clock-frequency' can be used if no clock is provided?
Difference by one property does not really justify new example. The
third case - clock frequency - is okay, but the property is going away.
Best regards,
Krzysztof
