2022-10-19 14:58:10

by Amjad Ouled-Ameur

[permalink] [raw]
Subject: [PATCH v3 0/2] spi: amlogic: meson-spicc: Use pinctrl to drive CLK line when idle

Between SPI transactions, all SPI pins are in HiZ state. When using the SS
signal from the SPICC controller it's not an issue because when the
transaction resumes all pins come back to the right state at the same time
as SS.

The problem is when we use CS as a GPIO. In fact, between the GPIO CS
state change and SPI pins state change from idle, you can have a missing or
spurious clock transition.

Set a bias on the clock depending on the clock polarity requested before CS
goes active, by passing a special "idle-low" and "idle-high" pinctrl state
and setting the right state at a start of a message.

Signed-off-by: Amjad Ouled-Ameur <[email protected]>
---
Changes in v3:
- Fixed documentation by removing pinctrl states as they are not mandatory.
- Link to v2: https://lore.kernel.org/r/[email protected]

---
Amjad Ouled-Ameur (2):
spi: dt-bindings: amlogic, meson-gx-spicc: Add pinctrl names for SPI signal states
spi: meson-spicc: Use pinctrl to drive CLK line when idle

.../bindings/spi/amlogic,meson-gx-spicc.yaml | 67 ++++++++++++++--------
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 14 +++++
drivers/spi/spi-meson-spicc.c | 39 ++++++++++++-
3 files changed, 94 insertions(+), 26 deletions(-)
---
base-commit: aae703b02f92bde9264366c545e87cec451de471
change-id: 20221004-up-aml-fix-spi-c2bb7e78e603

Best regards,
--
Amjad Ouled-Ameur <[email protected]>


2022-10-19 15:06:08

by Amjad Ouled-Ameur

[permalink] [raw]
Subject: [PATCH v3 1/2] spi: dt-bindings: amlogic, meson-gx-spicc: Add pinctrl names for SPI signal states

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 <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
Signed-off-by: Amjad Ouled-Ameur <[email protected]>
---
.../bindings/spi/amlogic,meson-gx-spicc.yaml | 67 ++++++++++++++--------
1 file changed, 42 insertions(+), 25 deletions(-)

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

--
b4 0.10.1

2022-10-19 23:54:03

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] spi: dt-bindings: amlogic, meson-gx-spicc: Add pinctrl names for SPI signal states

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 <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>
> Signed-off-by: Amjad Ouled-Ameur <[email protected]>
> ---
> .../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.