On 09/17/2012 11:35 AM, Kim, Milo wrote:

 TI LP8788 PMU provides regulators, battery charger, ADC,
 RTC, backlight driver and current sinks.

 This patch enables the LP8788 ADC functions.

 The LP8788 ADC has several ADC input selection and supports 12bit resolution.
 Internal operation of getting ADC is access to registers of LP8788.
 The LP8788 ADC uses exported functions for accessing these registers.
 (exported by LP8788 MFD device driver)

 This driver supports IIO_CHAN_INFO_RAW and SCALE.
 So the IIO consumer can calculate the value with raw and scale.
 The unit of scale is micro.

 (ADC Input Selection)

 Voltage: battery voltage (MAX 5.0, 5.5 and 6.0V)
          charger input voltage
          four general ADC inputs
          coin cell voltage
 Current: battery charging current
 Temperature: IC temperature

 (The IIO map for the IIO consumer)

 The ADC input is configurable in the platform side.
 Even though this platform data is not defined,
 the default IIO map is created for supporting the power supply driver.
 The battery voltage and temperature are used inside this driver.

 (History)

 Patch v6.
 (a) Fix scale value for each ADC input selection
 Voltage and current type are mili unit and temperature is degree.
 To calculate the IC temperature,
 temp = raw * scaleint + (raw * scalepart)/ 1000000, scaleint is always 0.
      = raw * 0.061050, raw: 0 ~ 4095
 Then range of IC temperature(ADC result) is 0 ~ 250'C

 (b) Reorganization of the IIO channel Spec
 Remove address, scan_type and scan_index and rollback the datasheet name.
 The reason why 'address' field is unnecessary is no relation with each channel.
 Moreover, to get the raw ADC value, the address info is not only one register
 but also several registers.
 Therefore specific function(lp8788_get_adc_result) is called rather than
 using one 'address' field.

 (c) Fix coding style
 Remove duplicated checking routine while unregistering the IIO map.
 Fix code for space and parenthesis.

 Patch v5.
 Fix default consumer name as 'lp8788-charger'.
 Add mutex for ADC read operation.
 Reorganization on lp8788_adc_read_raw().

 Patch v4.
 Fix adc_raw function: support RAW and SCALE channel info.
 Change LP8788 ADC platform data - iio map.
 Enables the default IIO map.

 Patch v3.
 Fix wrong size of allocating iio private data.
 Fix coding styles.

 Patch v2.
 Support RAW and SCALE interface for IIO consumer.
 Clean up the iio channel spec macro.

Signed-off-by: Milo(Woogyom) Kim <redacted>

Looks good to me,

Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>

One comment though, not sure if it is critical or not.

+static int lp8788_get_adc_result(struct lp8788_adc *adc, enum lp8788_adc_id id,
+				int *val)
+{
+	unsigned int msb;
+	unsigned int lsb;
+	unsigned int result;
+	u8 data;
+	u8 rawdata[2];
+	int size = ARRAY_SIZE(rawdata);
+	int retry = 5;
+	int ret;
+
+	data = (id << 1) | ADC_CONV_START;
+	ret = lp8788_write_byte(adc->lp, LP8788_ADC_CONF, data);
+	if (ret)
+		goto err_io;
+
+	/* retry until adc conversion is done */
+	data = 0;
+	while (retry--) {
+		usleep_range(100, 200);
+
+		ret = lp8788_read_byte(adc->lp, LP8788_ADC_DONE, &data);
+		if (ret)
+			goto err_io;
+
+		/* conversion done */
+		if (data)
+			break;

Could as well go into the while header like this:
    while (!data && retry--)

+	}
+

You still sample the data, even if there was a timeout and ADC_DONE is not set.

+	ret = lp8788_read_multi_bytes(adc->lp, LP8788_ADC_RAW, rawdata, size);
+	if (ret)
+		goto err_io;
+
+	msb = (rawdata[0] << 4) & 0x00000ff0;
+	lsb = (rawdata[1] >> 4) & 0x0000000f;
+	result = msb | lsb;
+	*val = result;
+
+	return 0;
+
+err_io:
+	return ret;
+}