On Wed, May 13, 2020 at 04:00:27PM +0200, Lars Povlsen wrote:
> 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.
>
> Reviewed-by: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx>
> Signed-off-by: Lars Povlsen <lars.povlsen@xxxxxxxxxxxxx>
> ---
> .../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
>
> + 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.
> +
> + 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.
> +
> 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";
> + 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.
-Sergey
> + 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@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
