All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] Add push based interfce for non userspace iio users
@ 2012-03-03 12:09 Jonathan Cameron
       [not found] ` <1330776551-20301-4-git-send-email-jic23@kernel.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2012-03-03 12:09 UTC (permalink / raw
  To: linux-iio
  Cc: greg, dmitry.torokhov, broonie, alan, arnd, linus.walleij,
	maxime.ripard, thomas.petazzoni, zdevai, w.sang, marek.vasut,
	Jonathan Cameron

This set is large and relatively invasive.  The first important thing
to verify is that the first patch doesn't break any of the existing
drivers.

Taking the patches in turn.  Firstly this currenty should only effect
devices with software buffers so when I say buffers that is what I mean
in this description.

1) Re route all pushing to software buffers via iio_push_to_buffers.
   We also need to modify drivers to make them aware of the fact that
   they may well be pushing to multiple buffers.  Where possible I
   have done this by making use of iio_sw_buffer_preenable and adding
   pre demux scan element size computation to the core (as this was
   repeated quite a few times in drivers).  Basically I've almost
   certainly broken someone's driver so please test this one!

2) Now all drivers (that support buffers at all) should handle multiple
   buffers we add a trivial call back buffer implementation. This simply
   calls a callback function with new scans of data as and when they
   are available.

3) A very rough and ready proof of concept for an input driver that
   uses iio in a similar way to the hwmon driver that merged a few
   days back. Clearly this needs more work!

One little question this 3rd patch raises is how we get board specific
details about channels into an iio consumer device.  Here I thinking
things like 'fuzz' values for input.

To that end what do people think of adding a void *consumer_data
pointer to the struct iio_chan_map structure?

Other than that it's worth noting that this interface uses
exactly the same interface from the point of view of board files
as the pull interface does. (Hopefully avoiding what proved the most
controversial bit last time!).

One other practical issue post this patch set is that we can currently
only have either polled or interrupt driven clients of iio devices
but not both.  I have some thoughts on how to deal with this as it
is obviously import to some SoC adc users.

Another future project will be making a cleaner break between those
buffering elements to do with the IIO direct userspace interface
and those used by other consumer drivers.  If it looks like we
bolted on the in kernel users side of things that is because we
did!

Jonathan

Jonathan Cameron (3):
  staging:iio: make all buffer access pass through the buffer_list
  staging:iio: add a callback buffer for in kernel push interface
  staging:iio: Proof of concept input driver.

 drivers/staging/iio/Kconfig                     |   16 +
 drivers/staging/iio/Makefile                    |    2 +
 drivers/staging/iio/accel/adis16201_ring.c      |    9 +-
 drivers/staging/iio/accel/adis16203_ring.c      |   11 +-
 drivers/staging/iio/accel/adis16204_ring.c      |    8 +-
 drivers/staging/iio/accel/adis16209_ring.c      |    9 +-
 drivers/staging/iio/accel/adis16240_ring.c      |    9 +-
 drivers/staging/iio/accel/lis3l02dq_ring.c      |    9 +-
 drivers/staging/iio/adc/ad7192.c                |   27 +-
 drivers/staging/iio/adc/ad7298.h                |    1 -
 drivers/staging/iio/adc/ad7298_ring.c           |   31 +-
 drivers/staging/iio/adc/ad7476.h                |    1 -
 drivers/staging/iio/adc/ad7476_ring.c           |   40 +-
 drivers/staging/iio/adc/ad7606_ring.c           |   11 +-
 drivers/staging/iio/adc/ad7793.c                |   26 +-
 drivers/staging/iio/adc/ad7887.h                |    1 -
 drivers/staging/iio/adc/ad7887_ring.c           |   28 +-
 drivers/staging/iio/adc/ad799x.h                |    1 -
 drivers/staging/iio/adc/ad799x_ring.c           |   27 +-
 drivers/staging/iio/adc/max1363_ring.c          |    6 +-
 drivers/staging/iio/buffer.h                    |   24 +-
 drivers/staging/iio/buffer_cb.c                 |  115 +++++
 drivers/staging/iio/consumer.h                  |   46 ++
 drivers/staging/iio/gyro/adis16260_ring.c       |    8 +-
 drivers/staging/iio/iio.h                       |    8 +
 drivers/staging/iio/iio_input.c                 |  106 +++++
 drivers/staging/iio/iio_simple_dummy_buffer.c   |   16 +-
 drivers/staging/iio/impedance-analyzer/ad5933.c |   14 +-
 drivers/staging/iio/imu/adis16400_ring.c        |    6 +-
 drivers/staging/iio/industrialio-buffer.c       |  553 ++++++++++++++---------
 drivers/staging/iio/industrialio-core.c         |    1 +
 drivers/staging/iio/meter/ade7758_ring.c        |   25 +-
 32 files changed, 740 insertions(+), 455 deletions(-)
 create mode 100644 drivers/staging/iio/buffer_cb.c
 create mode 100644 drivers/staging/iio/iio_input.c

-- 
1.7.9.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] staging:iio: Proof of concept input driver.
       [not found] ` <1330776551-20301-4-git-send-email-jic23@kernel.org>
@ 2012-03-04 16:11   ` Dmitry Torokhov
  2012-03-07 11:18     ` Jonathan Cameron
  2012-03-07 14:57   ` Linus Walleij
  1 sibling, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2012-03-04 16:11 UTC (permalink / raw
  To: Jonathan Cameron
  Cc: linux-iio, greg, broonie, alan, arnd, linus.walleij,
	maxime.ripard, thomas.petazzoni, zdevai, w.sang, marek.vasut,
	Jonathan Cameron

Hi Jonathan,

On Sat, Mar 03, 2012 at 12:09:11PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <jic23@cam.ac.uk>
> 
> This is no where near ready to merge.  Lots of stuff missing.
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/Kconfig     |   10 ++++
>  drivers/staging/iio/Makefile    |    1 +
>  drivers/staging/iio/iio_input.c |  106 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 117 insertions(+)
> 
> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
> index 4aff125..a599f16 100644
> --- a/drivers/staging/iio/Kconfig
> +++ b/drivers/staging/iio/Kconfig
> @@ -11,6 +11,16 @@ menuconfig IIO
>  	  number of different physical interfaces (i2c, spi, etc). See
>  	  drivers/staging/iio/Documentation for more information.
>  if IIO
> +
> +config IIO_ST_INPUT
> +	tristate "Input driver that uses channels specified via iio maps"
> +	depends on INPUT
> +	depends on IIO_BUFFER
> +	select IIO_BUFFER_CB
> +	help
> +	  Proof of concept user of the in kernel push interface.  Not anywhere
> +	  near ready for production use.
> +
>  config IIO_ST_HWMON
>  	tristate "Hwmon driver that uses channels specified via iio maps"
>  	depends on HWMON
> diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
> index f55de94..f64c93b 100644
> --- a/drivers/staging/iio/Makefile
> +++ b/drivers/staging/iio/Makefile
> @@ -19,6 +19,7 @@ iio_dummy-$(CONFIG_IIO_SIMPLE_DUMMY_BUFFER) += iio_simple_dummy_buffer.o
>  obj-$(CONFIG_IIO_DUMMY_EVGEN) += iio_dummy_evgen.o
>  
>  obj-$(CONFIG_IIO_ST_HWMON) += iio_hwmon.o
> +obj-$(CONFIG_IIO_ST_INPUT) += iio_input.o
>  
>  obj-y += accel/
>  obj-y += adc/
> diff --git a/drivers/staging/iio/iio_input.c b/drivers/staging/iio/iio_input.c
> new file mode 100644
> index 0000000..2481901
> --- /dev/null
> +++ b/drivers/staging/iio/iio_input.c
> @@ -0,0 +1,106 @@
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/input.h>
> +#include "buffer.h"
> +#include "consumer.h"
> +
> +struct iio_input_state {
> +	struct iio_cb_buffer *buff;
> +	struct input_dev *idev;
> +};
> +
> +static int iio_input_store_to(u8 *data, void *private)
> +{
> +	struct iio_input_state *st = private;
> +
> +	/* DUMMY - need to put boiler plate conversion code
> +	 * in place */
> +	input_report_abs(st->idev, ABS_X, data[0]);
> +	input_sync(st->idev);
> +
> +	return 0;
> +}
> +
> +static int __devinit iio_input_probe(struct platform_device *pdev)
> +{
> +	struct iio_input_state *st;
> +	int ret;
> +
> +	st = kzalloc(sizeof(*st), GFP_KERNEL);
> +	if (st == NULL)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, st);
> +	st->buff = iio_st_channel_get_all_cb(dev_name(&pdev->dev),
> +					     &iio_input_store_to,
> +					     st);
> +	if (IS_ERR(st->buff)) {
> +		ret = PTR_ERR(st->buff);
> +		goto error_free_state;
> +	}
> +
> +	st->idev = input_allocate_device();
> +	if (!st->idev) {
> +		ret = -ENOMEM;
> +		goto error_channels_release_all;
> +	}
> +
> +	__set_bit(EV_ABS, st->idev->evbit);
> +	/* DUMMY DATA - need to actually make this available */
> +	input_set_abs_params(st->idev, ABS_X, 0, 100, 0, 0);
> +
> +	ret = input_register_device(st->idev);
> +	if (ret < 0)
> +		goto error_free_idev;
> +
> +	/* NORMALLY IN THE OPEN */
> +	iio_st_channel_start_all_cb(st->buff);
> +
> +	return 0;
> +error_free_idev:
> +	input_free_device(st->idev);
> +error_channels_release_all:
> +	iio_st_channel_release_all_cb(st->buff);
> +error_free_state:
> +	kfree(st);
> +	return ret;
> +}
> +
> +static int __devexit iio_input_remove(struct platform_device *pdev)
> +{
> +	struct iio_input_state *st = platform_get_drvdata(pdev);
> +	/* NORMALLY IN THE CLOSE */
> +	iio_st_channel_stop_all_cb(st->buff);
> +	input_unregister_device(st->idev);
> +	iio_st_channel_release_all_cb(st->buff);
> +
> +	kfree(st);
> +	return 0;
> +}
> +
> +static struct platform_driver __refdata iio_input_driver = {

Why is this __refdata?

> +	.driver = {
> +		.name = "iio_snoop",
> +		.owner = THIS_MODULE,
> +	},
> +	.probe = iio_input_probe,
> +	.remove = __devexit_p(iio_input_remove),
> +};
> +
> +static int iio_input_init(void)

__init

> +{
> +	return platform_driver_register(&iio_input_driver);
> +}
> +module_init(iio_input_init);
> +
> +static void iio_input_exit(void)

__exit

> +{
> +	platform_driver_unregister(&iio_input_driver);
> +}
> +module_exit(iio_input_exit);
> +
> +MODULE_AUTHOR("Jonathan Cameron <jic23@cam.ac.uk>");
> +MODULE_DESCRIPTION("IIO input buffer driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.7.9.2
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] staging:iio: Proof of concept input driver.
  2012-03-04 16:11   ` [PATCH 3/3] staging:iio: Proof of concept input driver Dmitry Torokhov
@ 2012-03-07 11:18     ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2012-03-07 11:18 UTC (permalink / raw
  To: Dmitry Torokhov
  Cc: linux-iio, greg, broonie, alan, arnd, linus.walleij,
	maxime.ripard, thomas.petazzoni, zdevai, w.sang, marek.vasut,
	Jonathan Cameron

On 03/04/2012 04:11 PM, Dmitry Torokhov wrote:
> Hi Jonathan,
> 
> On Sat, Mar 03, 2012 at 12:09:11PM +0000, Jonathan Cameron wrote:
>> From: Jonathan Cameron <jic23@cam.ac.uk>
>>
>> This is no where near ready to merge.  Lots of stuff missing.
>>
>> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
>> ---
>>  drivers/staging/iio/Kconfig     |   10 ++++
>>  drivers/staging/iio/Makefile    |    1 +
>>  drivers/staging/iio/iio_input.c |  106 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 117 insertions(+)
>>
>> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
>> index 4aff125..a599f16 100644
>> --- a/drivers/staging/iio/Kconfig
>> +++ b/drivers/staging/iio/Kconfig
>> @@ -11,6 +11,16 @@ menuconfig IIO
>>  	  number of different physical interfaces (i2c, spi, etc). See
>>  	  drivers/staging/iio/Documentation for more information.
>>  if IIO
>> +
>> +config IIO_ST_INPUT
>> +	tristate "Input driver that uses channels specified via iio maps"
>> +	depends on INPUT
>> +	depends on IIO_BUFFER
>> +	select IIO_BUFFER_CB
>> +	help
>> +	  Proof of concept user of the in kernel push interface.  Not anywhere
>> +	  near ready for production use.
>> +
>>  config IIO_ST_HWMON
>>  	tristate "Hwmon driver that uses channels specified via iio maps"
>>  	depends on HWMON
>> diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
>> index f55de94..f64c93b 100644
>> --- a/drivers/staging/iio/Makefile
>> +++ b/drivers/staging/iio/Makefile
>> @@ -19,6 +19,7 @@ iio_dummy-$(CONFIG_IIO_SIMPLE_DUMMY_BUFFER) += iio_simple_dummy_buffer.o
>>  obj-$(CONFIG_IIO_DUMMY_EVGEN) += iio_dummy_evgen.o
>>  
>>  obj-$(CONFIG_IIO_ST_HWMON) += iio_hwmon.o
>> +obj-$(CONFIG_IIO_ST_INPUT) += iio_input.o
>>  
>>  obj-y += accel/
>>  obj-y += adc/
>> diff --git a/drivers/staging/iio/iio_input.c b/drivers/staging/iio/iio_input.c
>> new file mode 100644
>> index 0000000..2481901
>> --- /dev/null
>> +++ b/drivers/staging/iio/iio_input.c
>> @@ -0,0 +1,106 @@
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/module.h>
>> +#include <linux/err.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/input.h>
>> +#include "buffer.h"
>> +#include "consumer.h"
>> +
>> +struct iio_input_state {
>> +	struct iio_cb_buffer *buff;
>> +	struct input_dev *idev;
>> +};
>> +
>> +static int iio_input_store_to(u8 *data, void *private)
>> +{
>> +	struct iio_input_state *st = private;
>> +
>> +	/* DUMMY - need to put boiler plate conversion code
>> +	 * in place */
>> +	input_report_abs(st->idev, ABS_X, data[0]);
>> +	input_sync(st->idev);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __devinit iio_input_probe(struct platform_device *pdev)
>> +{
>> +	struct iio_input_state *st;
>> +	int ret;
>> +
>> +	st = kzalloc(sizeof(*st), GFP_KERNEL);
>> +	if (st == NULL)
>> +		return -ENOMEM;
>> +	platform_set_drvdata(pdev, st);
>> +	st->buff = iio_st_channel_get_all_cb(dev_name(&pdev->dev),
>> +					     &iio_input_store_to,
>> +					     st);
>> +	if (IS_ERR(st->buff)) {
>> +		ret = PTR_ERR(st->buff);
>> +		goto error_free_state;
>> +	}
>> +
>> +	st->idev = input_allocate_device();
>> +	if (!st->idev) {
>> +		ret = -ENOMEM;
>> +		goto error_channels_release_all;
>> +	}
>> +
>> +	__set_bit(EV_ABS, st->idev->evbit);
>> +	/* DUMMY DATA - need to actually make this available */
>> +	input_set_abs_params(st->idev, ABS_X, 0, 100, 0, 0);
>> +
>> +	ret = input_register_device(st->idev);
>> +	if (ret < 0)
>> +		goto error_free_idev;
>> +
>> +	/* NORMALLY IN THE OPEN */
>> +	iio_st_channel_start_all_cb(st->buff);
>> +
>> +	return 0;
>> +error_free_idev:
>> +	input_free_device(st->idev);
>> +error_channels_release_all:
>> +	iio_st_channel_release_all_cb(st->buff);
>> +error_free_state:
>> +	kfree(st);
>> +	return ret;
>> +}
>> +
>> +static int __devexit iio_input_remove(struct platform_device *pdev)
>> +{
>> +	struct iio_input_state *st = platform_get_drvdata(pdev);
>> +	/* NORMALLY IN THE CLOSE */
>> +	iio_st_channel_stop_all_cb(st->buff);
>> +	input_unregister_device(st->idev);
>> +	iio_st_channel_release_all_cb(st->buff);
>> +
>> +	kfree(st);
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver __refdata iio_input_driver = {
> 
> Why is this __refdata?
No idea :)  Will drop that.  Also can use the platform_device_module
stuff below to get rid of some of this boiler plate.

Will clear all this up for a version that actually does something
beyond the proof of concept here!

Thanks for taking a look,

Jonathan
> 
>> +	.driver = {
>> +		.name = "iio_snoop",
>> +		.owner = THIS_MODULE,
>> +	},
>> +	.probe = iio_input_probe,
>> +	.remove = __devexit_p(iio_input_remove),
>> +};
>> +
>> +static int iio_input_init(void)
> 
> __init
> 
>> +{
>> +	return platform_driver_register(&iio_input_driver);
>> +}
>> +module_init(iio_input_init);
>> +
>> +static void iio_input_exit(void)
> 
> __exit
good point. Thanks!
> 
>> +{
>> +	platform_driver_unregister(&iio_input_driver);
>> +}
>> +module_exit(iio_input_exit);
>> +
>> +MODULE_AUTHOR("Jonathan Cameron <jic23@cam.ac.uk>");
>> +MODULE_DESCRIPTION("IIO input buffer driver");
>> +MODULE_LICENSE("GPL v2");
>> -- 
>> 1.7.9.2
>>
> 
> Thanks.
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] staging:iio: Proof of concept input driver.
       [not found] ` <1330776551-20301-4-git-send-email-jic23@kernel.org>
  2012-03-04 16:11   ` [PATCH 3/3] staging:iio: Proof of concept input driver Dmitry Torokhov
@ 2012-03-07 14:57   ` Linus Walleij
  1 sibling, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2012-03-07 14:57 UTC (permalink / raw
  To: Jonathan Cameron
  Cc: linux-iio, greg, dmitry.torokhov, broonie, alan, arnd,
	maxime.ripard, thomas.petazzoni, zdevai, w.sang, marek.vasut,
	Jonathan Cameron

On Sat, Mar 3, 2012 at 1:09 PM, Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <jic23@cam.ac.uk>
>
> This is no where near ready to merge. =A0Lots of stuff missing.
>
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>

Overall this is looking very interesting, this is definately where we
want to go with this IMO.

> +static int iio_input_store_to(u8 *data, void *private)
> +{
> + =A0 =A0 =A0 struct iio_input_state *st =3D private;
> +
> + =A0 =A0 =A0 /* DUMMY - need to put boiler plate conversion code
> + =A0 =A0 =A0 =A0* in place */
> + =A0 =A0 =A0 input_report_abs(st->idev, ABS_X, data[0]);
> + =A0 =A0 =A0 input_sync(st->idev);
> +
> + =A0 =A0 =A0 return 0;
> +}

If I have understood correctly, a lot of contemporary input handling
is in need of strict timestamping such as available in IIO. (Strict
control feedback loops etc.)

I guess these applications should use the "real" IIO interfaces,
whereas the input interface should be kept as-is?

Or ... is it possible to get the input subsystem to propagate the
timestamps to userspace?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-03-07 14:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-03 12:09 [RFC PATCH 0/3] Add push based interfce for non userspace iio users Jonathan Cameron
     [not found] ` <1330776551-20301-4-git-send-email-jic23@kernel.org>
2012-03-04 16:11   ` [PATCH 3/3] staging:iio: Proof of concept input driver Dmitry Torokhov
2012-03-07 11:18     ` Jonathan Cameron
2012-03-07 14:57   ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.