lkml.org 
[lkml]   [2016]   [Aug]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V4 1/1] iio: as6200: add AS6200 temperature sensor driver from ams AG
On Mon, Aug 15, 2016 at 06:25:44PM +0100, Jonathan Cameron wrote:
> On 04/08/16 09:35, Florian Lobmaier wrote:
> > Hello Peter,
> >
> > Thanks again for your valuable feedback. We use now IIO_EV_THRESH to
> > set high and low limits for temperature. Also removed all the custom
> > ABI as this are mainly settings which will be set one-time only. For
> > the removed custom ABI init defines where introduced which will be
> > written to the registers in the probe function. The remaining custom
> > ABI is now documented as well as the device tree bindings.>
> > Br,
> > Florian
> >
> > Signed-off-by: Florian Lobmaier <florian.lobmaier@ams.com>
> > Signed-off-by: Elitsa Polizoeva <elitsa.polizoeva@ams.com>
> Please post as a fresh email thread with a clean title. Otherwise
> people will assume it is simply a reply to a comment on an earlier
> version. Also don't include earlier versions as you have here!
>
> i.e. drop the RE from the title as it's confusing!
>
> Anyhow, right back at v1 Peter mentioned that this might be more
> suitable as a hwmon driver than an IIO one. If you have a good reason
> for supporting this part via IIO you should put it in the patch
> description. I'm afraid I've been more or less offline for the last
> couple fo weeks or I'd have highlighted that this question was
> important. A superficial look suggest to me that this is definitely
> a part targeting hardware monitoring applications.
>
Conversion time, conversion rate, the presence of limit registers,
and the intended use cases suggest that this should be a hardware
monitoring driver.

Regarding the attributes, most of those would be standard attributes
in a hwmon driver. "alarm polarity" should be a devicetree / platform
data configuration parameter, and interrupt vs. comparator mode could
be determined based on the presence of an interrupt line.

> I'll only take a border line part with agreement form Guenter / Jean
> who are the hwmon maintainers.
>
> I'll do a quick review ignoring this question.
> Mostly pretty good though I really don't like the remaining
> custom ABI. Can't see a reason for its existence.

Me not either.

Guenter

\
 
 \ /
  Last update: 2016-09-17 09:57    [W:0.049 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site