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
From: Jean Delvare <hidden> Date: 2012-01-18 14:44:12
Hi Arnd and all,
The lis3lv02d driver was moved outside of drivers/hwmon to make it
clear that we (hwmon/lm-sensors people) are not involved in maintaining
this driver in any way. So please remove the lm-sensors list (and
myself) from this discussion.
Thanks,
--
Jean Delvare
Hi Arnd and all,
The lis3lv02d driver was moved outside of drivers/hwmon to make it
clear that we (hwmon/lm-sensors people) are not involved in maintaining
this driver in any way. So please remove the lm-sensors list (and
myself) from this discussion.
Ok, I see. What about the applesmc driver then? If we come up with
a solution that works for the other drivers, should the accelerometer
part of that be converted to use that interface?
Arnd
From: Jean Delvare <hidden> Date: 2012-01-18 15:01:12
On Wed, 18 Jan 2012 14:49:35 +0000, Arnd Bergmann wrote:
On Wednesday 18 January 2012, Jean Delvare wrote:
quoted
Hi Arnd and all,
The lis3lv02d driver was moved outside of drivers/hwmon to make it
clear that we (hwmon/lm-sensors people) are not involved in maintaining
this driver in any way. So please remove the lm-sensors list (and
myself) from this discussion.
Ok, I see. What about the applesmc driver then? If we come up with
a solution that works for the other drivers, should the accelerometer
part of that be converted to use that interface?
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)
For what it is worth there are a total of nine iio acceleration drivers. Some are special purpose though.
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?
Personal thought is that straight 3d devices very directed at input should go there. Iio has a few wrinkles left for the in kernel interfaces needed for a bridge to input. For starters the pull and push data interfaces have not merged yet (in staging) and we have not started on in kernel acess to non data events yet(thresholds). Still happy to take them when we can support usecases fully.
Of course any help getting there would be most welcome!
Jonathan
Arnd
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
On Wednesday 18 January 2012, Jonathan Cameron wrote:
Arnd Bergmann [off-list ref] wrote:
quoted
Any opinions where they should live? input, iio or a new subsystem?
Personal thought is that straight 3d devices very directed at input
should go there. Iio has a few wrinkles left for the in kernel
interfaces needed for a bridge to input. For starters the pull
and push data interfaces have not merged yet (in staging) and we
have not started on in kernel acess to non data events yet(thresholds).
Still happy to take them when we can support usecases fully.
Ok, thanks for the information, sounds all reasonable. I think we
can definitely say no to new accelerometer drivers in drivers/misc
then and refer them to iio and input.
In case of lis33ldlh, I would like to see it move out at some point
once the necessary parts of iio have graduated out of staging.
Hopefully it won't be a large amount of work to convert it to
a different internal API.
AnilKumar, what are the existing users of the driver? Is it still
possible to convert them to use iio without causing problems for
people that rely on this driver, or do we have to keep supporting the
current interface?
Arnd
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:
quoted
Hi All,
Recalling the patch, provide the comments if there are any if not please include
this patch to v3.3 kernel.
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/
Hi Arnd,
On Wed, Jan 18, 2012 at 22:41:15, Arnd Bergmann wrote:
On Wednesday 18 January 2012, Jonathan Cameron wrote:
quoted
Arnd Bergmann [off-list ref] wrote:
quoted
Any opinions where they should live? input, iio or a new subsystem?
Personal thought is that straight 3d devices very directed at input
should go there. Iio has a few wrinkles left for the in kernel
interfaces needed for a bridge to input. For starters the pull
and push data interfaces have not merged yet (in staging) and we
have not started on in kernel acess to non data events yet(thresholds).
Still happy to take them when we can support usecases fully.
Ok, thanks for the information, sounds all reasonable. I think we
can definitely say no to new accelerometer drivers in drivers/misc
then and refer them to iio and input.
In case of lis33ldlh, I would like to see it move out at some point
once the necessary parts of iio have graduated out of staging.
Hopefully it won't be a large amount of work to convert it to
a different internal API.
AnilKumar, what are the existing users of the driver? Is it still
possible to convert them to use iio without causing problems for
people that rely on this driver, or do we have to keep supporting the
current interface?
Android userspace running on TI AM335x EVM is using the interface
provided by lis3lv02d. They were asking some more interfaces from
lis3lvo2d driver.
There are multiple ways we can interface accelerometer to Android layers,
which is implemented on hardware abstraction layer (HAL) in Andriod.
1) Interrupt mode
2) Polling mode
2a) Kernel polling
2b) Timer polling
Based on the interfaces provided by the lis3lv02d as well as
lis331dlh (H/W not supporting the interrupts) they were implementing
the kernel polling mechanism.
So implementation on HAL is like this if accelerometer interface is
opened then kernel will start polling this driver periodically and
pass events to input subsystem. (It's a little bit over head)
Generally the device should be open but kernel should only poll
when an app that uses accelerometer is started.
The biggest requirement for them (Andriod people) is to allow user to
enable / disable accelerometer from user space and to configure
the accelerometer polling frequency.
Today there is no option in lis3lvo2d driver to provide this kind
of functionalities
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.
The problem is that we have some drivers using the input subsystem, some
drivers using the iio subsystem and some drivers using none of these but
interfacing with user space using some homegrown method, see my list:
quoted
$ 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)
If all drivers were using the input subsystem in the way you describe,
that would be a good solution, but right now the majority of the
drivers don't do this.
Arnd
On Thursday 19 January 2012, AnilKumar, Chimata wrote:
Android userspace running on TI AM335x EVM is using the interface
provided by lis3lv02d. They were asking some more interfaces from
lis3lvo2d driver.
There are multiple ways we can interface accelerometer to Android layers,
which is implemented on hardware abstraction layer (HAL) in Andriod.
1) Interrupt mode
2) Polling mode
2a) Kernel polling
2b) Timer polling
Based on the interfaces provided by the lis3lv02d as well as
lis331dlh (H/W not supporting the interrupts) they were implementing
the kernel polling mechanism.
So implementation on HAL is like this if accelerometer interface is
opened then kernel will start polling this driver periodically and
pass events to input subsystem. (It's a little bit over head)
Generally the device should be open but kernel should only poll
when an app that uses accelerometer is started.
The biggest requirement for them (Andriod people) is to allow user to
enable / disable accelerometer from user space and to configure
the accelerometer polling frequency.
Today there is no option in lis3lvo2d driver to provide this kind
of functionalities
Hi AnilKumar,
This all sounds like the interface is not completely thought through.
I did not realize that the driver actually uses the input subsystem
in addition to its own interfaces. This is definitely good, and it
means that we can move the files from drivers/misc to drivers/input/misc
or drivers/input/accelerometer, making it Dmitry's problem instead of
mine ;-)
Having custom user interfaces inside an input driver however is very
bad. I'm sure that other accelerometers will have the same requirements
regarding polling frequency and enable/disable in android as well as
anytwhere else and it should absolutely not be handled by a user space
HAL but instead inside of the kernel, using a common method for all
available drivers.
Based on that, I also doubt we should apply your patch to add the
"range" attribute (adding support for lis33ldlh is fine though).
Instead I would ask you to first fix what's there by moving the
user space interfaces into the input core from the driver
and documenting them.
I don't know what the preferred way for doing things is there
(joystick ioctl, sysfs attribute, ...) but Dmitry should
be able to provide advice there. Then add interfaces for the
additional stuff you need (range, disable, ...) in the same
place and implement them as callbacks in the driver itself.
Arnd
From: Jonathan Cameron <hidden> Date: 2012-01-19 17:15:44
Arnd Bergmann [off-list ref] wrote:
On Wednesday 18 January 2012, Andi wrote:
quoted
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
quoted
input driver.
The input driver provides already some accelerometer specific event
types, ABS_X, ABS_Y, ABS_Z,
quoted
in your opinion isn't it enough?
If you mean something like collecting common properties like g range
or frequency or whatever in a
quoted
standard interface, then I think that accelerometers are quite
different as devices and sometimes
quoted
it could be difficult to arrange a common interface.
The problem is that we have some drivers using the input subsystem,
some
drivers using the iio subsystem and some drivers using none of these
but
interfacing with user space using some homegrown method, see my list:
quoted
quoted
$ 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)
If all drivers were using the input subsystem in the way you describe,
that would be a good solution, but right now the majority of the
drivers don't do this.
Also worth noting that there are numerous devices that are completely inappropriate for human input. Take specialised impact and vibration accelerometers. Very high speed sampling for short time periods. I agree that your bog standard 3 axis sub 10 g accelerometer belongs in input. The kxsd9 and lis3lo2dq in iio are there for historical reasons and because they are very handy for testing our bridging to input. That said it should ultimately make very little difference to user space if they are accessing a true input device or a iio device via an input bridge. I also think that actually there is very little in the way of unusual controls for accelerometers particularly those designed for human input. Not standardizing gain or sampling frequency control is lazy. Even filtering and fft contr
ols can and should be standardised. Doing that is a large part of what iio is about.
Arnd
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
From: Mark Brown <hidden> Date: 2012-01-19 17:29:38
On Thu, Jan 19, 2012 at 05:10:45PM +0000, Arnd Bergmann wrote:
Having custom user interfaces inside an input driver however is very
bad. I'm sure that other accelerometers will have the same requirements
regarding polling frequency and enable/disable in android as well as
anytwhere else and it should absolutely not be handled by a user space
HAL but instead inside of the kernel, using a common method for all
available drivers.
The polling frequency thing isn't even accelerometer specific, there's a
general need for this on a wide range of other things like touchscreens
(where controlling the sample rate is interesting for power, there are
widely varying requirements depending on what the screen is doing).
Hi Arnd,
On Thu, Jan 19, 2012 at 22:40:45, Arnd Bergmann wrote:
On Thursday 19 January 2012, AnilKumar, Chimata wrote:
quoted
Android userspace running on TI AM335x EVM is using the interface
provided by lis3lv02d. They were asking some more interfaces from
lis3lvo2d driver.
There are multiple ways we can interface accelerometer to Android layers,
which is implemented on hardware abstraction layer (HAL) in Andriod.
1) Interrupt mode
2) Polling mode
2a) Kernel polling
2b) Timer polling
Based on the interfaces provided by the lis3lv02d as well as
lis331dlh (H/W not supporting the interrupts) they were implementing
the kernel polling mechanism.
So implementation on HAL is like this if accelerometer interface is
opened then kernel will start polling this driver periodically and
pass events to input subsystem. (It's a little bit over head)
Generally the device should be open but kernel should only poll
when an app that uses accelerometer is started.
The biggest requirement for them (Andriod people) is to allow user to
enable / disable accelerometer from user space and to configure
the accelerometer polling frequency.
Today there is no option in lis3lvo2d driver to provide this kind
of functionalities
Hi AnilKumar,
This all sounds like the interface is not completely thought through.
I did not realize that the driver actually uses the input subsystem
in addition to its own interfaces. This is definitely good, and it
means that we can move the files from drivers/misc to drivers/input/misc
or drivers/input/accelerometer, making it Dmitry's problem instead of
mine ;-)
Having custom user interfaces inside an input driver however is very
bad. I'm sure that other accelerometers will have the same requirements
regarding polling frequency and enable/disable in android as well as
anytwhere else and it should absolutely not be handled by a user space
HAL but instead inside of the kernel, using a common method for all
available drivers.
Based on that, I also doubt we should apply your patch to add the
"range" attribute (adding support for lis33ldlh is fine though).
Instead I would ask you to first fix what's there by moving the
user space interfaces into the input core from the driver
and documenting them.
I don't know what the preferred way for doing things is there
(joystick ioctl, sysfs attribute, ...) but Dmitry should
be able to provide advice there. Then add interfaces for the
additional stuff you need (range, disable, ...) in the same
place and implement them as callbacks in the driver itself.
I will remove the range attribute and I will submit v2. The rest of
the patch deals with adding support for lis33ldlh and should be fairly
non-controversial. We can revisit the range attribute addition at a
later time.
Regards
AnilKumar