| Message ID | 20221004-up-aml-fix-spi-v3-1-89de126fd163@baylibre.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: amlogic: meson-spicc: Use pinctrl to drive CLK line when idle | expand |
On Wed, 19 Oct 2022 16:01:03 +0200, Amjad Ouled-Ameur wrote: > SPI pins of the SPICC Controller in Meson-GX needs to be controlled by > pin biais when idle. Therefore define three pinctrl names: > - default: SPI pins are controlled by spi function. > - idle-high: SCLK pin is pulled-up, but MOSI/MISO are still controlled > by spi function. > - idle-low: SCLK pin is pulled-down, but MOSI/MISO are still controlled > by spi function. > > Reported-by: Da Xue <da@libre.computer> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > Signed-off-by: Amjad Ouled-Ameur <aouledameur@baylibre.com> > --- > .../bindings/spi/amlogic,meson-gx-spicc.yaml | 67 ++++++++++++++-------- > 1 file changed, 42 insertions(+), 25 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.yaml:46:1: [error] duplication of key "allOf" in mapping (key-duplicates) dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.yaml:46:1: found duplicate key "allOf" with value "[]" (original value: "[]") /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.yaml: ignoring, error parsing file make[1]: *** Deleting file 'Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.example.dts' Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.yaml:46:1: found duplicate key "allOf" with value "[]" (original value: "[]") make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1492: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
>>> + properties: >>> + pinctrl-names: >>> + minItems: 1 >>> + items: >>> + - const: default >>> + - const: idle-high >>> + - const: idle-low >> >> You should also define in such case pinctrl-0 and others. > > Ok I thought it would be covered by the pinctrl-consumer.yaml > but yeah we should allow pinctrl-1 and pinctrl-2 here aswell by adding: > > pinctrl-1: true > pinctrl-2: true > > Yes. Best regards, Krzysztof
Hi On 10/20/22 14:49, Krzysztof Kozlowski wrote: >>>> + properties: >>>> + pinctrl-names: >>>> + minItems: 1 >>>> + items: >>>> + - const: default >>>> + - const: idle-high >>>> + - const: idle-low >>> You should also define in such case pinctrl-0 and others. >> Ok I thought it would be covered by the pinctrl-consumer.yaml >> but yeah we should allow pinctrl-1 and pinctrl-2 here aswell by adding: >> >> pinctrl-1: true >> pinctrl-2: true >> >> In such case, should I define pinctrl- as part of the if statement, as shown below, or before allOf ? [...] - if: properties: compatible: contains: enum: - amlogic,meson-gx-spicc then: properties: pinctrl-0: true pinctrl-1: true pinctrl-2: true pinctrl-names: minItems: 1 items: - const: default - const: idle-high - const: idle-low [...] Regards Amjad > Yes. > > Best regards, > Krzysztof >
On 21/10/2022 08:54, Amjad Ouled-Ameur wrote: > Hi > > On 10/20/22 14:49, Krzysztof Kozlowski wrote: >>>>> + properties: >>>>> + pinctrl-names: >>>>> + minItems: 1 >>>>> + items: >>>>> + - const: default >>>>> + - const: idle-high >>>>> + - const: idle-low >>>> You should also define in such case pinctrl-0 and others. >>> Ok I thought it would be covered by the pinctrl-consumer.yaml >>> but yeah we should allow pinctrl-1 and pinctrl-2 here aswell by adding: >>> >>> pinctrl-1: true >>> pinctrl-2: true >>> >>> > In such case, should I define pinctrl- as part of the if statement, as shown below, > > or before allOf ? The same as pinctrl-names, so part of allOf. > > [...] > > - if: > properties: > compatible: > contains: > enum: > - amlogic,meson-gx-spicc > > then: > properties: > pinctrl-0: true > pinctrl-1: true > pinctrl-2: true > > pinctrl-names: > minItems: 1 > items: > - const: default > - const: idle-high > - const: idle-low Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.yaml b/Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.yaml index 0c10f7678178..3e47fe7760a8 100644 --- a/Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.yaml +++ b/Documentation/devicetree/bindings/spi/amlogic,meson-gx-spicc.yaml @@ -43,31 +43,48 @@ properties: minItems: 1 maxItems: 2 -if: - properties: - compatible: - contains: - enum: - - amlogic,meson-g12a-spicc - -then: - properties: - clocks: - minItems: 2 - - clock-names: - items: - - const: core - - const: pclk - -else: - properties: - clocks: - maxItems: 1 - - clock-names: - items: - - const: core +allOf: + - if: + properties: + compatible: + contains: + enum: + - amlogic,meson-g12a-spicc + + then: + properties: + clocks: + minItems: 2 + + clock-names: + items: + - const: core + - const: pclk + + else: + properties: + clocks: + maxItems: 1 + + clock-names: + items: + - const: core + + - if: + properties: + compatible: + contains: + enum: + - amlogic,meson-gx-spicc + + then: + properties: + pinctrl-names: + minItems: 1 + items: + - const: default + - const: idle-high + - const: idle-low required: - compatible