Re: [NEW DRIVER] iio: lps331ap: Add new driver for the device

[Jacek Anaszewski <j.anaszewski@samsung.com>]

On 05/18/2013 12:18 PM, Jonathan Cameron wrote:
> On 05/10/2013 10:09 AM, Jacek Anaszewski wrote:
>> Add new driver for the barometer device. The driver is
>> compiliant with IIO framework, and exposes two channels
>> for reading the pressure and the temperature. The output
>> data can be read either in 'one shot' mode or by exploiting
>> IIO events.
>>
>> Signed-off-by: Jacek Anaszewski<j.anaszewski@samsung.com>
>> Reviewed-by: Sylwester Nawrocki<s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
> Hi All,
>
> As I said the other day I'll review this driver irrespective
> of what is going on with Denis' alternative.  It is a useful
> exercise to look at alternative drivers for parts anyway
> and I hope the feedback may prove useful.
>
> This is mostly a nice driver.  Few bits and bobs inline.
>
> There are some places where a bit of simple reorganization can
> lead to code that is easier to review (thus making me happy).
> You can also hopefully assume the core code isn't calling
> your callbacks with invalid combinations so drop some error
> checking (if it is I certainly want to know about it!)
>
> Also I'd like to see the stanard i2c helper functions used
> (using the simple byte and word ones where possible).
> That will reduce the length of the code a fair bit by
> not reinventing the wheel...
>
> All in all a driver that did not make for an unpleasant hour on a
> Saturday morning :)
>
> Jonathan

Hi Jonathan,

Thanks for the thorough review. I will certainly keep your remarks
in mind and apply them in my future patches.

Thanks,
Jacek