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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue,  8 May 2018 14:38:21 -0700
Siddartha Mohanadoss <smohanad@xxxxxxxxxxxxxx> wrote:

> 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.
> 
> Signed-off-by: Siddartha Mohanadoss <smohanad@xxxxxxxxxxxxxx>
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

> ---
>  .../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
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
> new file mode 100644
> index 0000000..c9268ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-adc5.txt
> @@ -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.

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.

> +	    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,

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.

> +            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.

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

> +
> +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";
> +        };

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux