Re: [PATCH] lis3lv02d: Add STMicroelectronics lis33ldlh digital

[Andi <andi.shyti@gmail.com>]

Hi Arnd,

 
What do you mean with "kernel-wide policy for accelerometer drivers"?
As far as I know, accelerometer drivers are written between the i2c driver and the
input driver.
The input driver provides already some accelerometer specific event types, ABS_X, ABS_Y, ABS_Z,
in your opinion isn't it enough?
If you mean something like collecting common properties like g range or frequency or whatever in a
standard interface, then I think that accelerometers are quite different as devices and sometimes
it could be difficult to arrange a common interface.

Andi


On Wed, Jan 18, 2012 at 02:03:47PM +0000, Arnd Bergmann wrote:
> On Tuesday 17 January 2012, AnilKumar, Chimata wrote:
> > Hi All,
> > 
> > Recalling the patch, provide the comments if there are any if not please include
> > this patch to v3.3 kernel.
> 
> As Mark and Greg said, 3.4 would be appropriate.
> 
> > +static ssize_t lis3lv02d_range_set(struct device *dev,
> > +				struct device_attribute *attr, const char *buf,
> > +				size_t count)
> > +{
> > +	unsigned long range;
> > +
> > +	if (strict_strtoul(buf, 0, &range))
> > +		return -EINVAL;
> > +
> > +	lis3_dev.g_range = range;
> > +	lis3lv02d_update_g_range(&lis3_dev);
> > +
> > +	return count;
> > +}
> > +
> >  static DEVICE_ATTR(selftest, S_IRUSR, lis3lv02d_selftest_show, NULL);
> >  static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
> >  static DEVICE_ATTR(rate, S_IRUGO | S_IWUSR, lis3lv02d_rate_show,
> >  					    lis3lv02d_rate_set);
> > +static DEVICE_ATTR(range, S_IRUGO | S_IWUSR, lis3lv02d_range_show,
> > +					    lis3lv02d_range_set);
> 
> I think you need to document this new attribute in the Documentation
> directory, unless I missed the other patch doing this.
> 
> On a more general topic, do we have a kernel-wide policy for accelerometer
> drivers? AFAICT, we currently have three subsystems that contain 
> accelerometer drivers, plus a few ad-hoc ones like this, which is a
> rather unpleasant situation. What I found are these:
> 
> $ git grep -l accelerometer drivers/ | manual_grep
> drivers/hwmon/applesmc.c
>    (one hwmon sysfs attribute for x/y/z)
> drivers/input/misc/adxl34x.c
>    (lots of sysfs attributes, input_report_key)
> drivers/input/misc/cma3000_d0x.c
>    (input_report_abs)
> drivers/input/misc/kxtj9.c
>    (input_report_abs, plus one aux sysfs attribute)
> drivers/macintosh/ams/ams-core.c
>    (one sysfs attribute for x/y/z)
> drivers/misc/lis3lv02d/
>    (multiple sysfs attributes)
> drivers/platform/x86/hdaps.c
>    (multiple sysfs attributes, only x/y)
> drivers/platform/x86/hp_accel.c
>    (hooks into drivers/misc/lis3lv02d/)
> drivers/staging/iio/accel/kxsd9.c
>    (iio)
> drivers/staging/iio/accel/sca3000_core.c
>    (iio plus extra attributes)
> 
> While I'm not blaming you for the current situation, but I think the
> situation is no longer sustainable and we need to decide on one place
> and interface for these to go in the long run so we don't grow even
> more nonstandard interfaces.
> 
> Any opinions where they should live? input, iio or a new subsystem?
> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/