[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080710065639.GA16794@digi.com>
Date: Thu, 10 Jul 2008 08:56:39 +0200
From: Uwe Kleine-König <Uwe.Kleine-Koenig@...i.com>
To: Magnus Damm <magnus.damm@...il.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"gregkh@...e.de" <gregkh@...e.de>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"hjk@...utronix.de" <hjk@...utronix.de>,
"lethal@...ux-sh.org" <lethal@...ux-sh.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"alan@...rguk.ukuu.org.uk" <alan@...rguk.ukuu.org.uk>
Subject: Re: [PATCH] uio: uio_pdrv_genirq V2
Hi Magnus,
Magnus Damm wrote:
> This patch is V2 of uio_pdrv_genirq.c, a platform driver for UIO with
> generic IRQ handling code. This driver is very similar to the regular
> UIO platform driver, but is only suitable for devices that are
> connected to the interrupt controller using unique interrupt lines.
>
> The uio_pdrv_genirq driver includes generic interrupt handling code
> which disables the serviced interrupt in the interrupt controller
> and makes the user space driver responsible for acknowledging the
> interrupt in the device and reenabling the interrupt in the interrupt
> controller.
>
> Shared interrupts are not supported since the in-kernel interrupt
> handler will disable the interrupt line in the interrupt controller,
> and in a shared interrupt configuration this will stop other devices
> from delivering interrupts.
>
> Signed-off-by: Magnus Damm <damm@...l.co.jp>
> ---
>
> Changes since V1
> - allow user space to enable _and_ disable the interrupt
> - replaced bitops with spinlock + irq_disabled variable
> - renamed pdata variable name to priv
>
> drivers/uio/Kconfig | 13 ++
> drivers/uio/Makefile | 1
> drivers/uio/uio_pdrv_genirq.c | 186 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 200 insertions(+)
>
> --- 0014/drivers/uio/Kconfig
> +++ work/drivers/uio/Kconfig 2008-07-09 17:10:20.000000000 +0900
> @@ -33,6 +33,19 @@ config UIO_PDRV
>
> If you don't know what to do here, say N.
>
> +config UIO_PDRV_GENIRQ
> + tristate "Userspace I/O platform driver with generic IRQ handling"
> + help
> + Platform driver for Userspace I/O devices, including generic
> + interrupt handling code. Shared interrupts are not supported.
> +
> + This kernel driver requires that the matching userspace driver
> + handles interrupts in a special way. Userspace is responsible
> + for acknowledging the hardware device if needed, and re-enabling
> + interrupts in the interrupt controller using the write() syscall.
> +
> + If you don't know what to do here, say N.
> +
> config UIO_SMX
> tristate "SMX cryptengine UIO interface"
> depends on UIO
> --- 0014/drivers/uio/Makefile
> +++ work/drivers/uio/Makefile 2008-07-09 17:10:20.000000000 +0900
> @@ -1,4 +1,5 @@
> obj-$(CONFIG_UIO) += uio.o
> obj-$(CONFIG_UIO_CIF) += uio_cif.o
> obj-$(CONFIG_UIO_PDRV) += uio_pdrv.o
> +obj-$(CONFIG_UIO_PDRV_GENIRQ) += uio_pdrv_genirq.o
> obj-$(CONFIG_UIO_SMX) += uio_smx.o
> --- /dev/null
> +++ work/drivers/uio/uio_pdrv_genirq.c 2008-07-10 12:24:52.000000000 +0900
> @@ -0,0 +1,186 @@
> +/*
> + * drivers/uio/uio_pdrv_genirq.c
> + *
> + * Userspace I/O platform driver with generic IRQ handling code.
> + *
> + * Copyright (C) 2008 Magnus Damm
> + *
> + * Based on uio_pdrv.c by Uwe Kleine-Koenig,
> + * Copyright (C) 2008 by Digi International Inc.
> + * All rights reserved.
> + *
> + * 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.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/uio_driver.h>
> +#include <linux/spinlock.h>
> +#include <linux/interrupt.h>
> +#include <linux/stringify.h>
> +
> +#define DRIVER_NAME "uio_pdrv_genirq"
> +
> +struct uio_pdrv_genirq_platdata {
> + struct uio_info *uioinfo;
> + spinlock_t lock;
> + int irq_disabled;
> +};
> +
> +static irqreturn_t uio_pdrv_genirq_handler(int irq, struct uio_info *dev_info)
> +{
> + struct uio_pdrv_genirq_platdata *priv = dev_info->priv;
> + unsigned long flags;
> +
> + /* Just disable the interrupt in the interrupt controller, and
> + * remember the state so we can allow user space to enable it later.
> + */
> + spin_lock_irqsave(&priv->lock, flags);
> + if (!priv->irq_disabled) {
> + disable_irq_nosync(irq);
> + priv->irq_disabled = 1;
> + }
> + spin_unlock_irqrestore(&priv->lock, flags);
> + return IRQ_HANDLED;
> +}
> +
> +static int uio_pdrv_genirq_irqcontrol(struct uio_info *dev_info, s32 irq_on)
> +{
> + struct uio_pdrv_genirq_platdata *priv = dev_info->priv;
> + unsigned long flags;
> +
> + /* Allow user space to enable and disable the interrupt
> + * in the interrupt controller, but keep track of the
> + * state to prevent per-irq depth damage.
> + */
> +
> + spin_lock_irqsave(&priv->lock, flags);
> + if (irq_on && priv->irq_disabled)
> + enable_irq(dev_info->irq);
> + else if (!irq_on && !priv->irq_disabled)
> + disable_irq(dev_info->irq);
I'm not sure if this is a problem on SMP. Should you use
disable_irq_nosync here, too? Probably it's OK.
> +
> + priv->irq_disabled = !irq_on;
> + spin_unlock_irqrestore(&priv->lock, flags);
> + return 0;
> +}
> + ret = uio_register_device(&pdev->dev, priv->uioinfo);
> + if (ret) {
> + dev_err(&pdev->dev, "unable to register uio device\n");
> + goto bad1;
> + }
> +
> + platform_set_drvdata(pdev, priv);
This should probably go before uio_register_device. (Uups, this is an
issue for uio_pdrv, too.)
Best regards
Uwe
--
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists