On 04/01/2022 17:47, Rob Herring wrote: > On Sun, Jan 02, 2022 at 04:58:07PM +0100, Krzysztof Kozlowski wrote: >> Convert the Samsung SoC (S3C24xx, S3C64xx, S5Pv210, Exynos) SPI >> controller bindings to DT schema format >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx> >> --- >> .../devicetree/bindings/spi/samsung,spi.yaml | 220 ++++++++++++++++++ >> .../devicetree/bindings/spi/spi-samsung.txt | 122 ---------- >> MAINTAINERS | 2 +- >> 3 files changed, 221 insertions(+), 123 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/spi/samsung,spi.yaml >> delete mode 100644 Documentation/devicetree/bindings/spi/spi-samsung.txt >> >> diff --git a/Documentation/devicetree/bindings/spi/samsung,spi.yaml b/Documentation/devicetree/bindings/spi/samsung,spi.yaml >> new file mode 100644 >> index 000000000000..195bfafe05fd >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/spi/samsung,spi.yaml >> @@ -0,0 +1,220 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/spi/samsung,spi.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Samsung S3C/S5P/Exynos SoC SPI controller >> + >> +maintainers: >> + - Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx> >> + >> +description: >> + All the SPI controller nodes should be represented in the aliases node using >> + the following format 'spi{n}' where n is a unique number for the alias. >> + >> +properties: >> + compatible: >> + oneOf: >> + - enum: >> + - samsung,s3c2443-spi >> + - samsung,s3c6410-spi >> + - samsung,s5pv210-spi >> + - samsung,exynos5433-spi >> + - const: samsung,exynos7-spi >> + deprecated: true >> + >> + clocks: >> + minItems: 2 >> + maxItems: 3 >> + >> + clock-names: >> + minItems: 2 >> + maxItems: 3 >> + >> + cs-gpios: true >> + >> + dmas: >> + minItems: 2 >> + maxItems: 2 >> + >> + dma-names: >> + items: >> + - const: tx >> + - const: rx >> + >> + interrupts: >> + maxItems: 1 >> + >> + no-cs-readback: >> + description: >> + The CS line is disconnected, therefore the device should not operate >> + based on CS signalling. >> + type: boolean >> + >> + num-cs: >> + minimum: 1 >> + maximum: 4 >> + default: 1 >> + >> + samsung,spi-src-clk: >> + description: >> + If the spi controller includes a internal clock mux to select the clock >> + source for the spi bus clock, this property can be used to indicate the >> + clock to be used for driving the spi bus clock. If not specified, the >> + clock number 0 is used as default. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + default: 0 >> + >> + reg: >> + maxItems: 1 >> + >> +patternProperties: >> + "^.*@[0-9a-f]+$": >> + type: object >> + >> + allOf: >> + - $ref: spi-peripheral-props.yaml >> + >> + properties: >> + controller-data: >> + type: object >> + additionalProperties: false >> + >> + properties: >> + samsung,spi-feedback-delay: > > All this has to go in it's own schema which then needs to be added to > spi-peripheral-props.yaml. OK > >> + description: | >> + The sampling phase shift to be applied on the miso line (to account >> + for any lag in the miso line). Valid values: >> + - 0: No phase shift. >> + - 1: 90 degree phase shift sampling. >> + - 2: 180 degree phase shift sampling. >> + - 3: 270 degree phase shift sampling. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + enum: [0, 1, 2, 3] >> + >> + required: >> + - samsung,spi-feedback-delay >> + >> + required: >> + - controller-data > > Not really great that this is required and there's not some default that > works. I can make it optional with new kernel. I'll send a follow up patch for this. > >> + >> + unevaluatedProperties: false >> + >> +required: >> + - compatible >> + - clocks >> + - clock-names >> + - dmas >> + - dma-names >> + - interrupts >> + - reg >> + >> +allOf: >> + - $ref: spi-controller.yaml# >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: samsung,exynos5433-spi >> + then: >> + properties: >> + clocks: >> + minItems: 3 >> + maxItems: 3 >> + clock-names: >> + items: >> + - const: spi >> + - enum: >> + - spi_busclk0 >> + - spi_busclk1 >> + - spi_busclk2 >> + - spi_busclk3 >> + - const: spi_ioclk >> + else: >> + properties: >> + clocks: >> + minItems: 2 >> + maxItems: 2 >> + clock-names: >> + items: >> + - const: spi >> + - enum: >> + - spi_busclk0 >> + - spi_busclk1 >> + - spi_busclk2 >> + - spi_busclk3 >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/exynos5433.h> >> + #include <dt-bindings/clock/samsung,s2mps11.h> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/gpio/gpio.h> >> + >> + spi@14d30000 { >> + compatible = "samsung,exynos5433-spi"; >> + reg = <0x14d30000 0x100>; >> + interrupts = <GIC_SPI 433 IRQ_TYPE_LEVEL_HIGH>; >> + dmas = <&pdma0 11>, <&pdma0 10>; >> + dma-names = "tx", "rx"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + clocks = <&cmu_peric CLK_PCLK_SPI1>, >> + <&cmu_peric CLK_SCLK_SPI1>, >> + <&cmu_peric CLK_SCLK_IOCLK_SPI1>; >> + clock-names = "spi", >> + "spi_busclk0", >> + "spi_ioclk"; >> + samsung,spi-src-clk = <0>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&spi1_bus>; >> + num-cs = <1>; >> + >> + cs-gpios = <&gpd6 3 GPIO_ACTIVE_HIGH>; >> + status = "okay"; > > Don't need 'status' in examples. Sure > >> + audio-codec@0 { >> + compatible = "wlf,wm5110"; >> + reg = <0x0>; >> + spi-max-frequency = <20000000>; >> + interrupt-parent = <&gpa0>; >> + interrupts = <4 IRQ_TYPE_NONE>; >> + clocks = <&pmu_system_controller 0>, >> + <&s2mps13_osc S2MPS11_CLK_BT>; >> + clock-names = "mclk1", "mclk2"; >> + >> + gpio-controller; >> + #gpio-cells = <2>; >> + interrupt-controller; >> + #interrupt-cells = <2>; >> + >> + wlf,micd-detect-debounce = <300>; >> + wlf,micd-bias-start-time = <0x1>; >> + wlf,micd-rate = <0x7>; >> + wlf,micd-dbtime = <0x2>; >> + wlf,micd-force-micbias; >> + wlf,micd-configs = <0x0 1 0>; >> + wlf,hpdet-channel = <1>; >> + wlf,gpsw = <0x1>; >> + wlf,inmode = <2 0 2 0>; >> + >> + wlf,reset = <&gpc0 7 GPIO_ACTIVE_HIGH>; >> + wlf,ldoena = <&gpf0 0 GPIO_ACTIVE_HIGH>; >> + >> + /* core supplies */ >> + AVDD-supply = <&ldo18_reg>; >> + DBVDD1-supply = <&ldo18_reg>; >> + CPVDD-supply = <&ldo18_reg>; >> + DBVDD2-supply = <&ldo18_reg>; >> + DBVDD3-supply = <&ldo18_reg>; >> + SPKVDDL-supply = <&ldo18_reg>; >> + SPKVDDR-supply = <&ldo18_reg>; >> + >> + controller-data { > > The schema didn't get checked since spi-peripheral-props.yaml isn't > yet in my tree, but I'd expect this to be a warning. If not its schema > needs to be a bit stricter. The schema for "wlf,wm5110" will need to > reference spi-peripheral-props.yaml. (Actually, all SPI peripheral > device schemas will need a reference in theory, but really we only care > for devices used with controllers with extra properties). I did not see any warnings. I'll modify the wm5110. Best regards, Krzysztof