diff mbox series

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

Message ID 20200905173004.216081-19-jic23@kernel.org
State Superseded, archived
Headers show
Series dt-bindings:iio:adc: Another set of yaml conversions. | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 2 warnings, 145 lines checked
robh/dt-meta-schema success

Commit Message

Jonathan Cameron Sept. 5, 2020, 5:30 p.m. UTC 
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This binding is non trivial due to the range of different parts
supported having several subtle quirks.
I am far from sure I have these correct. For example:
1) Can we have the clock adc_sel without adc_clk?
2) 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         | 145 ++++++++++++++++++
 2 files changed, 145 insertions(+), 48 deletions(-)

Comments

Martin Blumenstingl Sept. 6, 2020, 10 a.m. UTC | #1 
Hi Jonathan,

On Sat, Sep 5, 2020 at 7:32 PM Jonathan Cameron <jic23@kernel.org> 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.
thank you for working on this!

> I am far from sure I have these correct. For example:
> 1) Can we have the clock adc_sel without adc_clk?
adc_sel and adc_clk only exist on GXBB and newer
Meson8, Meson8b and Meson8m2 have these clocks built into the SAR ADC IP block

> 2) 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.
yes, I think we can restrict this later

[...]
> +        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>,
> +                <&clkc CLKID_SAR_ADC_CLK>,
> +                <&clkc CLKID_SAR_ADC_SEL>;
> +            clock-names = "clkin", "core";
CLKID_SAR_ADC_CLK and CLKID_SAR_ADC_SEL should not be in this Meson8b example

I have attached a patch for this as well as for making the clock
selection more restrictive (to catch errors like this). feel free to
include it in your patch if you think that it improves things


Best regards,
Martin
Jonathan Cameron Sept. 6, 2020, 12:12 p.m. UTC | #2 
On Sun, 6 Sep 2020 12:00:02 +0200
Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Jonathan,
> 
> On Sat, Sep 5, 2020 at 7:32 PM Jonathan Cameron <jic23@kernel.org> 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.  
> thank you for working on this!
> 
> > I am far from sure I have these correct. For example:
> > 1) Can we have the clock adc_sel without adc_clk?  
> adc_sel and adc_clk only exist on GXBB and newer
> Meson8, Meson8b and Meson8m2 have these clocks built into the SAR ADC IP block

Ah. That makes sense.

> 
> > 2) 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.  
> yes, I think we can restrict this later
> 
> [...]
> > +        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>,
> > +                <&clkc CLKID_SAR_ADC_CLK>,
> > +                <&clkc CLKID_SAR_ADC_SEL>;
> > +            clock-names = "clkin", "core";  
> CLKID_SAR_ADC_CLK and CLKID_SAR_ADC_SEL should not be in this Meson8b example
> 
> I have attached a patch for this as well as for making the clock
> selection more restrictive (to catch errors like this). feel free to
> include it in your patch if you think that it improves things

Great thanks. I'll roll your patch into this for v2.

Thanks,

Jonathan

> 
> 
> Best regards,
> Martin
diff mbox series

Patch

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..5b2e07828c3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.yaml
@@ -0,0 +1,145 @@ 
+# 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:
+    minItems: 2
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: clkin
+      - const: core
+      - const: adc_clk
+      - const: adc_sel
+    minItems: 2
+    maxItems: 4
+
+  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
+
+  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
+    else:
+      properties:
+        nvmem-cells: false
+        mvmem-cel-names: false
+
+  - 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>,
+                <&clkc CLKID_SAR_ADC_CLK>,
+                <&clkc CLKID_SAR_ADC_SEL>;
+            clock-names = "clkin", "core";
+            nvmem-cells = <&tsens_caldata>;
+            nvmem-cell-names = "temperature_calib";
+            amlogic,hhi-sysctrl = <&hhi>;
+        };
+    };
+...