Accelerometer, Gyros and ADC's etc within the kernel.

Accelerometer, Gyros and ADC's etc within the kernel.

[Jonathan Cameron]

This email is basically a request for opinions on how and where such sensors
should be integrated into the kernel.

To set the scene...

Increasing numbers of embedded devices are being supplied attached MEMS
devices (www.xbow.com imote2 etc). Along with more traditional sensors such
as ADC's not being used for hardware monitoring, these do not really 
seem to
fit with in an particular subsystem of the kernel.  A previous 
discussion on
lkml in 2006 considered the accelerometers to be found within some laptop
hard drives, but I haven't been able to track down any more general 
discussions
of such non hardware monitoring sensors.

The obvious possibilities are:

* To place the various drivers within the spi / i2c etc subsystems as 
relevant.

* To place within the hwmon subsystem as this is probably closest.
(there is already at least one straight ADC driver in hwmon)

* To create a new subsystem, or perhaps merely sysfs class to contain these
  elements.

Typical requirements within an application include simply polling for 
current
readings, and using device triggered interrupts to grab data 
continuously to a
ring buffer, for collection by suitable userspace code.  Obviously it 
would be
desirable to standardize sysfs controls for various calibration 
parameters as
much as possible across the various devices.

Any other suggestions welcome!

To illustrate the sort of devices here are a few I have drivers written 
for or will
shortly be writing  (some submitted to hwmon and spi mailing lists, some 
not
finished as of yet)

ST Micro LIS3L02DQ 3D accelerometer.  SPI device, no buffering, 
interrupt pin
raised high on new data being available. Currently the driver assumes, if
interrupts are enabled, that this is connected to a specified gpio.
(http://www.st.com/stonline/books/pdf/docs/10175.pdf)

VTI SCA3000 E05 3D accelerometer equiped with substantial ring buffer. SPI
device.  Can operate either in buffered or direct mode.  In buffered 
mode, interrupts
can be used to indicate when the ring buffer is 3/4 full triggering a 
download to
a larger ring buffer in the kernel if necessary.
(http://www.vti.fi/en/products-solutions/products/accelerometers/sca3000-accelerometers/)

Analog Devices ADIS16350 combined 3D accelerometer and gyro unit.  SPI 
device.
(http://www.analog.com/en/prod/0,,764_801_ADIS16350%2C00.html)

Maxim MAX1363, MAX1238 ADC's.  I2C devices. Some SPI ADC's 
(www.analog.com for
examples)

Would be nice if practical to allow the framework to include RS232 
devices such
as those from www.xsens.com, www.isense.com and others.

--
Jonathan Cameron

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[Jean Delvare]

Hi Jonathan,

On Tue, 20 May 2008 11:04:01 +0100, Jonathan Cameron wrote:
> This email is basically a request for opinions on how and where such sensors
> should be integrated into the kernel.
> 
> To set the scene...
> 
> Increasing numbers of embedded devices are being supplied attached MEMS
> devices (www.xbow.com imote2 etc). Along with more traditional sensors such
> as ADC's not being used for hardware monitoring, these do not really 
> seem to
> fit with in an particular subsystem of the kernel.  A previous 
> discussion on
> lkml in 2006 considered the accelerometers to be found within some laptop
> hard drives, but I haven't been able to track down any more general 
> discussions
> of such non hardware monitoring sensors.
> 
> The obvious possibilities are:
> 
> * To place the various drivers within the spi / i2c etc subsystems as 
> relevant.

Bad idea. Grouping drivers by connectivity is almost always the wrong
thing to do, as it means that different persons will maintain them and
they have all chances to diverge. You really want to group drivers by
functionality. On top of that, I am busy enough maintaining the i2c
core and bus drivers without having more i2c device drivers added to my
plate. These days I am trying to _empty_ drivers/i2c/chips, if anything.

> * To place within the hwmon subsystem as this is probably closest.
> (there is already at least one straight ADC driver in hwmon)

Probably not the wisest choice, if nothing else, because the hwmon
maintainer is already overloaded. And I don't think that these devices
have much in common with the traditional hardware monitoring components
anyway.

I'm not sure what "straight ADC driver" you are referring to, but
anything which measures a voltage and exports the reading to user-space
is, well, a voltage sensor, and thus fix within hwmon. If the same chip
is used for a higher-level, dedicated function then we would probably
have a separate driver for it, outside of hwmon.

> * To create a new subsystem, or perhaps merely sysfs class to contain these
>   elements.

Would be OK. Or:

* Place these within the input subsystem. You might want to discuss
this with the input subsystem maintainer Dmitry Torokhov (Cc'd). The
Wii remote is essentially a joystick, and joysticks belong to the input
subsystem.

> Typical requirements within an application include simply polling for 
> current
> readings, and using device triggered interrupts to grab data 
> continuously to a
> ring buffer, for collection by suitable userspace code.  Obviously it 
> would be
> desirable to standardize sysfs controls for various calibration 
> parameters as
> much as possible across the various devices.
> 
> Any other suggestions welcome!

This all sounds quite different from our hwmon drivers. Our hwmon
drivers read all the sensor values at once and cache the readings for a
couple seconds, so you can't get an instant reading at any time, and
they also don't support interrupts in general.

-- 
Jean Delvare

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[Hans J. Koch]

On Tue, May 20, 2008 at 01:28:17PM +0200, Jean Delvare wrote:
> Hi Jonathan,
> 
> On Tue, 20 May 2008 11:04:01 +0100, Jonathan Cameron wrote:
> > This email is basically a request for opinions on how and where such sensors
> > should be integrated into the kernel.
> > 
> > To set the scene...
> > 
> > Increasing numbers of embedded devices are being supplied attached MEMS
> > devices (www.xbow.com imote2 etc). Along with more traditional sensors such
> > as ADC's not being used for hardware monitoring, these do not really 
> > seem to
> > fit with in an particular subsystem of the kernel.  A previous 
> > discussion on
> > lkml in 2006 considered the accelerometers to be found within some laptop
> > hard drives, but I haven't been able to track down any more general 
> > discussions
> > of such non hardware monitoring sensors.
> > 
> > The obvious possibilities are:
> > 
> > * To place the various drivers within the spi / i2c etc subsystems as 
> > relevant.
> 
> Bad idea. Grouping drivers by connectivity is almost always the wrong
> thing to do, as it means that different persons will maintain them and
> they have all chances to diverge. You really want to group drivers by
> functionality. On top of that, I am busy enough maintaining the i2c
> core and bus drivers without having more i2c device drivers added to my
> plate. These days I am trying to _empty_ drivers/i2c/chips, if anything.

100% ACK. And the functionality here is something like "industrial
control" or "automation I/O". If this sort of hardware appears as device
with mappable memory, we can handle it with UIO, but for SPI, I2C, USB,
serial, we should have a new subsystem. It should handle not only input,
but also similar output devices. It doesn't make sense to have ADCs in
one subsystem, and DACs in a different one.

> 
> > * To place within the hwmon subsystem as this is probably closest.
> > (there is already at least one straight ADC driver in hwmon)
> 
> Probably not the wisest choice, if nothing else, because the hwmon
> maintainer is already overloaded. And I don't think that these devices
> have much in common with the traditional hardware monitoring components
> anyway.

Agreed, hwmon devices are not really tuned for maximum performance, for
example. Performance is often critical in automation control
applications.

> 
> I'm not sure what "straight ADC driver" you are referring to, but
> anything which measures a voltage and exports the reading to user-space
> is, well, a voltage sensor, and thus fix within hwmon. If the same chip
> is used for a higher-level, dedicated function then we would probably
> have a separate driver for it, outside of hwmon.
> 
> > * To create a new subsystem, or perhaps merely sysfs class to contain these
> >   elements.
> 
> Would be OK.

Definitely.

> Or:
> 
> * Place these within the input subsystem. You might want to discuss
> this with the input subsystem maintainer Dmitry Torokhov (Cc'd). The
> Wii remote is essentially a joystick, and joysticks belong to the input
> subsystem.

This might apply to some devices, but not all. And the requirements are
quite different, I think.

> 
> This all sounds quite different from our hwmon drivers. Our hwmon
> drivers read all the sensor values at once and cache the readings for a
> couple seconds, so you can't get an instant reading at any time, and
> they also don't support interrupts in general.

Exactly.

Thanks,
Hans

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[Jonathan Cameron]

Hi Hans and Jean, 

Thanks for your responses.
> On Tue, May 20, 2008 at 01:28:17PM +0200, Jean Delvare wrote:
>   
>> Hi Jonathan,
>>
>> On Tue, 20 May 2008 11:04:01 +0100, Jonathan Cameron wrote:
>>     
>>> This email is basically a request for opinions on how and where such sensors
>>> should be integrated into the kernel.
>>>
>>> To set the scene...
>>>
>>> Increasing numbers of embedded devices are being supplied attached MEMS
>>> devices (www.xbow.com imote2 etc). Along with more traditional sensors such
>>> as ADC's not being used for hardware monitoring, these do not really 
>>> seem to
>>> fit with in an particular subsystem of the kernel.  A previous 
>>> discussion on
>>> lkml in 2006 considered the accelerometers to be found within some laptop
>>> hard drives, but I haven't been able to track down any more general 
>>> discussions
>>> of such non hardware monitoring sensors.
>>>
>>> The obvious possibilities are:
>>>
>>> * To place the various drivers within the spi / i2c etc subsystems as 
>>> relevant.
>>>       
>> Bad idea. Grouping drivers by connectivity is almost always the wrong
>> thing to do, as it means that different persons will maintain them and
>> they have all chances to diverge. You really want to group drivers by
>> functionality. On top of that, I am busy enough maintaining the i2c
>> core and bus drivers without having more i2c device drivers added to my
>> plate. These days I am trying to _empty_ drivers/i2c/chips, if anything.
>>     
I can certainly see your point there. 
>
> 100% ACK. And the functionality here is something like "industrial
> control" or "automation I/O".
Hmm.. Going to be interesting coming up with a name!   Guess that can 
come at a
later stage anyway.
>  If this sort of hardware appears as device
> with mappable memory, we can handle it with UIO, but for SPI, I2C, USB,
> serial, we should have a new subsystem. It should handle not only input,
> but also similar output devices. It doesn't make sense to have ADCs in
> one subsystem, and DACs in a different one.
>   
Definitely agreed that it makes sense to put DACs and ADCs in the same 
general place
and obviously there are plenty of devices out there that do both.
>>> * To place within the hwmon subsystem as this is probably closest.
>>> (there is already at least one straight ADC driver in hwmon)
>>>       
>> Probably not the wisest choice, if nothing else, because the hwmon
>> maintainer is already overloaded. And I don't think that these devices
>> have much in common with the traditional hardware monitoring components
>> anyway.
>>     
>
> Agreed, hwmon devices are not really tuned for maximum performance, for
> example. Performance is often critical in automation control
> applications.
>   
It would indeed by stretching the point to make drivers in that 
subsystem have more performance
critical drivers!  We seem to have a consensus that neither hwmon or 
i2c/chips is good idea.
>> I'm not sure what "straight ADC driver" you are referring to, but
>> anything which measures a voltage and exports the reading to user-space
>> is, well, a voltage sensor, and thus fix within hwmon. If the same chip
>> is used for a higher-level, dedicated function then we would probably
>> have a separate driver for it, outside of hwmon.
>>
>>     
>>> * To create a new subsystem, or perhaps merely sysfs class to contain these
>>>   elements.
>>>       
>> Would be OK.
>>     
> Definitely.
>
>   
>> Or:
>>
>> * Place these within the input subsystem. You might want to discuss
>> this with the input subsystem maintainer Dmitry Torokhov (Cc'd). The
>> Wii remote is essentially a joystick, and joysticks belong to the input
>> subsystem.
>>     
>
> This might apply to some devices, but not all. And the requirements are
> quite different, I think.
>   
Although I don't know much about the input subsystem it doesn't seem 
likely that all the devices
we are considering would share all that much functionality with the 
devices in there, particularly if
as you have suggested, we include DACs.  
>> This all sounds quite different from our hwmon drivers. Our hwmon
>> drivers read all the sensor values at once and cache the readings for a
>> couple seconds, so you can't get an instant reading at any time, and
>> they also don't support interrupts in general.
>>     
>
> Exactly.
>   
Ok, so the consensus so far is we are looking at a new subsystem.  Will 
give this a few more days
on the lists, then if the consensus is still heading that way, I'll have 
a think about exact functionality
this would need.  I guess it would be something that would probably 
evolve for some time, centered
around an initial half dozen or so drivers.

Thanks for all your comments,

--
Jonathan Cameron

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[Jean Delvare]

On Wed, 21 May 2008 11:04:38 +0100, Jonathan Cameron wrote:
> Definitely agreed that it makes sense to put DACs and ADCs in the same 
> general place and obviously there are plenty of devices out there that do
> both.

Note that hardware monitoring chips can include ADCs and DACs: the
former to monitor system voltages, and the later to control fans (most
chips use PWM for that but a few use DACs.) ADCs and DACs are, much
like GPIOs, usable for a variety of different purposes. I doubt that
there would be any benefit in abstracting DACs and ADCs the way we did
for GPIOs, but the important point here is that we really want to group
the drivers according to their functionality and not technical
implementation details.

-- 
Jean Delvare

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[Dmitry Torokhov]

On Tue, May 20, 2008 at 01:28:17PM +0200, Jean Delvare wrote:
> Hi Jonathan,
> 
> On Tue, 20 May 2008 11:04:01 +0100, Jonathan Cameron wrote:
> > This email is basically a request for opinions on how and where such sensors
> > should be integrated into the kernel.
> > 
> > To set the scene...
> > 
> > Increasing numbers of embedded devices are being supplied attached MEMS
> > devices (www.xbow.com imote2 etc). Along with more traditional sensors such
> > as ADC's not being used for hardware monitoring, these do not really 
> > seem to
> > fit with in an particular subsystem of the kernel.  A previous 
> > discussion on
> > lkml in 2006 considered the accelerometers to be found within some laptop
> > hard drives, but I haven't been able to track down any more general 
> > discussions
> > of such non hardware monitoring sensors.
> > 
> > The obvious possibilities are:
> > 
> > * To place the various drivers within the spi / i2c etc subsystems as 
> > relevant.
> 
> Bad idea. Grouping drivers by connectivity is almost always the wrong
> thing to do, as it means that different persons will maintain them and
> they have all chances to diverge. You really want to group drivers by
> functionality. On top of that, I am busy enough maintaining the i2c
> core and bus drivers without having more i2c device drivers added to my
> plate. These days I am trying to _empty_ drivers/i2c/chips, if anything.
> 
> > * To place within the hwmon subsystem as this is probably closest.
> > (there is already at least one straight ADC driver in hwmon)
> 
> Probably not the wisest choice, if nothing else, because the hwmon
> maintainer is already overloaded. And I don't think that these devices
> have much in common with the traditional hardware monitoring components
> anyway.
> 
> I'm not sure what "straight ADC driver" you are referring to, but
> anything which measures a voltage and exports the reading to user-space
> is, well, a voltage sensor, and thus fix within hwmon. If the same chip
> is used for a higher-level, dedicated function then we would probably
> have a separate driver for it, outside of hwmon.
> 
> > * To create a new subsystem, or perhaps merely sysfs class to contain these
> >   elements.
> 
> Would be OK. Or:
> 
> * Place these within the input subsystem. You might want to discuss
> this with the input subsystem maintainer Dmitry Torokhov (Cc'd). The
> Wii remote is essentially a joystick, and joysticks belong to the input
> subsystem.
> 

I don't think that input subsystem woudl be the best choice. While we
do support the event-driven mechanism for delivering information to
userspace input is mostly oriented for human interface devices, not
general data acquisition. If anything input_dev is too fat. Another
thing is that input layer anonymizes input devices, makes them
capability-oriented. I.e. we dont really care what model of joystick
we have, we want something that reports ABS_X, ABS_Y, BTN_FIRE and
userpsace can use it whether it a simple analog joystick or something
fancy and USB connected. For your purposes you probably do want to know
what the device is and what exactly being sampled/measured.

I'd go with a lean and mean new subsytem.

-- 
Dmitry

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[Henrique de Moraes Holschuh]

On Wed, 21 May 2008, Dmitry Torokhov wrote:
> I don't think that input subsystem woudl be the best choice. While we
> do support the event-driven mechanism for delivering information to
> userspace input is mostly oriented for human interface devices, not
> general data acquisition. If anything input_dev is too fat. Another
> thing is that input layer anonymizes input devices, makes them
> capability-oriented. I.e. we dont really care what model of joystick
> we have, we want something that reports ABS_X, ABS_Y, BTN_FIRE and
> userpsace can use it whether it a simple analog joystick or something
> fancy and USB connected. For your purposes you probably do want to know
> what the device is and what exactly being sampled/measured.
> 
> I'd go with a lean and mean new subsytem.

Agreed.  I have put some thought on it in the past, and while the input
subsystem is the only thing even remotely usable for this *right now*, a
new subsystem for data acquisition would be the best way forward.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Ben Dooks]

On Wed, May 21, 2008 at 09:49:42AM -0400, Dmitry Torokhov wrote:
> On Tue, May 20, 2008 at 01:28:17PM +0200, Jean Delvare wrote:
> > Hi Jonathan,
> > 
> > On Tue, 20 May 2008 11:04:01 +0100, Jonathan Cameron wrote:
> > > This email is basically a request for opinions on how and where such sensors
> > > should be integrated into the kernel.
> > > 
> > > To set the scene...
> > > 
> > > Increasing numbers of embedded devices are being supplied attached MEMS
> > > devices (www.xbow.com imote2 etc). Along with more traditional sensors such
> > > as ADC's not being used for hardware monitoring, these do not really 
> > > seem to
> > > fit with in an particular subsystem of the kernel.  A previous 
> > > discussion on
> > > lkml in 2006 considered the accelerometers to be found within some laptop
> > > hard drives, but I haven't been able to track down any more general 
> > > discussions
> > > of such non hardware monitoring sensors.
> > > 
> > > The obvious possibilities are:
> > > 
> > > * To place the various drivers within the spi / i2c etc subsystems as 
> > > relevant.
> > 
> > Bad idea. Grouping drivers by connectivity is almost always the wrong
> > thing to do, as it means that different persons will maintain them and
> > they have all chances to diverge. You really want to group drivers by
> > functionality. On top of that, I am busy enough maintaining the i2c
> > core and bus drivers without having more i2c device drivers added to my
> > plate. These days I am trying to _empty_ drivers/i2c/chips, if anything.

I concur for these sorts of devices, where the connectivity to the
device is quite an easy part of the driver, thus you end up with tiny
little bus drivers all over the place, and a main core driver. The kernel
build system should allow easy selection of interface depending on what
is available without huge #ifdef hacks. An example could be:

Makefile:
mysensor-y		:= mysensor-core.o
mysensor-$(CONFIG_SPI)	+= mysensor-spi.o
mysensor-$(CONFIG_I2C)	+= mysensor-i2c.o
obj-$(CONFIG_MYSENSOR)	+= mysensor.o

> > > * To place within the hwmon subsystem as this is probably closest.
> > > (there is already at least one straight ADC driver in hwmon)
> > 
> > Probably not the wisest choice, if nothing else, because the hwmon
> > maintainer is already overloaded. And I don't think that these devices
> > have much in common with the traditional hardware monitoring components
> > anyway.
> > 
> > I'm not sure what "straight ADC driver" you are referring to, but
> > anything which measures a voltage and exports the reading to user-space
> > is, well, a voltage sensor, and thus fix within hwmon. If the same chip
> > is used for a higher-level, dedicated function then we would probably
> > have a separate driver for it, outside of hwmon.
> > 
> > > * To create a new subsystem, or perhaps merely sysfs class to contain these
> > >   elements.
> > 
> > Would be OK. Or:
> > 
> > * Place these within the input subsystem. You might want to discuss
> > this with the input subsystem maintainer Dmitry Torokhov (Cc'd). The
> > Wii remote is essentially a joystick, and joysticks belong to the input
> > subsystem.
> > 
> 
> I don't think that input subsystem woudl be the best choice. While we
> do support the event-driven mechanism for delivering information to
> userspace input is mostly oriented for human interface devices, not
> general data acquisition. If anything input_dev is too fat. Another
> thing is that input layer anonymizes input devices, makes them
> capability-oriented. I.e. we dont really care what model of joystick
> we have, we want something that reports ABS_X, ABS_Y, BTN_FIRE and
> userpsace can use it whether it a simple analog joystick or something
> fancy and USB connected. For your purposes you probably do want to know
> what the device is and what exactly being sampled/measured.

Can you explain to me how the input system anonymizes things? I thought
it was quite easy to read the name of the device? You could easily add
a symlink from the driver's device binding in sysfs to the relevant
input device as well.

Also, why would you not want all your accelerometers grouped into a set
of accelerometers? If your application software just searches all input
devices for anything reporting itself as an accelerometer, then it would
get the right result... 

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Dmitry Torokhov]

Hi Ben,

On Tue, May 27, 2008 at 06:16:56PM +0100, Ben Dooks wrote:
> > 
> > I don't think that input subsystem woudl be the best choice. While we
> > do support the event-driven mechanism for delivering information to
> > userspace input is mostly oriented for human interface devices, not
> > general data acquisition. If anything input_dev is too fat. Another
> > thing is that input layer anonymizes input devices, makes them
> > capability-oriented. I.e. we dont really care what model of joystick
> > we have, we want something that reports ABS_X, ABS_Y, BTN_FIRE and
> > userpsace can use it whether it a simple analog joystick or something
> > fancy and USB connected. For your purposes you probably do want to know
> > what the device is and what exactly being sampled/measured.
> 
> Can you explain to me how the input system anonymizes things? I thought
> it was quite easy to read the name of the device? You could easily add
> a symlink from the driver's device binding in sysfs to the relevant
> input device as well.
> 

Yes, it is possible to read the name of the device, but I could see
with names like "Thermal probe blah" it may not be unique enough. Of
course input core also exports phys, sysfs and uniq attributes so
there are certainly possibilities for using it.

OTOH input_dev is quite fat and oriented for human interface. It could
made work if you create one device per sensor but devices with X
sensorts per one device (that provide related readings, like a generic
state) would be cludgy... Accelerometers could use ABS_X, ABS_Y.. but
what about other devices? ABS_RAW1, ABS_RAW2? And here we run into the
conceptual problem, as I see it - in input we try to keep meaning of
the event independent from the device it generated. We dont care what
generated ABS_X, ABS_Y, BTN_FIRE - it could be a joystick, gamepad,
something similar. A program can use it if it knows how handle the
above events.  The ABS_RAWx would mean different things for every
sensor... That's what I meant by anonymizing - the application
does not really care about the particular device and where and how it
is connected, but only about its capabilities.

Another thing is that (at least now) input supresses duplicate events,
which may be undesirable for generic data acquisition device.

I suppose it could be made work. I guess the size of input_dev is my
main concern. If you hyave something with hundreds of sensors
input_dev might be too expensive.

> Also, why would you not want all your accelerometers grouped into a set
> of accelerometers? If your application software just searches all input
> devices for anything reporting itself as an accelerometer, then it would
> get the right result... 

I never said I would not want them to be grouped together.

-- 
Dmitry

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[David Brownell]

On Tuesday 20 May 2008, Jean Delvare wrote:
> This all sounds quite different from our hwmon drivers. Our hwmon
> drivers read all the sensor values at once and cache the readings for a
> couple seconds, so you can't get an instant reading at any time, and
> they also don't support interrupts in general.

That's something of an issue when the hardware monitor
triggers alarms though, right?  I couldn't figure out
how to get a TMP75 alert out to any userspace code that
would care, to pick one example, even after teaching
the i2c core how to handle SMBus alerts.  ;)

I suppose this just emphasizes the point made by Hans
that we kind of need (industrial) "control" components
(or "data acquisition"?) not just "monitoring" ones.

Such alarms are of course different from the "data
ready" signals of an accelerometer.

- Dave

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Jonathan Cameron]

Hi David,
> On Tuesday 20 May 2008, Jean Delvare wrote:
>> This all sounds quite different from our hwmon drivers. Our hwmon
>> drivers read all the sensor values at once and cache the readings for a
>> couple seconds, so you can't get an instant reading at any time, and
>> they also don't support interrupts in general.
> 
> That's something of an issue when the hardware monitor
> triggers alarms though, right?  I couldn't figure out
> how to get a TMP75 alert out to any userspace code that
> would care, to pick one example, even after teaching
> the i2c core how to handle SMBus alerts.  ;)
> 
> I suppose this just emphasizes the point made by Hans
> that we kind of need (industrial) "control" components
> (or "data acquisition"?) not just "monitoring" ones.
I get the feeling even the name is going to evolve for some time to come, 
getting broader as and when we add more classes of device.
> Such alarms are of course different from the "data
> ready" signals of an accelerometer.
Whilst this is true, most accelerometers also have "alarm" triggers, for example
the ST LIS3L02DQ will raise a pin on configurable conditions such as thresholds
and even a double click detector (though I've never tried them!).  I suppose we
will ideally support this functionality as well.
--
Jonathan Cameron

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Jonathan Cameron]

Dear All,

This email is intended to summarize the results of the discussions over
the last week or so and to open up to further comments on the way foward.

The original question was related to garnering opinions on how and where to
support devices such as accelerometers, gyros, general purpose ADCs (and
possibly DACs).  Typical interfaces are SPI and I2C, rates varying from 10Hz
to many kHz.  Real time and buffered access needed depending on application.

Conclusions:

 1.  The hwmon subsystem is unsuitable as it is intended to provide
relatively low frequency cached updates of sensor readings.

2. Definitely a bad idea to simply put such drivers in the relevant bus
subsystems due to unmaintainability and probable long term interface
divergence due to differing maintainers.

3. Whilst input subsystem is probably the only place that these sorts
of device could be currently included this would come at the cost of
input_dev being too fat and undesirable anonymization of the devices

4. Some stuff to be learned from comedi project (www.comedi.org).
Based on a quick browse of their documentation it looks like there
may be  stuff to learn on what sort of userspace interfaces are interesting
but the drivers themselves are (as Matt originally stated) io-port based.

5. General consensus seems to be on a new subsytem (we'll bash out a
suitable name later - for now I don't want a name effectively guiding
the functionality!)

What's next

Obviously the exact functionality of this subsystem is going to be somewhat
up in the air until we have a wide range of devices supported.

The minimum needed seems to be a control interface to configure the devices
and some level of event interface (similar to input subsystem) to 
indicate to
userspace programs that something of interest to them has occured.
As configuration of the devices is a relatively infrequent event
(at least in the applications I work with), it seems to me that a 
suitable sys
interface, under a new class would provide this.

Types of parameter:

Chip related:
read/write -- Offsets, gains, sensor biases.
read only -- Factory set versions of the above, version numbers etc.

Mode related:
Some chips will operative in buffered and unbuffered modes.
Things like single ended vs differential modes on ADCs.

Interface related:
Like the input subsystem, things like parameters of an associated character
device, and the type of events this may return.  This would allow different
complexities of event depending on what is going on.  Down the line this 
may
allow the turning off and on of individual events as appropriate. For 
example,
if no-one is interested in an interrupt event such as freefall detection 
of an
accelerometer then depending on hardware support we may be a) able to turn
it off and free the relevant interrupt, or b) simply free the relevant 
interrupt
and ignore the event.

Simple read of current value:
As the devices will not always be generating interrupts, it probably 
also makes
sense to have interfaces here to get the current values of readings.   
Either this
will be from a cached value in an interrupt providing device, or 
actually initialise
a read from the device if appropriate.

The other interface element would be a stripped down /dev/ device 
similar to those
used by the input subsystem.  However, it may make sense to have an 
independent one
of these for each and every chip present?  Does it make sense to ever 
aggregate these,
or should this effectively be left to select calls in the userspace 
code?  There is also an
issue here of whether we want to group similar devices.  I.e. should we 
group all
accelerometers? My personal view is, not at the moment as it would 
complicate the
handling of devices combining several types of sensor.

The fun bit of this subsystem will be in how buffering of incoming data 
is accomplished.
As an example, lets say we want a 100Hz set of readings from the device. 
Several
possibilities exist:

a) The device itself has a ring buffer (the VTI accelerometers are an 
example of this). 
If this is the case, how would people suggest we go about reading from 
the ring buffer in
the event of an interrupt (typically 3/4 full).  We could have a the 
kernel module interpret
this interrupt and effectively fill a ring buffer in the kernel from the 
hardware one.  This
is efficient on interrupts and the kernel based ring buffer could have a 
size controllable
through the sys interface. 

b) The device triggers an interrupt on each 'data ready' after sampling 
some analog
measurement.  Do we generate an event on each of these? (seems like 
overkill to me)
or again use a ring buffer in the kernel to store the results and 
perhaps send an event
out after a (sysfs interface controlled) number of readings.  If a near 
real-time app is
wanting the data, then this would be set to zero to indicate need to 
send an event on each
reading. The ST LIS3L02DQ accelerometer falls into this category.  
Complexities here
are concerned with the need to look directly at the relevant gpio value 
in order to establish
whether another even has occured before the bottom half of the interrupt 
has run. See
the LIS3L02DQ driver I posted to the spi-devel mailing list for an example.

c) The device samples only on a request over the bus.  Either these 
could only be read
from via the sysfs interface or an event sent through the dev character 
device. 
Also feasible would be a board specific element of the platform_data, 
perhaps specifying
functions to initialize a hardware periodic clock interrupt or similar
(I've done this before, but it was clunky - at high rates you frequently 
miss interrupts.)
The MAX123x and MAX1363 ADCs fall into this category.

Obviously the challenge in all this is to keep the number of interrupts 
and indeed events (in
the input subsystem sense of sending to userspace via a char device) to 
a minimum.

For other issues such as time stamping, whilst vital, to a certain 
extent may need to be
handled within largely within the individual drivers (due to the 
different types of read above).
Clearly the subsystem should provide helper functions for this wherever 
possible.

All comments welcome and thanks to everyone who contributed to the 
original discussion.


--

Jonathan Cameron

Re: Accelerometer etc subsystem (Update on progress)

[Jonathan Cameron]

[-- Attachment #1: Type: text/plain, Size: 3031 bytes --]

Dear All,

This email is mainly to give people an idea of current progress towards
a new
subsystem as discussed in the thread starting with
http://lkml.org/lkml/2008/5/20/135

Sorry for the mass list bombardment, but clearly some elements of this
discussion
will end up in various different territories.

Some elements of a prototype subsystem are in place. It draws very heavily
on parts of the input and hwmon subsystems diverging only where necessary.

The only test driver currently integrated is for an ST LIS3L02DQ
accelerometer
which has more than a few quirks to make it tricky to handle (and some what
sketchy documentation.) More chips will follow over next week or so but
hopefully the driver for this chip gives enough of an idea of how I envision
the system working to encourage discussion / advice.

Note that I haven't dealt with anywhere near all the possible locking issues
etc and am well aware that this needs to be done. Other cleanups that will
need to be done include working out the layout in sysfs to make it more
intuitive. Also sorry for the somewhat rough and ready nature of the
attached
patch (against 2.6.26-rc4)

Ring buffer design is a large part of the attached patch. I'm not sure if
I am going about this the right way. Basically, we need ring buffers with
almost no write latency but can waste plenty of time reading from them
(in general case - we do however want reading the last available value to be
fast). What is there works, but probably has at least a few nasty corner
cases that I haven't prevented.

Interfaces (these are per device) - at somepoint a procfs interface similar
       to that used in the input subsystem would make device querying
       simpler.

Sysfs -    Parameter Control - gain / offsets etc
    State control, turn interrupts on and off etc.
    Interrupt control parameters (threshold etc)
    Ring buffer parameters as relevant (currently fixed)
    Individual input reading (acceleration values here)
    Minor numbers for various chrdevs associated with this device.

chrdev-    3 types of chrdev at the moment
    Ring buffer events
    Ring buffer access (currently ripping data off the buffer only)
    Interrupt events - for lis3l02dq these are only threshold breaks

Functionality yet to be implemented.
    Polled based capture (use a peroidic timer if available)

    Hardware ring buffering for devices that support it (two level ring
buffer -
    hard and soft may be appropriate)

    A chrdev for polling of whole device (with timestamps etc).

    Composite interrupt handling (some devices allow logical combinations
    of different interrupt signals to be used as the trigger condition).

    Documenation ;)

    Cleaner solution to data alignment in the ring buffer (currently I'm
cheating
    and manually doing it)

    Lots lots more....

Anyhow, all comments welcome.  Can anyone think of a better name?
(I'm not keen on industrialio. It's too long if nothing else!
 It will do as a working title for now)

Thanks,

--

Jonathan Cameron


[-- Attachment #2: indio.patch --]
[-- Type: text/x-patch, Size: 88443 bytes --]

diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/Kconfig b/drivers/industrialio/Kconfig
--- a/drivers/industrialio/Kconfig	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/Kconfig	2008-06-04 18:59:18.000000000 +0100
@@ -0,0 +1,27 @@
+#
+# Industrial I/O subsytem configuration
+#
+
+menuconfig INDUSTRIALIO
+	tristate "Industrial I/O support"
+	---help---
+	  The industrial IO subsystem provides a unified framework for drivers
+	  for many different types of embedded sensors using a number of
+	  different phyiscal interfaces (i2c, spi etc).
+
+	  This documentation will need some work!
+
+if INDUSTRIALIO
+
+source drivers/industrialio/accelerometer/Kconfig
+#source drivers/industrialio/adc/Kconfig
+#source drivers/industrialio/misc/Kconfig
+
+config INDUSTRIALIO_DEBUG_CORE
+	bool "Industrial I/O Core debugging messages"
+	help
+	  Say yes here if you want the Industrial I/O core to producte a bunch
+	  of debug messages to the system log. Select this if you are having a
+	  problem with industrial io support and want to see mor eof what is
+	  going on.
+endif
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/Makefile b/drivers/industrialio/Makefile
--- a/drivers/industrialio/Makefile	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/Makefile	2008-06-07 13:41:31.000000000 +0100
@@ -0,0 +1,11 @@
+#
+# Makefile for the industrial I/O core.
+#
+
+obj-$(CONFIG_INDUSTRIALIO)	+= industrial.o
+
+obj-y				+= accelerometer/ 
+
+ifeq ($(CONFIG_INDUSTRIALIO_DEBUG_CORE),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
\ No newline at end of file
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/accelerometer/Kconfig b/drivers/industrialio/accelerometer/Kconfig
--- a/drivers/industrialio/accelerometer/Kconfig	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/accelerometer/Kconfig	2008-06-04 18:59:47.000000000 +0100
@@ -0,0 +1,24 @@
+#
+# Accelerometer drivers
+#
+
+config LIS3L02DQ
+	tristate "ST Microelectronics LIS3L02DQ Accelerometer Driver"
+	help
+	  Say yes here to build generic support for the ST microelectronics
+	  accelerometer. You will also need to one or more of the bus specific
+	  elements below.
+
+config LIS3L02DQ_SPI
+	depends on LIS3L02DQ && SPI
+	tristate "SPI support"
+	help
+	  Say yes here to build support for the ST LIS3L02DQ accelerometer via
+	  an SPI bus.
+
+config LIS3L02DQ_I2C
+	depends on LIS3L02DQ && I2C
+	tristate "I2C support"
+	help
+	  Say yes here to build support for the ST LIS3L02DQ accelerometer via
+	  an I2C bus.
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/accelerometer/Makefile b/drivers/industrialio/accelerometer/Makefile
--- a/drivers/industrialio/accelerometer/Makefile	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/accelerometer/Makefile	2008-06-05 11:35:28.000000000 +0100
@@ -0,0 +1,5 @@
+#
+# Makefile for industrial I/O accelerometer drivers
+#
+
+obj-$(CONFIG_LIS3L02DQ_SPI)	+= lis3l02dq.o
\ No newline at end of file
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/accelerometer/lis3l02dq.c b/drivers/industrialio/accelerometer/lis3l02dq.c
--- a/drivers/industrialio/accelerometer/lis3l02dq.c	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/accelerometer/lis3l02dq.c	2008-06-26 18:05:57.000000000 +0100
@@ -0,0 +1,1305 @@
+/*
+ * lis3l02dq.c	support STMicroelectronics LISD02DQ
+ *		3d 2g Linear Accelerometers via SPI
+ *
+ * Copyright (c) 2007 Jonathan Cameron <jic23@cam.ac.uk>
+ *
+ * Loosely based upon tle62x0.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * FIXME: MORE DOCS
+ *
+ * Settings:
+ * Latch on interrupt generation enabled as it simplifies when to reenable
+ * the interrupt.
+ * 16 bit left justified mode used (makes little or no difference, but that
+ * is what I picked.
+ *
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/gpio.h>
+#include <linux/workqueue.h>
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/spi/spi.h>
+
+#include <linux/sysfs.h>
+#include <linux/list.h>
+#include <linux/industrialio.h>
+#include <linux/industrialio_sysfs.h>
+
+#include "lis3l02dq.h"
+
+/* Read all inputs in one spi message */
+static const char read_all_tx_array[] =
+{
+	0, LIS3L02DQ_READ_REG(LIS3L02DQ_REG_OUT_X_L_ADDRESS),
+	0, LIS3L02DQ_READ_REG(LIS3L02DQ_REG_OUT_X_H_ADDRESS),
+	0, LIS3L02DQ_READ_REG(LIS3L02DQ_REG_OUT_Y_L_ADDRESS),
+	0, LIS3L02DQ_READ_REG(LIS3L02DQ_REG_OUT_Y_H_ADDRESS),
+	0, LIS3L02DQ_READ_REG(LIS3L02DQ_REG_OUT_Z_L_ADDRESS),
+	0, LIS3L02DQ_READ_REG(LIS3L02DQ_REG_OUT_Z_H_ADDRESS),
+};
+
+static int lis3l02dq_read_all(struct lis3l02dq_state *st,
+			      unsigned char *rx_array)
+{
+	/* Sadly the device appears to require deselection between
+	 * reading the different registers - hence the somewhat
+	 * convoluted nature of this transfer */
+	struct spi_transfer xfers[] = {
+		/* x low byte */
+		{
+			.tx_buf = read_all_tx_array,
+			.rx_buf = rx_array,
+			.bits_per_word = 16,
+			.len = 2,
+			.cs_change = 1,
+		},
+		/* x high byte */
+		{
+			.tx_buf = read_all_tx_array+2,
+			.rx_buf = rx_array + 2,
+			.bits_per_word = 16,
+			.len = 2,
+			.cs_change = 1,
+		},
+		/* y low byte */
+		{
+			.tx_buf = read_all_tx_array+4,
+			.rx_buf = rx_array + 4,
+			.bits_per_word = 16,
+			.len = 2,
+			.cs_change = 1,
+		},
+		/* y high byte */
+		{
+			.tx_buf = read_all_tx_array+6,
+			.rx_buf = rx_array + 6,
+			.bits_per_word = 16,
+			.len = 2,
+			.cs_change = 1,
+		},
+		/* z low byte */
+		{
+			.tx_buf = read_all_tx_array+8,
+			.rx_buf = rx_array + 8,
+			.bits_per_word = 16,
+			.len = 2,
+			.cs_change = 1,
+		},
+		/* z high byte */
+		{
+			.tx_buf = read_all_tx_array+10,
+			.rx_buf = rx_array + 10,
+			.bits_per_word = 16,
+			.len = 2,
+			.cs_change = 0,
+		},
+	};
+	struct spi_message msg;
+	int ret;
+	/* After these are trasmitted, the rx_buff should have
+	 * values in alternate bytes */
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[2], &msg);
+	spi_message_add_tail(&xfers[4], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
+	spi_message_add_tail(&xfers[3], &msg);
+	spi_message_add_tail(&xfers[5], &msg);
+	ret = spi_sync(st->us, &msg);
+	if (ret) {
+		dev_err(&st->us->dev, "problem with get all accels");
+		goto err_ret;
+	}
+
+err_ret:
+	return ret;
+}
+
+static int lis3l02dq_spi_read_reg_int8_t(struct device *dev,
+					  uint8_t reg_address,
+					  int8_t *val)
+{
+	int ret;
+	struct spi_message msg;
+	struct industrialio_dev *indio_dev = dev_get_drvdata(dev);
+	struct lis3l02dq_state *st = indio_dev->dev_data;
+	struct spi_transfer xfer = {
+		.tx_buf = NULL,
+		.rx_buf = NULL,
+		.bits_per_word = 16,
+		.len = 2,
+	};
+
+	xfer.tx_buf = (unsigned char *)(kmalloc(4, GFP_KERNEL));
+	xfer.rx_buf = (unsigned char *)(kmalloc(4, GFP_KERNEL));
+	((unsigned char *)(xfer.tx_buf))[1] = LIS3L02DQ_READ_REG(reg_address);
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+	ret = spi_sync(st->us, &msg);
+	kfree(xfer.tx_buf);
+	if (ret) {
+		dev_err(&st->us->dev, "problem with get x offset");
+		goto err_ret;
+	}
+	*val = ((unsigned char *)(xfer.rx_buf))[0];
+	kfree(xfer.rx_buf);
+	return ret;
+err_ret:
+	kfree(xfer.rx_buf);
+	return 0;
+}
+
+/*Returns into to allow full 0/255 range with error codes in negative range */
+static int lis3l02dq_spi_read_reg_uint8_t(struct device *dev,
+					       uint8_t reg_address)
+{
+	uint8_t val;
+	int8_t ret;
+	struct spi_message msg;
+	struct industrialio_dev *indio_dev = dev_get_drvdata(dev);
+	struct lis3l02dq_state *st = indio_dev->dev_data;
+	/* FIXME clearer to leave these in, or use tx_buf in xfer directly? */
+	unsigned char *local_tx_buf;
+	unsigned char *local_rx_buf;
+	struct spi_transfer xfer = {
+		.tx_buf = NULL,
+		.rx_buf = NULL,
+		.bits_per_word = 16,
+		.len = 2,
+	};
+
+	local_tx_buf = (unsigned char *)(kmalloc(4, GFP_KERNEL));
+	local_rx_buf = (unsigned char *)(kmalloc(4, GFP_KERNEL));
+
+	local_tx_buf[1] = LIS3L02DQ_READ_REG(reg_address);
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+	ret = spi_sync(st->us, &msg);
+	kfree(local_tx_buf);
+	if (ret) {
+		dev_err(&st->us->dev, "problem with get x offset");
+		goto err_ret;
+	}
+	val = local_rx_buf[0];
+	kfree(local_rx_buf);
+	return val;
+err_ret:
+	kfree(local_rx_buf);
+	return ret;
+}
+
+static int lis3l02dq_spi_write_reg_int8_t(struct device *dev,
+					   uint8_t reg_address,
+					   int value)
+{
+	struct spi_message msg;
+	struct industrialio_dev *indio_dev = dev_get_drvdata(dev);
+	struct lis3l02dq_state *st = indio_dev->dev_data;
+	unsigned char *local_tx_buf;
+	struct spi_transfer xfer = {
+		.tx_buf = NULL,
+		.rx_buf = NULL,
+		.bits_per_word = 16,
+		.len = 2,
+	};
+	int ret;
+
+	local_tx_buf = (unsigned char *)(kmalloc(4, GFP_KERNEL));
+	xfer.tx_buf = local_tx_buf;
+	local_tx_buf[1] = LIS3L02DQ_WRITE_REG(reg_address);
+	local_tx_buf[0] = value;
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+	ret = spi_sync(st->us, &msg);
+	kfree(local_tx_buf);
+
+	if (ret) {
+		dev_err(&st->us->dev, "problem with writing 8 bit register");
+		return ret;
+	}
+	return 0;
+}
+
+/* Relies on the MSB being one higher adress than the LSB */
+static int lis3l02dq_spi_write_reg_int16_t(struct device *dev,
+						uint8_t lower_reg_address,
+						int value)
+{
+	struct spi_message msg;
+	struct industrialio_dev *indio_dev = dev_get_drvdata(dev);
+	struct lis3l02dq_state *st = indio_dev->dev_data;
+	unsigned char *local_tx_buf;
+	int ret;
+	struct spi_transfer xfers [] = { {
+			.tx_buf = NULL,
+			.rx_buf = NULL,
+			.bits_per_word = 16,
+			.len = 2,
+		}, {
+			.tx_buf = NULL,
+			.rx_buf = NULL,
+			.bits_per_word = 16,
+			.len = 2,
+		},
+	};
+
+	local_tx_buf = (unsigned char *)(kmalloc(4, GFP_KERNEL));
+	xfers[0].tx_buf = local_tx_buf;
+	xfers[1].tx_buf = local_tx_buf + 2;
+	local_tx_buf[1] = LIS3L02DQ_WRITE_REG(lower_reg_address);
+	local_tx_buf[0] = (value & 0xFF);
+	local_tx_buf[3] = LIS3L02DQ_WRITE_REG(lower_reg_address+1);
+	local_tx_buf[2] = (value & 0xFF00) >> 8;
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
+	ret = spi_sync(st->us, &msg);
+	kfree(local_tx_buf);
+	if (ret) {
+		dev_err(&st->us->dev, "problem when writing 16 bit register");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int lis3l02dq_spi_read_reg_int16_t(struct device *dev,
+					       uint8_t lower_reg_address,
+					       int16_t *val)
+{
+	struct spi_message msg;
+	struct industrialio_dev *indio_dev = dev_get_drvdata(dev);
+	struct lis3l02dq_state *st = indio_dev->dev_data;
+	unsigned char *local_tx_buf;
+	unsigned char *local_rx_buf;
+	int ret;
+	uint16_t temp;
+	struct spi_transfer xfers [] = { {
+			.tx_buf = NULL,
+			.rx_buf = NULL,
+			.bits_per_word = 16,
+			.len = 2,
+		}, {
+			.tx_buf = NULL,
+			.rx_buf = NULL,
+			.bits_per_word = 16,
+			.len = 2,
+		},
+	};
+	local_tx_buf = (unsigned char *)(kmalloc(4, GFP_KERNEL));
+	local_rx_buf = (unsigned char *)(kmalloc(4, GFP_KERNEL));
+	xfers[0].tx_buf = local_tx_buf;
+	xfers[0].rx_buf = local_rx_buf;
+	xfers[1].tx_buf = local_tx_buf + 2;
+	xfers[1].rx_buf = local_rx_buf + 2;
+	local_tx_buf[0] = 0;
+	local_tx_buf[1] = LIS3L02DQ_READ_REG(lower_reg_address);
+	local_tx_buf[2] = 0;
+	local_tx_buf[3] = LIS3L02DQ_READ_REG(lower_reg_address+1);
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfers[0], &msg);
+	spi_message_add_tail(&xfers[1], &msg);
+	ret = spi_sync(st->us, &msg);
+	kfree(local_tx_buf);
+	kfree(local_rx_buf);
+	if (ret) {
+		dev_err(&st->us->dev, "problem when reading 16 bit register");
+		return ret;
+	}
+	temp = (((uint16_t)((local_rx_buf[2]))) << 8)
+		| (uint16_t)(local_rx_buf[0]);
+	*val = *((int16_t *)(&temp));
+
+	return 0;
+}
+
+
+
+static ssize_t lis3l02dq_read_signed(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buf)
+{
+	int len, ret;
+	int8_t val;
+	struct industrialio_dev_attr *this_attr
+		= to_industrialio_dev_attr(attr);
+
+	ret = lis3l02dq_spi_read_reg_int8_t(dev,
+					    this_attr->address,
+					    &val);
+	if (ret < 0)
+		goto err_ret;
+	len = sprintf(buf, "%d\n", val);
+
+	return len;
+
+err_ret:
+	return ret;
+}
+
+static ssize_t lis3l02dq_read_unsigned(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	int val, len;
+	struct industrialio_dev_attr *this_attr
+		= to_industrialio_dev_attr(attr);
+
+	val = lis3l02dq_spi_read_reg_uint8_t(dev,
+					     this_attr->address);
+	if (val < 0) {
+		dev_err(dev, "problem reading an unsigned 8 bit value");
+		goto err_ret;
+	}
+
+	len = sprintf(buf, "%d\n", val);
+	return len;
+err_ret:
+	return val;
+}
+
+static ssize_t lis3l02dq_write_signed(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf,
+				      size_t len)
+{
+	long val;
+	int ret;
+	struct industrialio_dev_attr *this_attr
+		= to_industrialio_dev_attr(attr);
+
+	ret = strict_strtol(buf, 10, &val);
+	if (ret)
+		goto err_ret;
+
+	ret = lis3l02dq_spi_write_reg_int8_t(dev,
+					     this_attr->address,
+					     val);
+	if (ret)
+		goto err_ret;
+
+	return len;
+
+err_ret:
+	return ret;
+}
+
+static ssize_t lis3l02dq_write_unsigned(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf,
+					size_t len)
+{
+	int ret;
+	ulong val;
+	struct industrialio_dev_attr *this_attr
+		= to_industrialio_dev_attr(attr);
+
+	ret = strict_strtoul(buf, 10, &val);
+	if (ret)
+		goto err_ret;
+
+	ret = lis3l02dq_spi_write_reg_int8_t(dev,
+					      this_attr->address,
+					      val);
+	if (ret)
+		goto err_ret;
+
+	return len;
+err_ret:
+	return ret;
+}
+
+
+static ssize_t lis3l02dq_read_16bit_signed(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	int len, ret;
+	int16_t val;
+	struct industrialio_dev_attr *this_attr
+		= to_industrialio_dev_attr(attr);
+	ret = lis3l02dq_spi_read_reg_int16_t(dev,
+						  this_attr->address,
+						  &val);
+	if (ret < 0) {
+		dev_err(dev, "problem reading a signed 16 bit value from chip");
+		return ret;
+	}
+	len = sprintf(buf, "%d\n", val);
+	return len;
+}
+
+/* As the ring buffer contents are device dependent this functionality
+ * must remain part of the driver and not the ring buffer subsystem */
+static ssize_t
+lis3l02dq_read_accel_from_ring(struct industrialio_ring_buffer *ring,
+			       int element, char *buf)
+{
+	int val, ret, len;
+	uint16_t temp;
+	char *data, *datalock;
+
+	data = kmalloc(8, GFP_KERNEL);
+	if (data == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	ret = industrialio_read_last_from_ring(ring, data);
+	datalock = data + 2*element;
+	kfree(data);
+	temp = (((uint16_t)((datalock[1]))) << 8)
+		| (uint16_t)(datalock[0]);
+	val = *((int16_t *)(&temp));
+	len = sprintf(buf, "ring %d\n", val);
+
+	return len;
+error_ret:
+	return ret;
+}
+
+static ssize_t lis3l02dq_read_accel(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct industrialio_dev_attr *this_attr
+		= to_industrialio_dev_attr(attr);
+	struct industrialio_dev *dev_info = dev_get_drvdata(dev);
+	int element;
+	/* FIXME clunky*/
+	if (dev_info->currentmode == INDIO_RING_DATA_RDY) {
+		switch (this_attr->address) {
+		case LIS3L02DQ_REG_OUT_X_L_ADDRESS:
+			element = 0;
+			break;
+		case LIS3L02DQ_REG_OUT_Y_L_ADDRESS:
+			element = 1;
+			break;
+		case LIS3L02DQ_REG_OUT_Z_L_ADDRESS:
+			element = 2;
+			break;
+		default:
+			return -EINVAL;
+		}
+		return lis3l02dq_read_accel_from_ring(dev_info->ring,
+						      element, buf);
+	} else
+		return lis3l02dq_read_16bit_signed(dev, attr, buf);
+}
+
+/* For this device this is only relevant to the threshold for interrupt
+ * generation */
+static ssize_t lis3l02dq_write_16bit_signed(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf,
+					    size_t len)
+{
+	int ret;
+	long val;
+	struct industrialio_dev_attr *this_attr
+		= to_industrialio_dev_attr(attr);
+
+	ret = strict_strtol(buf, 10, &val);
+	if (ret)
+		return ret;
+	ret = lis3l02dq_spi_write_reg_int16_t(dev,
+						   this_attr->address,
+						   val);
+	if (ret)
+		return ret;
+	return len;
+}
+
+static ssize_t lis3l02dq_read_av_freq(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	return sprintf(buf, "280 560 1120 4480\n");
+}
+
+static ssize_t lis3l02dq_read_frequency(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	int ret, len;
+	int8_t t;
+	ret = lis3l02dq_spi_read_reg_int8_t(dev,
+					    LIS3L02DQ_REG_CTRL_1_ADDRESS,
+					    &t);
+	if (ret)
+		goto error_ret;
+	t &= LIS3L02DQ_DEC_MASK;
+	if (t == LIS3L02DQ_REG_CTRL_1_DF_128)
+		len = sprintf(buf, "280");
+	else if (t == LIS3L02DQ_REG_CTRL_1_DF_64)
+		len = sprintf(buf, "560");
+	else if (t == LIS3L02DQ_REG_CTRL_1_DF_32)
+		len = sprintf(buf, "1120");
+	else
+		len = sprintf(buf, "4480");
+	len += sprintf(buf+len, "\n");
+
+	return len;
+
+error_ret:
+	return ret;
+}
+
+static ssize_t lis3l02dq_write_frequency(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf,
+					 size_t len)
+{
+	long val;
+	int ret;
+	int8_t t;
+
+	ret = strict_strtol(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	ret = lis3l02dq_spi_read_reg_int8_t(dev,
+					    LIS3L02DQ_REG_CTRL_1_ADDRESS,
+					    &t);
+	if (ret)
+		goto error_ret;
+	/* Wipe the bits clean */
+	t |= ~LIS3L02DQ_DEC_MASK;
+	switch (val) {
+	case 280:
+		t |= LIS3L02DQ_REG_CTRL_1_DF_128;
+		break;
+	case 560:
+		t |= LIS3L02DQ_REG_CTRL_1_DF_64;
+		break;
+	case 1120:
+		t |= LIS3L02DQ_REG_CTRL_1_DF_32;
+		break;
+	case 4480:
+		t |= LIS3L02DQ_REG_CTRL_1_DF_8;
+		break;
+	default:
+		ret = -EINVAL;
+		goto error_ret;
+	};
+
+	ret = lis3l02dq_spi_write_reg_int8_t(dev,
+					     LIS3L02DQ_REG_CTRL_1_ADDRESS,
+					     t);
+	if (ret)
+		goto error_ret;
+
+	return len;
+
+error_ret:
+	return ret;
+
+}
+
+static int lis3l02dq_initial_setup(struct lis3l02dq_state *st)
+{
+	int ret;
+
+	st->us->mode = SPI_MODE_3;
+	spi_setup(st->us);
+
+	/* Write suitable defaults to ctrl1 */
+	ret = lis3l02dq_spi_write_reg_int8_t(&st->us->dev,
+					      LIS3L02DQ_REG_CTRL_1_ADDRESS,
+					      LIS3L02DQ_DEFAULT_CTRL1);
+	if (ret) {
+		dev_err(&st->us->dev, "problem with setup control register 1");
+		goto err_ret;
+	}
+	ret = lis3l02dq_spi_write_reg_int8_t(&st->us->dev,
+					      LIS3L02DQ_REG_CTRL_2_ADDRESS,
+					      LIS3L02DQ_DEFAULT_CTRL2);
+	if (ret) {
+		dev_err(&st->us->dev, "problem with setup control register 2");
+		goto err_ret;
+	}
+
+	ret = lis3l02dq_spi_write_reg_int8_t(&st->us->dev,
+					     LIS3L02DQ_REG_WAKE_UP_CFG_ADDRESS,
+					     LIS3L02DQ_REG_WAKE_UP_CFG_LATCH_SRC);
+	if (ret) {
+		dev_err(&st->us->dev, "problem with interrupt cfg register");
+		goto err_ret;
+	}
+err_ret:
+	return ret;
+}
+
+
+
+/* These are all a case of reading / writing directly to the chip */
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_X_OFFSET(S_IWUSR | S_IRUGO,
+					    lis3l02dq_read_signed,
+					    lis3l02dq_write_signed,
+					    LIS3L02DQ_REG_OFFSET_X_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_Y_OFFSET(S_IWUSR | S_IRUGO,
+					    lis3l02dq_read_signed,
+					    lis3l02dq_write_signed,
+					    LIS3L02DQ_REG_OFFSET_Y_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_Z_OFFSET(S_IWUSR | S_IRUGO,
+					    lis3l02dq_read_signed,
+					    lis3l02dq_write_signed,
+					    LIS3L02DQ_REG_OFFSET_Z_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_X_GAIN(S_IWUSR | S_IRUGO,
+					  lis3l02dq_read_unsigned,
+					  lis3l02dq_write_unsigned,
+					  LIS3L02DQ_REG_GAIN_X_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_Y_GAIN(S_IWUSR | S_IRUGO,
+					  lis3l02dq_read_unsigned,
+					  lis3l02dq_write_unsigned,
+					  LIS3L02DQ_REG_GAIN_Y_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_Z_GAIN(S_IWUSR | S_IRUGO,
+					  lis3l02dq_read_unsigned,
+					  lis3l02dq_write_unsigned,
+					  LIS3L02DQ_REG_GAIN_Z_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_THRESH(S_IWUSR | S_IRUGO,
+					  lis3l02dq_read_16bit_signed,
+					  lis3l02dq_write_16bit_signed,
+					  LIS3L02DQ_REG_THS_L_ADDRESS);
+
+/* Obviously the reading method for these will change depending on whether
+   ring buffer capture is in use. Allow specification here of alternate
+   function? Or take the approach used above of making this a driver issue
+   rather than part of industrialio */
+static INDUSTRIALIO_DEV_ATTR_ACCEL_X(lis3l02dq_read_accel,
+				     LIS3L02DQ_REG_OUT_X_L_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_Y(lis3l02dq_read_accel,
+				     LIS3L02DQ_REG_OUT_Y_L_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_ACCEL_Z(lis3l02dq_read_accel,
+				     LIS3L02DQ_REG_OUT_Z_L_ADDRESS);
+
+static INDUSTRIALIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO,
+				       lis3l02dq_read_frequency,
+				       lis3l02dq_write_frequency);
+
+static INDUSTRIALIO_DEV_ATTR_AVAIL_SAMP_FREQ(lis3l02dq_read_av_freq);
+
+static ssize_t
+lis3l02dq_read_interrupt_config(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	int len, ret, set;
+	int8_t val;
+	struct industrialio_event_attr *this_attr
+		= to_industrialio_event_attr(attr);
+
+	ret = lis3l02dq_spi_read_reg_int8_t(dev,
+					     LIS3L02DQ_REG_WAKE_UP_CFG_ADDRESS,
+					     &val);
+	if (ret < 0)
+		return ret;
+	set = (val & this_attr->mask) ? 1 : 0;
+	len = sprintf(buf, "%d\n", set);
+
+	return len;
+}
+
+static ssize_t
+lis3l02dq_write_interrupt_config(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf,
+				 size_t len)
+{
+	int ret, currentlyset, changed = 0;
+	int8_t valold;
+	int8_t controlold;
+
+	struct industrialio_event_attr *this_attr
+		= to_industrialio_event_attr(attr);
+	long val;
+	struct industrialio_dev *indio_dev = dev_get_drvdata(dev);
+
+	ret = strict_strtol(buf, 10, &val);
+
+	if (ret < 0)
+		return ret;
+	/* read current value */
+	ret = lis3l02dq_spi_read_reg_int8_t(dev,
+					    LIS3L02DQ_REG_WAKE_UP_CFG_ADDRESS,
+					    &valold);
+	if (ret < 0)
+		return ret;
+
+	/* read current control */
+	ret = lis3l02dq_spi_read_reg_int8_t(dev,
+					    LIS3L02DQ_REG_CTRL_2_ADDRESS,
+					    &controlold);
+	if (ret < 0)
+		return ret;
+	currentlyset = valold & this_attr->mask ? 1 : 0;
+	if (val == 0 && currentlyset) {
+		valold &= ~this_attr->mask;
+		changed = 1;
+		ret = industrialio_remove_event_from_list(this_attr->listel);
+		if (ret < 0)
+			return ret;
+	} else if (val == 1 && !currentlyset) {
+		changed = 1;
+		valold |= this_attr->mask;
+		/* move to a line spec rather than this? */
+		ret = industrialio_add_event_to_list(&indio_dev
+						     ->interrupts[0]->ev_list,
+						     this_attr->listel);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (changed) {
+		ret = lis3l02dq_spi_write_reg_int8_t(dev,
+						     LIS3L02DQ_REG_WAKE_UP_CFG_ADDRESS,
+						     valold);
+		if (ret < 0)
+			return ret;
+		/* This always enables the interrupt, even if we've remove the
+		 * last thing using it. For this device we can use the reference
+		 * count on the handler to tell us if anyone
+		 * wants the interrupt */
+		controlold = this_attr->listel->refcount ?
+			(controlold | LIS3L02DQ_REG_CTRL_2_ENABLE_INTERRUPT):
+			(controlold & ~LIS3L02DQ_REG_CTRL_2_ENABLE_INTERRUPT);
+		ret = lis3l02dq_spi_write_reg_int8_t(dev,
+							  LIS3L02DQ_REG_CTRL_2_ADDRESS,
+							  controlold);
+		if (ret < 0)
+			return ret;
+	}
+
+	return len;
+}
+
+static ssize_t lis3l02dq_read_data_ready_config(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
+{
+	int ret, set, len;
+	int8_t val;
+
+	ret = lis3l02dq_spi_read_reg_int8_t(dev,
+					    LIS3L02DQ_REG_CTRL_2_ADDRESS,
+					    &val);
+	if (ret < 0)
+		return ret;
+	set = (val & LIS3L02DQ_REG_CTRL_2_ENABLE_DATA_READY_GENERATION) ? 1 : 0;
+	len = sprintf(buf, "%d\n", set);
+
+	return len;
+}
+
+/* This function must enable data ready generation - whether this is going into
+   a ring buffer or directly generating events is dependant on the value input.
+   0 - no data ready
+   1 - into ring buffer
+   2 - generate events
+*/
+static ssize_t lis3l02dq_write_data_ready_config(struct device *dev,
+						 struct device_attribute *attr,
+						 const char *buf,
+						 size_t len)
+{
+	int ret, currentlyset, changed = 0;
+	int8_t valold;
+
+	struct industrialio_event_attr *this_attr
+		= to_industrialio_event_attr(attr);
+	long val;
+	struct industrialio_dev *indio_dev = dev_get_drvdata(dev);
+
+	ret = strict_strtol(buf, 10, &val);
+	if (ret < 0)
+		return ret;
+	if (val > 2 || val < 0)
+		return -EINVAL;
+
+	ret = lis3l02dq_spi_read_reg_int8_t(dev,
+					    LIS3L02DQ_REG_CTRL_2_ADDRESS,
+					    &valold);
+	if (ret < 0)
+		return ret;
+
+	currentlyset
+		= valold
+		& LIS3L02DQ_REG_CTRL_2_ENABLE_DATA_READY_GENERATION
+		? 1 : 0;
+	if (val == 0 && currentlyset) {
+		valold &= ~LIS3L02DQ_REG_CTRL_2_ENABLE_DATA_READY_GENERATION;
+		changed = 1;
+		ret = industrialio_remove_event_from_list(this_attr->listel);
+		indio_dev->currentmode = INDIO_DIRECT_MODE;
+	} else if (val != 0 && !currentlyset) {
+		valold |= LIS3L02DQ_REG_CTRL_2_ENABLE_DATA_READY_GENERATION;
+		changed = 2;
+		ret = industrialio_add_event_to_list(&indio_dev
+						     ->interrupts[0]->ev_list,
+						     this_attr->listel);
+	}
+	if (val == 1)
+		indio_dev->currentmode = INDIO_RING_DATA_RDY;
+	else if (val == 2) {
+		indio_dev->currentmode = INDIO_DIRECT_MODE;
+		valold |= LIS3L02DQ_REG_CTRL_2_BLOCK_UPDATE;
+	}
+	if (changed) {
+		ret = lis3l02dq_spi_write_reg_int8_t(dev,
+						     LIS3L02DQ_REG_CTRL_2_ADDRESS,
+						     valold);
+		if (ret < 0)
+			return ret;
+		/* Clear the other sources of interrupts - whilst not strictly
+		   necessary, this does avoid the need for maintaining state
+		   in software */
+		if (changed == 2) {
+			ret = lis3l02dq_spi_write_reg_int8_t(dev,
+							     LIS3L02DQ_REG_WAKE_UP_CFG_ADDRESS,
+							     LIS3L02DQ_REG_WAKE_UP_CFG_LATCH_SRC);
+
+			if (ret < 0)
+				return ret;
+		}
+	}
+
+	return len;
+}
+
+static void lis3l02dq_data_rdy_bh_to_ring(struct work_struct *work_s)
+{
+	struct lis3l02dq_state *st
+		= container_of(work_s, struct lis3l02dq_state,
+			       work_data_rdy_ring);
+	/* indicate we have begun to process the interrupt*/
+	unsigned char *rx_array, *ring_data;
+	int i;
+
+	/*FIXME both need tests for -enomem */
+	rx_array = (unsigned char *)(kmalloc(12, GFP_KERNEL));
+	ring_data = (unsigned char *)(kmalloc(6, GFP_KERNEL));
+
+	st->inter = 0;
+
+	if (lis3l02dq_read_all(st, rx_array) >= 0) {
+		for (i = 0; i < 6; i++)
+			ring_data[i] = rx_array[i*2];
+		industrialio_store_to_ring(&st->indio_dev->ring[0],
+					   ring_data,
+					   st->last_timestamp);
+	}
+	/* so the data should now be in the rx array */
+try_again:
+	while (gpio_get_value(irq_to_gpio(st->us->irq)))
+		if (lis3l02dq_read_all(st, rx_array) >= 0) {
+			for (i = 0; i < 3; i++)
+				ring_data[i] = rx_array[i*2];
+			/* Passing a negated time stamp to indicate that
+			 * we don't actually know when this occured! */
+			industrialio_store_to_ring(&st->indio_dev->ring[0],
+						   ring_data,
+						   -st->last_timestamp);
+		}
+	/* push data into ring buffer before trying renable */
+	enable_irq(st->us->irq);
+	if (gpio_get_value(irq_to_gpio(st->us->irq)))
+		if (st->inter == 0) {
+			disable_irq_nosync(st->us->irq);
+			goto try_again;
+		}
+	kfree(rx_array);
+	kfree(ring_data);
+	return;
+}
+
+/* This one is odd, I need to perform a full read in order to reset the
+ * interrupt - hence only real thing to make sense it to send the data
+ * as payload of an event? */
+
+static void lis3l02dq_data_rdy_bh_to_event(struct work_struct *work_s)
+{
+	struct lis3l02dq_state *st
+		= container_of(work_s, struct lis3l02dq_state,
+			       work_data_rdy_event);
+
+	/* send an event up to user space */
+	industrialio_put_event(st->indio_dev, 0,
+			       INDUSTRIALIO_EVENT_CODE_DATA_RDY,
+			       st->last_timestamp);
+	enable_irq(st->us->irq);
+	return;
+}
+
+static int lis3l02dq_data_rdy_th(struct industrialio_dev *dev_info,
+				 int index,
+				 s64 timestamp,
+				 int no_test)
+{
+	struct lis3l02dq_state *st = dev_info->dev_data;
+
+	/* Stash the timestamp for the bottom half of the handler*/
+	st->last_timestamp = timestamp;
+
+	if (dev_info->currentmode == INDIO_RING_DATA_RDY) {
+		schedule_work(&st->work_data_rdy_ring);
+		st->inter = 1;
+	} else
+		schedule_work(&st->work_data_rdy_event);
+
+	return IRQ_HANDLED;
+}
+
+static inline int lis3l02dq_thresh_th_impl(struct industrialio_dev *dev_info,
+					  int index,
+					  int timestamp,
+					  int no_test,
+					  struct lis3l02dq_work_cont *wc)
+{
+	/* Find out if this condition is true */
+	if (no_test == 0) {
+		schedule_work(&wc->ws);
+		/* can't do anything else yet */
+		return 0;
+	}
+	schedule_work(&wc->ws_nocheck);
+	/* So I'm set - send event to userspace */
+
+}
+static int lis3l02dq_thresh_handler_th(struct industrialio_dev *dev_info,
+				       int index,
+				       s64 timestamp,
+				       int no_test)
+{
+	struct lis3l02dq_state *st = dev_info->dev_data;
+	/* Stash the timestamp somewhere convenient for the bh */
+	st->last_timestamp = timestamp;
+	/* This is somewhat missleading as for this chip these are actually
+	   the same function */
+	if (no_test == 0)
+		schedule_work(&st->work_cont_thresh.ws);
+	else
+		schedule_work(&st->work_cont_thresh.ws_nocheck);
+	return 0;
+}
+
+
+/* Unforunately it appears the interrupt won't clear unless you read from the
+   src register */
+/* Could move the event list adding stuff into the interrupt, and just do the
+   stuff necessary to reenable the interrupt in here? */
+static void lis3l02dq_thresh_handler_bh_no_check(struct work_struct *work_s)
+{
+	struct lis3l02dq_work_cont *wc
+		= container_of(work_s, struct lis3l02dq_work_cont,
+			       ws_nocheck);
+	struct lis3l02dq_state *st = wc->st;
+
+	int8_t t;
+
+	lis3l02dq_spi_read_reg_int8_t(st->indio_dev->dev,
+					   LIS3L02DQ_REG_WAKE_UP_SRC_ADDRESS,
+					   &t);
+
+	if (t & LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_Z_HIGH)
+		industrialio_put_event(st->indio_dev, 0,
+				       INDUSTRIALIO_EVENT_CODE_ACCEL_Z_HIGH,
+				       st->last_timestamp);
+
+	if (t & LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_Z_LOW)
+		industrialio_put_event(st->indio_dev, 0,
+				       INDUSTRIALIO_EVENT_CODE_ACCEL_Z_LOW,
+				       st->last_timestamp);
+
+	if (t & LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_Y_HIGH)
+		industrialio_put_event(st->indio_dev, 0,
+				       INDUSTRIALIO_EVENT_CODE_ACCEL_Y_HIGH,
+				       st->last_timestamp);
+
+	if (t & LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_Y_LOW)
+		industrialio_put_event(st->indio_dev, 0,
+				       INDUSTRIALIO_EVENT_CODE_ACCEL_Y_LOW,
+				       st->last_timestamp);
+
+	if (t & LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_X_HIGH)
+		industrialio_put_event(st->indio_dev, 0,
+				       INDUSTRIALIO_EVENT_CODE_ACCEL_X_HIGH,
+				       st->last_timestamp);
+
+	if (t & LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_X_LOW)
+		industrialio_put_event(st->indio_dev, 0,
+				       INDUSTRIALIO_EVENT_CODE_ACCEL_X_LOW,
+				       st->last_timestamp);
+
+	/* For this chip, if this handler is running no other interrupts can
+	   occur so this will actually enable the irq  - in others it will
+	   effectively reference count */
+	enable_irq(st->us->irq);
+	/* Ack (and allow for new interrupts? (not clear on data sheet ) )*/
+	lis3l02dq_spi_read_reg_int8_t(st->indio_dev->dev,
+				      LIS3L02DQ_REG_WAKE_UP_ACK_ADDRESS,
+				      &t);
+
+	return;
+}
+
+/* A shared handler for a number of threshold types */
+INDUSTRIALIO_EVENT_SH(threshold, &lis3l02dq_thresh_handler_th);
+
+INDUSTRIALIO_EVENT_ATTR_ACCEL_X_HIGH_SH(industrialio_event_threshold,
+					lis3l02dq_read_interrupt_config,
+					lis3l02dq_write_interrupt_config,
+					LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_X_HIGH);
+
+INDUSTRIALIO_EVENT_ATTR_ACCEL_Y_HIGH_SH(industrialio_event_threshold,
+					lis3l02dq_read_interrupt_config,
+					lis3l02dq_write_interrupt_config,
+					LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_Y_HIGH);
+
+INDUSTRIALIO_EVENT_ATTR_ACCEL_Z_HIGH_SH(industrialio_event_threshold,
+					lis3l02dq_read_interrupt_config,
+					lis3l02dq_write_interrupt_config,
+					LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_Z_HIGH);
+
+INDUSTRIALIO_EVENT_ATTR_ACCEL_X_LOW_SH(industrialio_event_threshold,
+				       lis3l02dq_read_interrupt_config,
+				       lis3l02dq_write_interrupt_config,
+				       LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_X_LOW);
+
+INDUSTRIALIO_EVENT_ATTR_ACCEL_Y_LOW_SH(industrialio_event_threshold,
+				       lis3l02dq_read_interrupt_config,
+				       lis3l02dq_write_interrupt_config,
+				       LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_Y_LOW);
+
+INDUSTRIALIO_EVENT_ATTR_ACCEL_Z_LOW_SH(industrialio_event_threshold,
+				       lis3l02dq_read_interrupt_config,
+				       lis3l02dq_write_interrupt_config,
+				       LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_Z_LOW);
+
+INDUSTRIALIO_EVENT_ATTR_DATA_RDY(lis3l02dq_read_data_ready_config,
+				lis3l02dq_write_data_ready_config,
+				 0,
+				&lis3l02dq_data_rdy_th);
+
+
+
+static struct attribute *lis3l02dq_event_attributes[] = {
+	&industrialio_event_attr_x_high.dev_attr.attr,
+	&industrialio_event_attr_y_high.dev_attr.attr,
+	&industrialio_event_attr_z_high.dev_attr.attr,
+	&industrialio_event_attr_x_low.dev_attr.attr,
+	&industrialio_event_attr_y_low.dev_attr.attr,
+	&industrialio_event_attr_z_low.dev_attr.attr,
+	&industrialio_event_attr_data_rdy.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group lis3l02dq_event_attribute_group = {
+	.name = "event_sources",
+	.attrs = lis3l02dq_event_attributes,
+};
+
+/*standardize these attributes as much as possible*/
+
+static struct attribute *lis3l02dq_attributes[] = {
+	&industrialio_dev_attr_x_offset.dev_attr.attr,
+	&industrialio_dev_attr_y_offset.dev_attr.attr,
+	&industrialio_dev_attr_z_offset.dev_attr.attr,
+	&industrialio_dev_attr_x_gain.dev_attr.attr,
+	&industrialio_dev_attr_y_gain.dev_attr.attr,
+	&industrialio_dev_attr_z_gain.dev_attr.attr,
+	&industrialio_dev_attr_thresh.dev_attr.attr,
+	&industrialio_dev_attr_x.dev_attr.attr,
+	&industrialio_dev_attr_y.dev_attr.attr,
+	&industrialio_dev_attr_z.dev_attr.attr,
+	&industrialio_dev_attr_sampling_frequency.dev_attr.attr,
+	&industrialio_dev_attr_available_sampling_frequency.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group lis3l02dq_attribute_group = {
+	.attrs = lis3l02dq_attributes,
+};
+
+static int __devinit lis3l02dq_probe(struct spi_device *spi)
+{
+	struct lis3l02dq_state *st;
+	int ret;
+	st = kzalloc(sizeof(struct lis3l02dq_state), GFP_KERNEL);
+	if (st == NULL) {
+		dev_err(&spi->dev, "Memory allocation error \n");
+		return -ENOMEM;
+	}
+	st->us = spi;
+
+	/* setup the industrialio driver allocated elements */
+	st->indio_dev
+		= (struct industrialio_dev *)
+		(kzalloc(sizeof(struct industrialio_dev), GFP_KERNEL));
+	if (st->indio_dev == NULL) {
+		ret = -ENOMEM;
+		goto error_free_st;
+	}
+
+	/* WRAP THIS LOT INTO A COUPLE OF DEFINES! */
+	st->indio_dev->dev = &spi->dev;
+	st->indio_dev->num_interrupt_lines = 1;
+	st->indio_dev->event_attrs = &lis3l02dq_event_attribute_group;
+	st->indio_dev->attrs = &lis3l02dq_attribute_group;
+	st->indio_dev->dev_data = (void *)(st);
+	/* setup parameters of the ring buffer */
+/* CARE WITH WORD ALIGNMENT*/
+	st->indio_dev->ring_dimension = 4;
+	st->indio_dev->ring_bytes_per_reading = 2;
+	st->indio_dev->ring_length = 500;
+	st->indio_dev->driver_module = THIS_MODULE;
+	st->indio_dev->modes = INDIO_DIRECT_MODE | INDIO_RING_DATA_RDY;
+
+
+	ret = industrialio_device_register(st->indio_dev);
+	if (ret < 0)
+		goto error_free_dev;
+	/* FIXME: Due to intialization order the ring is created even if the irq
+	   is no good. Valid when polling is implemented */
+	if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0) {
+		INIT_WORK(&st->work_data_rdy_ring,
+			  lis3l02dq_data_rdy_bh_to_ring);
+		INIT_WORK(&st->work_data_rdy_event,
+			  lis3l02dq_data_rdy_bh_to_event);
+
+		/* This is a little unusual, in that the device seems
+		   to need a full read of the interrupt source reg before
+		   the interrupt will reset.
+		   Hence the two handlers are the same */
+
+		INIT_WORK_CONT(&(st->work_cont_thresh),
+			       lis3l02dq_thresh_handler_bh_no_check,
+			       lis3l02dq_thresh_handler_bh_no_check,
+			       LIS3L02DQ_REG_WAKE_UP_SRC_ADDRESS,
+			       0,
+			       st);
+		st->inter = 0;
+		ret = industrialio_register_interrupt_line(spi->irq,
+							   st->indio_dev,
+							   0,
+							   IRQF_TRIGGER_RISING,
+							   "lis3l02dq");
+		if (ret)
+			goto error_unregister_dev;
+	} else
+		st->indio_dev->modes &= ~INDIO_RING_DATA_RDY;
+
+	/* Get the device into a sane initial state */
+	ret = lis3l02dq_initial_setup(st);
+	if (ret)
+		goto error_unregister_line;
+
+	return 0;
+
+error_unregister_line:
+	if (st->indio_dev->modes & INDIO_RING_DATA_RDY)
+		industrialio_unregister_interrupt_line(st->indio_dev, 0);
+error_unregister_dev:
+	industrialio_device_unregister(st->indio_dev);
+error_free_dev:
+	kfree(st->indio_dev);
+error_free_st:
+	kfree(st);
+	return ret;
+}
+
+/* Power down the device */
+static int lis3l02dq_stop_device(struct lis3l02dq_state *st)
+{
+	int ret;
+
+	ret = lis3l02dq_spi_write_reg_int8_t(&st->us->dev,
+					      LIS3L02DQ_REG_CTRL_1_ADDRESS,
+					      0);
+	if (ret) {
+		dev_err(&st->us->dev, "problem with turning device off: ctrl1");
+		goto err_ret;
+	}
+
+	ret = lis3l02dq_spi_write_reg_int8_t(&st->us->dev,
+					      LIS3L02DQ_REG_CTRL_2_ADDRESS,
+					      0);
+	if (ret) {
+		dev_err(&st->us->dev, "problem with turning device off: ctrl2");
+		goto err_ret;
+	}
+	return 0;
+err_ret:
+	return ret;
+}
+
+/* fixme, confirm ordering in this function */
+static int lis3l02dq_remove(struct spi_device *spi)
+{
+	int ret;
+	struct industrialio_dev *indio_dev = spi_get_drvdata(spi);
+	struct lis3l02dq_state *st = indio_dev->dev_data;
+
+	/* stop the device
+	 * Move into industrialio - can then automating linking
+	 * to power type controls in sysfs? FUTURE WORK!
+	 */
+	/* Amongst other things this must ensure no more interrupts are generate
+	 * by the device */
+	ret = lis3l02dq_stop_device(st);
+	if (ret)
+		goto err_ret;
+	/* Fixme slightly misleading test condition - even if valid */
+	if (st->indio_dev->modes & INDIO_RING_DATA_RDY)
+		industrialio_unregister_interrupt_line(st->indio_dev, 0);
+	industrialio_device_unregister(st->indio_dev);
+	kfree(st->indio_dev);
+	kfree(st);
+	return 0;
+
+err_ret:
+	return ret;
+}
+
+static struct spi_driver lis3l02dq_driver = {
+	.driver = {
+		.name = "lis3l02dq",
+		.owner = THIS_MODULE,
+	},
+	.probe = lis3l02dq_probe,
+	.remove = __devexit_p(lis3l02dq_remove),
+};
+
+static __init int lis3l02dq_init(void)
+{
+	return spi_register_driver(&lis3l02dq_driver);
+}
+
+static __exit void lis3l02dq_exit(void)
+{
+	spi_unregister_driver(&lis3l02dq_driver);
+}
+
+module_init(lis3l02dq_init);
+module_exit(lis3l02dq_exit);
+
+MODULE_AUTHOR("Jonathan Cameron <jic23@cam.ac.uk>");
+MODULE_DESCRIPTION("ST LIS3L02DQ Accelerometer SPI driver");
+MODULE_LICENSE("GPL v2");
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/accelerometer/lis3l02dq.c_oldfuncs b/drivers/industrialio/accelerometer/lis3l02dq.c_oldfuncs
--- a/drivers/industrialio/accelerometer/lis3l02dq.c_oldfuncs	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/accelerometer/lis3l02dq.c_oldfuncs	2008-06-12 18:11:01.000000000 +0100
@@ -0,0 +1,265 @@
+/* A fairly inellegant way of ripping the contents of the
+ * ring buffer and ensuring only a valid set of readings are output */
+
+static ssize_t lis3l02dq_rip_buffer(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	int len = 0, elements, i, dead_offset = 0;
+	uint8_t data_dump[LIS3L02DQ_BUFFER_LENGTH*6];
+	uint8_t *initial_read_p, *initial_write_p,
+		*current_read_p, *end_read_p;
+	struct lis3l02dq_state *st = dev_get_drvdata(dev);
+	uint16_t temp;
+
+	/* Get a consistent pair of read and write pointers */
+	initial_read_p = st->read_pointer;
+
+	/* Occurs if nothing has yet been placed in the ring buffer */
+	if (unlikely(initial_read_p == 0))
+		goto err;
+
+	initial_write_p = st->write_pointer;
+	while (initial_read_p != st->read_pointer
+	       || initial_write_p != st->write_pointer) {
+		initial_read_p = st->read_pointer;
+		initial_write_p = st->write_pointer;
+	}
+	if (initial_write_p > initial_read_p) {
+		elements = (initial_write_p - initial_read_p);
+		memcpy(data_dump, initial_read_p, elements);
+	} else {
+		elements = st->ring_buffer
+			+ LIS3L02DQ_BUFFER_LENGTH*6
+			- initial_read_p;
+
+		memcpy(data_dump, initial_read_p, elements);
+		memcpy(data_dump+elements,
+		       st->ring_buffer,
+		       initial_write_p - st->ring_buffer);
+		elements += initial_write_p - st->ring_buffer;
+	}
+
+	end_read_p = st->read_pointer;
+
+	if (initial_read_p <= end_read_p)
+		dead_offset = end_read_p - initial_read_p;
+	else
+		dead_offset = st->ring_buffer
+			+ LIS3L02DQ_BUFFER_LENGTH*6 - initial_read_p
+			+ end_read_p - st->ring_buffer;
+
+	/* Possible issue here is the readpointer may have changed.
+	 * It could in theory have passed the initial write pointer.*/
+	st->read_pointer = initial_write_p;
+
+	for (current_read_p = data_dump + dead_offset;
+	    current_read_p < data_dump + elements;
+	    current_read_p += 6) {
+		for (i = 0; i < 3; i++) {
+			temp = (((uint16_t)((current_read_p[2*i+1]))) << 8)
+				| (uint16_t)(current_read_p[2*i]);
+			len += sprintf(len+buf, "%d ", *((int16_t *)(&temp)));
+		}
+	}
+	len += sprintf(len+buf, "\n");
+
+	return len;
+
+err:
+	return 0;
+}
+
+static int lis3l02dq_set_mode(struct device *dev, int val)
+{
+	int ret;
+	uint8_t tmp;
+	const uint8_t addr_ctrl2 = LIS3L02DQ_REG_CTRL_2_ADDRESS;
+	struct lis3l02dq_state *st = dev_get_drvdata(dev);
+
+	if (st->mode == val
+	    || st->mode == LIS3L02DQ_DIRECT_ONLY_MODE)
+		return 0;
+
+	switch (val) {
+	case 0:
+		/* disable interrupt generation */
+		ret = lis3l02dq_write_register_int8_t(&st->us->dev,
+						      addr_ctrl2,
+						      LIS3L02DQ_DEFAULT_CTRL2);
+		if (ret)	{
+			dev_err(&st->us->dev,
+				"problem with setup control register 2");
+			goto err_ret;
+		}
+
+		flush_scheduled_work();
+		free_irq(st->us->irq, st);
+		break;
+
+	case 1:
+		/* quick read to ensure that the interrupt line is low */
+		lis3l02dq_read_all(st);
+		ret = request_irq(st->us->irq,
+				  interrupthandler,
+				  IRQF_TRIGGER_RISING,
+				  "lis3l02dq",
+				  st);
+		if (ret < 0) {
+			dev_err(&st->us->dev, "Could not obtain irq ");
+			goto err_ret;
+		}
+		/* enable interrupt generation */
+	tmp =  LIS3L02DQ_DEFAULT_CTRL2	       
+			|  LIS3L02DQ_REG_CTRL_2_ENABLE_DATA_READY_GENERATION;
+		ret = lis3l02dq_write_register_int8_t(&st->us->dev,
+						      addr_ctrl2,
+						      tmp);
+		if (ret) {
+			dev_err(&st->us->dev,
+				"problem with setup control register 2");
+			goto err_ret;
+		}
+		break;
+	default:
+		ret = -EINVAL;
+		goto err_ret;
+		break;
+	};
+	st->mode = val;
+
+	return 0;
+err_ret:
+	return ret;
+}
+
+static ssize_t lis3l02dq_read_mode(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	int len = 0;
+	struct lis3l02dq_state *st = dev_get_drvdata(dev);
+
+	len += sprintf(buf + len, "%d\n", st->mode);
+
+	return len;
+}
+
+static ssize_t lis3l02dq_write_mode(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf,
+				    size_t len)
+{
+	int ret;
+	long val;
+
+	ret = strict_strtol(buf, 10, &val);
+	if (ret < 0)
+		goto err_ret;
+	ret = lis3l02dq_set_mode(dev, val);
+	if (ret < 0)
+		goto err_ret;
+	return len;
+err_ret:
+	return ret;
+}
+
+static DEVICE_ATTR(mode, S_IWUSR | S_IRUGO, lis3l02dq_read_mode,
+		   lis3l02dq_write_mode);
+
+
+
+static DEVICE_ATTR(rip_buffer, S_IRUGO, lis3l02dq_rip_buffer, NULL);
+
+
+
+static void lis3l02dq_store_to_ring(struct lis3l02dq_state *st)
+{
+	int i;
+	bool initread = true;
+
+	/* First use of ring */
+	if (unlikely(st->write_pointer == 0)) {
+		st->write_pointer = st->ring_buffer;
+		initread = false;
+	}
+	/*probably unnecessary */
+	barrier();
+	/*save data */
+	for (i = 0; i < 3; i++) {
+		st->write_pointer[i*2] = st->rx_buff[i*4];
+		st->write_pointer[i*2+1] = st->rx_buff[i*4+2];
+	}
+	barrier();
+	st->last_written_pointer = st->write_pointer;
+	barrier();
+	st->write_pointer += 6;
+	if (unlikely(st->write_pointer
+		     == st->ring_buffer + 6*LIS3L02DQ_BUFFER_LENGTH))
+		st->write_pointer = st->ring_buffer;
+
+	if (unlikely(st->read_pointer == 0))
+		st->read_pointer = st->ring_buffer;
+	else if (st->write_pointer == st->read_pointer)	{
+		if (unlikely((st->read_pointer+6
+			      == st->ring_buffer + 6*LIS3L02DQ_BUFFER_LENGTH)))
+			st->read_pointer = st->ring_buffer;
+		else
+			st->read_pointer += 6;
+	}
+	return;
+}
+
+static void lis3l02dq_data_ready_work(struct work_struct *work_s)
+{
+	struct lis3l02dq_state *st
+		= container_of(work_s, struct lis3l02dq_state, work);
+
+	st->inter = 0;
+	if (lis3l02dq_read_all(st) >= 0)
+		lis3l02dq_store_to_ring(st);
+
+try_again:
+	while (gpio_get_value(irq_to_gpio(st->us->irq)))
+		if (lis3l02dq_read_all(st) >= 0)
+			lis3l02dq_store_to_ring(st);
+	/* If we are lucky gpio should not be set now
+	 * Try reenabling interrupt. */
+	enable_irq(st->us->irq);
+	/* verify that either the gpio has not risen or that
+	 * the interrupt handler caught it */
+	if (gpio_get_value(irq_to_gpio(st->us->irq)))
+		if (st->inter == 0) {
+			disable_irq_nosync(st->us->irq);
+			goto try_again;
+		}
+	return;
+}
+
+
+/* can I make any of this generic - yes! */
+static irqreturn_t interrupthandler(int irq, void *_state)
+{
+
+	struct lis3l02dq_state *st = _state;
+	struct industrialio_event_list* p;
+
+	disable_irq_nosync(irq);
+	/* Could it have been us? */
+	if(list_empty(&st->interrupt_pin_event_list.list))
+		return IRQ_NONE;
+
+	/* Grab and store a time stamp - might take us a while to fill in the details */
+	/* Assume for now it could only have been us */
+	/* If only one possible source exists don't need to identify */
+	if(st->interrupt_pin_event_list.list.next->next == &st->interrupt_pin_event_list.list);
+	list_for_each_entry(p, &st->interrupt_pin_event_list.list, list)
+	{
+		/* fixme, need the index from somewhere safe! hang on, I'm in an interrupt, anywhere is fine */
+		/* Hang on, what's the point of the index?  If the time is fine, all is good */
+		p->handler(_state, 1, 1);
+	}
+	/*Otherwise we need to queue a query to find out what it was and then handle later */
+
+	return IRQ_HANDLED;
+}
\ No newline at end of file
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/accelerometer/lis3l02dq.h b/drivers/industrialio/accelerometer/lis3l02dq.h
--- a/drivers/industrialio/accelerometer/lis3l02dq.h	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/accelerometer/lis3l02dq.h	2008-06-26 18:07:34.000000000 +0100
@@ -0,0 +1,183 @@
+/*
+ * LISL02DQ.h -- support STMicroelectronics LISD02DQ
+ *               3d 2g Linear Accelerometers via SPI
+ *
+ * Copyright (c) 2007 Jonathan Cameron <jic23@cam.ac.uk>
+ *
+ * Loosely based upon tle62x0.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This driver has two modes, one is interrupt based, the other on demand
+ */
+#ifndef SPI_LIS3L02DQ_H_
+#define SPI_LIS3L02DQ_H_
+#define LIS3L02DQ_READ_REG(a) ((a) | 0x80)
+#define LIS3L02DQ_WRITE_REG(a) a
+
+/* Calibration parameters */
+#define LIS3L02DQ_REG_OFFSET_X_ADDRESS		0x16
+#define LIS3L02DQ_REG_OFFSET_Y_ADDRESS		0x17
+#define LIS3L02DQ_REG_OFFSET_Z_ADDRESS		0x18
+
+#define LIS3L02DQ_REG_GAIN_X_ADDRESS		0x19
+#define LIS3L02DQ_REG_GAIN_Y_ADDRESS		0x1A
+#define LIS3L02DQ_REG_GAIN_Z_ADDRESS		0x1B
+
+/* Control Register (1 of 2) */
+#define LIS3L02DQ_REG_CTRL_1_ADDRESS		0x20
+/* Power ctrl - either bit set corresponds to on*/
+#define LIS3L02DQ_REG_CTRL_1_PD_ON	0xC0
+
+/* Decimation Factor  */
+#define LIS3L02DQ_DEC_MASK			0x30
+#define LIS3L02DQ_REG_CTRL_1_DF_128		0x00
+#define LIS3L02DQ_REG_CTRL_1_DF_64		0x10
+#define LIS3L02DQ_REG_CTRL_1_DF_32		0x20
+#define LIS3L02DQ_REG_CTRL_1_DF_8		(0x10 | 0x20)
+
+/* Self Test Enable */
+#define LIS3L02DQ_REG_CTRL_1_SELF_TEST_ON	0x08
+
+/* Axes enable ctrls */
+#define LIS3L02DQ_REG_CTRL_1_AXES_Z_ENABLE	0x04
+#define LIS3L02DQ_REG_CTRL_1_AXES_Y_ENABLE	0x02
+#define LIS3L02DQ_REG_CTRL_1_AXES_X_ENABLE	0x01
+
+/* Control Register (2 of 2) */
+#define LIS3L02DQ_REG_CTRL_2_ADDRESS		0x21
+
+/* Block Data Update only after MSB and LSB read */
+#define LIS3L02DQ_REG_CTRL_2_BLOCK_UPDATE	0x40
+
+/* Set to big endian output */
+#define LIS3L02DQ_REG_CTRL_2_BIG_ENDIAN		0x20
+
+/* Reboot memory content */
+#define LIS3L02DQ_REG_CTRL_2_REBOOT_MEMORY	0x10
+
+/* Interupt Enable - applies data ready to the RDY pad */
+#define LIS3L02DQ_REG_CTRL_2_ENABLE_INTERRUPT	0x08
+
+/* Enable Data Ready Generation - relationship with previous unclear in docs */
+#define LIS3L02DQ_REG_CTRL_2_ENABLE_DATA_READY_GENERATION 0x04
+
+/* SPI 3 wire mode */
+#define LIS3L02DQ_REG_CTRL_2_THREE_WIRE_SPI_MODE	0x02
+
+/* Data alignment, default is 12 bit right justified
+ * - option for 16 bit left justified */
+#define LIS3L02DQ_REG_CTRL_2_DATA_ALIGNMENT_16_BIT_LEFT_JUSTIFIED	0x01
+
+/* Interupt related stuff */
+#define LIS3L02DQ_REG_WAKE_UP_CFG_ADDRESS		0x23
+
+/* Switch from or combination fo conditions to and */
+#define LIS3L02DQ_REG_WAKE_UP_CFG_BOOLEAN_AND		0x80
+
+/* Latch interupt request,
+ * if on ack must be given by reading the ack register */
+#define LIS3L02DQ_REG_WAKE_UP_CFG_LATCH_SRC		0x40
+
+/* Z Interupt on High (above threshold)*/
+#define LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_Z_HIGH	0x20
+/* Z Interupt on Low */
+#define LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_Z_LOW	0x10
+/* Y Interupt on High */
+#define LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_Y_HIGH	0x08
+/* Y Interupt on Low */
+#define LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_Y_LOW	0x04
+/* X Interupt on High */
+#define LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_X_HIGH	0x02
+/* X Interupt on Low */
+#define LIS3L02DQ_REG_WAKE_UP_CFG_INTERRUPT_X_LOW 0x01
+
+/* Register that gives description of what caused interupt
+ * - latched if set in CFG_ADDRES */
+#define LIS3L02DQ_REG_WAKE_UP_SRC_ADDRESS		0x24
+/* top bit ignored */
+/* Interupt Active */
+#define LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_ACTIVATED	0x40
+/* Interupts that have been triggered */
+#define LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_Z_HIGH	0x20
+#define LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_Z_LOW	0x10
+#define LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_Y_HIGH	0x08
+#define LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_Y_LOW	0x04
+#define LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_X_HIGH	0x02
+#define LIS3L02DQ_REG_WAKE_UP_SRC_INTERRUPT_X_LOW	0x01
+
+#define LIS3L02DQ_REG_WAKE_UP_ACK_ADDRESS		0x25
+
+/* Status register */
+#define LIS3L02DQ_REG_STATUS_ADDRESS			0x27
+/* XYZ axis data overrun - first is all overrun? */
+#define LIS3L02DQ_REG_STATUS_XYZ_OVERRUN		0x80
+#define LIS3L02DQ_REG_STATUS_Z_OVERRUN			0x40
+#define LIS3L02DQ_REG_STATUS_Y_OVERRUN			0x20
+#define LIS3L02DQ_REG_STATUS_X_OVERRUN			0x10
+/* XYZ new data available - first is all 3 available? */
+#define LIS3L02DQ_REG_STATUS_XYZ_NEW_DATA 0x08
+#define LIS3L02DQ_REG_STATUS_Z_NEW_DATA			0x04
+#define LIS3L02DQ_REG_STATUS_Y_NEW_DATA			0x02
+#define LIS3L02DQ_REG_STATUS_X_NEW_DATA			0x01
+
+/* The accelerometer readings - low and high bytes.
+Form of high byte dependant on justification set in ctrl reg */
+#define LIS3L02DQ_REG_OUT_X_L_ADDRESS			0x28
+#define LIS3L02DQ_REG_OUT_X_H_ADDRESS			0x29
+#define LIS3L02DQ_REG_OUT_Y_L_ADDRESS			0x2A
+#define LIS3L02DQ_REG_OUT_Y_H_ADDRESS			0x2B
+#define LIS3L02DQ_REG_OUT_Z_L_ADDRESS			0x2C
+#define LIS3L02DQ_REG_OUT_Z_H_ADDRESS			0x2D
+
+/* Threshold values for all axes and both above and below thresholds
+ * - i.e. there is only one value */
+#define LIS3L02DQ_REG_THS_L_ADDRESS			0x2E
+#define LIS3L02DQ_REG_THS_H_ADDRESS			0x2F
+
+#define LIS3L02DQ_DEFAULT_CTRL1 (LIS3L02DQ_REG_CTRL_1_PD_ON	      \
+				 | LIS3L02DQ_REG_CTRL_1_AXES_Z_ENABLE \
+				 | LIS3L02DQ_REG_CTRL_1_AXES_Y_ENABLE \
+				 | LIS3L02DQ_REG_CTRL_1_AXES_X_ENABLE \
+				 | LIS3L02DQ_REG_CTRL_1_DF_128)
+
+#define LIS3L02DQ_DEFAULT_CTRL2	0
+
+#define LIS3L02DQ_DIRECT_ONLY_MODE	-1
+#define LIS3L02DQ_DIRECT_MODE		0
+#define LIS3L02DQ_INTERRUPT_MODE	1
+
+#define LIS3L02DQ_BUFFER_LENGTH		100
+
+struct lis3l02dq_work_cont {
+	struct work_struct ws;
+	struct work_struct ws_nocheck;
+	int address;
+	int mask;
+	struct lis3l02dq_state *st;
+};
+
+#define INIT_WORK_CONT(cont, _checkfunc, _nocheckfunc, _add, _mask, _st)\
+	do {								\
+	INIT_WORK(&(cont)->ws, _checkfunc);				\
+	INIT_WORK(&(cont)->ws_nocheck, _nocheckfunc);			\
+	(cont)->address = _add;						\
+	(cont)->mask = _mask;						\
+	(cont)->st = _st;						\
+	} while (0)
+
+struct lis3l02dq_state {
+	struct spi_device		*us;
+	struct work_struct		work_data_rdy_ring;
+	struct work_struct		work_data_rdy_event;
+	struct lis3l02dq_work_cont	work_cont_thresh;
+
+	/* Interrupt caught event - used as part of the datardy to ring bh
+	   in ensuring interrupt line does not become locked high */
+	bool				inter;
+	s64				last_timestamp;
+	struct industrialio_dev		*indio_dev;
+};
+#endif /* SPI_LIS3L02DQ_H_ */
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/adc/Makefile b/drivers/industrialio/adc/Makefile
--- a/drivers/industrialio/adc/Makefile	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/adc/Makefile	2008-06-05 11:31:16.000000000 +0100
@@ -0,0 +1,3 @@
+
+# Makefile for industrial I/O ADC drivers
+#
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/industrial.c b/drivers/industrialio/industrial.c
--- a/drivers/industrialio/industrial.c	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/industrial.c	2008-06-26 18:26:12.000000000 +0100
@@ -0,0 +1,1172 @@
+/* The industrial I/O core
+ *
+ * Copyright (c) 2008 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * Based on elements of hwmon and input subsystems.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/idr.h>
+#include <linux/kdev_t.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/interrupt.h>
+#include <linux/poll.h>
+
+#include <linux/industrialio.h>
+
+MODULE_AUTHOR("Jonathan Cameron <jic23@cam.ac.uk>");
+MODULE_DESCRIPTION("Industrial I/O core");
+MODULE_LICENSE("GPL");
+
+#define INDUSTRIALIO_ID_PREFIX "industrialio"
+#define INDUSTRIALIO_ID_FORMAT INDUSTRIALIO_ID_PREFIX "%d"
+#define INDUSTRIALIO_MAJOR 244
+
+/* Integer id  - used to assign each registered device a unique id*/
+static DEFINE_IDR(industrialio_idr);
+
+/* Single spinlock used to protect all the IDRs */
+static DEFINE_SPINLOCK(industrialio_idr_lock);
+
+struct class industrialio_class = {
+	.name = "industrialio",
+};
+EXPORT_SYMBOL_GPL(industrialio_class);
+
+static DEFINE_SPINLOCK(industrialio_state_lock);
+
+/* Struct used to maintain internal state about industrialio.
+ * This will be used to handle the character device accesses
+ * and redirect them to the relevant driver.
+ * Will reduce this to the included table if nothing else comes
+ * up that should go in here!
+ */
+struct __industrialio_state {
+	/* All initially set to NULL in init */
+	struct industrialio_handler *fhs[256];
+};
+
+
+static struct __industrialio_state industrialio_state;
+
+/* Used to escalate shared event.
+ * currently this only used with ring buffer events */
+static inline void
+__industrialio_change_event(struct industrialio_detected_event_list *ev,
+			    int ev_code,
+			    s64 timestamp)
+{
+	ev->ev.id = ev_code;
+	ev->ev.timestamp = timestamp;
+}
+
+/* Used both in the interrupt line put events and the ring buffer ones */
+static inline int
+__industrialio_put_event(struct industrialio_event_interface *ev_int,
+			 int ev_code,
+			 s64 timestamp,
+			 struct industrialio_shared_ev_pointer*
+			 shared_pointer_p)
+{
+	struct industrialio_detected_event_list *ev;
+	int ret;
+	/* Does anyone care? */
+	if (test_bit(INDUSTRIALIO_BUSY_BIT_POS, &ev_int->handler.flags)) {
+		if (ev_int->current_events
+		   == ev_int->max_events)
+			return 0;
+		ev = (struct industrialio_detected_event_list *)
+			(kmalloc(sizeof(struct
+					industrialio_detected_event_list),
+				 GFP_KERNEL));
+		if (ev == NULL) {
+			ret = -ENOMEM;
+			goto error_ret;
+		}
+
+		INIT_LIST_HEAD(&ev->list);
+		ev->ev.id = ev_code;
+		ev->ev.timestamp = timestamp;
+		if (shared_pointer_p != NULL) {
+			ev->shared_pointer = shared_pointer_p;
+			shared_pointer_p->ev_p = ev;
+		} else
+			ev->shared_pointer = NULL;
+		list_add_tail(&ev->list, &ev_int->det_events.list);
+		ev_int->current_events++;
+		wake_up_interruptible(&ev_int->wait);
+	}
+
+	return 0;
+error_ret:
+	return ret;
+
+}
+
+int industrialio_put_event(struct industrialio_dev *dev_info,
+			    int ev_line,
+			    int ev_code,
+			    s64 timestamp)
+{
+	return __industrialio_put_event(&dev_info->event_interfaces[ev_line],
+					ev_code,
+					timestamp, NULL);
+}
+EXPORT_SYMBOL(industrialio_put_event);
+
+/* Confirming validity of supplied irq is left to drivers */
+int industrialio_register_interrupt_line(unsigned int irq,
+					 struct industrialio_dev *dev_info,
+					 int line_number,
+					 unsigned long type,
+					 const char *name)
+{
+	int ret;
+
+	dev_info->interrupts[line_number] = (struct industrialio_interrupt *)
+		kmalloc(sizeof(struct industrialio_interrupt), GFP_KERNEL);
+	if (dev_info->interrupts[line_number] == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+
+	INIT_LIST_HEAD(&dev_info->interrupts[line_number]->ev_list.list);
+	dev_info->interrupts[line_number]->line_number = line_number;
+	dev_info->interrupts[line_number]->irq = irq;
+	dev_info->interrupts[line_number]->dev_info = dev_info;
+
+	/* Possibly only request on demand?
+	 * Can see this may complicate the handling of interrupts.
+	 * However, with this approache we end up handling lots of
+	 * events noone cares about.*/
+	ret = request_irq(irq,
+			  &industrialio_interrupt_handler,
+			  type,
+			  name,
+			  dev_info->interrupts[line_number]);
+	if (ret < 0)
+		goto error_ret;
+
+	return 0;
+
+error_ret:
+	return ret;
+}
+EXPORT_SYMBOL(industrialio_register_interrupt_line);
+
+/* Before this runs the interrupt generator must have been disabled */
+void industrialio_unregister_interrupt_line(struct industrialio_dev *dev_info,
+					    int line_number)
+{
+	/* make sure the interrupt handlers are all done */
+	flush_scheduled_work();
+	free_irq(dev_info->interrupts[line_number]->irq,
+		 dev_info->interrupts[line_number]);
+	kfree(dev_info->interrupts[line_number]);
+}
+EXPORT_SYMBOL(industrialio_unregister_interrupt_line);
+
+/* Generic interrupt line interrupt handler */
+irqreturn_t industrialio_interrupt_handler(int irq, void *_int_info)
+{
+	struct industrialio_interrupt *int_info = _int_info;
+	struct industrialio_dev *dev_info = int_info->dev_info;
+	struct industrialio_event_handler_list *p;
+	s64 time_ns;
+
+	if (list_empty(&int_info->ev_list.list))
+		return IRQ_NONE;
+
+	time_ns = industrialio_get_time_ns();
+	/* detect single element list*/
+	if (int_info->ev_list.list.next->next == &int_info->ev_list.list) {
+		disable_irq_nosync(irq);
+		p = list_first_entry(&int_info->ev_list.list,
+				     struct industrialio_event_handler_list,
+				     list);
+		p->handler(dev_info, 1, time_ns, 1);
+	} else
+		list_for_each_entry(p, &int_info->ev_list.list, list)
+		{
+			disable_irq_nosync(irq);
+			p->handler(dev_info, 1, time_ns, 0);
+		}
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(industrialio_interrupt_handler);
+
+int industrialio_add_event_to_list(struct industrialio_event_handler_list *list,
+				   struct industrialio_event_handler_list *el)
+{
+	if (el->refcount == 0)
+		list_add(&list->list, &el->list);
+	el->refcount++;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(industrialio_add_event_to_list);
+
+int industrialio_remove_event_from_list(struct industrialio_event_handler_list
+					*el)
+{
+	el->refcount--;
+	if (el->refcount == 0)
+		list_del_init(&el->list);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(industrialio_remove_event_from_list);
+
+
+static int industrialio_allocate_chrdev(struct industrialio_handler *handler)
+{
+	int id;
+	spin_lock(industrialio_state_lock);
+	/* Find an unused device - fixme, more efficient options?*/
+	for (id = 0; id <= 256; id++)
+		if (industrialio_state.fhs[id] == NULL)
+			break;
+	if (id == 256) { /*FIXME incorrect error code ?*/
+		spin_unlock(industrialio_state_lock);
+		return -ENOMEM;
+	}
+	industrialio_state.fhs[id] = handler;
+	spin_unlock(industrialio_state_lock);
+	handler->id = id;
+
+	return 0;
+}
+
+
+static void industrialio_deallocate_chrdev(struct industrialio_handler *handler)
+{
+	spin_lock(industrialio_state_lock);
+	industrialio_state.fhs[handler->id] = NULL;
+	spin_unlock(industrialio_state_lock);
+}
+
+/* Upon open, switch in the correct file ops
+ * lifted directly from input subsystem */
+static int industrialio_open_file(struct inode *inode, struct file *file)
+{
+	struct industrialio_handler *handler;
+	const struct file_operations *old_fops, *new_fops = NULL;
+	int err;
+
+
+	/* This lock needed as unlike input we are dynamically allocating
+	 * chrdevs */
+	spin_lock(industrialio_state_lock);
+	handler = industrialio_state.fhs[iminor(inode)];
+	spin_unlock(industrialio_state_lock);
+	if (!handler || !(new_fops == fops_get(handler->fops)))
+		return -ENODEV;
+	/* cribbed from lp.c - not entirely certain if the fops_put is
+	 * necessary */
+
+	if (test_and_set_bit(INDUSTRIALIO_BUSY_BIT_POS, &handler->flags)) {
+		fops_put(file->f_op);
+		return -EBUSY;
+	}
+
+	if (!new_fops->open) {
+		fops_put(new_fops);
+		return -ENODEV;
+	}
+	old_fops = file->f_op;
+	file->f_op = new_fops;
+	/* use the private data pointer in file to give access to device
+	 * specific stuff */
+	file->private_data = handler->private;
+	err = new_fops->open(inode, file);
+
+	if (err) {
+		fops_put(file->f_op);
+		file->f_op = fops_get(old_fops);
+	}
+	fops_put(old_fops);
+
+	return err;
+}
+
+
+/* The main file ops structure. All open calls on the major number will
+ * be handled by this with fops for the actual minor number assigned by
+ * switching function above */
+
+static const struct file_operations industrialio_fops = {
+	.owner = THIS_MODULE,
+	.open = industrialio_open_file,
+};
+static int count;
+/* Ring buffer related functionality */
+int industrialio_store_to_ring(struct industrialio_ring_buffer *ring,
+			       unsigned char *data,
+			       s64 timestamp)
+{
+	bool init_read = true;
+	int ret;
+	int code;
+
+	/* initial store */
+	if (unlikely(ring->write_p == 0)) {
+		ring->write_p = ring->data;
+		/* doesn't actually matter if this is out of the set
+		 * FIXME deal with odd ring length */
+		ring->half_p = ring->data - ring->length*ring->skip / 2;
+		init_read = false;
+	}
+	memcpy(ring->write_p, data, ring->size);
+	memcpy(ring->write_p + ring->size, &timestamp, sizeof(s64));
+		barrier();
+	ring->last_written_p = ring->write_p;
+	barrier();
+	ring->write_p += ring->skip;
+	/* End of ring, back to the beginning */
+	if (ring->write_p == ring->data + ring->length*ring->skip) {
+		ring->write_p = ring->data;
+		ring->loopcount++;
+	}
+	if (ring->read_p == 0)
+		ring->read_p = ring->data;
+	/* Buffer full - move the read pointer and create / escalate
+	 * ring event */
+	else if (ring->write_p == ring->read_p) {
+		ring->read_p += ring->skip;
+		if (ring->read_p == ring->data + ring->length*ring->skip)
+			ring->read_p = ring->data;
+		/* Needs protection against unexpected delete? How? */
+		spin_lock(ring->shared_ev_pointer.lock);
+		if (ring->shared_ev_pointer.ev_p) {
+			/* Event escalation - probably quicker to let this
+			   keep running than check if it is necessary */
+			code = INDUSTRIALIO_EVENT_CODE_RING_100_FULL;
+			__industrialio_change_event(ring
+						    ->shared_ev_pointer.ev_p,
+						    code,
+						    timestamp);
+		} else {
+			code = INDUSTRIALIO_EVENT_CODE_RING_100_FULL;
+			ret = __industrialio_put_event(&ring->ev_int,
+						       code,
+						       timestamp,
+						       &ring
+						       ->shared_ev_pointer);
+			if (ret) {
+				spin_unlock(ring->shared_ev_pointer.lock);
+				goto error_ret;
+			}
+		}
+		spin_unlock(ring->shared_ev_pointer.lock);
+
+	}
+
+
+	/* investigate if our event barrier has been passed */
+	/* There are definite 'issues' with this and chances of
+	 * simultaneous read */
+	/* Also need to use loop count to ensure this only happens once */
+	ring->half_p += ring->skip;
+	if (ring->half_p == ring->data + ring->length*ring->skip)
+		ring->half_p = ring->data;
+	if (ring->half_p == ring->read_p) {
+		spin_lock(ring->shared_ev_pointer.lock);
+		code = INDUSTRIALIO_EVENT_CODE_RING_50_FULL;
+		ret = __industrialio_put_event(&ring->ev_int,
+					       code,
+					       timestamp,
+					       &ring->shared_ev_pointer);
+		spin_unlock(ring->shared_ev_pointer.lock);
+		if (ret)
+			goto error_ret;
+	}
+	return 0;
+error_ret:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(industrialio_store_to_ring);
+
+int industrialio_read_last_from_ring(struct industrialio_ring_buffer *ring,
+					    unsigned char *data)
+{
+	int loopcount_copy;
+	unsigned char *last_written_p_copy;
+again:
+	loopcount_copy = ring->loopcount;
+	barrier();
+	last_written_p_copy = ring->last_written_p;
+	barrier(); /*unnessecary? */
+
+	memcpy(data, last_written_p_copy, ring->size);
+
+	if (unlikely(loopcount_copy != ring->loopcount)) {
+		if (unlikely(ring->last_written_p >= last_written_p_copy))
+			goto again;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(industrialio_read_last_from_ring);
+
+ssize_t industrialio_interrupt_read(struct file *filep,
+				    char *buf,
+				    size_t count,
+				    loff_t *f_ps)
+{
+	struct industrialio_event_interface *ev_int = filep->private_data;
+	struct industrialio_detected_event_list *el;
+	int ret;
+	/* event interface has a list of events
+	 * - if empty and blocking, block */
+	/* need lock to protect this list ?*/
+
+	if (list_empty(&ev_int->det_events.list)) {
+		if (filep->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+		/* So we are blocking on this device waiting for something
+		 * to be there */
+		/* FIXME: Take into account risk that something may have
+		 * disappeared in meantime!*/
+		ret = wait_event_interruptible(ev_int->wait,
+					       !list_empty(&ev_int
+							   ->det_events.list));
+		if (ret)
+			return ret;
+	}
+
+	el = list_first_entry(&ev_int->det_events.list,
+			      struct industrialio_detected_event_list,
+			      list);
+
+	if (copy_to_user(buf, &(el->ev),
+			 sizeof(struct industrialio_event_data)))
+		return -EFAULT;
+
+	list_del(&el->list);
+	ev_int->current_events--;
+	/* Need to zero pointer to shared element*/
+	spin_lock(el->shared_pointer->lock);
+	if (el->shared_pointer)
+		(el->shared_pointer->ev_p) = NULL;
+	spin_unlock(el->shared_pointer->lock);
+	kfree(el);
+
+	return sizeof(struct industrialio_event_data);
+}
+
+int industrialio_interrupt_release(struct inode *inode, struct file *filep)
+{
+	struct industrialio_event_interface *ev_int = filep->private_data;
+
+	module_put(ev_int->owner);
+	clear_bit(INDUSTRIALIO_BUSY_BIT_POS, &ev_int->handler.flags);
+
+	return 0;
+}
+
+int industrialio_interrupt_open(struct inode *inode, struct file *filep)
+{
+	struct industrialio_event_interface *ev_int = filep->private_data;
+	try_module_get(ev_int->owner);
+
+	return 0;
+}
+static const struct file_operations industrialio_interrupt_fileops = {
+	.read =  industrialio_interrupt_read,
+	.release = industrialio_interrupt_release,
+	.open = industrialio_interrupt_open,
+	.owner = THIS_MODULE,
+};
+
+
+static ssize_t industrialio_show_attr_minor(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	int len;
+
+	struct industrialio_chrdev_minor_attr *_attr
+		= to_industrialio_chrdev_minor_attr(attr);
+	len = sprintf(buf, "%d\n", _attr->minor);
+
+	return len;
+}
+static inline int
+industrialio_setup_ev_int(struct industrialio_event_interface *ev_int,
+			  const char *name,
+			  struct module *owner,
+			  struct device *dev)
+{
+	int ret;
+	/* FIXME MAKE THIS VARIABLE! */
+	ev_int->max_events = 10;
+	ev_int->current_events = 0;
+	INIT_LIST_HEAD(&ev_int->det_events.list);
+	init_waitqueue_head(&ev_int->wait);
+	ev_int->handler.fops = &industrialio_interrupt_fileops;
+	ev_int->handler.private = ev_int;
+	ev_int->handler.flags = 0;
+	ret = industrialio_allocate_chrdev(&ev_int->handler);
+	if (ret)
+		goto error_ret;
+	ev_int->attr.dev_attr.attr.name = (const char *)(name);
+	ev_int->attr.dev_attr.attr.owner = owner;
+	ev_int->owner = owner;
+	ev_int->attr.minor = ev_int->handler.id;
+	ev_int->attr.dev_attr.attr.mode = S_IRUGO;
+	ev_int->attr.dev_attr.show = &industrialio_show_attr_minor;
+	ret = sysfs_create_file(&dev->kobj, &ev_int->attr.dev_attr.attr);
+	if (ret)
+		goto error_deallocate_chrdev;
+
+	return 0;
+error_deallocate_chrdev:
+	industrialio_deallocate_chrdev(&ev_int->handler);
+error_ret:
+	return ret;
+}
+
+static inline void
+industrialio_free_ev_int(struct industrialio_event_interface *ev_int,
+						     struct device *dev)
+{
+	sysfs_remove_file(&dev->kobj, &ev_int->attr.dev_attr.attr);
+	industrialio_deallocate_chrdev(&ev_int->handler);
+}
+
+
+
+
+/*
+ * Create a character device for reading from local ring buffer. One of
+ * these per device.
+ * Create this as part of a request for a ring buffer? Yes. If it's not
+ * there then the device does not have an associate ring buffer!
+ * How to make reading through to a hardware ring buffer transparent?
+ */
+
+
+int industrialio_ring_open(struct inode *inode, struct file *filp)
+{
+	/* So now we need to know where this should go !*/
+	struct industrialio_ring_buffer *ring = filp->private_data;
+	/*Fixme MESSY - should the ring have an owner?*/
+	try_module_get(ring->access_minor_attr.dev_attr.attr.owner);
+
+	return 0;
+}
+
+int industrialio_ring_release(struct inode *inode, struct file *filp)
+{
+	struct industrialio_ring_buffer *ring = filp->private_data;
+
+	module_put(ring->access_minor_attr.dev_attr.attr.owner);
+	clear_bit(INDUSTRIALIO_BUSY_BIT_POS, &ring->access_handler.flags);
+
+	return 0;
+}
+
+/* no point in ripping more than nearest number of whole records below count */
+/* Depending on movement of pointers in the meantime this may return a lot
+ * less than count*/
+/* Also, we aren't going to wait for enough data to be available */
+/* NEEDS CODE REVIEW */
+ssize_t industrialio_ring_rip(struct file *filp,
+			      char *buf,
+			      size_t count,
+			      loff_t *f_ps)
+{
+	unsigned char *initial_read_p, *initial_write_p,
+		*current_read_p, *end_read_p;
+
+	struct industrialio_ring_buffer *ring = filp->private_data;
+	unsigned char *data_cpy;
+	int dead_offset;
+	int bytes_to_rip = 0;
+	int max_copied;
+
+	bytes_to_rip = (count - count % ring->skip);
+	/*fixme, needed? */
+	if (bytes_to_rip > ring->skip*ring->length)
+		bytes_to_rip = ring->skip*ring->length;
+	data_cpy = (unsigned char *)(kmalloc(bytes_to_rip, GFP_KERNEL));
+	/* Handle -ENOMEM*/
+	/* build local copy */
+	initial_read_p = ring->read_p;
+	if (unlikely(initial_read_p == 0))
+		goto err;
+	initial_write_p = ring->write_p;
+
+	/* Need a consistent pair */
+	while (initial_read_p != ring->read_p
+	      || initial_write_p != ring->write_p) {
+		initial_read_p = ring->read_p;
+		initial_write_p = ring->write_p;
+	}
+	if (initial_write_p == initial_read_p)
+		goto err;
+	/* write is at later location than read */
+
+	if (initial_write_p > initial_read_p + bytes_to_rip) {
+		/* write_p is greater than necessary, all is easy */
+		max_copied = bytes_to_rip;
+		memcpy(data_cpy, initial_read_p, max_copied);
+		end_read_p = initial_read_p + max_copied;
+	} else if (initial_write_p > initial_read_p) {
+		/*not enough data to cpy */
+		max_copied = initial_write_p - initial_read_p;
+		memcpy(data_cpy, initial_read_p, max_copied);
+		end_read_p = initial_write_p;
+	} else {
+		max_copied = ring->data
+			+ ring->length*ring->skip - initial_read_p;
+		memcpy(data_cpy, initial_read_p, max_copied);
+		if (initial_write_p > ring->data + bytes_to_rip - max_copied) {
+			/* enough data to finish */
+			memcpy(data_cpy, ring->data, bytes_to_rip - max_copied);
+			max_copied = bytes_to_rip;
+			end_read_p = ring->data + (bytes_to_rip - max_copied);
+		} else {  /* not enough data */
+			memcpy(data_cpy, ring->data,
+			       initial_write_p - ring->data);
+			max_copied += initial_write_p - ring->data;
+			end_read_p = initial_write_p;
+		}
+	}
+	/* Now to verify which section was cleanly copied - i.e. how far
+	 * read pointer has been pushed */
+	current_read_p = ring->read_p;
+
+	if (initial_read_p <= current_read_p)
+		dead_offset = current_read_p - initial_read_p;
+	else
+		dead_offset = ring->length*ring->skip - (initial_read_p
+							 - current_read_p);
+	/* setup the next read position */
+	ring->read_p = end_read_p;
+	/* possible issue if the initial write has been lapped or indeed
+	 * the point we were reading to has been passed */
+	/* No valid data read */
+	if (max_copied - dead_offset < 0)
+		return 0;
+
+	if (copy_to_user(buf, data_cpy + dead_offset,
+			 max_copied - dead_offset))
+		return -EFAULT;
+	kfree(data_cpy);
+	return max_copied - dead_offset;
+err:
+	kfree(data_cpy);
+	return 0;
+}
+
+static const struct file_operations industrialio_ring_fileops = {
+	.read = industrialio_ring_rip,
+	.release = industrialio_ring_release,
+	.open = industrialio_ring_open,
+	/* true? */
+	.owner = THIS_MODULE,
+};
+
+void industrialio_free_ring_buffer(struct industrialio_ring_buffer *ring,
+				   struct device *dev)
+{
+	sysfs_remove_file(&dev->kobj, &ring->access_minor_attr.dev_attr.attr);
+	industrialio_deallocate_chrdev(&ring->access_handler);
+	kfree(ring->access_minor_name);
+	industrialio_free_ev_int(&ring->ev_int, dev);
+	kfree(ring->event_minor_name);
+	FREE_INDUSTRIALIO_RING_BUFFER(ring);
+	kfree(ring);
+}
+EXPORT_SYMBOL_GPL(industrialio_free_ring_buffer);
+
+int industrialio_request_ring_buffer(int dimension,
+				     int bytes_per_reading,
+				     int length,
+				     struct industrialio_ring_buffer **ring,
+				     int id,
+				     struct module *owner,
+				     struct device *dev)
+{
+	int ret;
+/* Actually do the ring buffer initialization */
+	*ring = (struct industrialio_ring_buffer *)
+		(kmalloc(sizeof(struct industrialio_ring_buffer),
+			 GFP_KERNEL));
+
+	if (*ring == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	INIT_INDUSTRIALIO_RING_BUFFER(*ring,
+				      dimension,
+				      bytes_per_reading,
+				      length);
+	if ((*ring)->data == NULL) {
+		ret = -ENOMEM;
+		goto error_free_ring;
+	}
+
+/* Create and register the event character device */
+	(*ring)->event_minor_name = kmalloc(20, GFP_KERNEL);
+	if ((*ring)->event_minor_name == NULL) {
+		ret = -ENOMEM;
+		goto error_free_ring_data;
+	}
+	sprintf((*ring)->event_minor_name, "ring_buffer%d_ev_minor", id);
+
+	ret = industrialio_setup_ev_int(&(*ring)->ev_int,
+					(const char *)
+					((*ring)->event_minor_name),
+					owner,
+					dev);
+	if (ret)
+		goto error_free_event_minor_name;
+	(*ring)->ev_int.private = (*ring);
+/* Create and register the access character device */
+	(*ring)->access_minor_name = kmalloc(20, GFP_KERNEL);
+	if ((*ring)->access_minor_name == NULL) {
+		ret = -ENOMEM;
+		goto error_free_event_interface;
+	}
+	sprintf((*ring)->access_minor_name, "ring_buffer%d_access_minor", id);
+
+	ret = industrialio_allocate_chrdev(&(*ring)->access_handler);
+	if (ret)
+		goto error_free_access_minor_name;
+	(*ring)->access_handler.fops = &industrialio_ring_fileops;
+	(*ring)->access_handler.private = (*ring);
+	(*ring)->access_minor_attr.dev_attr.attr.name
+		= (const char *)((*ring)->access_minor_name);
+	(*ring)->access_minor_attr.dev_attr.attr.owner = owner;
+	(*ring)->access_minor_attr.dev_attr.attr.mode = S_IRUGO;
+	(*ring)->access_minor_attr.minor = (*ring)->access_handler.id;
+	(*ring)->access_minor_attr.dev_attr.show
+		= &industrialio_show_attr_minor;
+	ret = sysfs_create_file(&dev->kobj,
+				&(*ring)->access_minor_attr.dev_attr.attr);
+	if (ret)
+		goto error_deallocate_chrdev;
+
+	return 0;
+error_deallocate_chrdev:
+	industrialio_deallocate_chrdev(&(*ring)->access_handler);
+error_free_access_minor_name:
+	kfree((*ring)->access_minor_name);
+error_free_event_interface:
+	industrialio_free_ev_int(&(*ring)->ev_int,
+					  dev);
+error_free_event_minor_name:
+	kfree((*ring)->event_minor_name);
+error_free_ring_data:
+	FREE_INDUSTRIALIO_RING_BUFFER(*ring);
+error_free_ring:
+	kfree(*ring);
+error_ret:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(industrialio_request_ring_buffer);
+
+
+static int __init industrialio_init(void)
+{
+	int ret;
+
+	memset(industrialio_state.fhs,
+	       sizeof(struct industrialio_handler *)*256,
+	       0);
+
+	/* Create sysfs class */
+	ret  = class_register(&industrialio_class);
+	if (ret < 0) {
+		printk(KERN_ERR
+		       "industrialio.c: could not create sysfs class\n");
+		goto error_nothing;
+	}
+
+	/* Register the industrialio major number character device */
+	/* Various things can then result in minor numbers being allocated
+	 * by the subsystem
+	 * 1) Event device - possibly more than one?
+	 * 2) Ring buffer read device
+	 */
+
+	ret = register_chrdev(INDUSTRIALIO_MAJOR, "bob", &industrialio_fops);
+	if (ret) {
+		printk(KERN_ERR
+		       "industrialio: unable to register a char major %d",
+		       INDUSTRIALIO_MAJOR);
+		goto error_unregister_class;
+	}
+
+
+	return 0;
+error_unregister_class:
+	class_unregister(&industrialio_class);
+error_nothing:
+	return ret;
+}
+
+static void __exit industrialio_exit(void)
+{
+	unregister_chrdev(INDUSTRIALIO_MAJOR, "bob");
+	class_unregister(&industrialio_class);
+}
+/* A series of functions that are effectively parts of
+ * device register.  This structure is simpler to debug than
+ * a unified function.
+ * Each has corresponding unregister function*/
+
+
+int industrialio_device_register_sysfs(struct industrialio_dev *dev_info)
+{
+	int ret;
+
+	dev_info->sysfs_dev = device_create(&industrialio_class,
+					    dev_info->dev,
+					    MKDEV(0, 0),
+					    INDUSTRIALIO_ID_FORMAT,
+					    dev_info->id);
+
+	if (IS_ERR(dev_info->sysfs_dev)) {
+		/* what would correct error here be?*/
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	/* register attributes */
+	ret = sysfs_create_group(&dev_info->dev->kobj, dev_info->attrs);
+	if (ret) {
+		dev_err(dev_info->dev, "Failed to register sysfs hooks\n");
+		goto error_free_sysfs_device;
+	}
+
+	return 0;
+error_free_sysfs_device:
+	device_unregister(dev_info->dev);
+
+error_ret:
+	return ret;
+}
+
+void industrialio_device_unregister_sysfs(struct industrialio_dev *dev_info)
+{
+	sysfs_remove_group(&dev_info->dev->kobj, dev_info->attrs);
+	device_unregister(dev_info->sysfs_dev);
+}
+
+int industrialio_device_register_id(struct industrialio_dev *dev_info)
+{
+	int ret;
+idr_again:
+	if (unlikely(idr_pre_get(&industrialio_idr, GFP_KERNEL) == 0))
+		return -ENOMEM;
+
+	spin_lock(&industrialio_idr_lock);
+	ret = idr_get_new(&industrialio_idr, NULL, &dev_info->id);
+	spin_unlock(&industrialio_idr_lock);
+	if (unlikely(ret == -EAGAIN))
+		goto idr_again;
+	else if (unlikely(ret))
+		return ret;
+
+	dev_info->id = dev_info->id & MAX_ID_MASK;
+	return 0;
+}
+void industrialio_device_unregister_id(struct industrialio_dev *dev_info)
+{
+	/* Can I use the save id? */
+	int id;
+	if (likely(sscanf(dev_info->sysfs_dev->bus_id,
+			  INDUSTRIALIO_ID_FORMAT, &id) == 1)) {
+		spin_lock(&industrialio_idr_lock);
+		idr_remove(&industrialio_idr, id);
+		spin_unlock(&industrialio_idr_lock);
+	} else
+		dev_dbg(dev_info->dev->parent,
+			"indio_device_unregister() failed: bad class ID!\n");
+}
+
+int industrialio_device_register_eventset(struct industrialio_dev *dev_info)
+{
+	int ret, i;
+	char *name;
+	struct device_attribute *devattr;
+	struct industrialio_event_attr *indio_devattr;
+	/* Establish whether interrupts are actually possible */
+	/*fixme - still in driver for now */
+
+	dev_info->event_interfaces = (struct industrialio_event_interface *)
+		(kzalloc(sizeof(struct industrialio_event_interface)
+			 *dev_info->num_interrupt_lines,
+			 GFP_KERNEL));
+	if (dev_info->event_interfaces == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	/* assign id's to the event_interface elements */
+	for (i = 0; i < dev_info->num_interrupt_lines; i++) {
+		dev_info->event_interfaces[i].id = i;
+		/* FIXME: done in event_setup as well? */
+		dev_info->event_interfaces[i].owner = dev_info->driver_module;
+	}
+	dev_info->interrupts
+		= (struct industrialio_interrupt **)
+		kmalloc(sizeof(struct industrialio_interrupt *)
+			*dev_info->num_interrupt_lines,
+			GFP_KERNEL);
+	if (dev_info->interrupts == NULL) {
+		dev_err(dev_info->dev,
+			"Failed to register sysfs hooks for events attributes");
+		ret = -ENOMEM;
+		goto error_free_event_interfaces;
+	}
+
+	for (i = 0; i < dev_info->num_interrupt_lines; i++) {
+		name = kmalloc(20, GFP_KERNEL);
+		if (name == NULL) {
+			ret = -ENOMEM;
+			goto error_ret;
+		}
+		sprintf(name, "event_line%d_minor", i);
+		ret = industrialio_setup_ev_int(&dev_info->event_interfaces[i],
+						(const char *)(name),
+						dev_info->driver_module,
+						dev_info->dev);
+		if (ret) {
+			dev_err(dev_info->dev,
+				"Could not get chrdev interface\n");
+		}
+	}
+	ret = sysfs_create_group(&dev_info->dev->kobj, dev_info->event_attrs);
+	if (ret) {
+		dev_err(dev_info->dev,
+			"Failed to register sysfs hooks for events attributes");
+		goto error_free_interrupts;
+	}
+	/* May double initialize lists in case of shared handlers,
+	   but other than slight overhead that isn't a problem */
+	i = 0;
+	while (1) {
+		if (dev_info->event_attrs->attrs[i] == NULL)
+			break;
+		devattr = container_of(dev_info->event_attrs->attrs[i],
+				       struct device_attribute, attr);
+		indio_devattr = to_industrialio_event_attr(devattr);
+		INIT_LIST_HEAD(&indio_devattr->listel->list);
+		i++;
+	}
+	return 0;
+
+error_free_interrupts:
+	kfree(dev_info->interrupts);
+error_free_event_interfaces:
+	kfree(dev_info->event_interfaces);
+error_ret:
+	return ret;
+}
+
+void industrialio_device_unregister_eventset(struct industrialio_dev *dev_info)
+{
+	int i;
+	for (i = 0; i < dev_info->num_interrupt_lines; i++)
+		industrialio_free_ev_int(&dev_info->event_interfaces[i],
+					 dev_info->dev);
+	sysfs_remove_group(&dev_info->dev->kobj, dev_info->event_attrs);
+	kfree(dev_info->event_interfaces);
+}
+
+/*Simple paramater read and set for ring buffer */
+static ssize_t industrialio_read_ring_dim(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	int len;
+	struct industrialio_dev *dev_info = dev_get_drvdata(dev);
+
+	len = sprintf(buf, "%d\n", dev_info->ring_dimension);
+
+	return len;
+}
+
+static ssize_t industrialio_write_ring_dim(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf,
+					   size_t len)
+{
+	/* FIXME: move to dynamically adjustable as and when ring is dynamically
+	   allocated */
+	return len;
+}
+
+static ssize_t industrialio_read_ring_length(struct device *dev,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	int len;
+	struct industrialio_dev *dev_info = dev_get_drvdata(dev);
+
+	len = sprintf(buf, "%d\n", dev_info->ring_length);
+
+	return len;
+
+	return 0;
+}
+
+static ssize_t industrialio_write_ring_length(struct device *dev,
+					      struct device_attribute *attr,
+					      const char *buf,
+					      size_t len)
+{
+	/* FIXME: move to user adjustable as and when ring is dynamically
+	   activated */
+	return len;
+}
+
+static ssize_t industrialio_read_ring_bps(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	int len;
+	struct industrialio_dev *dev_info = dev_get_drvdata(dev);
+
+	len = sprintf(buf, "%d\n", dev_info->ring_bytes_per_reading);
+
+	return len;
+}
+
+static ssize_t industrialio_write_ring_bps(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf,
+					   size_t len)
+{
+	/*Not sure it will ever make sense to make this user adjustable!*/
+	return len;
+}
+
+DEVICE_ATTR(dimension, S_IRUGO | S_IWUSR,
+	    industrialio_read_ring_dim,
+	    industrialio_write_ring_dim);
+DEVICE_ATTR(length, S_IRUGO | S_IWUSR,
+	    industrialio_read_ring_length,
+	    industrialio_write_ring_length);
+DEVICE_ATTR(bps, S_IRUGO | S_IWUSR,
+	    industrialio_read_ring_bps,
+	    industrialio_write_ring_bps);
+
+
+static struct attribute *industrialio_ring_attributes[] = {
+	&dev_attr_dimension.attr,
+	&dev_attr_length.attr,
+	&dev_attr_bps.attr,
+
+	NULL,
+};
+
+static const struct attribute_group industrialio_ring_attribute_group = {
+	.name = "ring_buffer_attributes",
+	.attrs = industrialio_ring_attributes,
+};
+
+
+
+int industrialio_device_register_ring(struct industrialio_dev *dev_info, int id)
+{
+	int ret;
+
+	/* FIXME: actual ring alloc should be on demand, not here
+	* For now I'm simply ignoring any ring buffer parameter changes!*/
+	ret = industrialio_request_ring_buffer(dev_info->ring_dimension,
+					       dev_info->ring_bytes_per_reading,
+					       dev_info->ring_length,
+					       &dev_info->ring,
+					       id,
+					       dev_info->driver_module,
+					       dev_info->dev);
+	if (ret < 0)
+		goto error_ret;
+	ret = sysfs_create_group(&dev_info->dev->kobj,
+				 &industrialio_ring_attribute_group);
+	if (ret < 0)
+		goto error_free_ring;
+
+	return 0;
+
+error_free_ring:
+	industrialio_free_ring_buffer(dev_info->ring, dev_info->dev);
+error_ret:
+	return ret;
+}
+
+void industrialio_device_unregister_ring(struct industrialio_dev *dev_info)
+{
+	sysfs_remove_group(&dev_info->dev->kobj,
+			   &industrialio_ring_attribute_group);
+	/* deallocate ring buffer related stuff */
+	if (dev_info->modes & (INDIO_RING_POLLED | INDIO_RING_DATA_RDY))
+		industrialio_free_ring_buffer(dev_info->ring, dev_info->dev);
+
+}
+
+int industrialio_device_register(struct industrialio_dev *dev_info)
+{
+	int ret;
+	dev_set_drvdata(dev_info->dev, (void *)(dev_info));
+
+/*Get a unique id */
+	ret = industrialio_device_register_id(dev_info);
+	if (ret)
+		goto error_nothing;
+
+/* Create sysfs device */
+	ret = industrialio_device_register_sysfs(dev_info);
+	if (ret)
+		goto error_free_idr;
+
+/* Interrupt triggered events setup */
+	ret = industrialio_device_register_eventset(dev_info);
+	if (ret)
+		goto error_free_sysfs;
+
+/* Ring buffer init if relevant */
+	/* FIXME: multiple ring buffers? */
+	if (dev_info->modes & (INDIO_RING_POLLED | INDIO_RING_DATA_RDY)) {
+		ret = industrialio_device_register_ring(dev_info, 0);
+		if (ret)
+			goto error_free_eventset;
+	}
+	return 0;
+/* Clean up the shrapnel of any failures */
+
+error_free_eventset:
+	industrialio_device_unregister_eventset(dev_info);
+
+error_free_sysfs:
+	industrialio_device_unregister_sysfs(dev_info);
+
+error_free_idr:
+	industrialio_device_unregister_id(dev_info);
+
+error_nothing:
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(industrialio_device_register);
+
+void industrialio_device_unregister(struct industrialio_dev *dev_info)
+{
+	if (dev_info->modes & (INDIO_RING_POLLED | INDIO_RING_DATA_RDY))
+		industrialio_device_unregister_ring(dev_info);
+	industrialio_device_unregister_eventset(dev_info);
+	industrialio_device_unregister_sysfs(dev_info);
+	industrialio_device_unregister_id(dev_info);
+
+}
+EXPORT_SYMBOL_GPL(industrialio_device_unregister);
+
+subsys_initcall(industrialio_init);
+module_exit(industrialio_exit);
diff -uprN -X a/Documentation/dontdiff a/drivers/industrialio/misc/Makefile b/drivers/industrialio/misc/Makefile
--- a/drivers/industrialio/misc/Makefile	1970-01-01 01:00:00.000000000 +0100
+++ b/drivers/industrialio/misc/Makefile	2008-06-05 11:31:46.000000000 +0100
@@ -0,0 +1,3 @@
+#
+# Makefile for industrial I/O misc drivers
+#
\ No newline at end of file

Re: Accelerometer etc subsystem (Update on progress)

[Jonathan Cameron]

[-- Attachment #1: Type: text/plain, Size: 3361 bytes --]

Sorry, forgot to include the headers in the original patch.  Obviously these
are very rough and ready at the moment and if nothing else their overall
layout isn't particularly logical.
> Dear All,
>
> This email is mainly to give people an idea of current progress towards
> a new
> subsystem as discussed in the thread starting with
> http://lkml.org/lkml/2008/5/20/135
>
> Sorry for the mass list bombardment, but clearly some elements of this
> discussion
> will end up in various different territories.
>
> Some elements of a prototype subsystem are in place. It draws very heavily
> on parts of the input and hwmon subsystems diverging only where necessary.
>
> The only test driver currently integrated is for an ST LIS3L02DQ
> accelerometer
> which has more than a few quirks to make it tricky to handle (and some what
> sketchy documentation.) More chips will follow over next week or so but
> hopefully the driver for this chip gives enough of an idea of how I envision
> the system working to encourage discussion / advice.
>
> Note that I haven't dealt with anywhere near all the possible locking issues
> etc and am well aware that this needs to be done. Other cleanups that will
> need to be done include working out the layout in sysfs to make it more
> intuitive. Also sorry for the somewhat rough and ready nature of the
> attached
> patch (against 2.6.26-rc4)
>
> Ring buffer design is a large part of the attached patch. I'm not sure if
> I am going about this the right way. Basically, we need ring buffers with
> almost no write latency but can waste plenty of time reading from them
> (in general case - we do however want reading the last available value to be
> fast). What is there works, but probably has at least a few nasty corner
> cases that I haven't prevented.
>
> Interfaces (these are per device) - at somepoint a procfs interface similar
>        to that used in the input subsystem would make device querying
>        simpler.
>
> Sysfs -    Parameter Control - gain / offsets etc
>     State control, turn interrupts on and off etc.
>     Interrupt control parameters (threshold etc)
>     Ring buffer parameters as relevant (currently fixed)
>     Individual input reading (acceleration values here)
>     Minor numbers for various chrdevs associated with this device.
>
> chrdev-    3 types of chrdev at the moment
>     Ring buffer events
>     Ring buffer access (currently ripping data off the buffer only)
>     Interrupt events - for lis3l02dq these are only threshold breaks
>
> Functionality yet to be implemented.
>     Polled based capture (use a peroidic timer if available)
>
>     Hardware ring buffering for devices that support it (two level ring
> buffer -
>     hard and soft may be appropriate)
>
>     A chrdev for polling of whole device (with timestamps etc).
>
>     Composite interrupt handling (some devices allow logical combinations
>     of different interrupt signals to be used as the trigger condition).
>
>     Documenation ;)
>
>     Cleaner solution to data alignment in the ring buffer (currently I'm
> cheating
>     and manually doing it)
>
>     Lots lots more....
>
> Anyhow, all comments welcome.  Can anyone think of a better name?
> (I'm not keen on industrialio. It's too long if nothing else!
>  It will do as a working title for now)
>
> Thanks,
>
> --
>
> Jonathan Cameron
>   


[-- Attachment #2: indio_headers.patch --]
[-- Type: text/x-patch, Size: 16882 bytes --]

--- a/include/linux/industrialio.h	1970-01-01 01:00:00.000000000 +0100
+++ b/include/linux/industrialio.h	2008-06-26 12:10:31.000000000 +0100
@@ -0,0 +1,275 @@
+/* The industrial I/O core
+ *
+ * Copyright (c) 2008 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef _INDUSTRIAL_IO_H_
+#define _INDUSTRIAL_IO_H_
+
+#include <linux/device.h>
+#include <linux/industrialio_sysfs.h>
+
+/* TODO LIST */
+/* Initial test drivers to implement
+   SCA3000 VTI Accelerometers (hardware ring buffers)
+   MAX1363 ADC (polled only for ring buffer )
+
+   Static device specific elements (conversion factors etc) should be exported via sysfs
+
+   Finish writing ring buffer character interface
+   Write general event character interfaces
+   Another type of chardev to allow direct reading (typically in response to data ready events)
+
+
+   Opinions sought on:
+   Shared interrupt lines.  Worth dealing with? (very time consuming to check
+   whether some devices caused an interrupt or not - in some states anyway)
+   Limiting length of event lists.  Could get silly numbers of them otherwise.
+*/
+
+
+/* Event interface flags */
+#define INDUSTRIALIO_BUSY_BIT_POS 1
+
+
+/* Could maintain a list of these for rapid clean up purposes,
+   but it doesn't exactly take long to scan the array */
+struct industrialio_handler {
+	const struct file_operations *fops;
+	int id;
+	unsigned long flags;
+	void *private;
+};
+
+
+/* The actual event being pushed ot userspace */
+struct industrialio_event_data {
+	int id;
+	s64 timestamp;
+};
+
+/* FIXME -WORK ON NAMING*/
+struct industrialio_detected_event_list {
+	struct list_head list;
+	struct industrialio_event_data ev;
+	/* Part of shared event handling - (typicaly ring buffers) */
+	struct industrialio_shared_ev_pointer *shared_pointer;
+};
+
+
+/* Requires high resolution timers */
+static inline s64 industrialio_get_time_ns(void)
+{
+	struct timespec ts;
+	ktime_get_ts(&ts);
+	return timespec_to_ns(&ts);
+}
+
+struct industrialio_dev;
+/* Each device has one of these per interrupt */
+struct industrialio_event_handler_list {
+	struct list_head list;
+	int (*handler)(struct industrialio_dev *dev_io, int index, s64 timestamp, int no_test);
+	/* This element may be shared */
+	int refcount;
+};
+/* wraps adding to lists and does reference counting to allowed shared handlers */
+/* FIXME CONFUSING NAMING */
+int industrialio_add_event_to_list(struct industrialio_event_handler_list *list,
+				   struct industrialio_event_handler_list *el);
+
+int industrialio_remove_event_from_list(struct industrialio_event_handler_list *el);
+
+
+
+/* This means that interrupts can be turned off when no events are being generated,
+   and also provides the interrupt handler the means to identify the incoming event */
+//int industrialio_register_event_list_to_interrupt(int interrupt, industrialio_event_list *list);
+
+/* Want this to be as transparrent as possible from the point of view of the driver! */
+
+/* JIC23: This is my first serious attempt at a lock free ring buffer for this sort of
+   situation so all suggestions on this code particularly welcome! */
+
+
+
+
+struct industrialio_ring_buffer;
+#define INIT_INDUSTRIALIO_RING_BUFFER(ring, _dim, _bytes, _length) {	\
+		(ring)->size = _dim*_bytes;				\
+		(ring)->skip = (ring)->size + sizeof(s64);		\
+		(ring)->length = _length;				\
+		(ring)->dimension = _dim;				\
+		(ring)->bytes = _bytes;					\
+		(ring)->read_p = 0;					\
+		(ring)->write_p = 0;					\
+		(ring)->last_written_p = 0;				\
+		(ring)->loopcount = 0;					\
+		(ring)->data						\
+			= (unsigned char*)				\
+			(kmalloc(_length*(ring)->skip,			\
+				 GFP_KERNEL));				\
+		(ring)->shared_ev_pointer.ev_p =0;			\
+		(ring)->shared_ev_pointer.lock =			\
+			__SPIN_LOCK_UNLOCKED((ring)->shared_ev_pointer->loc); \
+}
+
+#define FREE_INDUSTRIALIO_RING_BUFFER(ring)	\
+	kfree((ring)->data)
+
+int industrialio_store_to_ring(struct industrialio_ring_buffer *ring,
+			       unsigned char* data,
+			       s64 timestamp);
+
+/* Edge cases :
+   1) data at last_p is no longer valid - requires complete wrap around.
+      To detect, loop count has changed - if only by 1 then problem only
+      if current_lastp is equal to or greater than copy made at start.
+      If we have wrapped an entire int in this time (loopcount) then
+      something very very weird has occured!
+*/
+int industrialio_read_last_from_ring(struct industrialio_ring_buffer *ring,
+					    unsigned char* data);
+/* Dump the ring */
+
+int
+industrialio_request_ring_buffer(int dimension,
+				 int bytes_per_reading,
+				 int length,
+				 struct industrialio_ring_buffer **ring,
+				 int id,
+				 struct module *owner,
+				 struct device *dev );
+
+
+void industrialio_free_ring_buffer(struct industrialio_ring_buffer* ring, struct device *dev);
+/* Device operating modes */
+#define INDIO_DIRECT_MODE		0x01
+#define INDIO_RING_POLLED		0x02
+#define INDIO_RING_DATA_RDY		0x04
+#define INDIO_RING_HARDWARE_BUFFER	0x08
+
+
+
+struct industrialio_event_interface {
+	struct industrialio_handler		handler;
+	wait_queue_head_t			wait;
+	struct industrialio_detected_event_list	det_events;
+	int					max_events;
+	int					current_events;
+	/* Integer id, used to differentiate this one form any others */
+	int					id;
+	struct industrialio_chrdev_minor_attr	attr;
+	struct module				*owner;
+	void					*private;
+};
+
+
+struct industrialio_shared_ev_pointer {
+	struct industrialio_detected_event_list	*ev_p;
+	spinlock_t lock;
+};
+/* A general ring buffer structure
+   Intended to be completely lock free as we always want fills from the interrupt
+   handler to not have to wait.  This obviously increases the possible time required
+   to read from the buffer.  */
+struct industrialio_ring_buffer
+{
+	unsigned char*		data;
+	int			length;
+	int			dimension;
+	int			bytes;
+	int			size;
+	int			skip;
+	unsigned char		*read_p;
+	unsigned char		*write_p;
+	unsigned char		*last_written_p;
+	/* used to act as a point at which to signal an event */
+	unsigned char		*half_p;
+	int			loopcount;
+	/* accessing the ring buffer */
+	char*			access_minor_name;
+	struct industrialio_chrdev_minor_attr	access_minor_attr;
+	struct industrialio_handler		access_handler;
+	/* events triggered by the ring buffer */
+	char*			event_minor_name;
+	struct industrialio_event_interface ev_int;
+	/* a fully shared output event */
+	struct industrialio_shared_ev_pointer shared_ev_pointer;
+};
+/* Seperate registration functions were leading to very messy driver init */
+/* Vast majority of this is set by the industrialio subsystem.
+ * FIXME: Add a macro to set only the relevant stuff within a chip driver
+ */
+struct industrialio_dev {
+/* generic handling data used by ind io */
+	int id;
+/* device specific data */
+	void *dev_data;
+
+/* Modes the drivers supports */
+	int modes; /* Driver Set */
+	int currentmode;
+/* Direct sysfs related functionality */
+	struct device *sysfs_dev;
+	struct device *dev; /* Driver Set */
+	/* General attributes */
+	const struct attribute_group *attrs;
+
+/* Interrupt handling related */
+	/* FIXME: GETTING MESSY! */
+	struct module *driver_module;
+	int 				num_interrupt_lines; /* Driver Set */
+
+	struct industrialio_interrupt **interrupts;
+
+
+	/* Event control attributes */
+	const struct attribute_group *event_attrs;
+	/* The character device related elements */
+	struct industrialio_event_interface *event_interfaces;
+
+/* Software Ring Buffer - for now assuming only makes sense to have a single ring */
+	int ring_dimension;
+	int ring_bytes_per_reading;
+	int ring_length;
+	struct industrialio_ring_buffer *ring;
+	struct attribute_group *ring_attrs_group;
+	struct industrialio_ring_attr *ring_attrs;
+};
+
+int industrialio_device_register(struct industrialio_dev *dev_info);
+
+void industrialio_device_unregister(struct industrialio_dev *dev_info);
+
+/* Wrapper class used to allow easy specification of different line numbers */
+struct industrialio_interrupt {
+	struct industrialio_dev *dev_info;
+	int line_number;
+	int irq;
+	struct industrialio_event_handler_list ev_list;
+};
+
+irqreturn_t industrialio_interrupt_handler(int irq, void *_int_info);
+
+int industrialio_register_interrupt_line(unsigned int			irq,
+					 struct industrialio_dev	*dev_info,
+					 int				line_number,
+					 unsigned long			type,
+					 const char			*name);
+
+void industrialio_unregister_interrupt_line(struct industrialio_dev *dev_info,
+					    int line_number);
+
+
+/* Used to try inserting an event into the list for userspace reading via
+ * chrdev */
+int industrialio_put_event(struct industrialio_dev *dev_info,
+			   int ev_line,
+			   int ev_code,
+			   s64 timestamp);
+#endif /* _INDUSTRIAL_IO_H_ */
--- a/include/linux/industrialio_sysfs.h	1970-01-01 01:00:00.000000000 +0100
+++ b/include/linux/industrialio_sysfs.h	2008-06-26 16:44:50.000000000 +0100
@@ -0,0 +1,207 @@
+/* The industrial I/O core
+ *
+ *Copyright (c) 2008 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * General attributes
+ */
+
+#ifndef _INDUSTRIAL_IO_SYSFS_H_
+#define _INDUSTRIAL_IO_SYSFS_H_
+
+#include <linux/industrialio.h>
+
+
+struct industrialio_event_attr {
+	struct device_attribute dev_attr;
+	int mask;
+	struct industrialio_event_handler_list *listel;
+};
+
+
+#define to_industrialio_event_attr(_dev_attr) \
+	container_of(_dev_attr, struct industrialio_event_attr, dev_attr)
+
+
+struct industrialio_chrdev_minor_attr {
+	struct device_attribute dev_attr;
+	int minor;
+};
+
+#define to_industrialio_chrdev_minor_attr(_dev_attr) \
+	container_of(_dev_attr, struct industrialio_chrdev_minor_attr, dev_attr);
+
+struct industrialio_dev_attr {
+	struct device_attribute dev_attr;
+	int address;
+};
+
+
+#define to_industrialio_dev_attr(_dev_attr) \
+	container_of(_dev_attr, struct industrialio_dev_attr, dev_attr)
+
+/* Some attributes will be hard coded (device dependant) and not require an
+   address, in these cases pass a negative */
+#define INDUSTRIALIO_ATTR(_name, _mode, _show, _store, _addr)	\
+	{ .dev_attr = __ATTR(_name, _mode, _show, _store), \
+	  .address = _addr }
+
+#define INDUSTRIALIO_DEVICE_ATTR(_name, _mode, _show, _store, _addr)	\
+	struct industrialio_dev_attr industrialio_dev_attr_##_name	\
+	= INDUSTRIALIO_ATTR(_name, _mode, _show, _store, _addr)
+
+/* This may get broken down into separate files later */
+
+/* For devices with internal clocks - and possibly poling later */
+
+#define INDUSTRIALIO_DEV_ATTR_SAMP_FREQ(_mode, _show, _store) \
+	INDUSTRIALIO_DEVICE_ATTR(sampling_frequency, _mode,   \
+				 _show, _store, 0)
+
+#define INDUSTRIALIO_DEV_ATTR_AVAIL_SAMP_FREQ(_show)\
+	INDUSTRIALIO_DEVICE_ATTR(available_sampling_frequency,	\
+				 S_IRUGO, _show, NULL, 0)
+
+/* Accelerometer types of attribute */
+
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_X_OFFSET(_mode, _show, _store, _addr) \
+	INDUSTRIALIO_DEVICE_ATTR(x_offset, _mode, _show, _store, _addr)
+
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_Y_OFFSET(_mode, _show, _store, _addr) \
+	INDUSTRIALIO_DEVICE_ATTR(y_offset, _mode, _show, _store, _addr)
+
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_Z_OFFSET(_mode, _show, _store, _addr) \
+	INDUSTRIALIO_DEVICE_ATTR(z_offset, _mode, _show, _store, _addr)
+
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_X_GAIN(_mode, _show, _store, _addr)	\
+	INDUSTRIALIO_DEVICE_ATTR(x_gain, _mode, _show, _store, _addr)
+
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_Y_GAIN(_mode, _show, _store, _addr)	\
+	INDUSTRIALIO_DEVICE_ATTR(y_gain, _mode, _show, _store, _addr)
+
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_Z_GAIN(_mode, _show, _store, _addr)	\
+	INDUSTRIALIO_DEVICE_ATTR(z_gain, _mode, _show, _store, _addr)
+
+
+/* The actual device readings are always going to be read only */
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_X(_show, _addr)			\
+	INDUSTRIALIO_DEVICE_ATTR(x, S_IRUGO, _show, NULL, _addr)
+
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_Y(_show, _addr)			\
+	INDUSTRIALIO_DEVICE_ATTR(y, S_IRUGO, _show, NULL, _addr)
+
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_Z(_show, _addr)			\
+	INDUSTRIALIO_DEVICE_ATTR(z, S_IRUGO, _show, NULL, _addr)
+
+/* Thresholds are somewhat chip dependent - may need quite a few defs here */
+#define INDUSTRIALIO_DEV_ATTR_ACCEL_THRESH(_mode, _show, _store, _addr) \
+	INDUSTRIALIO_DEVICE_ATTR(thresh, _mode, _show, _store, _addr)
+
+
+
+/* Events that the device may generate */
+/* How to do this. Is it valid to have sysfs elements which can be neither
+   read nor written? */
+/*  GOING TO NEED a usage count */
+#define INDUSTRIALIO_EVENT_SH(_name, _handler)			\
+	static struct industrialio_event_handler_list			\
+	industrialio_event_##_name = {				\
+		.handler=_handler,				\
+		.refcount = 0,					\
+	};
+#define INDUSTRIALIO_EVENT_ATTR_SH(_name, _ev_list, _show, _store, _mask) \
+	static struct industrialio_event_attr				\
+	industrialio_event_attr_##_name					\
+	= { .dev_attr = __ATTR(_name, S_IRUGO | S_IWUSR, _show, _store),\
+	    .mask = _mask,\
+	    .listel = &_ev_list };
+
+/*FIXME use the above to define this */
+#define INDUSTRIALIO_EVENT_ATTR(_name, _show, _store, _mask, _handler)	\
+	static struct industrialio_event_handler_list				\
+	industrialio_event_##_name = {				\
+		.handler=_handler,					\
+	};								\
+	static struct							\
+	industrialio_event_attr						\
+	industrialio_event_attr_##_name					\
+	= { .dev_attr = __ATTR(_name, S_IRUGO | S_IWUSR, _show, _store), \
+	    .mask = _mask,						\
+	    .listel = &industrialio_event_##_name };		\
+/*FIXME, add line number to the above?*/
+
+/* In most of these cases, this actually corresponds to something with a
+   value attached */
+
+/* For some devices you can select whether all conditions or any condition
+   must be met for interrupt generation */
+#define INDUSTRIALIO_EVENT_ATTR_DATA_RDY(_show, _store, _mask, _handler) \
+	INDUSTRIALIO_EVENT_ATTR(data_rdy, _show, _store, _mask, _handler)
+
+#define INDUSTRIALIO_EVENT_CODE_DATA_RDY  100
+
+/* Threshold pass events */
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_X_HIGH(_show, _store, _mask, _handler) \
+	INDUSTRIALIO_EVENT_ATTR(x_high, _show, _store, _mask, _handler)
+
+#define INDUSTRIALIO_EVENT_CODE_ACCEL_X_HIGH 1
+
+/* Shared handler version */
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_X_HIGH_SH(_evlist, _show, _store, _mask)\
+	INDUSTRIALIO_EVENT_ATTR_SH(x_high, _evlist, _show, _store, _mask)
+
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_Y_HIGH(_show, _store, _mask, _handler) \
+	INDUSTRIALIO_EVENT_ATTR(y_high, _show, _store, _mask, _handler)
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_Y_HIGH_SH(_evlist, _show, _store, _mask)\
+	INDUSTRIALIO_EVENT_ATTR_SH(y_high, _evlist, _show, _store, _mask)
+
+#define INDUSTRIALIO_EVENT_CODE_ACCEL_Y_HIGH 2
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_Z_HIGH(_show, _store, _mask, _handler) \
+	INDUSTRIALIO_EVENT_ATTR(z_high, _show, _store, _mask, _handler)
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_Z_HIGH_SH(_evlist, _show, _store, _mask)\
+	INDUSTRIALIO_EVENT_ATTR_SH(z_high, _evlist, _show, _store, _mask)
+
+#define INDUSTRIALIO_EVENT_CODE_ACCEL_Z_HIGH 3
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_X_LOW(_show, _store, _mask, _handler) \
+	INDUSTRIALIO_EVENT_ATTR(x_low, _show, _store, _mask, _handler)
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_X_LOW_SH(_evlist, _show, _store, _mask)\
+	INDUSTRIALIO_EVENT_ATTR_SH(x_low, _evlist, _show, _store, _mask)
+
+#define INDUSTRIALIO_EVENT_CODE_ACCEL_X_LOW 4
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_Y_LOW(_show, _store, _mask, _handler) \
+	INDUSTRIALIO_EVENT_ATTR(y_low, _show, _store, _mask, _handler)
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_Y_LOW_SH(_evlist,_show, _store, _mask)\
+	INDUSTRIALIO_EVENT_ATTR_SH(y_low, _evlist, _show, _store, _mask)
+
+#define INDUSTRIALIO_EVENT_CODE_ACCEL_Y_LOW 5
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_Z_LOW(_show, _store, _mask, _handler) \
+	INDUSTRIALIO_EVENT_ATTR(z_low, _show, _store, _mask, _handler)
+
+#define INDUSTRIALIO_EVENT_ATTR_ACCEL_Z_LOW_SH(_evlist, _show, _store, _mask)\
+	INDUSTRIALIO_EVENT_ATTR_SH(z_low, _evlist, _show, _store, _mask)
+
+#define INDUSTRIALIO_EVENT_CODE_ACCEL_Z_LOW 6
+
+
+#define INDUSTRIALIO_EVENT_CODE_RING_50_FULL 100
+#define INDUSTRIALIO_EVENT_CODE_RING_100_FULL 101
+/* HOW TO HANDLE COMPOSITE EVENTS? */
+
+
+
+
+/* function that takes a list of these and puts them in an events directory? */
+
+#endif /* _INDUSTRIAL_IO_SYSFS_H_ */
--- a/include/linux/spi/lis3l02dq.h	1970-01-01 01:00:00.000000000 +0100
+++ b/include/linux/spi/lis3l02dq.h	2008-05-27 20:18:00.000000000 +0100
@@ -0,0 +1,6 @@
+
+
+struct LIS3L02DQ_platform_data {
+  unsigned data_ready_gpio;
+};
+

Re: Accelerometer etc subsystem (Update on progress)

[Randy Dunlap]

On Thu, 26 Jun 2008 19:01:30 +0100 Jonathan Cameron wrote:

>     Documenation ;)

and Documentation ;)


Ugh.  Attachment.  Please send patches inline so that they can be reviewed
without having to do tons of copy-and-paste.  Possibly Documentation/email-clients.txt
can help you with thunderbird.

There are several typos in the Kconfig text parts...
and in source code comments.

>From Documentation/CodingStyle:

The preferred style for long (multi-line) comments is:

	/*
	 * This is the preferred style for multi-line
	 * comments in the Linux kernel source code.
	 * Please use it consistently.
	 *
	 * Description:  A column of asterisks on the left side,
	 * with beginning and ending almost-blank lines.
	 */

Please review/use Documentation/CodingStyle.

This Makefile (drivers/industrialio/misc/Makefile)
doesn't seem to do anything.

---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

Re: Accelerometer etc subsystem (Update on progress)

[Ben Nizette]

On Thu, 2008-06-26 at 19:01 +0100, Jonathan Cameron wrote:

> Sysfs -    Parameter Control - gain / offsets etc
>     State control, turn interrupts on and off etc.

As in turn userspace [interrupt] event notification on and off?  I would
have thought it'd be the kernel driver's responsibility to turn the
device's interrupt generation on and off according to needs for
data/events etc.

I know this is in a rough state but a few comments anyway.  First, there
are heaps of casts-of-void-pointers (eg casts of kmalloc returns) which
are superfluous and hard readability 'coz you have to split the line to
fit in 80cols.

And hardcoded type-size assumptions everywhere.  Don't be scared of
sizeof ;-)


> Anyhow, all comments welcome.  Can anyone think of a better name?
> (I'm not keen on industrialio. It's too long if nothing else!
>  It will do as a working title for now)

I don't mind industrial io but as a function prefix, iio works better.
I can't see it being used elsewhere.

also:

+/* As the ring buffer contents are device dependent this functionality
+ * must remain part of the driver and not the ring buffer subsystem */
+static ssize_t
+lis3l02dq_read_accel_from_ring(struct industrialio_ring_buffer *ring,
+			       int element, char *buf)
+{
+	int val, ret, len;
+	uint16_t temp;
+	char *data, *datalock;
+
+	data = kmalloc(8, GFP_KERNEL);
+	if (data == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+	ret = industrialio_read_last_from_ring(ring, data);
+	datalock = data + 2*element;

I haven't looked deeply at the ringbuffer code but can you guarantee
that later elements are at higher addresses than the lower ones?  As in,
can one datum in the the ring buffer wrap to the beginning again?

+	kfree(data);

You free the data before you use it?  Though you are using it through a
different pointer below.  I wouldn't be scared of allocating 8 bytes on
the stack rather than kmalloc'ing (unless you expect this to be called
in a deep callchain)

+	temp = (((uint16_t)((datalock[1]))) << 8)
+		| (uint16_t)(datalock[0]);
+	val = *((int16_t *)(&temp));

All this data/datalock/bitshuffle nonsense would be nicer if you just
used structs and unions, yeah?

union channel {
	char data[2];
	int16_t val;
}

struct datum {
	union channel elements[3];
}

or something.

+	len = sprintf(buf, "ring %d\n", val);
+
+	return len;
+error_ret:
+	return ret;
+}


Incidentally, is there much that your ringbuffer can do which kfifo
can't?  Apart from having a bunch of extra nice accessor-helpers sitting
on the top.

Overall looking good and useful, can't wait 'till it's done :-)
	--Ben.

Re: [lm-sensors] Accelerometer etc subsystem (Update on progress)

[Jonathan Cameron]

Ben Nizette wrote:
> On Thu, 2008-06-26 at 19:01 +0100, Jonathan Cameron wrote:
> 
>> Sysfs -    Parameter Control - gain / offsets etc
>>     State control, turn interrupts on and off etc.
> 
> As in turn userspace [interrupt] event notification on and off?  I would
> have thought it'd be the kernel driver's responsibility to turn the
> device's interrupt generation on and off according to needs for
> data/events etc.
Ok, there is a division here between interrupt handling on the host side
which indeed should be turned on and off transparently by the driver 
and actually telling the sensor which interrupts to generate.  In a sense
this is simply a case of terminology and what is actually being requested
is event notifications (which then match with those sent up to userspace).
> 
> I know this is in a rough state but a few comments anyway.  First, there
> are heaps of casts-of-void-pointers (eg casts of kmalloc returns) which
> are superfluous and hard readability 'coz you have to split the line to
> fit in 80cols.
Oops, silly habit that one - I'll clean those out.
> And hardcoded type-size assumptions everywhere.  Don't be scared of
> sizeof ;-)
Yup, definitely need to clean them up.
>> Anyhow, all comments welcome.  Can anyone think of a better name?
>> (I'm not keen on industrialio. It's too long if nothing else!
>>  It will do as a working title for now)
> 
> I don't mind industrial io but as a function prefix, iio works better.
> I can't see it being used elsewhere.
Good point - I'd droped to indio in some cases but iio will make those 80
character limits even easier ;)

> also:
> 
> +/* As the ring buffer contents are device dependent this functionality
> + * must remain part of the driver and not the ring buffer subsystem */
> +static ssize_t
> +lis3l02dq_read_accel_from_ring(struct industrialio_ring_buffer *ring,
> +			       int element, char *buf)
> +{
> +	int val, ret, len;
> +	uint16_t temp;
> +	char *data, *datalock;
> +
> +	data = kmalloc(8, GFP_KERNEL);
> +	if (data == NULL) {
> +		ret = -ENOMEM;
> +		goto error_ret;
> +	}
> +	ret = industrialio_read_last_from_ring(ring, data);
> +	datalock = data + 2*element;
> 
> I haven't looked deeply at the ringbuffer code but can you guarantee
> that later elements are at higher addresses than the lower ones?  As in,
> can one datum in the the ring buffer wrap to the beginning again?
At the moment the ring buffer has to be a whole number of datums big.
I'm inclined to keep it way and move more towards dynamic allocation
such that it is true than to try handling split data reading sets.
 
> +	kfree(data);
> 
> You free the data before you use it?  Though you are using it through a
> different pointer below.  I wouldn't be scared of allocating 8 bytes on
> the stack rather than kmalloc'ing (unless you expect this to be called
> in a deep callchain)
Ouch.  That's an out and out bug!
> 
> +	temp = (((uint16_t)((datalock[1]))) << 8)
> +		| (uint16_t)(datalock[0]);
> +	val = *((int16_t *)(&temp));
> 
> All this data/datalock/bitshuffle nonsense would be nicer if you just
> used structs and unions, yeah?
> 
> union channel {
> 	char data[2];
> 	int16_t val;
> }
Good point, I'd forgotten you could do that with unions.
> 
> struct datum {
> 	union channel elements[3];
> }
> 
> or something.
> 
> +	len = sprintf(buf, "ring %d\n", val);
> +
> +	return len;
> +error_ret:
> +	return ret;
> +}
Good approach, I'll switch to that.

> 
> Incidentally, is there much that your ringbuffer can do which kfifo
> can't?  Apart from having a bunch of extra nice accessor-helpers sitting
> on the top.
Not sure, I'll look into it.
> 
> Overall looking good and useful, can't wait 'till it's done :-)
Thanks for the comments.

Jonathan

Re: [lm-sensors] Accelerometer etc subsystem (Update on progress)

[Ben Nizette]

On Fri, 2008-06-27 at 10:45 +0100, Jonathan Cameron wrote:
> Ben Nizette wrote:
> > On Thu, 2008-06-26 at 19:01 +0100, Jonathan Cameron wrote:
> > 
> >> Sysfs -    Parameter Control - gain / offsets etc
> >>     State control, turn interrupts on and off etc.
> > 
> > As in turn userspace [interrupt] event notification on and off?  I would
> > have thought it'd be the kernel driver's responsibility to turn the
> > device's interrupt generation on and off according to needs for
> > data/events etc.
> Ok, there is a division here between interrupt handling on the host side
> which indeed should be turned on and off transparently by the driver 
> and actually telling the sensor which interrupts to generate.  In a sense
> this is simply a case of terminology and what is actually being requested
> is event notifications (which then match with those sent up to userspace).

Righteo, I suspected as much :-)

< snip good replies :-) >

> 
> > also:
> > 
> > +/* As the ring buffer contents are device dependent this functionality
> > + * must remain part of the driver and not the ring buffer subsystem */
> > +static ssize_t
> > +lis3l02dq_read_accel_from_ring(struct industrialio_ring_buffer *ring,
> > +			       int element, char *buf)
> > +{
> > +	int val, ret, len;
> > +	uint16_t temp;
> > +	char *data, *datalock;
> > +
> > +	data = kmalloc(8, GFP_KERNEL);
> > +	if (data == NULL) {
> > +		ret = -ENOMEM;
> > +		goto error_ret;
> > +	}
> > +	ret = industrialio_read_last_from_ring(ring, data);
> > +	datalock = data + 2*element;
> > 
> > I haven't looked deeply at the ringbuffer code but can you guarantee
> > that later elements are at higher addresses than the lower ones?  As in,
> > can one datum in the the ring buffer wrap to the beginning again?
> At the moment the ring buffer has to be a whole number of datums big.
> I'm inclined to keep it way and move more towards dynamic allocation
> such that it is true than to try handling split data reading sets.

Yup, agreed.  Just so long as your code thinks about :-)

>  
> > +	kfree(data);
> > 
> > You free the data before you use it?  Though you are using it through a
> > different pointer below.  I wouldn't be scared of allocating 8 bytes on
> > the stack rather than kmalloc'ing (unless you expect this to be called
> > in a deep callchain)
> Ouch.  That's an out and out bug!
> > 
> > +	temp = (((uint16_t)((datalock[1]))) << 8)
> > +		| (uint16_t)(datalock[0]);
> > +	val = *((int16_t *)(&temp));
> > 
> > All this data/datalock/bitshuffle nonsense would be nicer if you just
> > used structs and unions, yeah?
> > 
> > union channel {
> > 	char data[2];
> > 	int16_t val;
> > }
> Good point, I'd forgotten you could do that with unions.

Cool, just watch endianness of course :-)

> > 
> > struct datum {
> > 	union channel elements[3];
> > }
> > 
> > or something.
> > 
> > +	len = sprintf(buf, "ring %d\n", val);
> > +
> > +	return len;
> > +error_ret:
> > +	return ret;
> > +}
> Good approach, I'll switch to that.
> 
> > 
> > Incidentally, is there much that your ringbuffer can do which kfifo
> > can't?  Apart from having a bunch of extra nice accessor-helpers sitting
> > on the top.
> Not sure, I'll look into it.

kfifo won't be a drop in replacement, it's just a very simple ring fifo.
I suspect your higher level ring buffer accessors and allocators could
live on top of it though.

> > 
> > Overall looking good and useful, can't wait 'till it's done :-)
> Thanks for the comments.
> 
> Jonathan

np,
	--Ben.

Re: [lm-sensors] Accelerometer etc subsystem (Update on progress)

[Jonathan Cameron]

Hi Ben,
>>> union channel {
>>> 	char data[2];
>>> 	int16_t val;
>>> }
>> Good point, I'd forgotten you could do that with unions.
> 
> Cool, just watch endianness of course :-)
> 
That and the annoyance of alignment issues making that approach taking way more
space that you'd think.
>>> Incidentally, is there much that your ringbuffer can do which kfifo
>>> can't?  Apart from having a bunch of extra nice accessor-helpers sitting
>>> on the top.
>> Not sure, I'll look into it.
> 
> kfifo won't be a drop in replacement, it's just a very simple ring fifo.
> I suspect your higher level ring buffer accessors and allocators could
> live on top of it though.
Sure, but from my understanding of kfifo it takes a much more symmetric
approach to reading and writing with locking used to prevent them occuring
concurrently.  You obviously can use it without locking but I don't believe
that it provide any facility for coping with the nasty case, (buffer full
and hence during read attempts a certain amount of what is copied out may
have become invalid).  It maybe the case that, as you say suitable high level
functions on top of a kfifo would be a good way to proceed (afterall,  kfifo
is well tested etc), but I fear, given how little of kfifo's code would actually
be used it would be more likely to cause problems than not. 

It might be best to leave this decision until the exact requirements of the ring
buffer are actually known.

Jonathan

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[mark gross]

On Tue, May 20, 2008 at 11:04:01AM +0100, Jonathan Cameron wrote:
> This email is basically a request for opinions on how and where such 
> sensors
> should be integrated into the kernel.
>
> To set the scene...
>
> Increasing numbers of embedded devices are being supplied attached MEMS
> devices (www.xbow.com imote2 etc). Along with more traditional sensors such
> as ADC's not being used for hardware monitoring, these do not really seem 
> to
> fit with in an particular subsystem of the kernel.  A previous discussion 
> on
> lkml in 2006 considered the accelerometers to be found within some laptop
> hard drives, but I haven't been able to track down any more general 
> discussions
> of such non hardware monitoring sensors.
>
> The obvious possibilities are:
>
> * To place the various drivers within the spi / i2c etc subsystems as 
> relevant.
>
> * To place within the hwmon subsystem as this is probably closest.
> (there is already at least one straight ADC driver in hwmon)
>
> * To create a new subsystem, or perhaps merely sysfs class to contain these
>  elements.
>
> Typical requirements within an application include simply polling for 
> current
> readings, and using device triggered interrupts to grab data continuously 
> to a
> ring buffer, for collection by suitable userspace code.  Obviously it would 
> be
> desirable to standardize sysfs controls for various calibration parameters 
> as
> much as possible across the various devices.
>
> Any other suggestions welcome!
>
> To illustrate the sort of devices here are a few I have drivers written for 
> or will
> shortly be writing  (some submitted to hwmon and spi mailing lists, some 
> not
> finished as of yet)
>
> ST Micro LIS3L02DQ 3D accelerometer.  SPI device, no buffering, interrupt 
> pin
> raised high on new data being available. Currently the driver assumes, if
> interrupts are enabled, that this is connected to a specified gpio.
> (http://www.st.com/stonline/books/pdf/docs/10175.pdf)

FWIW: I have this device talking to a PIC-18 and pushing the results
over the serial port.

WRT linux support, I can't think of a generalized way to create a driver
that would be able to be used with this device in Linux.  You need to
know witch IRQ line the DR line is connected too, and if you are
bit-banging the SPI data off the thing, then you need to know which
GPIO's of the host CPU you'll be using.  If you have SPI hardware then
you need to know where to pull the data from.  The problem doesn't seem to
generalize well.

Also, If you are playing with accelerometer data, you likely need some
real time support or at lest a reliable time stamping of the data to do
anything interesting.

Another problem area is around SPI itself.  There are variations of
device implementations around chip select polarity, clock biasing
(rising,falling, or midpoint) sampling from one SPI part to the next.

> VTI SCA3000 E05 3D accelerometer equiped with substantial ring buffer. SPI
> device.  Can operate either in buffered or direct mode.  In buffered mode, 
> interrupts
> can be used to indicate when the ring buffer is 3/4 full triggering a 
> download to
> a larger ring buffer in the kernel if necessary.
> (http://www.vti.fi/en/products-solutions/products/accelerometers/sca3000-accelerometers/)
>

ring buffered data can make RT applications harder...

> Analog Devices ADIS16350 combined 3D accelerometer and gyro unit.  SPI 
> device.
> (http://www.analog.com/en/prod/0,,764_801_ADIS16350%2C00.html)
>
> Maxim MAX1363, MAX1238 ADC's.  I2C devices. Some SPI ADC's (www.analog.com 
> for
> examples)
>
> Would be nice if practical to allow the framework to include RS232 devices 
> such
> as those from www.xsens.com, www.isense.com and others.


I'm not sure what you are asking for, you started off with SPI driver
for interfacing a handful of accelerometer devices.  Now your talking
about the serial port.

Besides the consumer of accelerometer data is user space applications
(mine attempt to be somewhat RT applications) 

--mgross

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Jonathan Cameron]

Hi Mark,

>> ST Micro LIS3L02DQ 3D accelerometer.  SPI device, no buffering, interrupt 
>> pin
>> raised high on new data being available. Currently the driver assumes, if
>> interrupts are enabled, that this is connected to a specified gpio.
>> (http://www.st.com/stonline/books/pdf/docs/10175.pdf)
> 
> FWIW: I have this device talking to a PIC-18 and pushing the results
> over the serial port.

That's certainly interesting and I guess one of the easiest ways of getting data 
from spi to a desktop machine. I'll admit my interest is more with embedded 
machines.

> WRT linux support, I can't think of a generalized way to create a driver
> that would be able to be used with this device in Linux.  You need to
> know witch IRQ line the DR line is connected too, and if you are
> bit-banging the SPI data off the thing, then you need to know which
> GPIO's of the host CPU you'll be using. 
Yes, and that means we would use the delights of platform data and the generic 
gpio subsystem.  Although I believe generic gpio isn't available on every 
platform as yet, it is getting there and is certainly in place for ARM (and 
cleans up an awful lot of hardware interfaces!)
> If you have SPI hardware then
> you need to know where to pull the data from.  The problem doesn't seem to
> generalize well.
Yes, the intention would not be to generalize the hardware comms, but rather the
interface on the user side.  This is similar to a number of other subsystems 
such as hwmon, where you share functionality as much as possible, but allow very 
different approaches to things like actual hardware register reads as and when 
they are necessary.

> Also, If you are playing with accelerometer data, you likely need some
> real time support or at lest a reliable time stamping of the data to do
> anything interesting.
Definitely. Obviously the accuracy of this time stamp is going to be limited by
the variable nature of when an interrupt handler picks it up, but knowledge of 
the device specs can allow filtering of this timing data.  This is certainly 
something we would want to consider at a later date although it may well be more 
sensible to leave this to userspace applications.

> Another problem area is around SPI itself.  There are variations of
> device implementations around chip select polarity, clock biasing
> (rising,falling, or midpoint) sampling from one SPI part to the next.
Indeed.  This is well handled by the SPI subsystem. I've spent far too much time 
with a scope recently fiddling with these parameters for badly documented chips!

>> VTI SCA3000 E05 3D accelerometer equiped with substantial ring buffer. SPI
>> device.  Can operate either in buffered or direct mode.  In buffered mode, 
>> interrupts
>> can be used to indicate when the ring buffer is 3/4 full triggering a 
>> download to
>> a larger ring buffer in the kernel if necessary.
>> (http://www.vti.fi/en/products-solutions/products/accelerometers/sca3000-accelerometers/)
>>
> 
> ring buffered data can make RT applications harder...
Agreed.  With this sort of device two modes would be needed (and are supported 
by the hardware).  The first gives direct data access and the second uses the 
ring buffer.  Which you use would obviously be dependent on the application. 
Some applications want the most recent data on all occasions, whilst some will 
be more interested in ensuring that all data is captured.

>>
>> Would be nice if practical to allow the framework to include RS232 devices 
>> such
>> as those from www.xsens.com, www.isense.com and others.
> 
> 
> I'm not sure what you are asking for, you started off with SPI driver
> for interfacing a handful of accelerometer devices.  Now your talking
> about the serial port.

The question here is concerned with standardising mainly the userspace 
interfaces (and hardware interfaces only when appropriate). Some of these 
devices (the ST accel above for example) support multiple hardware interfaces 
specs (I2C and SPI for that one).  So in a similar way to it making sense to 
group all TV adapters irrespective of how they are interfaced to the computer, 
in order to ensure a consistent interface to user space, it makes sense to share 
these interfaces amongst drivers talking to these devices irrespective of what 
their hardware interface is.
> 
> Besides the consumer of accelerometer data is user space applications
> (mine attempt to be somewhat RT applications) 

Thanks,

--
Jonathan Cameron

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[mark gross]

On Wed, May 21, 2008 at 10:40:50AM +0100, Jonathan Cameron wrote:

First I'm sorry for not checking my email for a week and the late reply.

> Hi Mark,
>
>>> ST Micro LIS3L02DQ 3D accelerometer.  SPI device, no buffering, interrupt 
>>> pin
>>> raised high on new data being available. Currently the driver assumes, if
>>> interrupts are enabled, that this is connected to a specified gpio.
>>> (http://www.st.com/stonline/books/pdf/docs/10175.pdf)
>> FWIW: I have this device talking to a PIC-18 and pushing the results
>> over the serial port.
>
> That's certainly interesting and I guess one of the easiest ways of getting 
> data from spi to a desktop machine. I'll admit my interest is more with 
> embedded machines.
>
>> WRT linux support, I can't think of a generalized way to create a driver
>> that would be able to be used with this device in Linux.  You need to
>> know witch IRQ line the DR line is connected too, and if you are
>> bit-banging the SPI data off the thing, then you need to know which
>> GPIO's of the host CPU you'll be using. 
> Yes, and that means we would use the delights of platform data and the 
> generic gpio subsystem.  Although I believe generic gpio isn't available on 
> every platform as yet, it is getting there and is certainly in place for 
> ARM (and cleans up an awful lot of hardware interfaces!)
>> If you have SPI hardware then
>> you need to know where to pull the data from.  The problem doesn't seem to
>> generalize well.
> Yes, the intention would not be to generalize the hardware comms, but 
> rather the
> interface on the user side.  This is similar to a number of other 
> subsystems such as hwmon, where you share functionality as much as 
> possible, but allow very different approaches to things like actual 
> hardware register reads as and when they are necessary.

Ok, say the HW SPI is pushed down to some lower level driver.  These
things send streaming data, and sometimes signals (free-fall detect) out
of band WRT the SPI comms.

I would like to see how a higher level API could encapsulate the
generalization of a class of such devices.

Also, I'm not sure how one could generalize SPI communications at a high
level given its bi-directional nature.

I guess I need to see some header files showing what you are thinking
about.

Keep in mind that one abstraction of these devices is they are simply
streaming A2D 16 bit data values over SPI (or I2C).  The size of the
data set is a function of the sensor axes.


>
>> Also, If you are playing with accelerometer data, you likely need some
>> real time support or at lest a reliable time stamping of the data to do
>> anything interesting.
> Definitely. Obviously the accuracy of this time stamp is going to be 
> limited by
> the variable nature of when an interrupt handler picks it up, but knowledge 
> of the device specs can allow filtering of this timing data.  This is 
> certainly something we would want to consider at a later date although it 
> may well be more sensible to leave this to userspace applications.
>
>> Another problem area is around SPI itself.  There are variations of
>> device implementations around chip select polarity, clock biasing
>> (rising,falling, or midpoint) sampling from one SPI part to the next.
> Indeed.  This is well handled by the SPI subsystem. I've spent far too much 
> time with a scope recently fiddling with these parameters for badly 
> documented chips!
>
>>> VTI SCA3000 E05 3D accelerometer equiped with substantial ring buffer. 
>>> SPI
>>> device.  Can operate either in buffered or direct mode.  In buffered 
>>> mode, interrupts
>>> can be used to indicate when the ring buffer is 3/4 full triggering a 
>>> download to
>>> a larger ring buffer in the kernel if necessary.
>>> (http://www.vti.fi/en/products-solutions/products/accelerometers/sca3000-accelerometers/)
>>>
>> ring buffered data can make RT applications harder...
> Agreed.  With this sort of device two modes would be needed (and are 
> supported by the hardware).  The first gives direct data access and the 
> second uses the ring buffer.  Which you use would obviously be dependent on 
> the application. Some applications want the most recent data on all 
> occasions, whilst some will be more interested in ensuring that all data is 
> captured.
>
>>>
>>> Would be nice if practical to allow the framework to include RS232 
>>> devices such
>>> as those from www.xsens.com, www.isense.com and others.
>> I'm not sure what you are asking for, you started off with SPI driver
>> for interfacing a handful of accelerometer devices.  Now your talking
>> about the serial port.
>
> The question here is concerned with standardising mainly the userspace 
> interfaces (and hardware interfaces only when appropriate). Some of these 
> devices (the ST accel above for example) support multiple hardware 
> interfaces specs (I2C and SPI for that one).  So in a similar way to it 
> making sense to group all TV adapters irrespective of how they are 
> interfaced to the computer, in order to ensure a consistent interface to 
> user space, it makes sense to share these interfaces amongst drivers 
> talking to these devices irrespective of what their hardware interface is.

I guess I need to see some header files to see where you are going with
this idea.  To be useful you'll also need to handle rotation sensors and
communicate the temperature bias's up the stack too.


good luck keep me posted on where your going with this, it is
interesting to me.

--mgross

>> Besides the consumer of accelerometer data is user space applications
>> (mine attempt to be somewhat RT applications) 
>
> Thanks,
>
> --
> Jonathan Cameron

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Jonathan Cameron]

> 
> Ok, say the HW SPI is pushed down to some lower level driver.  These
> things send streaming data, and sometimes signals (free-fall detect) out
> of band WRT the SPI comms.
Indeed.  This would have to handled using interrupts based on the
generic gpio interface. (see the LIS3L02DQ driver on the SPI-devel
mailing list for an example of how this might work.)
 
> I would like to see how a higher level API could encapsulate the
> generalization of a class of such devices.
> 
> Also, I'm not sure how one could generalize SPI communications at a high
> level given its bi-directional nature.
> 
> I guess I need to see some header files showing what you are thinking
> about.
Sure.  I'll get something put together and sent out asap. The general
form of this is going to look like a combination of the hwmon
interface via sysfs and something similar to the input subsystems
approach of using events sent over character devices.  The interesting
bit is going to be working out how to provide fairly generic interface
to ring buffers, whether these reside in the kernel or on the device
itself.

> 
> Keep in mind that one abstraction of these devices is they are simply
> streaming A2D 16 bit data values over SPI (or I2C).  The size of the
> data set is a function of the sensor axes.
This is of course true, but there still seem to be some advantages in
providing some common interfaces for these devices.  Almost all
digital interfaced accelerometers share a number of common elements
not typically found in ADC's.  These include bias and gain as well as
commonly found alarm signals.  

Actually, on this point, it may well make sense to provide an
accelerometer driver that is designed to run on top of any generic ADC
driver, thus providing (based on a suitable board specific
configuration) userspace access to analog accelerometers connected in
this fashion.  This is definitely one for some way down the line!

> 
>>> Also, If you are playing with accelerometer data, you likely need some
>>> real time support or at lest a reliable time stamping of the data to do
>>> anything interesting.
>> Definitely. Obviously the accuracy of this time stamp is going to be 
>> limited by
>> the variable nature of when an interrupt handler picks it up, but knowledge 
>> of the device specs can allow filtering of this timing data.  This is 
>> certainly something we would want to consider at a later date although it 
>> may well be more sensible to leave this to userspace applications.
>>
>>> Another problem area is around SPI itself.  There are variations of
>>> device implementations around chip select polarity, clock biasing
>>> (rising,falling, or midpoint) sampling from one SPI part to the next.
>> Indeed.  This is well handled by the SPI subsystem. I've spent far too much 
>> time with a scope recently fiddling with these parameters for badly 
>> documented chips!
>>
>>>> VTI SCA3000 E05 3D accelerometer equiped with substantial ring buffer. 
>>>> SPI
>>>> device.  Can operate either in buffered or direct mode.  In buffered 
>>>> mode, interrupts
>>>> can be used to indicate when the ring buffer is 3/4 full triggering a 
>>>> download to
>>>> a larger ring buffer in the kernel if necessary.
>>>> (http://www.vti.fi/en/products-solutions/products/accelerometers/sca3000-accelerometers/)
>>>>
>>> ring buffered data can make RT applications harder...
>> Agreed.  With this sort of device two modes would be needed (and are 
>> supported by the hardware).  The first gives direct data access and the 
>> second uses the ring buffer.  Which you use would obviously be dependent on 
>> the application. Some applications want the most recent data on all 
>> occasions, whilst some will be more interested in ensuring that all data is 
>> captured.
>>
>>>> Would be nice if practical to allow the framework to include RS232 
>>>> devices such
>>>> as those from www.xsens.com, www.isense.com and others.
>>> I'm not sure what you are asking for, you started off with SPI driver
>>> for interfacing a handful of accelerometer devices.  Now your talking
>>> about the serial port.
>> The question here is concerned with standardising mainly the userspace 
>> interfaces (and hardware interfaces only when appropriate). Some of these 
>> devices (the ST accel above for example) support multiple hardware 
>> interfaces specs (I2C and SPI for that one).  So in a similar way to it 
>> making sense to group all TV adapters irrespective of how they are 
>> interfaced to the computer, in order to ensure a consistent interface to 
>> user space, it makes sense to share these interfaces amongst drivers 
>> talking to these devices irrespective of what their hardware interface is.
> 
> I guess I need to see some header files to see where you are going with
> this idea.  To be useful you'll also need to handle rotation sensors and
> communicate the temperature bias's up the stack too.

Definitely, although to get a reasonable number of devices integrated
quickly they may not all support access to everything we would like
initially. 

> 
> good luck keep me posted on where your going with this, it is
> interesting to me.
Will do,
--
Jonathan Cameron

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[David Brownell]

On Tuesday 20 May 2008, mark gross wrote:
> > ST Micro LIS3L02DQ 3D accelerometer.  ...
> 
> FWIW: I have this device talking to a PIC-18 and pushing the results
> over the serial port.

Jonathan neglected to mention that he's already sent a driver
for this to the SPI list.  ;)

Which is part of the reason for asking this question.  Right
now that driver sits in drivers/spi/lis3l02dq.c but that is
probably not its best long-term domicile.


> WRT linux support, I can't think of a generalized way to create a driver
> that would be able to be used with this device in Linux.  You need to
> know witch IRQ line the DR line is connected too, and if you are
> bit-banging the SPI data off the thing, then you need to know which
> GPIO's of the host CPU you'll be using.  If you have SPI hardware then
> you need to know where to pull the data from.  The problem doesn't seem to
> generalize well.

I guess I don't follow.  The SPI framework handles all that
stuff already, even if you're bitbanging.  Though to be
sure, if you're bitbanging SPI you want your platform to
be able to inline those GPIO calls so those inner loops
only take a few instructions per bit ... also solved!  :)


> Also, If you are playing with accelerometer data, you likely need some
> real time support or at lest a reliable time stamping of the data to do
> anything interesting.

True.  I imagine a few other such issues will appear once
more folk than Jonathan are using these sorts of sensors
on Linux.  I'd expect the sample stream to have internal
timestamps for truly critical systems, unless variability
in when the host makes timestamps is not really an issue.
 

> Another problem area is around SPI itself.  There are variations of
> device implementations around chip select polarity, clock biasing
> (rising,falling, or midpoint) sampling from one SPI part to the next.

Midpoint?  That's not one I've come across before.  All four
standard SPI clock/sample/shift modes are already supported
in the SPI framework though.  Ditto active-high chipselects
(vs normal active-low) etc.

- Dave

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[mark gross]

On Wed, May 21, 2008 at 05:53:38PM -0700, David Brownell wrote:
> On Tuesday 20 May 2008, mark gross wrote:
> > > ST Micro LIS3L02DQ 3D accelerometer.  ...
> > 
> > FWIW: I have this device talking to a PIC-18 and pushing the results
> > over the serial port.
> 
> Jonathan neglected to mention that he's already sent a driver
> for this to the SPI list.  ;)
> 
> Which is part of the reason for asking this question.  Right
> now that driver sits in drivers/spi/lis3l02dq.c but that is
> probably not its best long-term domicile.
> 
> 
> > WRT linux support, I can't think of a generalized way to create a driver
> > that would be able to be used with this device in Linux.  You need to
> > know witch IRQ line the DR line is connected too, and if you are
> > bit-banging the SPI data off the thing, then you need to know which
> > GPIO's of the host CPU you'll be using.  If you have SPI hardware then
> > you need to know where to pull the data from.  The problem doesn't seem to
> > generalize well.
> 
> I guess I don't follow.  The SPI framework handles all that
> stuff already, even if you're bitbanging.  Though to be
> sure, if you're bitbanging SPI you want your platform to
> be able to inline those GPIO calls so those inner loops
> only take a few instructions per bit ... also solved!  :)
>

I'll take a look sometime soon.
 
> 
> > Also, If you are playing with accelerometer data, you likely need some
> > real time support or at lest a reliable time stamping of the data to do
> > anything interesting.
> 
> True.  I imagine a few other such issues will appear once
> more folk than Jonathan are using these sorts of sensors
> on Linux.  I'd expect the sample stream to have internal
> timestamps for truly critical systems, unless variability
> in when the host makes timestamps is not really an issue.
>  

Its on my list of things to do..

> 
> > Another problem area is around SPI itself.  There are variations of
> > device implementations around chip select polarity, clock biasing
> > (rising,falling, or midpoint) sampling from one SPI part to the next.
> 
> Midpoint?  That's not one I've come across before.  All four
> standard SPI clock/sample/shift modes are already supported
> in the SPI framework though.  Ditto active-high chipselects
> (vs normal active-low) etc.

Yeah, its one of the sampling modes for the PIC18F4455 its one of the
master mode sampling options  (see page 194 of
http://ww1.microchip.com/downloads/en/DeviceDoc/39632D.pdf )

--mgross

Re: Accelerometer, Gyros and ADC's etc within the kernel.

[David Brownell]

On Tuesday 27 May 2008, mark gross wrote:
> 
> > > Another problem area is around SPI itself.  There are variations of
> > > device implementations around chip select polarity, clock biasing
> > > (rising,falling, or midpoint) sampling from one SPI part to the next.
> > 
> > Midpoint?  That's not one I've come across before.  All four
> > standard SPI clock/sample/shift modes are already supported
> > in the SPI framework though.  Ditto active-high chipselects
> > (vs normal active-low) etc.
> 
> Yeah, its one of the sampling modes for the PIC18F4455 its one of the
> master mode sampling options  (see page 194 of
> http://ww1.microchip.com/downloads/en/DeviceDoc/39632D.pdf )

Actually figure 19-3 (p. 198) may be a bit more clear.

Curious.  That PIC18F controller has *three* bits ... equivalents
of CPOL and CPHA, plus a bit saying whether to wait a whole or
half clock after the leading clock edge before sampling it.

A quick survey of some other SPI chips suggests that the most
conventional interpretation is to sample on the trailing edge
of the clock, and not presume long hold times.

I would surely hope the SPI interface spec doesn't have to get
into the mess of timing variations which typify anyone trying to
make memory controllers do the Right thing ... *shudder* ...

- Dave

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Anton Vorontsov]

Hi Jonathan,

On Tue, May 20, 2008 at 11:04:01AM +0100, Jonathan Cameron wrote:
> This email is basically a request for opinions on how and where such sensors
> should be integrated into the kernel.
> 
> To set the scene...
> 
> Increasing numbers of embedded devices are being supplied attached MEMS
> devices (www.xbow.com imote2 etc). Along with more traditional sensors such
> as ADC's not being used for hardware monitoring, these do not really 
> seem to
> fit with in an particular subsystem of the kernel.  A previous 
> discussion on
> lkml in 2006 considered the accelerometers to be found within some laptop
> hard drives, but I haven't been able to track down any more general 
> discussions
> of such non hardware monitoring sensors.
> 
> The obvious possibilities are:
> 
> * To place the various drivers within the spi / i2c etc subsystems as 
> relevant.
> 
> * To place within the hwmon subsystem as this is probably closest.
> (there is already at least one straight ADC driver in hwmon)
> 
> * To create a new subsystem, or perhaps merely sysfs class to contain these
>   elements.
> 
> Typical requirements within an application include simply polling for 
> current
> readings, and using device triggered interrupts to grab data 
> continuously to a
> ring buffer, for collection by suitable userspace code.  Obviously it 
> would be
> desirable to standardize sysfs controls for various calibration 
> parameters as
> much as possible across the various devices.

Also, I'd mention that most ADC devices could report in "bunched" mode,
i.e.

1. Request ADC readings from pins X, Y, Z1, Z2.
2. Wait for single IRQ
3. Read all the results

At handhelds.org, we've wrote quite good (I think) ADC subsystem,
that keeps in mind ADC capabilities. It implements two interfaces:
in-kernel (e.g. for touchscreen drivers), and userspace interface via
sysfs. I was planning to implement drivers/input/ interface too.

I always don't find time to clean it up and submit, though.

Here is ADC subsystem itself:
http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/adc.c
http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/include/linux/adc.h
^ It is in drivers/misc/, but I think better placement would be
  drivers/adc.

Some drivers for this subsystem:
http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/ads7846_adc_ssp.c
^ ADC driver for ADS7846 chips (HP iPaq hx4700 and some HTC phones, the
  driver is using SSP subsystem, switching it to the SPI is still in my enless
  TODO list).

http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/samcop_adc.c
^ SAMCOP (HP iPaq H5xxx) ADC driver.

http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/tsc2200_adc_ssp.c
^ TSC2200 ADC driver (used AFAIK in some HTC phones, or ASUS handhelds).

http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/ad7877.c
^ AD7877 ADC driver (HTC phones too, IIRC).

Now, The Generic ADC Touchscreen Driver (tested to work quite good with all
above ADC drivers on appropriate hardware):
http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/input/touchscreen/ts-adc.c

And finally, ADC Battery driver ("backup" batteries in the iPaq devices
usually report their voltage via ADC chip, the same used by the
touchscreen).
http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/power/adc_battery.c

p.s.
I have more recent (for current Linus' tree) code for ADC subsystem, but
it still needs some love to be submittable.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Ben Dooks]

On Tue, May 27, 2008 at 08:44:15PM +0400, Anton Vorontsov wrote:
> Hi Jonathan,
> 
> On Tue, May 20, 2008 at 11:04:01AM +0100, Jonathan Cameron wrote:
> > This email is basically a request for opinions on how and where such sensors
> > should be integrated into the kernel.
> > 
> > To set the scene...
> > 
> > Increasing numbers of embedded devices are being supplied attached MEMS
> > devices (www.xbow.com imote2 etc). Along with more traditional sensors such
> > as ADC's not being used for hardware monitoring, these do not really 
> > seem to
> > fit with in an particular subsystem of the kernel.  A previous 
> > discussion on
> > lkml in 2006 considered the accelerometers to be found within some laptop
> > hard drives, but I haven't been able to track down any more general 
> > discussions
> > of such non hardware monitoring sensors.
> > 
> > The obvious possibilities are:
> > 
> > * To place the various drivers within the spi / i2c etc subsystems as 
> > relevant.
> > 
> > * To place within the hwmon subsystem as this is probably closest.
> > (there is already at least one straight ADC driver in hwmon)
> > 
> > * To create a new subsystem, or perhaps merely sysfs class to contain these
> >   elements.
> > 
> > Typical requirements within an application include simply polling for 
> > current
> > readings, and using device triggered interrupts to grab data 
> > continuously to a
> > ring buffer, for collection by suitable userspace code.  Obviously it 
> > would be
> > desirable to standardize sysfs controls for various calibration 
> > parameters as
> > much as possible across the various devices.

The two drivers i've seen so far use the input subsystem to report
their data to the user. I'm working on an driver for the SMB380 which
can be both i2c and spi.
 
> Also, I'd mention that most ADC devices could report in "bunched" mode,
> i.e.
> 
> 1. Request ADC readings from pins X, Y, Z1, Z2.
> 2. Wait for single IRQ
> 3. Read all the results
> 
> At handhelds.org, we've wrote quite good (I think) ADC subsystem,
> that keeps in mind ADC capabilities. It implements two interfaces:
> in-kernel (e.g. for touchscreen drivers), and userspace interface via
> sysfs. I was planning to implement drivers/input/ interface too.

handhelds.org's track history of getting things into the kernel
is poor.

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Anton Vorontsov]

On Tue, May 27, 2008 at 05:50:21PM +0100, Ben Dooks wrote:
> On Tue, May 27, 2008 at 08:44:15PM +0400, Anton Vorontsov wrote:
> > Hi Jonathan,
> > 
> > On Tue, May 20, 2008 at 11:04:01AM +0100, Jonathan Cameron wrote:
> > > This email is basically a request for opinions on how and where such sensors
> > > should be integrated into the kernel.
> > > 
> > > To set the scene...
> > > 
> > > Increasing numbers of embedded devices are being supplied attached MEMS
> > > devices (www.xbow.com imote2 etc). Along with more traditional sensors such
> > > as ADC's not being used for hardware monitoring, these do not really 
> > > seem to
> > > fit with in an particular subsystem of the kernel.  A previous 
> > > discussion on
> > > lkml in 2006 considered the accelerometers to be found within some laptop
> > > hard drives, but I haven't been able to track down any more general 
> > > discussions
> > > of such non hardware monitoring sensors.
> > > 
> > > The obvious possibilities are:
> > > 
> > > * To place the various drivers within the spi / i2c etc subsystems as 
> > > relevant.
> > > 
> > > * To place within the hwmon subsystem as this is probably closest.
> > > (there is already at least one straight ADC driver in hwmon)
> > > 
> > > * To create a new subsystem, or perhaps merely sysfs class to contain these
> > >   elements.
> > > 
> > > Typical requirements within an application include simply polling for 
> > > current
> > > readings, and using device triggered interrupts to grab data 
> > > continuously to a
> > > ring buffer, for collection by suitable userspace code.  Obviously it 
> > > would be
> > > desirable to standardize sysfs controls for various calibration 
> > > parameters as
> > > much as possible across the various devices.
> 
> The two drivers i've seen so far use the input subsystem to report
> their data to the user. I'm working on an driver for the SMB380 which
> can be both i2c and spi.
>  
> > Also, I'd mention that most ADC devices could report in "bunched" mode,
> > i.e.
> > 
> > 1. Request ADC readings from pins X, Y, Z1, Z2.
> > 2. Wait for single IRQ
> > 3. Read all the results
> > 
> > At handhelds.org, we've wrote quite good (I think) ADC subsystem,
> > that keeps in mind ADC capabilities. It implements two interfaces:
> > in-kernel (e.g. for touchscreen drivers), and userspace interface via
> > sysfs. I was planning to implement drivers/input/ interface too.
> 
> handhelds.org's track history of getting things into the kernel
> is poor.

Yes, mostly. Because lack of human resources. But something gets done
anyway, e.g. drivers/power/ is already in the mainline. Philipp Zabel is
doing great work for the HTC Magician support (already in mainline, too).

I think there are only few of us who think that mainlining is important,
this is bad. But the thinking improves as times goes by.

So, don't be so pessimistic. :-)

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Jonathan Cameron]

>
> The two drivers i've seen so far use the input subsystem to report
> their data to the user. I'm working on an driver for the SMB380 which
> can be both i2c and spi.
Yes, the ST accel I'm using (LIS3L02DQ) is like that as well, so we 
definitely need
to implement the ability to do this cleanly! 

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Ben Dooks]

On Tue, May 27, 2008 at 07:00:56PM +0100, Jonathan Cameron wrote:
> 
> >
> > The two drivers i've seen so far use the input subsystem to report
> > their data to the user. I'm working on an driver for the SMB380 which
> > can be both i2c and spi.
> Yes, the ST accel I'm using (LIS3L02DQ) is like that as well, so we 
> definitely need
> to implement the ability to do this cleanly! 

this would be so much easier to read if word-wrapped to 77 characters
per line.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [spi-devel-general] Accelerometer, Gyros and ADC's etc within the kernel.

[Jonathan Cameron]

Hi Anton
>> Typical requirements within an application include simply polling for 
>> current
>> readings, and using device triggered interrupts to grab data 
>> continuously to a
>> ring buffer, for collection by suitable userspace code.  Obviously it 
>> would be
>> desirable to standardize sysfs controls for various calibration 
>> parameters as
>> much as possible across the various devices.
>>     
>
> Also, I'd mention that most ADC devices could report in "bunched" mode,
> i.e.
>
> 1. Request ADC readings from pins X, Y, Z1, Z2.
> 2. Wait for single IRQ
> 3. Read all the results
>   
Good point. I guess it's going to be a while before we work out an 
anywhere near
exhaustive list of such events that we might want to be able to control.
> At handhelds.org, we've wrote quite good (I think) ADC subsystem,
> that keeps in mind ADC capabilities. It implements two interfaces:
> in-kernel (e.g. for touchscreen drivers), and userspace interface via
> sysfs. I was planning to implement drivers/input/ interface too.
> I always don't find time to clean it up and submit, though.
>   
I know that feeling!
> Here is ADC subsystem itself:
> http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/adc.c
> http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/include/linux/adc.h
> ^ It is in drivers/misc/, but I think better placement would be
>   drivers/adc.
>   
Looking at the code (briefly admittedly, I'll have a closer look 
tomorrow) it looks like a nice simple and
effective interface. 

The question here, is what functionality of the system under discussion 
does it fulfill.  Again we are in the
territory of possibly having multiple drivers for a device depending on 
how it is being used in the kernel.
Not ideal, but may in fact be more maintainable in the long run.

The sysfs interface looks similar to that I was intending to implement, 
but obviously for your application you
don't need any of the buffering support etc and it makes sense to keep 
everything as simple as possible.

Perhaps if we aim to keep in mind the possibility of combining your ADC 
system with a more general
input device system, it may make sense to do so at a later stage. 

Anything I write is certainly going to share a considerable amount of 
functionality and approach with your code.
> Some drivers for this subsystem:
> http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/ads7846_adc_ssp.c
> ^ ADC driver for ADS7846 chips (HP iPaq hx4700 and some HTC phones, the
>   driver is using SSP subsystem, switching it to the SPI is still in my enless
>   TODO list).
>   
I'll keep these part in mind, as a the moment adding any component for 
testing involves me getting my soldering iron out
and I hate soldering!

> http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/samcop_adc.c
> ^ SAMCOP (HP iPaq H5xxx) ADC driver.
>
> http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/tsc2200_adc_ssp.c
> ^ TSC2200 ADC driver (used AFAIK in some HTC phones, or ASUS handhelds).
>
> http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/misc/adc/ad7877.c
> ^ AD7877 ADC driver (HTC phones too, IIRC).
>
> Now, The Generic ADC Touchscreen Driver (tested to work quite good with all
> above ADC drivers on appropriate hardware):
> http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/input/touchscreen/ts-adc.c
>
> And finally, ADC Battery driver ("backup" batteries in the iPaq devices
> usually report their voltage via ADC chip, the same used by the
> touchscreen).
> http://handhelds.org/cgi-bin/cvsweb.cgi/~checkout~/linux/kernel26/drivers/power/adc_battery.c
>
> p.s.
> I have more recent (for current Linus' tree) code for ADC subsystem, but
> it still needs some love to be submittable.
>   
Thanks.  I'll have a look at the stuff above and pester you if I need a 
more recent version.

Jonathan