[PATCH] (repost) lis302dl-queue-work-in-interrupt-handler.patch
Sean McNeil
sean at mcneil.com
Mon Sep 22 18:35:12 CEST 2008
2 things:
1) If you change the interrupt to a level instead of edge trigger, then
you can get rid of the code to jump start and the while loop. It also
prevents lockup when the system is extremely busy. I don't think your
implementation would prevent that.
2) I was under the impression that the reg_read didn't prevent
simultaneous access of the two accelerometers. This would cause problems
since the chip selects need to be mutually exclusive when accessing them.
Cheers,
Sean
Simon Kagstrom wrote:
> Use work queue for lis302dl interrupts, fix accelerometer hang.
>
> From: Simon Kagstrom <simon.kagstrom at gmail.com>
>
> Use work queue for interrupts (since bitbang cannot be used in the
> interrupt handler). This gets rid of the special bitbang_read in
> mach-gta02.c as well, so that function has been removed.
>
> The accelerometer "hang" seems to be caused by missed interrupts, so the
> interrupt handler now reads the x,y,z values until the status shows no
> outstanding data.
>
> Signed-off-by: Simon Kagstrom <simon.kagstrom at gmail.com>
> ---
>
> arch/arm/mach-s3c2440/mach-gta02.c | 104 ------------------------------------
> drivers/input/misc/lis302dl.c | 41 +++++++++++++-
> include/linux/lis302dl.h | 4 +
> 3 files changed, 42 insertions(+), 107 deletions(-)
>
> diff --git a/arch/arm/mach-s3c2440/mach-gta02.c b/arch/arm/mach-s3c2440/mach-gta02.c
> index c15f072..1da48cb 100644
> --- a/arch/arm/mach-s3c2440/mach-gta02.c
> +++ b/arch/arm/mach-s3c2440/mach-gta02.c
> @@ -1034,109 +1034,9 @@ static struct platform_device gta02_vibrator_dev = {
>
> /*
> * Situation is that Linux SPI can't work in an interrupt context, so we
> - * implement our own bitbang here. Arbitration is needed because not only
> - * can this interrupt happen at any time even if foreground wants to use
> - * the bitbang API from Linux, but multiple motion sensors can be on the
> - * same SPI bus, and multiple interrupts can happen.
> - *
> - * Foreground / interrupt arbitration is okay because the interrupts are
> - * disabled around all the foreground SPI code.
> - *
> - * Interrupt / Interrupt arbitration is evidently needed, otherwise we
> - * lose edge-triggered service after a while due to the two sensors sharing
> - * the SPI bus having irqs at the same time eventually.
> - *
> - * Servicing is typ 75 - 100us at 400MHz.
> + * implement our own bitbang here.
> */
>
> -/* #define DEBUG_SPEW_MS */
> -#define MG_PER_SAMPLE 18
> -
> -struct lis302dl_platform_data lis302_pdata[];
> -
> -void gta02_lis302dl_bitbang_read(struct lis302dl_info *lis)
> -{
> - struct lis302dl_platform_data *pdata = lis->pdata;
> - u8 shifter = 0xc0 | LIS302DL_REG_OUT_X; /* read, autoincrement */
> - int n, n1;
> - unsigned long other_cs;
> - unsigned long flags;
> -#ifdef DEBUG_SPEW_MS
> - s8 x, y, z;
> -#endif
> -
> - local_irq_save(flags);
> -
> - /*
> - * Huh.. "quirk"... CS on this device is not really "CS" like you can
> - * expect. Instead when 1 it selects I2C interface mode. Because we
> - * have 2 devices on one interface, the "disabled" device when we talk
> - * to an "enabled" device sees the clocks as I2C clocks, creating
> - * havoc.
> - * I2C sees MOSI going LOW while CLK HIGH as a START action, we must
> - * ensure this is never issued.
> - */
> -
> - if (&lis302_pdata[0] == pdata)
> - other_cs = lis302_pdata[1].pin_chip_select;
> - else
> - other_cs = lis302_pdata[0].pin_chip_select;
> -
> - s3c2410_gpio_setpin(other_cs, 1);
> - s3c2410_gpio_setpin(pdata->pin_chip_select, 1);
> - s3c2410_gpio_setpin(pdata->pin_clk, 1);
> - s3c2410_gpio_setpin(pdata->pin_chip_select, 0);
> - for (n = 0; n < 8; n++) { /* write the r/w, inc and address */
> - s3c2410_gpio_setpin(pdata->pin_clk, 0);
> - s3c2410_gpio_setpin(pdata->pin_mosi, (shifter >> 7) & 1);
> - s3c2410_gpio_setpin(pdata->pin_clk, 0);
> - s3c2410_gpio_setpin(pdata->pin_clk, 1);
> - s3c2410_gpio_setpin(pdata->pin_clk, 1);
> - shifter <<= 1;
> - }
> -
> - for (n = 0; n < 5; n++) { /* 5 consequetive registers */
> - for (n1 = 0; n1 < 8; n1++) { /* 8 bits each */
> - s3c2410_gpio_setpin(pdata->pin_clk, 0);
> - s3c2410_gpio_setpin(pdata->pin_clk, 0);
> - shifter <<= 1;
> - if (s3c2410_gpio_getpin(pdata->pin_miso))
> - shifter |= 1;
> - s3c2410_gpio_setpin(pdata->pin_clk, 1);
> - s3c2410_gpio_setpin(pdata->pin_clk, 1);
> - }
> - switch (n) {
> - case 0:
> -#ifdef DEBUG_SPEW_MS
> - x = shifter;
> -#endif
> - input_report_rel(lis->input_dev, REL_X, MG_PER_SAMPLE * (s8)shifter);
> - break;
> - case 2:
> -#ifdef DEBUG_SPEW_MS
> - y = shifter;
> -#endif
> - input_report_rel(lis->input_dev, REL_Y, MG_PER_SAMPLE * (s8)shifter);
> - break;
> - case 4:
> -#ifdef DEBUG_SPEW_MS
> - z = shifter;
> -#endif
> - input_report_rel(lis->input_dev, REL_Z, MG_PER_SAMPLE * (s8)shifter);
> - break;
> - }
> - }
> - s3c2410_gpio_setpin(pdata->pin_chip_select, 1);
> - s3c2410_gpio_setpin(other_cs, 1);
> - local_irq_restore(flags);
> -
> - input_sync(lis->input_dev);
> -#ifdef DEBUG_SPEW_MS
> - printk("%s: %d %d %d\n", pdata->name, x, y, z);
> -#endif
> -}
> -
> -
> void gta02_lis302dl_suspend_io(struct lis302dl_info *lis, int resume)
> {
> struct lis302dl_platform_data *pdata = lis->pdata;
> @@ -1169,7 +1069,6 @@ struct lis302dl_platform_data lis302_pdata[] = {
> .pin_mosi = S3C2410_GPG6,
> .pin_miso = S3C2410_GPG5,
> .open_drain = 1, /* altered at runtime by PCB rev */
> - .lis302dl_bitbang_read = gta02_lis302dl_bitbang_read,
> .lis302dl_suspend_io = gta02_lis302dl_suspend_io,
> }, {
> .name = "lis302-2 (bottom)",
> @@ -1178,7 +1077,6 @@ struct lis302dl_platform_data lis302_pdata[] = {
> .pin_mosi = S3C2410_GPG6,
> .pin_miso = S3C2410_GPG5,
> .open_drain = 1, /* altered at runtime by PCB rev */
> - .lis302dl_bitbang_read = gta02_lis302dl_bitbang_read,
> .lis302dl_suspend_io = gta02_lis302dl_suspend_io,
> },
> };
> diff --git a/drivers/input/misc/lis302dl.c b/drivers/input/misc/lis302dl.c
> index b01ca04..c725e72 100644
> --- a/drivers/input/misc/lis302dl.c
> +++ b/drivers/input/misc/lis302dl.c
> @@ -160,12 +160,42 @@ static void _report_btn_double(struct input_dev *inp, int btn)
> }
> #endif
>
> +#define MG_PER_SAMPLE 18
> +
> +static void lis302dl_read_input(struct lis302dl_info *lis)
> +{
> + s8 x, y, z;
> + u8 status;
> +
> + /* Loop until there is no data available */
> + while ((status = reg_read(lis, LIS302DL_REG_STATUS)) &
> + (LIS302DL_STATUS_XYZOR | LIS302DL_STATUS_XDA |
> + LIS302DL_STATUS_YDA | LIS302DL_STATUS_ZDA)) {
> + x = (s8)reg_read(lis, LIS302DL_REG_OUT_X);
> + y = (s8)reg_read(lis, LIS302DL_REG_OUT_Y);
> + z = (s8)reg_read(lis, LIS302DL_REG_OUT_Z);
> +
> + input_report_rel(lis->input_dev, REL_X, MG_PER_SAMPLE * x);
> + input_report_rel(lis->input_dev, REL_Y, MG_PER_SAMPLE * y);
> + input_report_rel(lis->input_dev, REL_Z, MG_PER_SAMPLE * z);
> + input_sync(lis->input_dev);
> + }
> +}
> +
> +static void lis302dl_queued_interrupt(struct work_struct *work)
> +{
> + struct lis302dl_info *lis = container_of(work,
> + struct lis302dl_info, interrupt_work);
> +
> + lis302dl_read_input(lis);
> +}
>
> static irqreturn_t lis302dl_interrupt(int irq, void *_lis)
> {
> struct lis302dl_info *lis = _lis;
>
> - (lis->pdata->lis302dl_bitbang_read)(lis);
> + queue_work(lis->interrupt_queue, &lis->interrupt_work);
> +
> return IRQ_HANDLED;
> }
>
> @@ -478,8 +508,8 @@ static int lis302dl_input_open(struct input_dev *inp)
> LIS302DL_CTRL1_Yen | LIS302DL_CTRL1_Zen;
> unsigned long flags;
>
> - local_irq_save(flags);
> /* make sure we're powered up and generate data ready */
> + local_irq_save(flags);
> reg_set_bit_mask(lis, LIS302DL_REG_CTRL1, ctrl1, ctrl1);
> local_irq_restore(flags);
>
> @@ -487,7 +517,7 @@ static int lis302dl_input_open(struct input_dev *inp)
>
> /* kick it off -- since we are edge triggered, if we missed the edge
> * permanent low interrupt is death for us */
> - (lis->pdata->lis302dl_bitbang_read)(lis);
> + lis302dl_read_input(lis);
>
> return 0;
> }
> @@ -546,6 +576,9 @@ static int __devinit lis302dl_probe(struct spi_device *spi)
> if (!lis)
> return -ENOMEM;
>
> + INIT_WORK(&lis->interrupt_work, lis302dl_queued_interrupt);
> + lis->interrupt_queue = create_workqueue("LIS302dl interrupt queue");
> +
> local_irq_save(flags);
>
> mutex_init(&lis->lock);
> @@ -682,6 +715,8 @@ static int __devexit lis302dl_remove(struct spi_device *spi)
> local_irq_restore(flags);
>
> /* Cleanup resources */
> + destroy_workqueue(lis->interrupt_queue);
> +
> free_irq(lis->spi_dev->irq, lis);
> sysfs_remove_group(&spi->dev.kobj, &lis302dl_attr_group);
> input_unregister_device(lis->input_dev);
> diff --git a/include/linux/lis302dl.h b/include/linux/lis302dl.h
> index 7daa8b3..c708bdc 100644
> --- a/include/linux/lis302dl.h
> +++ b/include/linux/lis302dl.h
> @@ -4,6 +4,7 @@
> #include <linux/types.h>
> #include <linux/spi/spi.h>
> #include <linux/input.h>
> +#include <linux/workqueue.h>
>
>
> struct lis302dl_info;
> @@ -15,7 +16,6 @@ struct lis302dl_platform_data {
> unsigned long pin_mosi;
> unsigned long pin_miso;
> int open_drain;
> - void (*lis302dl_bitbang_read)(struct lis302dl_info *);
> void (*lis302dl_suspend_io)(struct lis302dl_info *, int resuming);
> };
>
> @@ -23,6 +23,8 @@ struct lis302dl_info {
> struct lis302dl_platform_data *pdata;
> struct spi_device *spi_dev;
> struct input_dev *input_dev;
> + struct workqueue_struct *interrupt_queue;
> + struct work_struct interrupt_work;
> struct mutex lock;
> unsigned int flags;
> u_int8_t regs[0x40];
>
>
More information about the openmoko-kernel
mailing list
