Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754827AbaKEN5U (ORCPT ); Wed, 5 Nov 2014 08:57:20 -0500 Received: from ns.mm-sol.com ([37.157.136.199]:56789 "EHLO extserv.mm-sol.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753898AbaKEN5S (ORCPT ); Wed, 5 Nov 2014 08:57:18 -0500 Message-ID: <1415195855.3660.4.camel@mm-sol.com> Subject: Re: [PATCH v4 2/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver From: "Ivan T. Ivanov" To: Jonathan Cameron Cc: Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald , Stanimir Varbanov , Angelo Compagnucci , Grant Likely , linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org Date: Wed, 05 Nov 2014 15:57:35 +0200 In-Reply-To: <545A218F.5060301@kernel.org> References: <1415028270-25860-1-git-send-email-iivanov@mm-sol.com> <1415028270-25860-3-git-send-email-iivanov@mm-sol.com> <545A218F.5060301@kernel.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.13.6-fta3 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2014-11-05 at 13:09 +0000, Jonathan Cameron wrote: > On 03/11/14 15:24, Ivan T. Ivanov wrote: > > From: Stanimir Varbanov > > > > 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 > > Signed-off-by: Ivan T. Ivanov > 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 > > + > > +/* > > + * 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(). > > > + * @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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/