| Message ID | 20231011222105.2587175-1-hugo@hugovil.com |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: max310x: convert to YAML | expand |
On Thu, 12 Oct 2023 09:53:02 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 12/10/2023 00:21, Hugo Villeneuve wrote: > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > Convert binding from text format to YAML. > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. Missing prefix: serial: Hi, will add it for V2. > > > > Additions to original text binding: > > - add rs485 reference. > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > --- > > > > diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml > > new file mode 100644 > > index 000000000000..05fd00d95260 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml > > @@ -0,0 +1,107 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/serial/maxim,max310x.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Maxim MAX310X Advanced Universal Asynchronous Receiver-Transmitter (UART) > > + > > +maintainers: > > + - Hugo Villeneuve <hvilleneuve@dimonoff.com> > > + > > +properties: > > + compatible: > > + enum: > > + - maxim,max3107 > > + - maxim,max3108 > > + - maxim,max3109 > > + - maxim,max14830 > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + 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. > > > + > > + 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? Hugo.
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
On Thu, 12 Oct 2023 16:01:54 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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.
diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.txt b/Documentation/devicetree/bindings/serial/maxim,max310x.txt deleted file mode 100644 index 79e10a05a96a..000000000000 --- a/Documentation/devicetree/bindings/serial/maxim,max310x.txt +++ /dev/null @@ -1,48 +0,0 @@ -* Maxim MAX310X advanced Universal Asynchronous Receiver-Transmitter (UART) - -Required properties: -- compatible: Should be one of the following: - - "maxim,max3107" for Maxim MAX3107, - - "maxim,max3108" for Maxim MAX3108, - - "maxim,max3109" for Maxim MAX3109, - - "maxim,max14830" for Maxim MAX14830. -- reg: SPI chip select number. -- interrupts: Specifies the interrupt source of the parent interrupt - controller. The format of the interrupt specifier depends on the - parent interrupt controller. -- clocks: phandle to the IC source clock. -- clock-names: Should be "xtal" if clock is an external crystal or - "osc" if an external clock source is used. - -Optional properties: -- gpio-controller: Marks the device node as a GPIO controller. -- #gpio-cells: Should be two. The first cell is the GPIO number and - the second cell is used to specify the GPIO polarity: - 0 = active high, - 1 = active low. - -Example: - -/ { - clocks { - spi_uart_clk: osc_max14830 { - compatible = "fixed-clock"; - #clock-cells = <0>; - clock-frequency = <3686400>; - }; - - }; -}; - -&spi0 { - max14830: max14830@0 { - compatible = "maxim,max14830"; - reg = <0>; - clocks = <&spi_uart_clk>; - clock-names = "osc"; - interrupt-parent = <&gpio3>; - interrupts = <7 IRQ_TYPE_LEVEL_LOW>; - gpio-controller; - #gpio-cells = <2>; - }; -}; diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml new file mode 100644 index 000000000000..05fd00d95260 --- /dev/null +++ b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml @@ -0,0 +1,107 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/serial/maxim,max310x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Maxim MAX310X Advanced Universal Asynchronous Receiver-Transmitter (UART) + +maintainers: + - Hugo Villeneuve <hvilleneuve@dimonoff.com> + +properties: + compatible: + enum: + - maxim,max3107 + - maxim,max3108 + - maxim,max3109 + - maxim,max14830 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + + 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 + + clock-names: + enum: + - xtal # External crystal + - osc # External clock source + + gpio-controller: true + + "#gpio-cells": + const: 2 + + gpio-line-names: + minItems: 1 + maxItems: 16 + +allOf: + - $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 + +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>; + }; + + serial@60 { + compatible = "maxim,max3108"; + reg = <0x60>; + clocks = <&clk20m>; + clock-names = "osc"; + interrupt-parent = <&gpio3>; + interrupts = <7 IRQ_TYPE_LEVEL_LOW>; + gpio-controller; + #gpio-cells = <2>; + }; + + serial@6f { + compatible = "maxim,max14830"; + reg = <0x6f>; + clock-frequency = <3000000>; + interrupt-parent = <&gpio3>; + interrupts = <7 IRQ_TYPE_LEVEL_LOW>; + gpio-controller; + #gpio-cells = <2>; + }; + };