[PATCH][RESEND] uio: uio_pdrv_genirq V3

[PATCH][RESEND] uio: uio_pdrv_genirq V3

[Magnus Damm]

From: Magnus Damm <damm@igel.co.jp>

This is V3 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@igel.co.jp>
Signed-off-by: Hans J. Koch <hjk@linutronix.de>
---

 Patch sent to lkml on July 11th, please pick up.
 Platform data already in place in arch/sh/kernel/cpu/sh4a/...

 Changes since V2
  - readded bitops
  - removed irq handler spinlock to fix potential deadlock
  - kept irq control spinlock to serialize user space access

 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 |  188 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 202 insertions(+)

--- 0001/drivers/uio/Kconfig
+++ work/drivers/uio/Kconfig	2008-07-29 20:59:36.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"
 	default n
--- 0001/drivers/uio/Makefile
+++ work/drivers/uio/Makefile	2008-07-29 20:59:36.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-29 20:59:36.000000000 +0900
@@ -0,0 +1,188 @@
+/*
+ * 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/bitops.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;
+	unsigned long flags;
+};
+
+static irqreturn_t uio_pdrv_genirq_handler(int irq, struct uio_info *dev_info)
+{
+	struct uio_pdrv_genirq_platdata *priv = dev_info->priv;
+
+	/* Just disable the interrupt in the interrupt controller, and
+	 * remember the state so we can allow user space to enable it later.
+	 */
+
+	if (!test_and_set_bit(0, &priv->flags))
+		disable_irq_nosync(irq);
+
+	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.
+	 *
+	 * Serialize this operation to support multiple tasks.
+	 */
+
+	spin_lock_irqsave(&priv->lock, flags);
+	if (irq_on) {
+		if (test_and_clear_bit(0, &priv->flags))
+			enable_irq(dev_info->irq);
+	} else {
+		if (!test_and_set_bit(0, &priv->flags))
+			disable_irq(dev_info->irq);
+	}
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int uio_pdrv_genirq_probe(struct platform_device *pdev)
+{
+	struct uio_info *uioinfo = pdev->dev.platform_data;
+	struct uio_pdrv_genirq_platdata *priv;
+	struct uio_mem *uiomem;
+	int ret = -EINVAL;
+	int i;
+
+	if (!uioinfo || !uioinfo->name || !uioinfo->version) {
+		dev_err(&pdev->dev, "missing platform_data\n");
+		goto bad0;
+	}
+
+	if (uioinfo->handler || uioinfo->irqcontrol || uioinfo->irq_flags) {
+		dev_err(&pdev->dev, "interrupt configuration error\n");
+		goto bad0;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		dev_err(&pdev->dev, "unable to kmalloc\n");
+		goto bad0;
+	}
+
+	priv->uioinfo = uioinfo;
+	spin_lock_init(&priv->lock);
+	priv->flags = 0; /* interrupt is enabled to begin with */
+
+	uiomem = &uioinfo->mem[0];
+
+	for (i = 0; i < pdev->num_resources; ++i) {
+		struct resource *r = &pdev->resource[i];
+
+		if (r->flags != IORESOURCE_MEM)
+			continue;
+
+		if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
+			dev_warn(&pdev->dev, "device has more than "
+					__stringify(MAX_UIO_MAPS)
+					" I/O memory resources.\n");
+			break;
+		}
+
+		uiomem->memtype = UIO_MEM_PHYS;
+		uiomem->addr = r->start;
+		uiomem->size = r->end - r->start + 1;
+		++uiomem;
+	}
+
+	while (uiomem < &uioinfo->mem[MAX_UIO_MAPS]) {
+		uiomem->size = 0;
+		++uiomem;
+	}
+
+	/* This driver requires no hardware specific kernel code to handle
+	 * interrupts. Instead, the interrupt handler simply disables the
+	 * interrupt in the interrupt controller. User space is responsible
+	 * for performing hardware specific acknowledge and re-enabling of
+	 * the interrupt in the interrupt controller.
+	 *
+	 * Interrupt sharing is not supported.
+	 */
+
+	uioinfo->irq_flags = IRQF_DISABLED;
+	uioinfo->handler = uio_pdrv_genirq_handler;
+	uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
+	uioinfo->priv = priv;
+
+	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);
+	return 0;
+ bad1:
+	kfree(priv);
+ bad0:
+	return ret;
+}
+
+static int uio_pdrv_genirq_remove(struct platform_device *pdev)
+{
+	struct uio_pdrv_genirq_platdata *priv = platform_get_drvdata(pdev);
+
+	uio_unregister_device(priv->uioinfo);
+	kfree(priv);
+	return 0;
+}
+
+static struct platform_driver uio_pdrv_genirq = {
+	.probe = uio_pdrv_genirq_probe,
+	.remove = uio_pdrv_genirq_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.owner = THIS_MODULE,
+	},
+};
+
+static int __init uio_pdrv_genirq_init(void)
+{
+	return platform_driver_register(&uio_pdrv_genirq);
+}
+
+static void __exit uio_pdrv_genirq_exit(void)
+{
+	platform_driver_unregister(&uio_pdrv_genirq);
+}
+
+module_init(uio_pdrv_genirq_init);
+module_exit(uio_pdrv_genirq_exit);
+
+MODULE_AUTHOR("Magnus Damm");
+MODULE_DESCRIPTION("Userspace I/O platform driver with generic IRQ handling");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);

Re: [PATCH][RESEND] uio: uio_pdrv_genirq V3

[Hans J. Koch]

On Tue, Jul 29, 2008 at 09:10:15PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@igel.co.jp>
> 
> This is V3 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.

Hi Magnus,
I already sent this to Greg on July, 15. I guess this will be merged
soon. Greg was probably busy with tons of other stuff during the merge
window.

Thanks,
Hans

> 
> 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@igel.co.jp>
> Signed-off-by: Hans J. Koch <hjk@linutronix.de>
> ---
> 
>  Patch sent to lkml on July 11th, please pick up.
>  Platform data already in place in arch/sh/kernel/cpu/sh4a/...
> 
>  Changes since V2
>   - readded bitops
>   - removed irq handler spinlock to fix potential deadlock
>   - kept irq control spinlock to serialize user space access
> 
>  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 |  188 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 202 insertions(+)
> 
> --- 0001/drivers/uio/Kconfig
> +++ work/drivers/uio/Kconfig	2008-07-29 20:59:36.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"
>  	default n
> --- 0001/drivers/uio/Makefile
> +++ work/drivers/uio/Makefile	2008-07-29 20:59:36.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-29 20:59:36.000000000 +0900
> @@ -0,0 +1,188 @@
> +/*
> + * 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/bitops.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;
> +	unsigned long flags;
> +};
> +
> +static irqreturn_t uio_pdrv_genirq_handler(int irq, struct uio_info *dev_info)
> +{
> +	struct uio_pdrv_genirq_platdata *priv = dev_info->priv;
> +
> +	/* Just disable the interrupt in the interrupt controller, and
> +	 * remember the state so we can allow user space to enable it later.
> +	 */
> +
> +	if (!test_and_set_bit(0, &priv->flags))
> +		disable_irq_nosync(irq);
> +
> +	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.
> +	 *
> +	 * Serialize this operation to support multiple tasks.
> +	 */
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	if (irq_on) {
> +		if (test_and_clear_bit(0, &priv->flags))
> +			enable_irq(dev_info->irq);
> +	} else {
> +		if (!test_and_set_bit(0, &priv->flags))
> +			disable_irq(dev_info->irq);
> +	}
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> +{
> +	struct uio_info *uioinfo = pdev->dev.platform_data;
> +	struct uio_pdrv_genirq_platdata *priv;
> +	struct uio_mem *uiomem;
> +	int ret = -EINVAL;
> +	int i;
> +
> +	if (!uioinfo || !uioinfo->name || !uioinfo->version) {
> +		dev_err(&pdev->dev, "missing platform_data\n");
> +		goto bad0;
> +	}
> +
> +	if (uioinfo->handler || uioinfo->irqcontrol || uioinfo->irq_flags) {
> +		dev_err(&pdev->dev, "interrupt configuration error\n");
> +		goto bad0;
> +	}
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		ret = -ENOMEM;
> +		dev_err(&pdev->dev, "unable to kmalloc\n");
> +		goto bad0;
> +	}
> +
> +	priv->uioinfo = uioinfo;
> +	spin_lock_init(&priv->lock);
> +	priv->flags = 0; /* interrupt is enabled to begin with */
> +
> +	uiomem = &uioinfo->mem[0];
> +
> +	for (i = 0; i < pdev->num_resources; ++i) {
> +		struct resource *r = &pdev->resource[i];
> +
> +		if (r->flags != IORESOURCE_MEM)
> +			continue;
> +
> +		if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
> +			dev_warn(&pdev->dev, "device has more than "
> +					__stringify(MAX_UIO_MAPS)
> +					" I/O memory resources.\n");
> +			break;
> +		}
> +
> +		uiomem->memtype = UIO_MEM_PHYS;
> +		uiomem->addr = r->start;
> +		uiomem->size = r->end - r->start + 1;
> +		++uiomem;
> +	}
> +
> +	while (uiomem < &uioinfo->mem[MAX_UIO_MAPS]) {
> +		uiomem->size = 0;
> +		++uiomem;
> +	}
> +
> +	/* This driver requires no hardware specific kernel code to handle
> +	 * interrupts. Instead, the interrupt handler simply disables the
> +	 * interrupt in the interrupt controller. User space is responsible
> +	 * for performing hardware specific acknowledge and re-enabling of
> +	 * the interrupt in the interrupt controller.
> +	 *
> +	 * Interrupt sharing is not supported.
> +	 */
> +
> +	uioinfo->irq_flags = IRQF_DISABLED;
> +	uioinfo->handler = uio_pdrv_genirq_handler;
> +	uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
> +	uioinfo->priv = priv;
> +
> +	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);
> +	return 0;
> + bad1:
> +	kfree(priv);
> + bad0:
> +	return ret;
> +}
> +
> +static int uio_pdrv_genirq_remove(struct platform_device *pdev)
> +{
> +	struct uio_pdrv_genirq_platdata *priv = platform_get_drvdata(pdev);
> +
> +	uio_unregister_device(priv->uioinfo);
> +	kfree(priv);
> +	return 0;
> +}
> +
> +static struct platform_driver uio_pdrv_genirq = {
> +	.probe = uio_pdrv_genirq_probe,
> +	.remove = uio_pdrv_genirq_remove,
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.owner = THIS_MODULE,
> +	},
> +};
> +
> +static int __init uio_pdrv_genirq_init(void)
> +{
> +	return platform_driver_register(&uio_pdrv_genirq);
> +}
> +
> +static void __exit uio_pdrv_genirq_exit(void)
> +{
> +	platform_driver_unregister(&uio_pdrv_genirq);
> +}
> +
> +module_init(uio_pdrv_genirq_init);
> +module_exit(uio_pdrv_genirq_exit);
> +
> +MODULE_AUTHOR("Magnus Damm");
> +MODULE_DESCRIPTION("Userspace I/O platform driver with generic IRQ handling");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRIVER_NAME);

Re: [PATCH][RESEND] uio: uio_pdrv_genirq V3

[Paul Mundt]

On Tue, Jul 29, 2008 at 02:28:06PM +0200, Hans J. Koch wrote:
> On Tue, Jul 29, 2008 at 09:10:15PM +0900, Magnus Damm wrote:
> > From: Magnus Damm <damm@igel.co.jp>
> > 
> > This is V3 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.
> 
> Hi Magnus,
> I already sent this to Greg on July, 15. I guess this will be merged
> soon. Greg was probably busy with tons of other stuff during the merge
> window.
> 
Note that I've already pushed the platform bits, so this is the only
outstanding part.

Re: [PATCH][RESEND] uio: uio_pdrv_genirq V3

[Greg KH]

On Tue, Jul 29, 2008 at 02:28:06PM +0200, Hans J. Koch wrote:
> On Tue, Jul 29, 2008 at 09:10:15PM +0900, Magnus Damm wrote:
> > From: Magnus Damm <damm@igel.co.jp>
> > 
> > This is V3 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.
> 
> Hi Magnus,
> I already sent this to Greg on July, 15. I guess this will be merged
> soon. Greg was probably busy with tons of other stuff during the merge
> window.

Yeah, it's in my queue, which _sucks_ right now for movement:
	$ mdfrm -c ~/mail/todo
	594 messages in ~/mail/todo

I'm slowly digging out, I have to get through the -stable backlog first:
	$ mdfrm -c ~/mail/todo-stable/
	68 messages in ~/mail/todo-stable/

ah, for more time in a day...

thanks for your patience,

greg k-h

Re: [PATCH][RESEND] uio: uio_pdrv_genirq V3

[Greg KH]

On Tue, Jul 29, 2008 at 09:40:07PM +0900, Paul Mundt wrote:
> On Tue, Jul 29, 2008 at 02:28:06PM +0200, Hans J. Koch wrote:
> > On Tue, Jul 29, 2008 at 09:10:15PM +0900, Magnus Damm wrote:
> > > From: Magnus Damm <damm@igel.co.jp>
> > > 
> > > This is V3 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.
> > 
> > Hi Magnus,
> > I already sent this to Greg on July, 15. I guess this will be merged
> > soon. Greg was probably busy with tons of other stuff during the merge
> > window.
> > 
> Note that I've already pushed the platform bits, so this is the only
> outstanding part.

Now merged into my tree, sorry for the delay.

greg k-h