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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
Reviewed-by: Sylwester Nawrocki<s.nawrocki@xxxxxxxxxxx>
Signed-off-by: Kyungmin Park<kyungmin.park@xxxxxxxxxxx>
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

--
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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux