Skip to contentSkip to conversation

dt-bindings: iio: adc: Add DT binding document for PMIC5 ADC

PATCH In review Siddartha Mohanadoss 3 participants 5 messages May 8, 2018 – May 25, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 dt-bindings: iio: adc: Add DT binding document for PMIC5 ADC

PMIC5 ADC has support for clients to measure voltage and current on inputs connected to the PMIC. Clients include reading voltage phone power and on board system thermistors for thermal management. ADC5 on certain PMIC has support to read battery current.

This change adds documentation.

---
 .../devicetree/bindings/iio/adc/qcom,spmi-adc5.txt | 137 +++++++++++++++++++++
 1 file changed, 137 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +137 -0
new file mode 100644
index 0000000..c9268ba
@@ -0,0 +1,137 @@
+Qualcomm Technologies Inc. SPMI PMIC5 voltage and current ADC
+
+SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
+voltage. The VADC is a 16-bit sigma-delta ADC.
+
+ADC node:
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "qcom,spmi-adc5" for PMIC5 ADC driver.
+		Should contain "qcom,spmi-adc-rev2" for PMIC refresh ADC driver.
+
+- reg:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: VADC base address and length in the SPMI PMIC register map.
+
+- #address-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. Child node 'reg' property should define ADC
+            channel number.
+
+- #size-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be zero.
+
+- #io-channel-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. For details about IIO bindings see:
+            Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+- interrupts:
+    Usage: optional
+    Value type: <prop-encoded-array>
+    Definition: End of conversion interrupt.
+
+Channel node properties:
+
+- reg:
+    Usage: required
+    Value type: <u32>
+    Definition: ADC channel number.
+            See include/dt-bindings/iio/qcom,spmi-vadc.h
+
+- label:
+    Usage: required
+    Value type: <empty>
+    Definition: ADC datasheet channel name.
+            For thermistor inputs connected to generic AMUX or GPIO inputs
+            these can vary across platform for the same pins. Hence select
+            the datasheet name for this channel.
+
+- qcom,pre-scaling:
+    Usage: required
+    Value type: <u32 array>
+    Definition: Used for scaling the channel input signal before the signal is
+            fed to VADC. The configuration for this node is to know the
+            pre-determined ratio and use it for post scaling. Select one from
+            the following options.
+            <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10>
+            If property is not found default value depending on chip will be used.
+
+- qcom,decimation:
+    Usage: optional
+    Value type: <u32>
+    Definition: This parameter is used to decrease ADC sampling rate.
+            Quicker measurements can be made by reducing decimation ratio.
+            For PMIC5 ADC, combined two step decimation values are 250, 420 and 840.
+            If property is not found, default value of 840 will be used.
+	    For PMIC refresh ADC, supported decimation values are 256, 512, 1024.
+	    If property is not found, default value of 1024 will be used.
+
+- qcom,ratiometric:
+    Usage: optional
+    Value type: <empty>
+    Definition: Channel calibration type. If this property is specified
+            VADC will use the VDD reference (1.875V) and GND for channel
+            calibration. If property is not found, channel will be
+            calibrated with 0V and 1.25V reference channels, also
+            known as absolute calibration.
+
+- qcom,hw-settle-time:
+    Usage: optional
+    Value type: <u32>
+    Definition: Time between AMUX getting configured and the ADC starting
+            conversion.
+	    For PMIC5, delay = 15us for value 0,
+			100us * (value) for values 0 < value < 11, and
+            		2ms * (value - 10) otherwise.
+            Valid values are: 15, 100, 200, 300, 400, 500, 600, 700, 800,
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 15us.
+	    For PMIC rev2, delay = 100us * (value) for values 0 < value < 11, and
+			2ms * (value - 10) otherwise.
+            Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800,
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 0 us.
+
+- qcom,avg-samples:
+    Usage: optional
+    Value type: <u32>
+    Definition: Number of samples to be used for measurement.
+            Averaging provides the option to obtain a single measurement
+            from the ADC that is an average of multiple samples. The value
+            selected is 2^(value).
+            Valid values are: 1, 2, 4, 8, 16
+            If property is not found, 1 sample will be used.
+
+Example:
+
+        /* VADC node */
+        pmic_vadc: vadc@3100 {
+                compatible = "qcom,spmi-adc5";
+                reg = <0x3100 0x100>;
+                interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+                #io-channel-cells = <1>;
+                io-channel-ranges;
+
+                /* Channel node */
+                vph_pwr {
+                        reg = <ADC_VPH_PWR>;
+                        label = "vph_pwr";
+                        qcom,pre-scaling = <1 3>;
+                };
+        };
+
+        /* IIO client node */
+        usb {
+                io-channels = <&pmic_vadc ADC_VPH_PWR>;
+                io-channel-names = "vadc";
+        };
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#1 lore raw
Quoted 8 lines · On Tue, 8 May 2018 14:38:21 -0700 Siddartha Mohanadoss…

PMIC5 ADC has support for clients to measure voltage and current on inputs connected to the PMIC. Clients include reading voltage phone power and on board system thermistors for thermal management. ADC5 on certain PMIC has support to read battery current.

This change adds documentation.

Hi Siddartha,

Some complexity in here! Anyhow, a few comments inline and we will definitely be wanting guidance from the devicetree people for this one.

Jonathan

Quoted 74 lines
---
 .../devicetree/bindings/iio/adc/qcom,spmi-adc5.txt | 137 +++++++++++++++++++++
 1 file changed, 137 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +63 -0
new file mode 100644
index 0000000..c9268ba
@@ -0,0 +1,137 @@
+Qualcomm Technologies Inc. SPMI PMIC5 voltage and current ADC
+
+SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
+voltage. The VADC is a 16-bit sigma-delta ADC.
+
+ADC node:
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "qcom,spmi-adc5" for PMIC5 ADC driver.
+		Should contain "qcom,spmi-adc-rev2" for PMIC refresh ADC driver.
+
+- reg:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: VADC base address and length in the SPMI PMIC register map.
+
+- #address-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. Child node 'reg' property should define ADC
+            channel number.
+
+- #size-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be zero.
+
+- #io-channel-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. For details about IIO bindings see:
+            Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+- interrupts:
+    Usage: optional
+    Value type: <prop-encoded-array>
+    Definition: End of conversion interrupt.
+
+Channel node properties:
+
+- reg:
+    Usage: required
+    Value type: <u32>
+    Definition: ADC channel number.
+            See include/dt-bindings/iio/qcom,spmi-vadc.h
+
+- label:
+    Usage: required
+    Value type: <empty>
+    Definition: ADC datasheet channel name.
+            For thermistor inputs connected to generic AMUX or GPIO inputs
+            these can vary across platform for the same pins. Hence select
+            the datasheet name for this channel.
+
+- qcom,pre-scaling:
+    Usage: required
+    Value type: <u32 array>
+    Definition: Used for scaling the channel input signal before the signal is
+            fed to VADC. The configuration for this node is to know the
+            pre-determined ratio and use it for post scaling. Select one from
+            the following options.

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 73

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +10 -0
+            <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10>
+            If property is not found default value depending on chip will be used.
+
+- qcom,decimation:
+    Usage: optional
+    Value type: <u32>
+    Definition: This parameter is used to decrease ADC sampling rate.
+            Quicker measurements can be made by reducing decimation ratio.
+            For PMIC5 ADC, combined two step decimation values are 250, 420 and 840.
+            If property is not found, default value of 840 will be used.

The odd indenting here needs sorting. Mixture of spaces and tabs at the moment.

Hmm. In someways this is a policy decision so should be pushed up to userspace, but given the 'right' value will be somewhat dependent on what you are doing with the channel and what is wired to it, it could arguably have a 'right' value for a given circuit. This is really just the sampling frequency wrapped up in decimation of something, I'm guessing some input clock?

Let's see what the Device-tree people think on this one! Personally I have never really minded devicetree providing sensible defaults. We can put control on these things later, if there is a usecase for changing them.

Quoted 11 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +11 -0
+	    For PMIC refresh ADC, supported decimation values are 256, 512, 1024.
+	    If property is not found, default value of 1024 will be used.
+
+- qcom,ratiometric:
+    Usage: optional
+    Value type: <empty>
+    Definition: Channel calibration type. If this property is specified
+            VADC will use the VDD reference (1.875V) and GND for channel
+            calibration. If property is not found, channel will be
+            calibrated with 0V and 1.25V reference channels, also
+            known as absolute calibration.

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 94

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +10 -0
+
+- qcom,hw-settle-time:
+    Usage: optional
+    Value type: <u32>
+    Definition: Time between AMUX getting configured and the ADC starting
+            conversion.
+	    For PMIC5, delay = 15us for value 0,
+			100us * (value) for values 0 < value < 11, and
+            		2ms * (value - 10) otherwise.
+            Valid values are: 15, 100, 200, 300, 400, 500, 600, 700, 800,

This description is very confusing given the different uses of 'value' None of the values you have allowed is less than 11 so the first condition doesn't apply.

Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +7 -0
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 15us.
+	    For PMIC rev2, delay = 100us * (value) for values 0 < value < 11, and
+			2ms * (value - 10) otherwise.
+            Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800,
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 0 us.

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 111

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +10 -0
+
+- qcom,avg-samples:
+    Usage: optional
+    Value type: <u32>
+    Definition: Number of samples to be used for measurement.
+            Averaging provides the option to obtain a single measurement
+            from the ADC that is an average of multiple samples. The value
+            selected is 2^(value).
+            Valid values are: 1, 2, 4, 8, 16
+            If property is not found, 1 sample will be used.

As with decimation, this is arguably not a feature of the hardware, but a software decision...

Quoted 26 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +26 -0
+
+Example:
+
+        /* VADC node */
+        pmic_vadc: vadc@3100 {
+                compatible = "qcom,spmi-adc5";
+                reg = <0x3100 0x100>;
+                interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+                #io-channel-cells = <1>;
+                io-channel-ranges;
+
+                /* Channel node */
+                vph_pwr {
+                        reg = <ADC_VPH_PWR>;
+                        label = "vph_pwr";
+                        qcom,pre-scaling = <1 3>;
+                };
+        };
+
+        /* IIO client node */
+        usb {
+                io-channels = <&pmic_vadc ADC_VPH_PWR>;
+                io-channel-names = "vadc";
+        };
#2 lore raw

Hi Jonathan,

Thanks for the comments.

Quoted 95 lines · Jonathan Cameron
Quoted 8 lines · On Tue, 8 May 2018 14:38:21 -0700 Siddartha Mohanadoss…

PMIC5 ADC has support for clients to measure voltage and current on inputs connected to the PMIC. Clients include reading voltage phone power and on board system thermistors for thermal management. ADC5 on certain PMIC has support to read battery current.

This change adds documentation.

Hi Siddartha,

Some complexity in here! Anyhow, a few comments inline and we will definitely be wanting guidance from the devicetree people for this one.

Jonathan

Quoted 77 lines
---
  .../devicetree/bindings/iio/adc/qcom,spmi-adc5.txt | 137 +++++++++++++++++++++
  1 file changed, 137 insertions(+)

create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +66 -0
new file mode 100644
index 0000000..c9268ba
@@ -0,0 +1,137 @@
+Qualcomm Technologies Inc. SPMI PMIC5 voltage and current ADC
+
+SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
+voltage. The VADC is a 16-bit sigma-delta ADC.
+
+ADC node:
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "qcom,spmi-adc5" for PMIC5 ADC driver.
+		Should contain "qcom,spmi-adc-rev2" for PMIC refresh ADC driver.
+
+- reg:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: VADC base address and length in the SPMI PMIC register map.
+
+- #address-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. Child node 'reg' property should define ADC
+            channel number.
+
+- #size-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be zero.
+
+- #io-channel-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. For details about IIO bindings see:
+            Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+- interrupts:
+    Usage: optional
+    Value type: <prop-encoded-array>
+    Definition: End of conversion interrupt.
+
+Channel node properties:
+
+- reg:
+    Usage: required
+    Value type: <u32>
+    Definition: ADC channel number.
+            See include/dt-bindings/iio/qcom,spmi-vadc.h
+
+- label:
+    Usage: required
+    Value type: <empty>
+    Definition: ADC datasheet channel name.
+            For thermistor inputs connected to generic AMUX or GPIO inputs
+            these can vary across platform for the same pins. Hence select
+            the datasheet name for this channel.
+
+- qcom,pre-scaling:
+    Usage: required
+    Value type: <u32 array>
+    Definition: Used for scaling the channel input signal before the signal is
+            fed to VADC. The configuration for this node is to know the
+            pre-determined ratio and use it for post scaling. Select one from
+            the following options.
+            <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10>
+            If property is not found default value depending on chip will be used.
+
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 73

Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +7 -0
+- qcom,decimation:
+    Usage: optional
+    Value type: <u32>
+    Definition: This parameter is used to decrease ADC sampling rate.
+            Quicker measurements can be made by reducing decimation ratio.
+            For PMIC5 ADC, combined two step decimation values are 250, 420 and 840.
+            If property is not found, default value of 840 will be used.

The odd indenting here needs sorting. Mixture of spaces and tabs at the moment.

Ok, will take a look.

Quoted 5 lines

Hmm. In someways this is a policy decision so should be pushed up to userspace, but given the 'right' value will be somewhat dependent on what you are doing with the channel and what is wired to it, it could arguably have a 'right' value for a given circuit. This is really just the sampling frequency wrapped up in decimation of something, I'm guessing some input clock?

Yes. It's number of samples collected over a 4.8MHz clock.
The only reason to update this value would be if client wants to get the conversion results back sooner. Hence left this as an optional property.

Quoted 21 lines

Let's see what the Device-tree people think on this one! Personally I have never really minded devicetree providing sensible defaults. We can put control on these things later, if there is a usecase for changing them.

Quoted 16 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +16 -0
+	    For PMIC refresh ADC, supported decimation values are 256, 512, 1024.
+	    If property is not found, default value of 1024 will be used.
+
+- qcom,ratiometric:
+    Usage: optional
+    Value type: <empty>
+    Definition: Channel calibration type. If this property is specified
+            VADC will use the VDD reference (1.875V) and GND for channel
+            calibration. If property is not found, channel will be
+            calibrated with 0V and 1.25V reference channels, also
+            known as absolute calibration.
+
+- qcom,hw-settle-time:
+    Usage: optional
+    Value type: <u32>
+    Definition: Time between AMUX getting configured and the ADC starting
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 94

Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +5 -0
+            conversion.
+	    For PMIC5, delay = 15us for value 0,
+			100us * (value) for values 0 < value < 11, and
+            		2ms * (value - 10) otherwise.
+            Valid values are: 15, 100, 200, 300, 400, 500, 600, 700, 800,

This description is very confusing given the different uses of 'value' None of the values you have allowed is less than 11 so the first condition doesn't apply.

The 'value' is an index programmed in the hardware to achieve the hardware settling delay specified under valid values. I will update the documentation here.

Quoted 47 lines
Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +7 -0
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 15us.
+	    For PMIC rev2, delay = 100us * (value) for values 0 < value < 11, and
+			2ms * (value - 10) otherwise.
+            Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800,
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 0 us.

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 111

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +10 -0
+
+- qcom,avg-samples:
+    Usage: optional
+    Value type: <u32>
+    Definition: Number of samples to be used for measurement.
+            Averaging provides the option to obtain a single measurement
+            from the ADC that is an average of multiple samples. The value
+            selected is 2^(value).
+            Valid values are: 1, 2, 4, 8, 16
+            If property is not found, 1 sample will be used.

As with decimation, this is arguably not a feature of the hardware, but a software decision...

Quoted 26 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +26 -0
+
+Example:
+
+        /* VADC node */
+        pmic_vadc: vadc@3100 {
+                compatible = "qcom,spmi-adc5";
+                reg = <0x3100 0x100>;
+                interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+                #io-channel-cells = <1>;
+                io-channel-ranges;
+
+                /* Channel node */
+                vph_pwr {
+                        reg = <ADC_VPH_PWR>;
+                        label = "vph_pwr";
+                        qcom,pre-scaling = <1 3>;
+                };
+        };
+
+        /* IIO client node */
+        usb {
+                io-channels = <&pmic_vadc ADC_VPH_PWR>;
+                io-channel-names = "vadc";
+        };
maintainer
#3 lore raw
Quoted 39 lines · Siddartha Mohanadoss

Hi Jonathan,

Thanks for the comments.

Quoted 33 lines · Jonathan Cameron
Quoted 8 lines · On Tue, 8 May 2018 14:38:21 -0700 Siddartha Mohanadoss…

PMIC5 ADC has support for clients to measure voltage and current on inputs connected to the PMIC. Clients include reading voltage phone power and on board system thermistors for thermal management. ADC5 on certain PMIC has support to read battery current.

This change adds documentation.

Hi Siddartha,

Some complexity in here! Anyhow, a few comments inline and we will definitely be wanting guidance from the devicetree people for this one.

Jonathan

Quoted 15 lines
---
  .../devicetree/bindings/iio/adc/qcom,spmi-adc5.txt | 137 +++++++++++++++++++++
  1 file changed, 137 insertions(+)

create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +4 -0
new file mode 100644
index 0000000..c9268ba
@@ -0,0 +1,137 @@
+Qualcomm Technologies Inc. SPMI PMIC5 voltage and current ADC
+
+SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
+voltage. The VADC is a 16-bit sigma-delta ADC.
Quoted 8 lines
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 12

Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +8 -0
+
+ADC node:
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "qcom,spmi-adc5" for PMIC5 ADC driver.
+		Should contain "qcom,spmi-adc-rev2" for PMIC refresh ADC driver.

Chip specific compatible strings please unless you convince me there are a large number of chips per above compatible.

Bindings are for hardware, not drivers.

Quoted 35 lines
Quoted 35 lines
Quoted 35 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +35 -0
+
+- reg:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: VADC base address and length in the SPMI PMIC register map.
+
+- #address-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. Child node 'reg' property should define ADC
+            channel number.
+
+- #size-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be zero.
+
+- #io-channel-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. For details about IIO bindings see:
+            Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+- interrupts:
+    Usage: optional
+    Value type: <prop-encoded-array>
+    Definition: End of conversion interrupt.
+
+Channel node properties:
+
+- reg:
+    Usage: required
+    Value type: <u32>
+    Definition: ADC channel number.
+            See include/dt-bindings/iio/qcom,spmi-vadc.h
Quoted 8 lines
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 55

Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +8 -0
+
+- label:
+    Usage: required
+    Value type: <empty>
+    Definition: ADC datasheet channel name.
+            For thermistor inputs connected to generic AMUX or GPIO inputs
+            these can vary across platform for the same pins. Hence select
+            the datasheet name for this channel.

Why do you need this? If the name comes from a datasheet list, then perhaps you should list the exact string here. Otherwise, there's a lot left to the user in terms of capitalization, etc.

Quoted 20 lines
Quoted 11 lines
Quoted 11 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +11 -0
+
+- qcom,pre-scaling:
+    Usage: required
+    Value type: <u32 array>
+    Definition: Used for scaling the channel input signal before the signal is
+            fed to VADC. The configuration for this node is to know the
+            pre-determined ratio and use it for post scaling. Select one from
+            the following options.
+            <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10>
+            If property is not found default value depending on chip will be used.
+
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 73

Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +7 -0
+- qcom,decimation:
+    Usage: optional
+    Value type: <u32>
+    Definition: This parameter is used to decrease ADC sampling rate.
+            Quicker measurements can be made by reducing decimation ratio.
+            For PMIC5 ADC, combined two step decimation values are 250, 420 and 840.
+            If property is not found, default value of 840 will be used.

The odd indenting here needs sorting. Mixture of spaces and tabs at the moment.

Ok, will take a look.

Quoted 12 lines
Quoted 5 lines

Hmm. In someways this is a policy decision so should be pushed up to userspace, but given the 'right' value will be somewhat dependent on what you are doing with the channel and what is wired to it, it could arguably have a 'right' value for a given circuit. This is really just the sampling frequency wrapped up in decimation of something, I'm guessing some input clock?

Yes. It's number of samples collected over a 4.8MHz clock.
The only reason to update this value would be if client wants to get the conversion results back sooner. Hence left this as an optional property.

Quoted 4 lines

Let's see what the Device-tree people think on this one! Personally I have never really minded devicetree providing sensible defaults. We can put control on these things later, if there is a usecase for changing them.

I don't have a problem with this in DT, though my first thought was it should be common. Then after reading some on decimation, I'm not sure it would always just be a single 32-bit value?

Quoted 25 lines
Quoted 17 lines
Quoted 17 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +16 -0
+	    For PMIC refresh ADC, supported decimation values are 256, 512, 1024.
+	    If property is not found, default value of 1024 will be used.
+
+- qcom,ratiometric:
+    Usage: optional
+    Value type: <empty>
+    Definition: Channel calibration type. If this property is specified
+            VADC will use the VDD reference (1.875V) and GND for channel
+            calibration. If property is not found, channel will be
+            calibrated with 0V and 1.25V reference channels, also
+            known as absolute calibration.
+
+- qcom,hw-settle-time:
+    Usage: optional
+    Value type: <u32>
+    Definition: Time between AMUX getting configured and the ADC starting
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 94

Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +5 -0
+            conversion.
+	    For PMIC5, delay = 15us for value 0,
+			100us * (value) for values 0 < value < 11, and
+            		2ms * (value - 10) otherwise.
+            Valid values are: 15, 100, 200, 300, 400, 500, 600, 700, 800,

This description is very confusing given the different uses of 'value' None of the values you have allowed is less than 11 so the first condition doesn't apply.

Quoted 23 lines

The 'value' is an index programmed in the hardware to achieve the hardware settling delay specified under valid values. I will update the documentation here.

Quoted 20 lines
Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +7 -0
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 15us.
+	    For PMIC rev2, delay = 100us * (value) for values 0 < value < 11, and
+			2ms * (value - 10) otherwise.
+            Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800,
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 0 us.

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 111

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +10 -0
+
+- qcom,avg-samples:
+    Usage: optional
+    Value type: <u32>
+    Definition: Number of samples to be used for measurement.
+            Averaging provides the option to obtain a single measurement
+            from the ADC that is an average of multiple samples. The value
+            selected is 2^(value).
+            Valid values are: 1, 2, 4, 8, 16
+            If property is not found, 1 sample will be used.

As with decimation, this is arguably not a feature of the hardware, but a software decision...

We already have a common property for touchscreens and vendor properties for a few ADCs, so we should define a common one.

Now, sadly, I've just found that all these properties are already defined in bindings/iio/adc/qcom,spmi-vadc.txt. Why didn't you add these compatibles to the existing binding. Then we're not reviewing the same thing again...

Rob

#4 lore raw

Hi Rob,

Thanks for the comments.

Quoted 42 lines · Rob Herring
Quoted 41 lines · Siddartha Mohanadoss

Hi Jonathan,

Thanks for the comments.

Quoted 35 lines · Jonathan Cameron
Quoted 8 lines · On Tue, 8 May 2018 14:38:21 -0700 Siddartha Mohanadoss…

PMIC5 ADC has support for clients to measure voltage and current on inputs connected to the PMIC. Clients include reading voltage phone power and on board system thermistors for thermal management. ADC5 on certain PMIC has support to read battery current.

This change adds documentation.

Hi Siddartha,

Some complexity in here! Anyhow, a few comments inline and we will definitely be wanting guidance from the devicetree people for this one.

Jonathan

Quoted 17 lines
---
   .../devicetree/bindings/iio/adc/qcom,spmi-adc5.txt | 137 +++++++++++++++++++++
   1 file changed, 137 insertions(+)

create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +6 -0
new file mode 100644
index 0000000..c9268ba
@@ -0,0 +1,137 @@
+Qualcomm Technologies Inc. SPMI PMIC5 voltage and current ADC
+
+SPMI PMIC5 voltage ADC (ADC) provides interface to clients to read
+voltage. The VADC is a 16-bit sigma-delta ADC.
+
+ADC node:
Quoted 8 lines
Quoted 6 lines
Quoted 6 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 12

Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +6 -0
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "qcom,spmi-adc5" for PMIC5 ADC driver.
+		Should contain "qcom,spmi-adc-rev2" for PMIC refresh ADC driver.

Chip specific compatible strings please unless you convince me there are a large number of chips per above compatible.

The above compatible property supports at least 5 different PMIC's each so far, hence split it as a separate property to address the differences.

Quoted 41 lines

Bindings are for hardware, not drivers.

Quoted 38 lines
Quoted 38 lines
Quoted 38 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +38 -0
+
+- reg:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: VADC base address and length in the SPMI PMIC register map.
+
+- #address-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. Child node 'reg' property should define ADC
+            channel number.
+
+- #size-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be zero.
+
+- #io-channel-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Must be one. For details about IIO bindings see:
+            Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+- interrupts:
+    Usage: optional
+    Value type: <prop-encoded-array>
+    Definition: End of conversion interrupt.
+
+Channel node properties:
+
+- reg:
+    Usage: required
+    Value type: <u32>
+    Definition: ADC channel number.
+            See include/dt-bindings/iio/qcom,spmi-vadc.h
+
+- label:
+    Usage: required
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 55

Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +5 -0
+    Value type: <empty>
+    Definition: ADC datasheet channel name.
+            For thermistor inputs connected to generic AMUX or GPIO inputs
+            these can vary across platform for the same pins. Hence select
+            the datasheet name for this channel.

Why do you need this? If the name comes from a datasheet list, then perhaps you should list the exact string here. Otherwise, there's a lot left to the user in terms of capitalization, etc.

I will fix the comment. The label property name used is from the schematics for the respective platform and not the data sheet. Some of the channel names on the PMIC datasheet are generic such as ADC_AMUX_THM1 and ADC_GPIO1. Depending on the platform these pins can be connected to measure different thermistors or inputs across the platforms.

Quoted 29 lines
Quoted 29 lines
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +11 -0
+
+- qcom,pre-scaling:
+    Usage: required
+    Value type: <u32 array>
+    Definition: Used for scaling the channel input signal before the signal is
+            fed to VADC. The configuration for this node is to know the
+            pre-determined ratio and use it for post scaling. Select one from
+            the following options.
+            <1 1>, <1 3>, <1 4>, <1 6>, <1 20>, <1 8>, <10 81>, <1 10>
+            If property is not found default value depending on chip will be used.
+
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 73

Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +7 -0
+- qcom,decimation:
+    Usage: optional
+    Value type: <u32>
+    Definition: This parameter is used to decrease ADC sampling rate.
+            Quicker measurements can be made by reducing decimation ratio.
+            For PMIC5 ADC, combined two step decimation values are 250, 420 and 840.
+            If property is not found, default value of 840 will be used.

The odd indenting here needs sorting. Mixture of spaces and tabs at the moment.

Ok, will take a look.

Quoted 5 lines

Hmm. In someways this is a policy decision so should be pushed up to userspace, but given the 'right' value will be somewhat dependent on what you are doing with the channel and what is wired to it, it could arguably have a 'right' value for a given circuit. This is really just the sampling frequency wrapped up in decimation of something, I'm guessing some input clock?

Yes. It's number of samples collected over a 4.8MHz clock.
The only reason to update this value would be if client wants to get the conversion results back sooner. Hence left this as an optional property.

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

Let's see what the Device-tree people think on this one! Personally I have never really minded devicetree providing sensible defaults. We can put control on these things later, if there is a usecase for changing them.

I don't have a problem with this in DT, though my first thought was it should be common. Then after reading some on decimation, I'm not sure it would always just be a single 32-bit value?

For this PMIC family, the decimation and fast averaging are common settings used across all the channels. In this case the decimation ratio values specified are programmed as an index to select the above decimation value. Not sure if i answered the question. Please let me know if this needs further clarification.

Quoted 47 lines
Quoted 25 lines
Quoted 17 lines
Quoted 17 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +16 -0
+	    For PMIC refresh ADC, supported decimation values are 256, 512, 1024.
+	    If property is not found, default value of 1024 will be used.
+
+- qcom,ratiometric:
+    Usage: optional
+    Value type: <empty>
+    Definition: Channel calibration type. If this property is specified
+            VADC will use the VDD reference (1.875V) and GND for channel
+            calibration. If property is not found, channel will be
+            calibrated with 0V and 1.25V reference channels, also
+            known as absolute calibration.
+
+- qcom,hw-settle-time:
+    Usage: optional
+    Value type: <u32>
+    Definition: Time between AMUX getting configured and the ADC starting
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 94

Quoted 5 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +5 -0
+            conversion.
+	    For PMIC5, delay = 15us for value 0,
+			100us * (value) for values 0 < value < 11, and
+            		2ms * (value - 10) otherwise.
+            Valid values are: 15, 100, 200, 300, 400, 500, 600, 700, 800,

This description is very confusing given the different uses of 'value' None of the values you have allowed is less than 11 so the first condition doesn't apply.

Quoted 22 lines

The 'value' is an index programmed in the hardware to achieve the hardware settling delay specified under valid values. I will update the documentation here.

Quoted 19 lines
Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +7 -0
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 15us.
+	    For PMIC rev2, delay = 100us * (value) for values 0 < value < 11, and
+			2ms * (value - 10) otherwise.
+            Valid values are: 0, 100, 200, 300, 400, 500, 600, 700, 800,
+            900 us and 1, 2, 4, 6, 8, 10 ms
+            If property is not found, channel will use 0 us.

Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt line 111

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt new file +10 -0
+
+- qcom,avg-samples:
+    Usage: optional
+    Value type: <u32>
+    Definition: Number of samples to be used for measurement.
+            Averaging provides the option to obtain a single measurement
+            from the ADC that is an average of multiple samples. The value
+            selected is 2^(value).
+            Valid values are: 1, 2, 4, 8, 16
+            If property is not found, 1 sample will be used.

As with decimation, this is arguably not a feature of the hardware, but a software decision...

Quoted 7 lines

We already have a common property for touchscreens and vendor properties for a few ADCs, so we should define a common one.

Now, sadly, I've just found that all these properties are already defined in bindings/iio/adc/qcom,spmi-vadc.txt. Why didn't you add these compatibles to the existing binding. Then we're not reviewing the same thing again...

I figured it would be easier for the client to follow and cleaner to separate it out for a per family otherwise it needs a separate definition for each of the property based on the PMIC type.

The earlier PMIC ADC family has capability to specify the decimation and fast averaging for each channel. On PMIC5 ADC these two properties are common across all channels and are required to be programmed only once. The hardware settling delays have differences within the family. I do not have a preference either way, i can try and merge these with the existing bindings for the next patch revision and based on the comments i can either revert or use the existing one.

Quoted 6 lines

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html