| Message ID | 20240428070258.4121-1-five231003@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] spi: dt-bindings: ti,qspi: convert to dtschema | expand |
On 28/04/2024 08:58, Kousik Sanagavarapu wrote: > Convert txt binding of TI's qspi controller (found on their omap SoCs) to > dtschema to allow for validation. > > It is however to be noted that it is not a one-to-one conversion, in the > sense that the original txt binding needed to be updated, but these > changes are included in the dtschema and are mentioned below. Drop this paragraph. > > The changes, w.r.t. the original txt binding, are: You also dropped qspi_ctrlmod during conversion. > > - Introduce "clocks" and "clock-names" which was never mentioned. > - Reflect that "ti,hwmods" is deprecated and is not a "required" > property anymore. > - Introduce "num-cs" which allows for setting the number of chip > selects. > > Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com> > --- > I'm a bit iffy about this one as I guess the original txt binding failed > to cover some things about the properties. I added the properties based > on their use in the *.dtsi files when I grepped for the compatible string > > arch/arm/boot/dts/ti/omap/dra7.dtsi > arch/arm/boot/dts/ti/omap/am4372.dtsi > > I also looked at the probe function in the driver for it, which can be > found at > > drivers/spi/spi-ti-qspi.c > > .../devicetree/bindings/spi/ti,qspi.yaml | 94 +++++++++++++++++++ > .../devicetree/bindings/spi/ti_qspi.txt | 53 ----------- > 2 files changed, 94 insertions(+), 53 deletions(-) > create mode 100644 Documentation/devicetree/bindings/spi/ti,qspi.yaml > delete mode 100644 Documentation/devicetree/bindings/spi/ti_qspi.txt > > diff --git a/Documentation/devicetree/bindings/spi/ti,qspi.yaml b/Documentation/devicetree/bindings/spi/ti,qspi.yaml > new file mode 100644 > index 000000000000..77cabd7158f5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/ti,qspi.yaml > @@ -0,0 +1,94 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/spi/ti,qspi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TI QSPI controller > + > +maintainers: > + - Kousik Sanagavarapu <five231003@gmail.com> > + > +allOf: > + - $ref: spi-controller.yaml# > + > +properties: > + compatible: > + enum: > + - ti,am4372-qspi > + - ti,dra7xxx-qspi > + > + reg: > + items: > + - description: base registers > + - description: mapped memory > + > + reg-names: > + items: > + - const: qspi_base > + - const: qspi_mmap > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: fck > + > + interrupts: > + maxItems: 1 > + > + num-cs: > + maxItems: 1 > + > + ti,hwmods: > + description: > + Name of the hwmod associated to the QSPI. This is for legacy > + platforms only. > + $ref: /schemas/types.yaml#/definitions/string > + deprecated: true > + > + syscon-chipselects: > + description: > + Handle to system control region contains QSPI chipselect register > + and offset of that register. > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + items: > + - description: phandle to system control register > + - description: register offset > + > + spi-max-frequency: > + description: Maximum SPI clocking speed of the controller in Hz. > + $ref: /schemas/types.yaml#/definitions/uint32 Are you sure that's actually needed? That's not a property of controller. > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - clock-names > + - interrupts > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/dra7.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + qspi: spi@0 { > + compatible = "ti,dra7xxx-qspi"; > + reg = <0x4b300000 0x100>, > + <0x5c000000 0x4000000>; > + reg-names = "qspi_base", "qspi_mmap"; > + syscon-chipselects = <&scm_conf 0x558>; > + #address-cells = <1>; > + #size-cells = <0>; > + clocks = <&l4per2_clkctrl DRA7_L4PER2_QSPI_CLKCTRL 25>; > + clock-names = "fck"; > + num-cs = <4>; > + spi-max-frequency = <48000000>; Drop. Are you sure driver parses it? > + interrupts = <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>; > + }; Best regards, Krzysztof
On Mon, Apr 29, 2024 at 07:07:38AM +0200, Krzysztof Kozlowski wrote: > On 28/04/2024 08:58, Kousik Sanagavarapu wrote: > > Convert txt binding of TI's qspi controller (found on their omap SoCs) to > > dtschema to allow for validation. [...] > > + spi-max-frequency: > > + description: Maximum SPI clocking speed of the controller in Hz. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > Are you sure that's actually needed? That's not a property of controller. [...] > > + num-cs = <4>; > > + spi-max-frequency = <48000000>; > > Drop. Are you sure driver parses it? The driver does parse it though. Looking at drivers/spi/spi-ti-qspi.c::ti_qspi_probe(), if (!of_property_read_u32(np, "spi-max-frequency", &max_freq)) host->max_speed_hz = max_freq; So I included it in the dtschema as well. Please let me know if including it in the dtschema in this case is wrong. Thanks
On 29/04/2024 09:10, Kousik Sanagavarapu wrote: > On Mon, Apr 29, 2024 at 07:07:38AM +0200, Krzysztof Kozlowski wrote: >> On 28/04/2024 08:58, Kousik Sanagavarapu wrote: >>> Convert txt binding of TI's qspi controller (found on their omap SoCs) to >>> dtschema to allow for validation. > > [...] > >>> + spi-max-frequency: >>> + description: Maximum SPI clocking speed of the controller in Hz. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >> >> >> Are you sure that's actually needed? That's not a property of controller. > > [...] > >>> + num-cs = <4>; >>> + spi-max-frequency = <48000000>; >> >> Drop. Are you sure driver parses it? > > The driver does parse it though. Looking at > drivers/spi/spi-ti-qspi.c::ti_qspi_probe(), > > if (!of_property_read_u32(np, "spi-max-frequency", &max_freq)) > host->max_speed_hz = max_freq; > > So I included it in the dtschema as well. Please let me know if > including it in the dtschema in this case is wrong. Ah, indeed. It is fine there. Keep it in the binidng and in the example. Best regards, Krzysztof
On Sun, Apr 28, 2024 at 12:28:59PM +0530, Kousik Sanagavarapu wrote: > Convert txt binding of TI's qspi controller (found on their omap SoCs) to > dtschema to allow for validation. > > It is however to be noted that it is not a one-to-one conversion, in the > sense that the original txt binding needed to be updated, but these > changes are included in the dtschema and are mentioned below. > > The changes, w.r.t. the original txt binding, are: > > - Introduce "clocks" and "clock-names" which was never mentioned. > - Reflect that "ti,hwmods" is deprecated and is not a "required" > property anymore. > - Introduce "num-cs" which allows for setting the number of chip > selects. > > Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com> > --- > I'm a bit iffy about this one as I guess the original txt binding failed > to cover some things about the properties. I added the properties based > on their use in the *.dtsi files when I grepped for the compatible string > > arch/arm/boot/dts/ti/omap/dra7.dtsi > arch/arm/boot/dts/ti/omap/am4372.dtsi > > I also looked at the probe function in the driver for it, which can be > found at > > drivers/spi/spi-ti-qspi.c > > .../devicetree/bindings/spi/ti,qspi.yaml | 94 +++++++++++++++++++ > .../devicetree/bindings/spi/ti_qspi.txt | 53 ----------- > 2 files changed, 94 insertions(+), 53 deletions(-) > create mode 100644 Documentation/devicetree/bindings/spi/ti,qspi.yaml > delete mode 100644 Documentation/devicetree/bindings/spi/ti_qspi.txt > > diff --git a/Documentation/devicetree/bindings/spi/ti,qspi.yaml b/Documentation/devicetree/bindings/spi/ti,qspi.yaml > new file mode 100644 > index 000000000000..77cabd7158f5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/ti,qspi.yaml > @@ -0,0 +1,94 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/spi/ti,qspi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TI QSPI controller > + > +maintainers: > + - Kousik Sanagavarapu <five231003@gmail.com> > + > +allOf: > + - $ref: spi-controller.yaml# > + > +properties: > + compatible: > + enum: > + - ti,am4372-qspi > + - ti,dra7xxx-qspi > + > + reg: > + items: > + - description: base registers > + - description: mapped memory > + > + reg-names: > + items: > + - const: qspi_base > + - const: qspi_mmap > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: fck > + > + interrupts: > + maxItems: 1 > + > + num-cs: > + maxItems: 1 This is not an array, so maxItems is not appropriate. If there are only certain values supported (default is up to 2^32), then you should define them with enum or maximum. > + > + ti,hwmods: > + description: > + Name of the hwmod associated to the QSPI. This is for legacy > + platforms only. > + $ref: /schemas/types.yaml#/definitions/string > + deprecated: true > + > + syscon-chipselects: > + description: > + Handle to system control region contains QSPI chipselect register > + and offset of that register. > + $ref: /schemas/types.yaml#/definitions/phandle-array > + items: > + items: > + - description: phandle to system control register > + - description: register offset This allows any number of phandle+offset entries. Is that what you want? If not, you need either '- items' for the 2nd 'items' or 'maxItems: 1'. > + > + spi-max-frequency: > + description: Maximum SPI clocking speed of the controller in Hz. > + $ref: /schemas/types.yaml#/definitions/uint32 > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - clock-names > + - interrupts > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/dra7.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + qspi: spi@0 { Drop unused labels. Unit-address is wrong. > + compatible = "ti,dra7xxx-qspi"; > + reg = <0x4b300000 0x100>, > + <0x5c000000 0x4000000>; > + reg-names = "qspi_base", "qspi_mmap"; > + syscon-chipselects = <&scm_conf 0x558>; > + #address-cells = <1>; > + #size-cells = <0>; > + clocks = <&l4per2_clkctrl DRA7_L4PER2_QSPI_CLKCTRL 25>; > + clock-names = "fck"; > + num-cs = <4>; > + spi-max-frequency = <48000000>; > + interrupts = <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>; > + }; > +...
diff --git a/Documentation/devicetree/bindings/spi/ti,qspi.yaml b/Documentation/devicetree/bindings/spi/ti,qspi.yaml new file mode 100644 index 000000000000..77cabd7158f5 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/ti,qspi.yaml @@ -0,0 +1,94 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/ti,qspi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI QSPI controller + +maintainers: + - Kousik Sanagavarapu <five231003@gmail.com> + +allOf: + - $ref: spi-controller.yaml# + +properties: + compatible: + enum: + - ti,am4372-qspi + - ti,dra7xxx-qspi + + reg: + items: + - description: base registers + - description: mapped memory + + reg-names: + items: + - const: qspi_base + - const: qspi_mmap + + clocks: + maxItems: 1 + + clock-names: + items: + - const: fck + + interrupts: + maxItems: 1 + + num-cs: + maxItems: 1 + + ti,hwmods: + description: + Name of the hwmod associated to the QSPI. This is for legacy + platforms only. + $ref: /schemas/types.yaml#/definitions/string + deprecated: true + + syscon-chipselects: + description: + Handle to system control region contains QSPI chipselect register + and offset of that register. + $ref: /schemas/types.yaml#/definitions/phandle-array + items: + items: + - description: phandle to system control register + - description: register offset + + spi-max-frequency: + description: Maximum SPI clocking speed of the controller in Hz. + $ref: /schemas/types.yaml#/definitions/uint32 + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - interrupts + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/dra7.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + qspi: spi@0 { + compatible = "ti,dra7xxx-qspi"; + reg = <0x4b300000 0x100>, + <0x5c000000 0x4000000>; + reg-names = "qspi_base", "qspi_mmap"; + syscon-chipselects = <&scm_conf 0x558>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&l4per2_clkctrl DRA7_L4PER2_QSPI_CLKCTRL 25>; + clock-names = "fck"; + num-cs = <4>; + spi-max-frequency = <48000000>; + interrupts = <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>; + }; +... diff --git a/Documentation/devicetree/bindings/spi/ti_qspi.txt b/Documentation/devicetree/bindings/spi/ti_qspi.txt deleted file mode 100644 index 47b184bce414..000000000000 --- a/Documentation/devicetree/bindings/spi/ti_qspi.txt +++ /dev/null @@ -1,53 +0,0 @@ -TI QSPI controller. - -Required properties: -- compatible : should be "ti,dra7xxx-qspi" or "ti,am4372-qspi". -- reg: Should contain QSPI registers location and length. -- reg-names: Should contain the resource reg names. - - qspi_base: Qspi configuration register Address space - - qspi_mmap: Memory mapped Address space - - (optional) qspi_ctrlmod: Control module Address space -- interrupts: should contain the qspi interrupt number. -- #address-cells, #size-cells : Must be present if the device has sub-nodes -- ti,hwmods: Name of the hwmod associated to the QSPI - -Recommended properties: -- spi-max-frequency: Definition as per - Documentation/devicetree/bindings/spi/spi-bus.txt - -Optional properties: -- syscon-chipselects: Handle to system control region contains QSPI - chipselect register and offset of that register. - -NOTE: TI QSPI controller requires different pinmux and IODelay -parameters for Mode-0 and Mode-3 operations, which needs to be set up by -the bootloader (U-Boot). Default configuration only supports Mode-0 -operation. Hence, "spi-cpol" and "spi-cpha" DT properties cannot be -specified in the slave nodes of TI QSPI controller without appropriate -modification to bootloader. - -Example: - -For am4372: -qspi: qspi@47900000 { - compatible = "ti,am4372-qspi"; - reg = <0x47900000 0x100>, <0x30000000 0x4000000>; - reg-names = "qspi_base", "qspi_mmap"; - #address-cells = <1>; - #size-cells = <0>; - spi-max-frequency = <25000000>; - ti,hwmods = "qspi"; -}; - -For dra7xx: -qspi: qspi@4b300000 { - compatible = "ti,dra7xxx-qspi"; - reg = <0x4b300000 0x100>, - <0x5c000000 0x4000000>, - reg-names = "qspi_base", "qspi_mmap"; - syscon-chipselects = <&scm_conf 0x558>; - #address-cells = <1>; - #size-cells = <0>; - spi-max-frequency = <48000000>; - ti,hwmods = "qspi"; -};
Convert txt binding of TI's qspi controller (found on their omap SoCs) to dtschema to allow for validation. It is however to be noted that it is not a one-to-one conversion, in the sense that the original txt binding needed to be updated, but these changes are included in the dtschema and are mentioned below. The changes, w.r.t. the original txt binding, are: - Introduce "clocks" and "clock-names" which was never mentioned. - Reflect that "ti,hwmods" is deprecated and is not a "required" property anymore. - Introduce "num-cs" which allows for setting the number of chip selects. Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com> --- I'm a bit iffy about this one as I guess the original txt binding failed to cover some things about the properties. I added the properties based on their use in the *.dtsi files when I grepped for the compatible string arch/arm/boot/dts/ti/omap/dra7.dtsi arch/arm/boot/dts/ti/omap/am4372.dtsi I also looked at the probe function in the driver for it, which can be found at drivers/spi/spi-ti-qspi.c .../devicetree/bindings/spi/ti,qspi.yaml | 94 +++++++++++++++++++ .../devicetree/bindings/spi/ti_qspi.txt | 53 ----------- 2 files changed, 94 insertions(+), 53 deletions(-) create mode 100644 Documentation/devicetree/bindings/spi/ti,qspi.yaml delete mode 100644 Documentation/devicetree/bindings/spi/ti_qspi.txt