mbox series

[v2,00/10] dt-bindings: iio: use spi-peripheral-props.yaml

Message ID 20220727164646.387541-1-krzysztof.kozlowski@linaro.org
Headers show
Series dt-bindings: iio: use spi-peripheral-props.yaml | expand

Message

Krzysztof Kozlowski July 27, 2022, 4:46 p.m. UTC
Technically, this depends on [1] merged to SPI tree, if we want to
preserve existing behavior of not allowing SPI CPHA and CPOL in each of
schemas in this patch.

If this patch comes independently via different tree, the SPI CPHA and
CPOL will be allowed for brief period of time, before [1] is merged.
This will not have negative impact, just DT schema checks will be
loosened for that period.

[1] https://lore.kernel.org/all/20220722191539.90641-2-krzysztof.kozlowski@linaro.org/

Changes since v1
================
1. Continue the rework for entire IIO.
v1: https://lore.kernel.org/all/20220715095302.214276-1-krzysztof.kozlowski@linaro.org/

Best regards,
Krzysztof

Krzysztof Kozlowski (10):
  dt-bindings: iio: adc: use spi-peripheral-props.yaml
  dt-bindings: iio: accel: use spi-peripheral-props.yaml
  dt-bindings: iio: amplifiers: adi,ada4250: use
    spi-peripheral-props.yaml
  dt-bindings: iio: dac: use spi-peripheral-props.yaml
  dt-bindings: iio: frequency: adf4371: use spi-peripheral-props.yaml
  dt-bindings: iio: health: ti,afe4403: use spi-peripheral-props.yaml
  dt-bindings: iio: imu: use spi-peripheral-props.yaml
  dt-bindings: iio: potentiometer: use spi-peripheral-props.yaml
  dt-bindings: iio: samsung,sensorhub-rinato: use
    spi-peripheral-props.yaml
  dt-bindings: iio: temperature: use spi-peripheral-props.yaml

 .../bindings/iio/accel/adi,adis16201.yaml     |  7 ++---
 .../bindings/iio/accel/adi,adis16240.yaml     |  7 ++---
 .../bindings/iio/accel/adi,adxl313.yaml       |  9 +++----
 .../bindings/iio/accel/adi,adxl345.yaml       |  7 ++---
 .../bindings/iio/accel/adi,adxl355.yaml       |  7 ++---
 .../bindings/iio/accel/adi,adxl367.yaml       |  7 ++---
 .../bindings/iio/accel/adi,adxl372.yaml       |  7 ++---
 .../bindings/iio/accel/bosch,bma220.yaml      |  7 ++---
 .../bindings/iio/accel/bosch,bma255.yaml      |  5 +++-
 .../bindings/iio/accel/bosch,bmi088.yaml      |  7 ++---
 .../bindings/iio/accel/fsl,mma7455.yaml       |  7 ++---
 .../bindings/iio/accel/kionix,kxsd9.yaml      |  7 ++---
 .../bindings/iio/accel/murata,sca3300.yaml    |  5 +++-
 .../bindings/iio/accel/nxp,fxls8962af.yaml    |  7 ++---
 .../bindings/iio/adc/adi,ad7124.yaml          |  7 ++---
 .../bindings/iio/adc/adi,ad7192.yaml          |  7 ++---
 .../bindings/iio/adc/adi,ad7280a.yaml         |  7 ++---
 .../bindings/iio/adc/adi,ad7292.yaml          |  7 ++---
 .../bindings/iio/adc/adi,ad7298.yaml          |  6 +++--
 .../bindings/iio/adc/adi,ad7476.yaml          |  8 +++---
 .../bindings/iio/adc/adi,ad7606.yaml          |  7 ++---
 .../bindings/iio/adc/adi,ad7768-1.yaml        |  7 ++---
 .../bindings/iio/adc/adi,ad7923.yaml          |  7 ++---
 .../bindings/iio/adc/adi,ad7949.yaml          |  7 ++---
 .../bindings/iio/adc/holt,hi8435.yaml         |  7 ++---
 .../bindings/iio/adc/lltc,ltc2496.yaml        |  8 +++---
 .../bindings/iio/adc/maxim,max1027.yaml       |  5 +++-
 .../bindings/iio/adc/maxim,max11100.yaml      |  7 +++--
 .../bindings/iio/adc/maxim,max1118.yaml       | 26 ++++++++++---------
 .../bindings/iio/adc/maxim,max1241.yaml       |  7 ++---
 .../bindings/iio/adc/microchip,mcp3201.yaml   |  6 +++--
 .../bindings/iio/adc/microchip,mcp3911.yaml   |  5 +++-
 .../bindings/iio/adc/ti,adc0832.yaml          |  7 ++---
 .../bindings/iio/adc/ti,adc084s021.yaml       |  7 ++---
 .../bindings/iio/adc/ti,adc108s102.yaml       |  6 +++--
 .../bindings/iio/adc/ti,adc12138.yaml         |  7 ++---
 .../bindings/iio/adc/ti,adc128s052.yaml       |  7 ++---
 .../bindings/iio/adc/ti,adc161s626.yaml       |  7 ++---
 .../bindings/iio/adc/ti,ads124s08.yaml        |  7 ++---
 .../bindings/iio/adc/ti,ads131e08.yaml        |  7 ++---
 .../bindings/iio/adc/ti,ads8344.yaml          |  7 ++---
 .../bindings/iio/adc/ti,ads8688.yaml          |  7 ++---
 .../bindings/iio/adc/ti,tlc4541.yaml          |  7 ++---
 .../bindings/iio/adc/ti,tsc2046.yaml          |  7 ++---
 .../bindings/iio/amplifiers/adi,ada4250.yaml  |  7 ++---
 .../bindings/iio/dac/adi,ad5064.yaml          |  7 +++--
 .../bindings/iio/dac/adi,ad5360.yaml          |  7 +++--
 .../bindings/iio/dac/adi,ad5380.yaml          |  9 ++++---
 .../bindings/iio/dac/adi,ad5421.yaml          |  7 ++---
 .../bindings/iio/dac/adi,ad5449.yaml          |  7 +++--
 .../bindings/iio/dac/adi,ad5624r.yaml         |  9 ++++---
 .../bindings/iio/dac/adi,ad5686.yaml          |  9 ++++---
 .../bindings/iio/dac/adi,ad5755.yaml          |  9 ++++---
 .../bindings/iio/dac/adi,ad5758.yaml          |  4 +--
 .../bindings/iio/dac/adi,ad5761.yaml          |  7 +++--
 .../bindings/iio/dac/adi,ad5764.yaml          |  7 +++--
 .../bindings/iio/dac/adi,ad5770r.yaml         |  7 ++---
 .../bindings/iio/dac/adi,ad5791.yaml          |  9 ++++---
 .../bindings/iio/dac/adi,ad8801.yaml          |  7 +++--
 .../bindings/iio/dac/microchip,mcp4922.yaml   |  9 ++++---
 .../bindings/iio/dac/ti,dac082s085.yaml       |  9 ++++---
 .../bindings/iio/dac/ti,dac7311.yaml          |  7 ++---
 .../bindings/iio/dac/ti,dac7612.yaml          |  7 ++---
 .../bindings/iio/frequency/adf4371.yaml       |  7 ++---
 .../bindings/iio/health/ti,afe4403.yaml       |  9 ++++---
 .../bindings/iio/imu/adi,adis16460.yaml       |  7 ++---
 .../bindings/iio/imu/adi,adis16480.yaml       |  9 ++++---
 .../bindings/iio/imu/bosch,bmi160.yaml        |  7 ++---
 .../bindings/iio/imu/invensense,icm42600.yaml |  6 +++--
 .../bindings/iio/imu/invensense,mpu6050.yaml  |  5 ++--
 .../bindings/iio/imu/nxp,fxos8700.yaml        |  7 ++---
 .../bindings/iio/imu/st,lsm6dsx.yaml          |  9 ++++---
 .../iio/potentiometer/microchip,mcp41010.yaml |  9 ++++---
 .../iio/potentiometer/microchip,mcp4131.yaml  |  9 ++++---
 .../iio/samsung,sensorhub-rinato.yaml         |  9 ++++---
 .../iio/temperature/maxim,max31855k.yaml      |  4 +--
 .../iio/temperature/maxim,max31856.yaml       |  6 +++--
 .../iio/temperature/maxim,max31865.yaml       |  6 +++--
 78 files changed, 324 insertions(+), 249 deletions(-)

Comments

Marcus Folkesson July 27, 2022, 4:56 p.m. UTC | #1
On Wed, Jul 27, 2022 at 06:46:37PM +0200, Krzysztof Kozlowski wrote:
> Instead of listing directly properties typical for SPI peripherals,
> reference the spi-peripheral-props.yaml schema.  This allows using all
> properties typical for SPI-connected devices, even these which device
> bindings author did not tried yet.
> 
> Remove the spi-* properties which now come via spi-peripheral-props.yaml
> schema, except for the cases when device schema adds some constraints
> like maximum frequency.
> 
> While changing additionalProperties->unevaluatedProperties, put it in
> typical place, just before example DTS.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
[...]
>  .../bindings/iio/adc/microchip,mcp3911.yaml   |  5 +++-

FWIW: 
Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Lukas Wunner July 30, 2022, 10:43 p.m. UTC | #2
On Wed, Jul 27, 2022 at 06:46:40PM +0200, Krzysztof Kozlowski wrote:
> diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac082s085.yaml b/Documentation/devicetree/bindings/iio/dac/ti,dac082s085.yaml
> index b0157050f1ee..201b04af2b22 100644
> --- a/Documentation/devicetree/bindings/iio/dac/ti,dac082s085.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac082s085.yaml
> @@ -33,21 +33,22 @@ properties:
>    vref-supply:
>      description: Needed to provide output scaling.
>  
> -  spi-max-frequency: true
> -
>  required:
>    - compatible
>    - reg
>    - vref-supply
>  
> -additionalProperties: false
> -
>  oneOf:
>    - required:
>        - spi-cpha
>    - required:
>        - spi-cpol
>  
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
>  examples:
>    - |
>      vref_2v5_reg: regulator-vref {

Just to double-check because I'm unsure how the allOf interacts with
the oneOf above:  The DT must specify either spi-cpha or spi-cpol
for this device, but not both.  Hopefully that's still enforced
after your change?

Thanks,

Lukas
Lukas Wunner July 30, 2022, 10:46 p.m. UTC | #3
On Wed, Jul 27, 2022 at 06:46:36PM +0200, Krzysztof Kozlowski wrote:
>  78 files changed, 324 insertions(+), 249 deletions(-)

Pardon me for being dense, but what is the benefit of this series
that justifies inflating the schema definitions by a total of 75 lines?

Thanks,

Lukas
Krzysztof Kozlowski Aug. 1, 2022, 3:45 p.m. UTC | #4
On 31/07/2022 00:46, Lukas Wunner wrote:
> On Wed, Jul 27, 2022 at 06:46:36PM +0200, Krzysztof Kozlowski wrote:
>>  78 files changed, 324 insertions(+), 249 deletions(-)
> 
> Pardon me for being dense, but what is the benefit of this series
> that justifies inflating the schema definitions by a total of 75 lines?

The commits were explaining rationale, so let me bring it here. The
benefits are:
This allows using all properties typical for SPI-connected devices, even
these which device bindings author did not tried yet.

Also, what I did not mention in commit msg, this makes sure, that
spi-xxx properties have a type, which is validated by
spi-peripheral-props.yaml. Otherwise, when someone puts bogus data as
spi-max-frequency (e.g. phandle) and checks only with that device
schema, no errors are reported.

Best regards,
Krzysztof
Lukas Wunner Aug. 1, 2022, 4:04 p.m. UTC | #5
On Mon, Aug 01, 2022 at 05:45:07PM +0200, Krzysztof Kozlowski wrote:
> On 31/07/2022 00:46, Lukas Wunner wrote:
> > On Wed, Jul 27, 2022 at 06:46:36PM +0200, Krzysztof Kozlowski wrote:
> >>  78 files changed, 324 insertions(+), 249 deletions(-)
> > 
> > Pardon me for being dense, but what is the benefit of this series
> > that justifies inflating the schema definitions by a total of 75 lines?
> 
> The commits were explaining rationale, so let me bring it here. The
> benefits are:
> This allows using all properties typical for SPI-connected devices, even
> these which device bindings author did not tried yet.

How do you know these untested properties work with the devices to which
you're adding them?

Thanks,

Lukas
Krzysztof Kozlowski Aug. 2, 2022, 8 a.m. UTC | #6
On 01/08/2022 18:04, Lukas Wunner wrote:
> On Mon, Aug 01, 2022 at 05:45:07PM +0200, Krzysztof Kozlowski wrote:
>> On 31/07/2022 00:46, Lukas Wunner wrote:
>>> On Wed, Jul 27, 2022 at 06:46:36PM +0200, Krzysztof Kozlowski wrote:
>>>>  78 files changed, 324 insertions(+), 249 deletions(-)
>>>
>>> Pardon me for being dense, but what is the benefit of this series
>>> that justifies inflating the schema definitions by a total of 75 lines?
>>
>> The commits were explaining rationale, so let me bring it here. The
>> benefits are:
>> This allows using all properties typical for SPI-connected devices, even
>> these which device bindings author did not tried yet.
> 
> How do you know these untested properties work with the devices to which
> you're adding them?

These properties should be device independent and instead
controller-dependent. At least some of them (that's why CPHA/CPOL was
moved away and maybe the same we need to do with spi-3wire, spi-cs-high,
spi-lsb-first).

My approach here is no different than other subsystems. Take a look at
regulator - we allow all regulator.yaml properties, even though several
are not applicable (e.g. current for voltage regulators) and for sure no
tested.

Best regards,
Krzysztof
Rob Herring (Arm) Aug. 3, 2022, 9:40 p.m. UTC | #7
On Mon, Aug 01, 2022 at 06:04:10PM +0200, Lukas Wunner wrote:
> On Mon, Aug 01, 2022 at 05:45:07PM +0200, Krzysztof Kozlowski wrote:
> > On 31/07/2022 00:46, Lukas Wunner wrote:
> > > On Wed, Jul 27, 2022 at 06:46:36PM +0200, Krzysztof Kozlowski wrote:
> > >>  78 files changed, 324 insertions(+), 249 deletions(-)
> > > 
> > > Pardon me for being dense, but what is the benefit of this series
> > > that justifies inflating the schema definitions by a total of 75 lines?
> > 
> > The commits were explaining rationale, so let me bring it here. The
> > benefits are:
> > This allows using all properties typical for SPI-connected devices, even
> > these which device bindings author did not tried yet.
> 
> How do you know these untested properties work with the devices to which
> you're adding them?

How do we know anything DT works? We don't without testing on h/w. 
That's not what the schemas provide.

The spi-peripheral-props.yaml reference is needed in order to allow 
controller specific timing properties and to prevent random 
other undocumented properties from being present. There is not another 
way to do both of those.

Do I wish we didn't have these controller specific timing parameters, 
yes! But that ship has sailed.

Rob