Hello,
2015-04-27 21:55 GMT+03:00 Peter Meerwald <pmeerw@xxxxxxxxxx>:
>
>> m62332 is a simple 2-channel DAC used on several Sharp Zaurus boards to
>> control LCD voltage, backlight and sound. The driver use regulators to
>> control the reference voltage and enabling/disabling the DAC.
>
> some minor comments below
Thank you for the review. I had a followup question, see below.
I'll have to submit a V4 anyway -- adding IIO_CHAN_INFO_OFFSET
to reflect correct values being generated by DAC.
>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>
>> ---
>> Changes since v2:
>> * Added M62332_CHANNELS to be used instead of magic value 2
>> * Changed the probe funciton handles error cases
>>
[skipped]
>> +
>> + if (val)
>> + res = regulator_enable(data->vcc);
>> + if (res)
>> + goto out;
>
> I would find it marginally clearer to do
> if (val) {
> res = regulator_enable(..);
> if (res)
> goto out;
> }
> and leave res uninitialized (feel free to ignore)
Ack
>
>> +
>> + res = i2c_master_send(client, outbuf, 2);
>> + if (res >= 0 && res != 2)
[skipped]
>> +
>> +static int m62332_write_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan, int val, int val2, long mask)
>> +{
>> + int ret;
>> +
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>
> see write_raw_get_fmt, the default is IIO_VAL_INT_PLUS_MICRO; probably you
> want to overwrite that function to return IIO_VAL_INT
>
> or at least check val2 == 0
This is strange. First, according to iio_write_channel_info(),
write_raw_get_fmt()
should not return IIO_VAL_INT. Also none of the DAC write_raw functions that
I have checked actually make use of val2 or check that it is 0.
[skipped]
--
With best wishes
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
