Hello Guenter, hello Jonathan,

we went for developing an IIO driver as the intended use-cases for the AS6200 
temperature sensor are biosensors as well as environmental sensors. It is 
currently designed-in in an Android wearable device to measure the skin 
temperature. So we did not think hwmon is the right place. Of course it may 
also be used for such purposes, but due to its tiny size its more intended for 
wearable devices and smartphones to measure skin- or outside air temperature.

Regarding the remaining custom ABI you are right. We will move this setting to 
the device tree. Makes much more sense there ;)

Maybe you can give an answer regarding the hwmon/iio topic before we submit an 
update?

Thank you for your help,
Florian Lobmaier

-----Original Message-----
From: Guenter Roeck [mailto:[email protected]] 
Sent: Montag, 15. August 2016 21:43
To: Jonathan Cameron <[email protected]>
Cc: Florian Lobmaier <[email protected]>; Peter Meerwald-Stadler 
<[email protected]>; Elitsa Polizoeva <[email protected]>; 
[email protected]; [email protected]; [email protected]; 
Lars-Peter Clausen <[email protected]>; Jean Delvare <[email protected]>; 
[email protected]
Subject: Re: [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 <[email protected]>
> > Signed-off-by: Elitsa Polizoeva <[email protected]>
> 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

Reply via email to