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 -- Lars Povlsen, Microchip