On Thu, 12 Oct 2023 16:01:54 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> 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.
Hi,
ok, no problem. I will drop it. I will also drop a similar property
from nxp,sc16is7xx.yaml which was added by me a few weeks ago (in a
separate patch, of course).
> >
> >
> >>
> >>> +
> >>> + 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.
Ok, I will drop examples 2 and 3.
Hugo.
