| Message ID | 20200702101331.26375-6-lars.povlsen@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: Adding support for Microchip Sparx5 SoC | expand |
On Thu, Jul 02, 2020 at 12:13:28PM +0200, Lars Povlsen wrote: > This has the following changes for the snps,dw-apb-ss DT bindings: > > - Add "microchip,sparx5-spi" as the compatible for the Sparx5 SoC > controller > > - Add the property "mux-controls" for the above compatible string > > - Add the property "snps,rx-sample-delay-ns" for SPI slaves > > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> > --- > .../bindings/spi/snps,dw-apb-ssi.yaml | 28 +++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > index c62cbe79f00dd..9d9208391fae3 100644 > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > @@ -36,6 +36,8 @@ properties: > - mscc,ocelot-spi > - mscc,jaguar2-spi > - const: snps,dw-apb-ssi > + - description: Microchip Sparx5 SoC SPI Controller > + const: microchip,sparx5-spi > - description: Amazon Alpine SPI Controller > const: amazon,alpine-dw-apb-ssi > - description: Renesas RZ/N1 SPI Controller > @@ -93,6 +95,19 @@ properties: > - const: tx > - const: rx > > +if: > + properties: > + compatible: > + contains: > + const: microchip,sparx5-spi > + > +then: > + properties: > + mux-controls: > + description: A mux controller node for selecting SPI bus interface. > + maxItems: 1 > + $ref: '/schemas/types.yaml#/definitions/phandle' Can drop the type. You can assume common properties already have a defined type. > + > patternProperties: > "^.*@[0-9a-f]+$": > type: object > @@ -107,6 +122,14 @@ patternProperties: > spi-tx-bus-width: > const: 1 > > + snps,rx-sample-delay-ns: We already have 'rx-sample-delay-ns' from Rockchip SPI, so use that. But note that it applies to the SPI node. Does this need to be per SPI child? BTW, the Rockchip controller appears to be a version of the DW controller. > + description: SPI Rx sample delay offset, unit is nanoseconds. > + The delay from the default sample time before the actual > + sample of the rxd input signal occurs. The "rx_sample_delay" > + is an optional feature of the designware controller, and the > + upper limit is also subject to controller configuration. > + $ref: /schemas/types.yaml#/definitions/uint32 > + > unevaluatedProperties: false > > required: > @@ -129,5 +152,10 @@ examples: > num-cs = <2>; > cs-gpios = <&gpio0 13 0>, > <&gpio0 14 0>; > + spi-flash@1 { > + compatible = "spi-nand"; > + reg = <1>; > + snps,rx-sample-delay-ns = <7>; > + }; > }; > ... > -- > 2.27.0
On Mon, Jul 13, 2020 at 01:22:59PM -0600, Rob Herring wrote: > On Thu, Jul 02, 2020 at 12:13:28PM +0200, Lars Povlsen wrote: ... > > > + > > patternProperties: > > "^.*@[0-9a-f]+$": > > type: object > > @@ -107,6 +122,14 @@ patternProperties: > > spi-tx-bus-width: > > const: 1 > > > > + snps,rx-sample-delay-ns: > > We already have 'rx-sample-delay-ns' from Rockchip SPI, so use that. But > note that it applies to the SPI node. Does this need to be per SPI > child? It was me, who suggested to Lars to have that parameter moved to the SPI sub-nodes. As I see it the property is highly dependent on the SPI slave device the controller is communicating to. Some of the them may need the delay some may not. It's not always the capacitance thing, but also depends on how good the MISO signal a particular slave generates. So IMO the Rockchip SPI driver developer should have moved that property to the sub-nodes too. On the other hand if the Rx errors are caused by the MISO lane capacitance, then it will be cumbersome to have the same property duplicated for each sub-node. Then what about having the property supported by both the SPI controller and the SPI-child nodes? For instance the SPI-controller "rx-sample-delay-ns" will provide a default sample delay for each sub-node instead of zero by default, while the individual sub-node "rx-sample-delay-ns" property can be used to override the default value. -Sergey > > BTW, the Rockchip controller appears to be a version of the DW > controller. > > > + description: SPI Rx sample delay offset, unit is nanoseconds. > > + The delay from the default sample time before the actual > > + sample of the rxd input signal occurs. The "rx_sample_delay" > > + is an optional feature of the designware controller, and the > > + upper limit is also subject to controller configuration. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + > > unevaluatedProperties: false > > > > required: > > @@ -129,5 +152,10 @@ examples: > > num-cs = <2>; > > cs-gpios = <&gpio0 13 0>, > > <&gpio0 14 0>; > > + spi-flash@1 { > > + compatible = "spi-nand"; > > + reg = <1>; > > + snps,rx-sample-delay-ns = <7>; > > + }; > > }; > > ... > > -- > > 2.27.0
Serge Semin writes: > On Mon, Jul 13, 2020 at 01:22:59PM -0600, Rob Herring wrote: >> On Thu, Jul 02, 2020 at 12:13:28PM +0200, Lars Povlsen wrote: > ... >> >> > + >> > patternProperties: >> > "^.*@[0-9a-f]+$": >> > type: object >> > @@ -107,6 +122,14 @@ patternProperties: >> > spi-tx-bus-width: >> > const: 1 >> > >> > + snps,rx-sample-delay-ns: >> > >> We already have 'rx-sample-delay-ns' from Rockchip SPI, so use that. But >> note that it applies to the SPI node. Does this need to be per SPI >> child? > > It was me, who suggested to Lars to have that parameter moved to the SPI > sub-nodes. As I see it the property is highly dependent on the SPI slave device > the controller is communicating to. Some of the them may need the delay some > may not. It's not always the capacitance thing, but also depends on how good > the MISO signal a particular slave generates. So IMO the Rockchip SPI driver > developer should have moved that property to the sub-nodes too. > In the case with sparx5, having two bus interfaces, spi slaves may be on different busses - making it obvious why you may need different settings. But I guess even on the same bus the physical length of SPI connections and device response delay to each device could play in as well. Nevertheless, it *does* make sense to be able to specify per slave, but a default could make the DT more terse. Should I add this to my patch? I will remove the "snps," prefix now that the property is globally established. ---Lars > On the other hand if the Rx errors are caused by the MISO lane capacitance, > then it will be cumbersome to have the same property duplicated for each > sub-node. Then what about having the property supported by both the SPI > controller and the SPI-child nodes? For instance the SPI-controller > "rx-sample-delay-ns" will provide a default sample delay for each sub-node > instead of zero by default, while the individual sub-node "rx-sample-delay-ns" > property can be used to override the default value. > > -Sergey > >> >> BTW, the Rockchip controller appears to be a version of the DW >> controller. >> >> > + description: SPI Rx sample delay offset, unit is nanoseconds. >> > + The delay from the default sample time before the actual >> > + sample of the rxd input signal occurs. The "rx_sample_delay" >> > + is an optional feature of the designware controller, and the >> > + upper limit is also subject to controller configuration. >> > + $ref: /schemas/types.yaml#/definitions/uint32 >> > + >> > unevaluatedProperties: false >> > >> > required: >> > @@ -129,5 +152,10 @@ examples: >> > num-cs = <2>; >> > cs-gpios = <&gpio0 13 0>, >> > <&gpio0 14 0>; >> > + spi-flash@1 { >> > + compatible = "spi-nand"; >> > + reg = <1>; >> > + snps,rx-sample-delay-ns = <7>; >> > + }; >> > }; >> > ... >> > -- >> > 2.27.0
diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml index c62cbe79f00dd..9d9208391fae3 100644 --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml @@ -36,6 +36,8 @@ properties: - mscc,ocelot-spi - mscc,jaguar2-spi - const: snps,dw-apb-ssi + - description: Microchip Sparx5 SoC SPI Controller + const: microchip,sparx5-spi - description: Amazon Alpine SPI Controller const: amazon,alpine-dw-apb-ssi - description: Renesas RZ/N1 SPI Controller @@ -93,6 +95,19 @@ properties: - const: tx - const: rx +if: + properties: + compatible: + contains: + const: microchip,sparx5-spi + +then: + properties: + mux-controls: + description: A mux controller node for selecting SPI bus interface. + maxItems: 1 + $ref: '/schemas/types.yaml#/definitions/phandle' + patternProperties: "^.*@[0-9a-f]+$": type: object @@ -107,6 +122,14 @@ patternProperties: spi-tx-bus-width: const: 1 + snps,rx-sample-delay-ns: + description: SPI Rx sample delay offset, unit is nanoseconds. + The delay from the default sample time before the actual + sample of the rxd input signal occurs. The "rx_sample_delay" + is an optional feature of the designware controller, and the + upper limit is also subject to controller configuration. + $ref: /schemas/types.yaml#/definitions/uint32 + unevaluatedProperties: false required: @@ -129,5 +152,10 @@ examples: num-cs = <2>; cs-gpios = <&gpio0 13 0>, <&gpio0 14 0>; + spi-flash@1 { + compatible = "spi-nand"; + reg = <1>; + snps,rx-sample-delay-ns = <7>; + }; }; ...
This has the following changes for the snps,dw-apb-ss DT bindings: - Add "microchip,sparx5-spi" as the compatible for the Sparx5 SoC controller - Add the property "mux-controls" for the above compatible string - Add the property "snps,rx-sample-delay-ns" for SPI slaves Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com> --- .../bindings/spi/snps,dw-apb-ssi.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) -- 2.27.0