[PATCH] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml

[PATCH] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml

[Rob Herring]

SPI bus per device properties must be defined in spi-peripheral-props.yaml
for unevaluatedProperties checks to work correctly on device nodes.

This has the side effect of promoting 'rx-sample-delay-ns' to be a
common property, but functionally it's no different if it was defined in
a Synopsys specific schema file.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../bindings/spi/snps,dw-apb-ssi.yaml          | 18 +++++++++---------
 .../bindings/spi/spi-peripheral-props.yaml     |  5 +++++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index d7e08b03e204..e25d44c218f2 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -124,9 +124,16 @@ properties:
 
   rx-sample-delay-ns:
     default: 0
-    description: Default value of the rx-sample-delay-ns property.
+    description: |
+      Default value of the rx-sample-delay-ns property.
       This value will be used if the property is not explicitly defined
-      for a SPI slave device. See below.
+      for a SPI slave device.
+
+      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.
 
 patternProperties:
   "^.*@[0-9a-f]+$":
@@ -142,13 +149,6 @@ patternProperties:
       spi-tx-bus-width:
         const: 1
 
-      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.
-
 unevaluatedProperties: false
 
 required:
diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
index 5e32928c4fc3..6ffb74352bef 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -71,6 +71,11 @@ properties:
     description:
       Delay, in microseconds, after a read transfer.
 
+  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.
+
   spi-tx-bus-width:
     description:
       Bus width to the SPI bus used for write transfers.
-- 
2.34.1

Re: [PATCH] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml

[Pratyush Yadav]

Hi Rob,

On 25/05/22 04:00PM, Rob Herring wrote:
> SPI bus per device properties must be defined in spi-peripheral-props.yaml
> for unevaluatedProperties checks to work correctly on device nodes.
> 
> This has the side effect of promoting 'rx-sample-delay-ns' to be a
> common property, but functionally it's no different if it was defined in
> a Synopsys specific schema file.

Functionally it is no different, but does this property make sense for 
other controllers? If not then I don't see why we should pollute the 
common list with controller-specific ones. For one, this now no longer 
makes it obvious that this property should only be used with the 
Synopsys controller. And if you keep making small exceptions for other 
controllers too, soon the common list will be full of controller 
properties and it will be a mess finding out what belongs to who.

> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/spi/snps,dw-apb-ssi.yaml          | 18 +++++++++---------
>  .../bindings/spi/spi-peripheral-props.yaml     |  5 +++++
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index d7e08b03e204..e25d44c218f2 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -124,9 +124,16 @@ properties:
>  
>    rx-sample-delay-ns:
>      default: 0
> -    description: Default value of the rx-sample-delay-ns property.
> +    description: |
> +      Default value of the rx-sample-delay-ns property.
>        This value will be used if the property is not explicitly defined
> -      for a SPI slave device. See below.
> +      for a SPI slave device.
> +
> +      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.
>  
>  patternProperties:
>    "^.*@[0-9a-f]+$":
> @@ -142,13 +149,6 @@ patternProperties:
>        spi-tx-bus-width:
>          const: 1
>  
> -      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.
> -
>  unevaluatedProperties: false
>  
>  required:
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index 5e32928c4fc3..6ffb74352bef 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -71,6 +71,11 @@ properties:
>      description:
>        Delay, in microseconds, after a read transfer.
>  
> +  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.
> +
>    spi-tx-bus-width:
>      description:
>        Bus width to the SPI bus used for write transfers.
> -- 
> 2.34.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml

[Rob Herring]

On Thu, May 26, 2022 at 11:16:42AM +0530, Pratyush Yadav wrote:
> Hi Rob,
> 
> On 25/05/22 04:00PM, Rob Herring wrote:
> > SPI bus per device properties must be defined in spi-peripheral-props.yaml
> > for unevaluatedProperties checks to work correctly on device nodes.
> > 
> > This has the side effect of promoting 'rx-sample-delay-ns' to be a
> > common property, but functionally it's no different if it was defined in
> > a Synopsys specific schema file.
> 
> Functionally it is no different, but does this property make sense for 
> other controllers? If not then I don't see why we should pollute the 
> common list with controller-specific ones. For one, this now no longer 
> makes it obvious that this property should only be used with the 
> Synopsys controller. And if you keep making small exceptions for other 
> controllers too, soon the common list will be full of controller 
> properties and it will be a mess finding out what belongs to who.

There's at least one other case already:

  cdns,read-delay:
    $ref: /schemas/types.yaml#/definitions/uint32
    description:
      Delay for read capture logic, in clock cycles.


Too many common properties is not a problem we have. Too many custom 
properties doing the same thing is the problem.

Rob

Re: [PATCH] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml

[Serge Semin]

On Thu, May 26, 2022 at 08:54:04AM -0500, Rob Herring wrote:
> On Thu, May 26, 2022 at 11:16:42AM +0530, Pratyush Yadav wrote:
> > Hi Rob,
> > 
> > On 25/05/22 04:00PM, Rob Herring wrote:
> > > SPI bus per device properties must be defined in spi-peripheral-props.yaml
> > > for unevaluatedProperties checks to work correctly on device nodes.
> > > 
> > > This has the side effect of promoting 'rx-sample-delay-ns' to be a
> > > common property, but functionally it's no different if it was defined in
> > > a Synopsys specific schema file.
> > 
> > Functionally it is no different, but does this property make sense for 
> > other controllers? If not then I don't see why we should pollute the 
> > common list with controller-specific ones. For one, this now no longer 
> > makes it obvious that this property should only be used with the 
> > Synopsys controller. And if you keep making small exceptions for other 
> > controllers too, soon the common list will be full of controller 
> > properties and it will be a mess finding out what belongs to who.
> 

> There's at least one other case already:
> 
>   cdns,read-delay:
>     $ref: /schemas/types.yaml#/definitions/uint32
>     description:
>       Delay for read capture logic, in clock cycles.

What about creating the schemas hierarchy for the device-specific
properties as I already suggested in the other thread? Like this:

https://lore.kernel.org/linux-ide/20220527101057.b5z7ase6y4naoxvk@mobilestation

-Sergey

> 
> 
> Too many common properties is not a problem we have. Too many custom 
> properties doing the same thing is the problem.
> 
> Rob

Re: [PATCH] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml

[Rob Herring]

On Fri, May 27, 2022 at 02:32:26PM +0300, Serge Semin wrote:
> On Thu, May 26, 2022 at 08:54:04AM -0500, Rob Herring wrote:
> > On Thu, May 26, 2022 at 11:16:42AM +0530, Pratyush Yadav wrote:
> > > Hi Rob,
> > > 
> > > On 25/05/22 04:00PM, Rob Herring wrote:
> > > > SPI bus per device properties must be defined in spi-peripheral-props.yaml
> > > > for unevaluatedProperties checks to work correctly on device nodes.
> > > > 
> > > > This has the side effect of promoting 'rx-sample-delay-ns' to be a
> > > > common property, but functionally it's no different if it was defined in
> > > > a Synopsys specific schema file.
> > > 
> > > Functionally it is no different, but does this property make sense for 
> > > other controllers? If not then I don't see why we should pollute the 
> > > common list with controller-specific ones. For one, this now no longer 
> > > makes it obvious that this property should only be used with the 
> > > Synopsys controller. And if you keep making small exceptions for other 
> > > controllers too, soon the common list will be full of controller 
> > > properties and it will be a mess finding out what belongs to who.
> > 
> 
> > There's at least one other case already:
> > 
> >   cdns,read-delay:
> >     $ref: /schemas/types.yaml#/definitions/uint32
> >     description:
> >       Delay for read capture logic, in clock cycles.
> 
> What about creating the schemas hierarchy for the device-specific
> properties as I already suggested in the other thread? Like this:
> 
> https://lore.kernel.org/linux-ide/20220527101057.b5z7ase6y4naoxvk@mobilestation

Because that doesn't work. I'll explain in that thread.

Rob

Re: [PATCH] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml

[Pratyush Yadav]

On 26/05/22 08:54AM, Rob Herring wrote:
> On Thu, May 26, 2022 at 11:16:42AM +0530, Pratyush Yadav wrote:
> > Hi Rob,
> > 
> > On 25/05/22 04:00PM, Rob Herring wrote:
> > > SPI bus per device properties must be defined in spi-peripheral-props.yaml
> > > for unevaluatedProperties checks to work correctly on device nodes.
> > > 
> > > This has the side effect of promoting 'rx-sample-delay-ns' to be a
> > > common property, but functionally it's no different if it was defined in
> > > a Synopsys specific schema file.
> > 
> > Functionally it is no different, but does this property make sense for 
> > other controllers? If not then I don't see why we should pollute the 
> > common list with controller-specific ones. For one, this now no longer 
> > makes it obvious that this property should only be used with the 
> > Synopsys controller. And if you keep making small exceptions for other 
> > controllers too, soon the common list will be full of controller 
> > properties and it will be a mess finding out what belongs to who.
> 
> There's at least one other case already:
> 
>   cdns,read-delay:
>     $ref: /schemas/types.yaml#/definitions/uint32
>     description:
>       Delay for read capture logic, in clock cycles.
> 
> 
> Too many common properties is not a problem we have. Too many custom 
> properties doing the same thing is the problem.

I agree. But in this case these two properties have different units. 
rx-sample-delay-ns is obviously in nanoseconds. cdns,read-delay is in 
number of ref clock cycles. If other controllers also use this property, 
it could make sense to make rx-sample-delay-ns the default/common 
property and drivers can then make conversions between the units that 
should actually be programmed.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml

[Mark Brown]

On Wed, 25 May 2022 16:00:53 -0500, Rob Herring wrote:
> SPI bus per device properties must be defined in spi-peripheral-props.yaml
> for unevaluatedProperties checks to work correctly on device nodes.
> 
> This has the side effect of promoting 'rx-sample-delay-ns' to be a
> common property, but functionally it's no different if it was defined in
> a Synopsys specific schema file.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: dt-bindings: Move 'rx-sample-delay-ns' to spi-peripheral-props.yaml
      commit: b658be56e867061a0d5496e837f350974ada5c89

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark