Thread (43 messages) 43 messages, 5 authors, 2020-06-10

Re: [PATCH 06/10] dt-bindings: spi: spi-dw-mchp: Add Sparx5 support

From: Lars Povlsen <hidden>
Date: 2020-06-10 12:27:46
Also in: linux-devicetree, linux-spi, lkml

Serge Semin writes:
On Wed, May 13, 2020 at 04:00:27PM +0200, Lars Povlsen wrote:
quoted
This add DT bindings for the Sparx5 SPI driver.
This whole file can be easily merged in to the generic DW APB SSI DT
binding file. Just use "if: properties: compatible: const: ..." construction
to distinguish ocelot, jaguar, sparx5 and non-sparx5 nodes.
quoted
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <redacted>
---
 .../bindings/spi/mscc,ocelot-spi.yaml         | 49 +++++++++++++++----
 1 file changed, 39 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
index a3ac0fa576553..8beecde4b0880 100644
--- a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
@@ -23,15 +23,23 @@ properties:
     enum:
       - mscc,ocelot-spi
       - mscc,jaguar2-spi
+      - microchip,sparx5-spi

   interrupts:
     maxItems: 1

   reg:
     minItems: 2
-    items:
-      - description: Designware SPI registers
-      - description: CS override registers
+    maxItems: 3
+    oneOf:
+      - items:
+          - description: Designware SPI registers
+          - description: CS override registers (Not sparx5).
+      - items:
+          - description: Designware SPI registers
+          - description: CS override registers (Not sparx5).
+          - description: Direct mapped SPI read area. If provided, the
+              driver will register spi_mem_op's to take advantage of it.

   clocks:
     maxItems: 1
@@ -43,6 +51,23 @@ properties:
        enum: [ 2, 4 ]
     maxItems: 1
quoted
+  spi-rx-delay-us:
+    description: |
+      The delay (in usec) of the RX signal sample position. This can
+      be used to tne the RX timing in order to acheive higher
+      speeds. This is used for all devices on the bus.
+    default: 0
+    maxItems: 1
spi-rx-delay-us is defined for a particular SPI-slave. Please see the
DT binding file: Documentation/devicetree/bindings/spi/spi-controller.yaml .
Although as I suggested before this delay isn't what the Dw APB SSI RX sample
delay functionality does. Probably a vendor-specific property would be better
here. But I'd also define it on a SPI-slave basis, not for all devices on the
bus.
Right, I was hunting for something "similar". As pointed out, this is
really different in nature, and the unit is also too coarse.

I will change this to "snps,rx-sample-delay-ns" as suggested in your
other comments.
quoted
+
+  interface-mapping-mask:
+    description: |
+      On the Sparx5 variant, two different busses are connected to the
+      controller. This property is a mask per chip-select, indicating
+      whether the CS should go to one or the other interface.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0
+    maxItems: 1
As Mark rightfully suggested this seems like an SPI-slave related property, then
most likely it should be defined on the SPI-slave basis (probably as a bool
property). Additionally it's vendor-specific, so the property name should be
accordingly prefixed.
Yes, I'll change this to a per-device property. I need the same for the
above as well.
quoted
+
 required:
   - compatible
   - reg
@@ -50,11 +75,15 @@ required:

 examples:
   - |
-    spi0: spi@101000 {
-      compatible = "mscc,ocelot-spi";
-      #address-cells = <1>;
-      #size-cells = <0>;
-      reg = <0x101000 0x100>, <0x3c 0x18>;
-      interrupts = <9>;
-      clocks = <&ahb_clk>;
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    spi0: spi@600104000 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        compatible = "microchip,sparx5-spi";
quoted
+        reg = <0x00104000 0x40>, <0 0>, <0x3000000 0x4000000>;
I have a doubt that defining an empty reg region is a good idea, since you can
detect the reg requirements by the node compatible string.
Yes, its probably better that way. It looks ugly too :-)

Thanks for your comments!

---Lars

-Sergey
quoted
+        num-cs = <16>;
+        reg-io-width = <4>;
+        reg-shift = <2>;
+        clocks = <&ahb_clk>;
+        interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
     };
--
2.26.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-- 
Lars Povlsen,
Microchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help