Re: [PATCH v2 18/20] dt-bindings:iio:adc:amlogic,meson-saradc yaml conversion

[Rob Herring <robh@kernel.org>]

On Wed, Sep 09, 2020 at 06:59:44PM +0100, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This binding is non trivial due to the range of different parts
> supported having several subtle quirks. Martin has helped
> clarify some of them.
> 
> Note, I haven't restricted the amlogic,hhi-sysctrl to only
> be present on the relevant parts if nvmem stuff also is, but
> it would seem to be rather odd if it were otherwise.
> 
> Perhaps we look to make this binding more restrictive at a later date.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  .../bindings/iio/adc/amlogic,meson-saradc.txt |  48 ------
>  .../iio/adc/amlogic,meson-saradc.yaml         | 149 ++++++++++++++++++
>  2 files changed, 149 insertions(+), 48 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
> deleted file mode 100644
> index d57e9df25f4f..000000000000
> --- a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
> +++ /dev/null
> @@ -1,48 +0,0 @@
> -* Amlogic Meson SAR (Successive Approximation Register) A/D converter
> -
> -Required properties:
> -- compatible:	depending on the SoC this should be one of:
> -			- "amlogic,meson8-saradc" for Meson8
> -			- "amlogic,meson8b-saradc" for Meson8b
> -			- "amlogic,meson8m2-saradc" for Meson8m2
> -			- "amlogic,meson-gxbb-saradc" for GXBB
> -			- "amlogic,meson-gxl-saradc" for GXL
> -			- "amlogic,meson-gxm-saradc" for GXM
> -			- "amlogic,meson-axg-saradc" for AXG
> -			- "amlogic,meson-g12a-saradc" for AXG
> -		along with the generic "amlogic,meson-saradc"
> -- reg:		the physical base address and length of the registers
> -- interrupts:	the interrupt indicating end of sampling
> -- clocks:	phandle and clock identifier (see clock-names)
> -- clock-names:	mandatory clocks:
> -			- "clkin" for the reference clock (typically XTAL)
> -			- "core" for the SAR ADC core clock
> -		optional clocks:
> -			- "adc_clk" for the ADC (sampling) clock
> -			- "adc_sel" for the ADC (sampling) clock mux
> -- vref-supply:	the regulator supply for the ADC reference voltage
> -- #io-channel-cells: must be 1, see ../iio-bindings.txt
> -
> -Optional properties:
> -- amlogic,hhi-sysctrl:	phandle to the syscon which contains the 5th bit
> -			of the TSC (temperature sensor coefficient) on
> -			Meson8b and Meson8m2 (which used to calibrate the
> -			temperature sensor)
> -- nvmem-cells:		phandle to the temperature_calib eFuse cells
> -- nvmem-cell-names:	if present (to enable the temperature sensor
> -			calibration) this must contain "temperature_calib"
> -
> -
> -Example:
> -	saradc: adc@8680 {
> -		compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
> -		#io-channel-cells = <1>;
> -		reg = <0x0 0x8680 0x0 0x34>;
> -		interrupts = <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>;
> -		clocks = <&xtal>,
> -			 <&clkc CLKID_SAR_ADC>,
> -			 <&clkc CLKID_SANA>,
> -			 <&clkc CLKID_SAR_ADC_CLK>,
> -			 <&clkc CLKID_SAR_ADC_SEL>;
> -		clock-names = "clkin", "core", "sana", "adc_clk", "adc_sel";
> -	};
> diff --git a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.yaml
> new file mode 100644
> index 000000000000..6d94bf76900e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.yaml
> @@ -0,0 +1,149 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/amlogic,meson-saradc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic Meson SAR (Successive Approximation Register) A/D converter
> +
> +maintainers:
> +  - Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> +
> +description:
> +  Binding covers a range of ADCs found on amlogic meson SoCs.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: amlogic,meson-saradc
> +      - items:
> +          - enum:
> +              - amlogic,meson8-saradc
> +              - amlogic,meson8b-saradc
> +              - amlogic,meson8m2-saradc
> +              - amlogic,meson-gxbb-saradc
> +              - amlogic,meson-gxl-saradc
> +              - amlogic,meson-gxm-saradc
> +              - amlogic,meson-axg-saradc
> +              - amlogic,meson-g12a-saradc
> +          - const: amlogic,meson-saradc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description: Interrupt indicates end of sampling.
> +    maxItems: 1
> +
> +  clocks: true
> +
> +  clock-names: true

Since the names are the same, I prefer to have a schema that covers both 
cases here and then...

> +
> +  vref-supply: true
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +  amlogic,hhi-sysctrl:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Syscon which contains the 5th bit of the TSC (temperature sensor
> +      coefficient) on Meson8b and Meson8m2 (which used to calibrate the
> +      temperature sensor)
> +
> +  nvmem-cells:
> +    description: phandle to the temperature_calib eFuse cells

maxItems: 1

> +
> +  nvmem-cell-names:
> +    const: temperature_calib
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - amlogic,meson8-saradc
> +              - amlogic,meson8b-saradc
> +              - amlogic,meson8m2-saradc
> +    then:
> +      properties:
> +        nvmem-cells: true
> +        nvmem-cell-names: true

You can drop these 2.

> +        clock-names:
> +          items:
> +            - const: clkin
> +            - const: core
> +          minItems: 2
> +        clocks:
> +          minItems: 2

This case just needs 'maxItems: 2' for clock-names and clocks.

> +    else:
> +      properties:
> +        nvmem-cells: false
> +        mvmem-cel-names: false
> +        clock-names:
> +          items:
> +            - const: clkin
> +            - const: core
> +            - const: adc_clk
> +            - const: adc_sel
> +          minItems: 4

And just minItems here.

> +        clocks:
> +          minItems: 4
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - amlogic,meson8b-saradc
> +              - amlogic,meson8m2-saradc
> +    then:
> +      properties:
> +        amlogic,hhi-sysctrl: true
> +    else:
> +      properties:
> +        amlogic,hhi-sysctrl: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - "#io-channel-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/clock/gxbb-clkc.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        adc@8680 {
> +            compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
> +            #io-channel-cells = <1>;
> +            reg = <0x0 0x8680 0x0 0x34>;
> +            interrupts = <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>;
> +            clocks = <&xtal>,
> +                <&clkc CLKID_SAR_ADC>,
> +                <&clkc CLKID_SAR_ADC_CLK>,
> +                <&clkc CLKID_SAR_ADC_SEL>;
> +            clock-names = "clkin", "core", "adc_clk", "adc_sel";
> +        };
> +        adc@9680 {
> +            compatible = "amlogic,meson8b-saradc", "amlogic,meson-saradc";
> +            #io-channel-cells = <1>;
> +            reg = <0x0 0x9680 0x0 0x34>;
> +            interrupts = <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>;
> +            clocks = <&xtal>, <&clkc CLKID_SAR_ADC>;
> +            clock-names = "clkin", "core";
> +            nvmem-cells = <&tsens_caldata>;
> +            nvmem-cell-names = "temperature_calib";
> +            amlogic,hhi-sysctrl = <&hhi>;
> +        };
> +    };
> +...
> -- 
> 2.28.0
>