Re: UIO device tree bindings.

From: Pavel Machek
Date: Tue Apr 02 2013 - 09:19:58 EST


Hi!

> > Or maybe we can do some magic with module parameter. That should be
> > enough for expected use.
> >
> I don't think that would make a difference. I mean, just take ns16550 as another
> example. No one has problems declaring some block of hardware addresses to be
> compatible with "ns16550", even though it can be anything including a memory
> block on one of the FPGAs or ASICs we are talking about here, it can be anything
> but a NS16550, and many of the actual "compatible" strings are not defined
> anythere either.
>
> So there is no problem with "ata-generic" and "ns16550", and no one cares if
> "fsl,mpc8349emitx-pata" or "xlnx,xps-uart16550-2.00.b" is defined or not, but
> "generic-uio" together with "ptx,c64fpga001" is unacceptable.
>
> I think it has more to do with the uio driver not being an actual driver, but
> the kernel part of a user-space driver, though that is just a wild guess.

Well, it turned out that adding module parameter was not that much
work... and yes I believe it is a tiny bit cleaner.

Here's updated patch, if you can please test this one.

[You can just pass "uio_of_genirq.of_id=generic-uio" to get previous
behaviour. But don't tell anyone :-)]

Signed-off-by: Pavel Machek <pavel@xxxxxxx>
Pavel

diff --git a/Documentation/devicetree/bindings/uio-generic.txt b/Documentation/devicetree/bindings/uio-generic.txt
new file mode 100644
index 0000000..f3c53fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/uio-generic.txt
@@ -0,0 +1,16 @@
+UIO for custom devices
+
+A device which will be mapped using the UIO subsystem.
+
+Properties:
+ - compatible : should contain the specific model used, followed by
+ "generic-uio".
+ - reg : address range(s) of the device (up to MAX_UIO_MAPS)
+ - interrupts : interrupt of the device
+
+Example:
+ c64fpga at 0 {
+ compatible = "ptx,c64fpga001", "generic-uio";
+ reg = <0x0 0x10000>;
+ interrupts = <0 0 3>;
+ };
diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dts b/arch/arm/boot/dts/socfpga_cyclone5.dts
index 13aaf09..fcc5f75 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5.dts
@@ -76,5 +76,11 @@
sysmgr@ffd08000 {
cpu1-start-addr = <0xffd080c4>;
};
+
+ xps-gpio@81400000 {
+ compatible = "generic-uio";
+ reg = < 0x81400000 0x10000 >;
+ };
};
};
+
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index e92eeaf..65807a9 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -59,6 +59,12 @@ config UIO_DMEM_GENIRQ

If you don't know what to do here, say N.

+config UIO_OF_GENIRQ
+ tristate "Userspace I/O device tree driver with generic IRQ handling"
+ depends on UIO_PDRV_GENIRQ && OF
+ help
+ Device tree wrapper for the above platform driver.
+
config UIO_AEC
tristate "AEC video timestamp device"
depends on PCI
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index b354c53..e1f2c4b 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -3,6 +3,7 @@ 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_DMEM_GENIRQ) += uio_dmem_genirq.o
+obj-$(CONFIG_UIO_OF_GENIRQ) += uio_of_genirq.o
obj-$(CONFIG_UIO_AEC) += uio_aec.o
obj-$(CONFIG_UIO_SERCOS3) += uio_sercos3.o
obj-$(CONFIG_UIO_PCI_GENERIC) += uio_pci_generic.o
diff --git a/drivers/uio/uio_of_genirq.c b/drivers/uio/uio_of_genirq.c
new file mode 100644
index 0000000..6a2b208
--- /dev/null
+++ b/drivers/uio/uio_of_genirq.c
@@ -0,0 +1,98 @@
+/*
+ * OF wrapper to make use of the uio_pdrv_genirq-driver.
+ *
+ * Copyright (C) 2009 Wolfram Sang, Pengutronix
+ * Copyright (C) 2013 Pavel Machek, Denx
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/uio_driver.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of.h>
+#include "uio_pdrv_genirq.h"
+
+#define OF_DRIVER_VERSION "1"
+
+static int uio_of_genirq_probe(struct platform_device *op,
+ const struct of_device_id *match)
+{
+ struct uio_info *uioinfo;
+ struct resource resources[MAX_UIO_MAPS];
+ int i, ret;
+
+ uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL);
+ if (!uioinfo)
+ return -ENOMEM;
+
+ uioinfo->name = op->dev.of_node->name;
+ uioinfo->version = OF_DRIVER_VERSION;
+ uioinfo->irq = irq_of_parse_and_map(op->dev.of_node, 0);
+ if (!uioinfo->irq)
+ uioinfo->irq = UIO_IRQ_NONE;
+
+ for (i = 0; i < MAX_UIO_MAPS; ++i)
+ if (of_address_to_resource(op->dev.of_node, i, &resources[i]))
+ break;
+
+ ret = __uio_pdrv_genirq_probe(&op->dev, uioinfo, &resources, i);
+ if (ret)
+ goto err_cleanup;
+
+ return 0;
+
+err_cleanup:
+ if (uioinfo->irq != UIO_IRQ_NONE)
+ irq_dispose_mapping(uioinfo->irq);
+
+ kfree(uioinfo);
+ return ret;
+}
+
+static int uio_of_genirq_remove(struct platform_device *op)
+{
+ struct uio_pdrv_genirq_platdata *priv = dev_get_drvdata(&op->dev);
+
+ uio_unregister_device(priv->uioinfo);
+
+ if (priv->uioinfo->irq != UIO_IRQ_NONE)
+ irq_dispose_mapping(priv->uioinfo->irq);
+
+ kfree(priv->uioinfo);
+ kfree(priv);
+ return 0;
+}
+
+/* Match table for of_platform binding */
+static struct of_device_id uio_of_genirq_match[] = {
+ { /* This is filled with module_parm */ },
+ { /* Sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, uio_of_genirq_match);
+
+module_param_string(of_id, uio_of_genirq_match[0].compatible, 128, 0);
+MODULE_PARM_DESC(of_id, "Openfirmware id of the device to be handled by uio");
+
+static struct platform_driver uio_of_genirq_driver = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "uio_of_genirq",
+ .of_match_table = uio_of_genirq_match,
+ },
+ .probe = uio_of_genirq_probe,
+ .remove = uio_of_genirq_remove,
+};
+
+module_platform_driver(uio_of_genirq_driver);
+
+MODULE_AUTHOR("Wolfram Sang, Pavel Machek");
+MODULE_DESCRIPTION("Userspace I/O OF driver with generic IRQ handling");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
index c122bca..4a50ead 100644
--- a/drivers/uio/uio_pdrv_genirq.c
+++ b/drivers/uio/uio_pdrv_genirq.c
@@ -9,6 +9,9 @@
* Copyright (C) 2008 by Digi International Inc.
* All rights reserved.
*
+ * Copyright (C) 2009 Wolfram Sang, Pengutronix
+ * Copyright (C) 2013 Pavel Machek, Denx
+ *
* 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.
@@ -27,22 +30,16 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/of_address.h>
+#include "uio_pdrv_genirq.h"

#define DRIVER_NAME "uio_pdrv_genirq"

-struct uio_pdrv_genirq_platdata {
- struct uio_info *uioinfo;
- spinlock_t lock;
- unsigned long flags;
- struct platform_device *pdev;
-};
-
static int uio_pdrv_genirq_open(struct uio_info *info, struct inode *inode)
{
struct uio_pdrv_genirq_platdata *priv = info->priv;

/* Wait until the Runtime PM code has woken up the device */
- pm_runtime_get_sync(&priv->pdev->dev);
+ pm_runtime_get_sync(priv->dev);
return 0;
}

@@ -51,7 +48,7 @@ static int uio_pdrv_genirq_release(struct uio_info *info, struct inode *inode)
struct uio_pdrv_genirq_platdata *priv = info->priv;

/* Tell the Runtime PM code that the device has become idle */
- pm_runtime_put_sync(&priv->pdev->dev);
+ pm_runtime_put_sync(priv->dev);
return 0;
}

@@ -94,76 +91,34 @@ static int uio_pdrv_genirq_irqcontrol(struct uio_info *dev_info, s32 irq_on)
return 0;
}

-static int uio_pdrv_genirq_probe(struct platform_device *pdev)
+int __uio_pdrv_genirq_probe(struct device *dev, struct uio_info *uioinfo,
+ struct resource *resources, unsigned int num_resources)
{
- struct uio_info *uioinfo = pdev->dev.platform_data;
struct uio_pdrv_genirq_platdata *priv;
struct uio_mem *uiomem;
- int ret = -EINVAL;
+ int ret;
int i;

- if (pdev->dev.of_node) {
- int irq;
-
- /* alloc uioinfo for one device */
- uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL);
- if (!uioinfo) {
- ret = -ENOMEM;
- dev_err(&pdev->dev, "unable to kmalloc\n");
- goto bad2;
- }
- uioinfo->name = pdev->dev.of_node->name;
- uioinfo->version = "devicetree";
-
- /* Multiple IRQs are not supported */
- irq = platform_get_irq(pdev, 0);
- if (irq == -ENXIO)
- uioinfo->irq = UIO_IRQ_NONE;
- else
- uioinfo->irq = irq;
- }
-
- if (!uioinfo || !uioinfo->name || !uioinfo->version) {
- dev_err(&pdev->dev, "missing platform_data\n");
- goto bad0;
- }
-
- if (uioinfo->handler || uioinfo->irqcontrol ||
- uioinfo->irq_flags & IRQF_SHARED) {
- 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;
+ dev_err(dev, "unable to kmalloc\n");
+ return -ENOMEM;
}

priv->uioinfo = uioinfo;
spin_lock_init(&priv->lock);
priv->flags = 0; /* interrupt is enabled to begin with */
- priv->pdev = pdev;
+ priv->dev = dev;

- if (!uioinfo->irq) {
- ret = platform_get_irq(pdev, 0);
- if (ret < 0) {
- dev_err(&pdev->dev, "failed to get IRQ\n");
- goto bad0;
- }
- uioinfo->irq = ret;
- }
uiomem = &uioinfo->mem[0];
-
- for (i = 0; i < pdev->num_resources; ++i) {
- struct resource *r = &pdev->resource[i];
+ for (i = 0; i < num_resources; ++i) {
+ struct resource *r = resources + i;

if (r->flags != IORESOURCE_MEM)
continue;

if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
- dev_warn(&pdev->dev, "device has more than "
+ dev_warn(dev, "device has more than "
__stringify(MAX_UIO_MAPS)
" I/O memory resources.\n");
break;
@@ -201,19 +156,71 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
* turned off by default. The Runtime PM bus code should power on the
* hardware and enable clocks at open().
*/
- pm_runtime_enable(&pdev->dev);
+ pm_runtime_enable(dev);

- ret = uio_register_device(&pdev->dev, priv->uioinfo);
+ ret = uio_register_device(dev, priv->uioinfo);
if (ret) {
- dev_err(&pdev->dev, "unable to register uio device\n");
- goto bad1;
+ dev_err(dev, "unable to register uio device\n");
+ kfree(priv);
+ pm_runtime_disable(dev);
+ return ret;
}

- platform_set_drvdata(pdev, priv);
+ dev_set_drvdata(dev, priv);
return 0;
- bad1:
- kfree(priv);
- pm_runtime_disable(&pdev->dev);
+}
+
+EXPORT_SYMBOL_GPL(__uio_pdrv_genirq_probe);
+
+static int uio_pdrv_genirq_probe(struct platform_device *pdev)
+{
+ struct uio_info *uioinfo = NULL;
+ int ret = 0;
+
+ if (pdev->dev.of_node) {
+ int irq;
+
+ /* alloc uioinfo for one device */
+ uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL);
+ if (!uioinfo) {
+ ret = -ENOMEM;
+ dev_err(&pdev->dev, "unable to kmalloc\n");
+ goto bad2;
+ }
+ uioinfo->name = pdev->dev.of_node->name;
+ uioinfo->version = "devicetree";
+
+ /* Multiple IRQs are not supported */
+ irq = platform_get_irq(pdev, 0);
+ if (irq == -ENXIO)
+ uioinfo->irq = UIO_IRQ_NONE;
+ else
+ uioinfo->irq = irq;
+ }
+
+ if (!uioinfo || !uioinfo->name || !uioinfo->version) {
+ dev_err(&pdev->dev, "missing platform_data\n");
+ goto bad0;
+ }
+
+ if (uioinfo->handler || uioinfo->irqcontrol ||
+ uioinfo->irq_flags & IRQF_SHARED) {
+ dev_err(&pdev->dev, "interrupt configuration error\n");
+ goto bad0;
+ }
+
+ if (!uioinfo->irq) {
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to get IRQ\n");
+ goto bad0;
+ }
+ uioinfo->irq = ret;
+ }
+
+ return __uio_pdrv_genirq_probe(&pdev->dev, uioinfo, pdev->resource,
+ pdev->num_resources);
+
bad0:
/* kfree uioinfo for OF */
if (pdev->dev.of_node)
diff --git a/drivers/uio/uio_pdrv_genirq.h b/drivers/uio/uio_pdrv_genirq.h
new file mode 100644
index 0000000..d5a7c60
--- /dev/null
+++ b/drivers/uio/uio_pdrv_genirq.h
@@ -0,0 +1,14 @@
+#ifndef _LINUX_UIO_PDRV_GENIRQ_H
+#define _LINUX_UIO_PDRV_GENIRQ_H
+
+struct uio_pdrv_genirq_platdata {
+ struct uio_info *uioinfo;
+ spinlock_t lock;
+ unsigned long flags;
+ struct device *dev;
+};
+
+extern int __uio_pdrv_genirq_probe(struct device *dev, struct uio_info *uioinfo,
+ struct resource *resources, unsigned int num_resources);
+
+#endif
diff --git a/include/linux/of_platform.h b/include/linux/of_platform.h
index 3863a4d..77a26e4 100644
--- a/include/linux/of_platform.h
+++ b/include/linux/of_platform.h
@@ -60,13 +60,13 @@ struct of_dev_auxdata {
*/
struct of_platform_driver
{
- int (*probe)(struct platform_device* dev,
+ int (*probe)(struct platform_device *dev,
const struct of_device_id *match);
- int (*remove)(struct platform_device* dev);
+ int (*remove)(struct platform_device *dev);

- int (*suspend)(struct platform_device* dev, pm_message_t state);
- int (*resume)(struct platform_device* dev);
- int (*shutdown)(struct platform_device* dev);
+ int (*suspend)(struct platform_device *dev, pm_message_t state);
+ int (*resume)(struct platform_device *dev);
+ int (*shutdown)(struct platform_device *dev);

struct device_driver driver;
};

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/