Re: [PATCH v4 2/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver

On Wed, 2014-11-05 at 13:09 +0000, Jonathan Cameron wrote:

On 03/11/14 15:24, Ivan T. Ivanov wrote:

quoted

From: Stanimir Varbanov <redacted>

The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has
15 bits resolution and register space inside PMIC accessible across
SPMI bus.

The vadc driver registers itself through IIO interface.

Signed-off-by: Stanimir Varbanov <redacted>
Signed-off-by: Ivan T. Ivanov <redacted>

Hi Ivan,

Couple of utterly tiny bits inline.  The biggest one is that
you store some info about the calibration that you never actually
use... Left over from some debugging perhaps?

Jonathan

<snip>

quoted

+
+/*
+ * VADC_CALIB_ABSOLUTE: uses the 625mV and 1.25V as reference channels.
+ * VADC_CALIB_RATIOMETRIC: uses the reference voltage (1.8V) and GND for
+ * calibration.
+ */
+enum vadc_calibration {
+       VADC_CALIB_ABSOLUTE = 0,
+       VADC_CALIB_RATIOMETRIC
+};
+
+/**
+ * struct vadc_linear_graph - Represent ADC characteristics.
+ * @dy: numerator slope to calculate the gain.

As dy is always equal to vref-gnd you could drop it and use those
directly...

Conversly you store vref or grnd and never use them...

I am not sure I am following you. Please take a look in 
vadc_measure_ref_points() and vadc_calibrate().

quoted

+ * @dx: denominator slope to calculate the gain.
+ * @vref: A/D word of the voltage reference used for the channel.
+ * @gnd: A/D word of the ground reference used for the channel.
+ *
+ * Each ADC device has different offset and gain parameters which are
+ * computed to calibrate the device.
+ */
+struct vadc_linear_graph {
+       s32 dy;
+       s32 dx;
+       s32 vref;
+       s32 gnd;
+};
+

Will address blank line comment.

Regards,
Ivan