lkml.org 
[lkml]   [2020]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property
Date

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

\
 
 \ /
  Last update: 2020-07-14 10:31    [W:1.041 / U:0.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site