Re: [PATCH] lis3lv02d: Add STMicroelectronics lis33ldlh digital

[Arnd Bergmann]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/