diff mbox series

[1/7] dt-bindings: spi: zynqmp-qspi: Split the bus

Message ID 20250116232118.2694169-2-sean.anderson@linux.dev
State New
Headers show
Series spi: zynqmp-gqspi: Split the bus and add GPIO support | expand

Commit Message

Sean Anderson Jan. 16, 2025, 11:21 p.m. UTC
This device supports two separate SPI busses: "lower" (SPI0) and "upper"
(SPI1). Each SPI bus has separate clock and data lines, as well as a
hardware-controlled chip select. The current binding does not model this
situation. It exposes one bus, where CS 0 uses the lower bus and the
lower chip select, and CS 1 uses the upper bus and the upper chip
select. It is not possible to use the upper chip select with the lower
bus (or vice versa). GPIO chip selects are unsupported, and there would
be no way to specify which bus to use if they were.

Split the "merged" bus into an upper and lower bus, each with their own
subnodes.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 .../bindings/spi/spi-zynqmp-qspi.yaml         | 43 +++++++++++++++++--
 1 file changed, 40 insertions(+), 3 deletions(-)

Comments

David Lechner Jan. 22, 2025, 12:16 a.m. UTC | #1
On 1/16/25 5:21 PM, Sean Anderson wrote:
> This device supports two separate SPI busses: 

...

> @@ -84,5 +94,32 @@ examples:
>          resets = <&zynqmp_reset ZYNQMP_RESET_QSPI>;
>          reg = <0x0 0xff0f0000 0x0 0x1000>,
>                <0x0 0xc0000000 0x0 0x8000000>;
> +
> +        spi-lower {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +          num-cs = <2>;
> +          cs-gpios = <0>, <&gpio 5>;
> +
> +          flash@0 {
> +            reg = <0>;
> +            compatible = "jedec,spi-nor";
> +          };
> +
> +          flash@1 {
> +            reg = <1>;
> +            compatible = "jedec,spi-nor";
> +          };
> +        };
> +
> +        spi-upper {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          flash@0 {
> +            reg = <0>;
> +            compatible = "jedec,spi-nor";
> +          };
> +        };
>        };
>      };

In the IIO subsystem, we've been recently working on several "advanced" ADCs
that could use a SPI controller like this. These ADCs have multiple input
channels that perform conversions in parallel and the data for each channel
needs to be read back on a separate serial line (MISO) at the same time. Another
similar case is to have two separate chips, but they share a conversion trigger
and essentially operate as a single composite device rather than two distinct
devices [1]. This would be similar to some ADCs that are daisy-chained where we
consider all of the chips in the chain as a single composite device, but they
would be in parallel rather than chained.

[1]: https://lore.kernel.org/linux-iio/e5e8eba7-2455-488b-a36f-e246844e11fd@baylibre.com/

For those use cases though, as mentioned above, we only have a single device
that would be connected to both buses. So for such a SPI controller with
multiple buses, I was envisioning that instead of adding child nodes for each
of the child buses, that we would do something like add a spi-buses property
to the spi peripheral bindings where you could specify one or more buses that
a device was connected to.

e.g. a device connected to the lower bus would be spi-buses = <0>; one connected
to the upper bus would be spi-buses = <1>; and a device connected to both would
be spi-buses = <0>, <1>;.  This would also work for SPI controllers that have
4 or 8 busses.

SPI controllers like these have a striping feature that allows to control both
buses at the same to either mirror the same data on both buses at the same
time when writing, e.g. for configuration or to read and write two different
bytes at the same time. A peripheral driver for device that was connected to
both buses could make use of this feature to craft a single SPI message with
transfers containing (new) parameters that specify which bus to use (one or
both) and, in the case of using both buses, to mirror or stripe the data.

Could we make a single device connected to both buses like this work using
the proposed spi-lower and spi-upper or should we consider a different binding
like the one I suggested?
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
index 901e15fcce2d..12c547c4f1ba 100644
--- a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
@@ -39,6 +39,18 @@  properties:
   resets:
     maxItems: 1
 
+  spi-lower:
+    type: object
+    $ref: spi-controller.yaml#
+    unevaluatedProperties: false
+    description: The "lower" bus (SPI0). On the ZynqMP this uses MIO pins 0-5.
+
+  spi-upper:
+    type: object
+    $ref: spi-controller.yaml#
+    unevaluatedProperties: false
+    description: The "upper" bus (SPI1). On the ZynqMP this uses MIO pins 7-12.
+
 required:
   - compatible
   - reg
@@ -50,8 +62,6 @@  required:
 unevaluatedProperties: false
 
 allOf:
-  - $ref: spi-controller.yaml#
-
   - if:
       properties:
         compatible:
@@ -75,7 +85,7 @@  examples:
       #address-cells = <2>;
       #size-cells = <2>;
 
-      qspi: spi@ff0f0000 {
+      qspi: spi-controller@ff0f0000 {
         compatible = "xlnx,zynqmp-qspi-1.0";
         clocks = <&zynqmp_clk QSPI_REF>, <&zynqmp_clk LPD_LSBUS>;
         clock-names = "ref_clk", "pclk";
@@ -84,5 +94,32 @@  examples:
         resets = <&zynqmp_reset ZYNQMP_RESET_QSPI>;
         reg = <0x0 0xff0f0000 0x0 0x1000>,
               <0x0 0xc0000000 0x0 0x8000000>;
+
+        spi-lower {
+          #address-cells = <1>;
+          #size-cells = <0>;
+          num-cs = <2>;
+          cs-gpios = <0>, <&gpio 5>;
+
+          flash@0 {
+            reg = <0>;
+            compatible = "jedec,spi-nor";
+          };
+
+          flash@1 {
+            reg = <1>;
+            compatible = "jedec,spi-nor";
+          };
+        };
+
+        spi-upper {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          flash@0 {
+            reg = <0>;
+            compatible = "jedec,spi-nor";
+          };
+        };
       };
     };