public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* uio: add the uio_aec driver
@ 2009-01-20 20:47 Brandon Philips
  2009-01-20 21:48 ` Hans J. Koch
  0 siblings, 1 reply; 7+ messages in thread
From: Brandon Philips @ 2009-01-20 20:47 UTC (permalink / raw)
  To: hjk, Greg KH; +Cc: linux-kernel

UIO driver for the Adrienne Electronics Corporation PCI time code
device.

This device differs from other UIO devices since it uses I/O ports instead of
memory mapped I/O. In order to make it possible for UIO to work with this
device a utility, uioport, can be used to read and write the ports.

uioport is designed to be a setuid program and checks the permissions of
the /dev/uio* node and if the user has write permissions it will use
iopl and out*/in* to access the device.

[1] git clone git://ifup.org/philips/uioport.git

Signed-off-by: Brandon Philips <brandon@ifup.org>

---
 drivers/uio/Kconfig        |   18 ++++
 drivers/uio/Makefile       |    1 
 drivers/uio/uio_aec.c      |  177 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/uio_driver.h |    1 
 4 files changed, 197 insertions(+)

Index: linux-2.6/drivers/uio/Makefile
===================================================================
--- linux-2.6.orig/drivers/uio/Makefile
+++ linux-2.6/drivers/uio/Makefile
@@ -3,4 +3,5 @@ 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
+obj-$(CONFIG_UIO_AEC)	+= uio_aec.o
 obj-$(CONFIG_UIO_SERCOS3)	+= uio_sercos3.o
Index: linux-2.6/include/linux/uio_driver.h
===================================================================
--- linux-2.6.orig/include/linux/uio_driver.h
+++ linux-2.6/include/linux/uio_driver.h
@@ -91,5 +91,6 @@ extern void uio_event_notify(struct uio_
 #define UIO_MEM_PHYS	1
 #define UIO_MEM_LOGICAL	2
 #define UIO_MEM_VIRTUAL 3
+#define UIO_MEM_PORT	4
 
 #endif /* _LINUX_UIO_DRIVER_H_ */
Index: linux-2.6/drivers/uio/Kconfig
===================================================================
--- linux-2.6.orig/drivers/uio/Kconfig
+++ linux-2.6/drivers/uio/Kconfig
@@ -58,6 +58,24 @@ config UIO_SMX
 
 	  If you compile this as a module, it will be called uio_smx.
 
+config UIO_AEC
+	tristate "AEC video timestamp device"
+	depends on PCI
+	default n
+	help
+
+	  UIO driver for the Adrienne Electronics Corporation PCI time
+	  code device.
+
+	  This device differs from other UIO devices since it uses I/O
+	  ports instead of memory mapped I/O. In order to make it
+	  possible for UIO to work with this device a utility, uioport,
+	  can be used to read and write the ports:
+
+	    git clone git://ifup.org/philips/uioport.git
+
+	  If you compile this as a module, it will be called uio_aec.
+
 config UIO_SERCOS3
 	tristate "Automata Sercos III PCI card driver"
 	default n
Index: linux-2.6/drivers/uio/uio_aec.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/uio/uio_aec.c
@@ -0,0 +1,177 @@
+/*
+ * uio_aec.c -- simple driver for Adrienne Electronics Corp time code PCI device
+ *
+ * Copyright (C) 2008 Brandon Philips <brandon@ifup.org>
+ *
+ *   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.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License along
+ *   with this program; if not, write to the Free Software Foundation, Inc., 59
+ *   Temple Place, Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/cdev.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <asm/system.h>
+#include <linux/uaccess.h>
+#include <linux/uio_driver.h>
+
+#define PCI_VENDOR_ID_AEC 0xaecb
+#define PCI_DEVICE_ID_AEC_VITCLTC 0x6250
+
+#define INT_ENABLE_ADDR		0xFC
+#define INT_ENABLE		0x10
+#define INT_DISABLE		0x0
+
+#define INT_MASK_ADDR		0x2E
+#define INT_MASK_ALL		0x3F
+
+#define INTA_DRVR_ADDR		0xFE
+#define INTA_ENABLED_FLAG	0x08
+#define INTA_FLAG		0x01
+
+#define MAILBOX			0x0F
+
+static struct pci_device_id ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_AEC, PCI_DEVICE_ID_AEC_VITCLTC), },
+	{ 0, }
+};
+MODULE_DEVICE_TABLE(pci, ids);
+
+static irqreturn_t aectc_irq(int irq, struct uio_info *dev_info)
+{
+	void __iomem *int_flag = dev_info->mem[0].internal_addr
+					+ INTA_DRVR_ADDR;
+	unsigned char status = ioread8(int_flag);
+
+
+	if ((status & INTA_ENABLED_FLAG) && (status & INTA_FLAG)) {
+		/* application writes 0x00 to 0x2F to get next interrupt */
+		status = ioread8(dev_info->mem[0].internal_addr + MAILBOX);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static void print_board_data(struct uio_info *i)
+{
+	printk(KERN_INFO "PCI-TC board vendor: %x%x number: %x%x"
+		" revision: %c%c\n",
+		ioread8(i->mem[0].internal_addr + 0x01),
+		ioread8(i->mem[0].internal_addr + 0x00),
+		ioread8(i->mem[0].internal_addr + 0x03),
+		ioread8(i->mem[0].internal_addr + 0x02),
+		ioread8(i->mem[0].internal_addr + 0x06),
+		ioread8(i->mem[0].internal_addr + 0x07));
+}
+
+static int __devinit probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	struct uio_info *info;
+	int ret;
+
+	info = kzalloc(sizeof(struct uio_info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	if (pci_enable_device(dev))
+		goto out_free;
+
+	if (pci_request_regions(dev, "aectc"))
+		goto out_disable;
+
+	info->name = "aectc";
+	info->mem[0].addr = pci_resource_start(dev, 0);
+	if (!info->mem[0].addr)
+		goto out_release;
+	info->mem[0].internal_addr = pci_iomap(dev, 0, 0);
+	if (!info->mem[0].internal_addr)
+		goto out_release;
+	info->mem[0].size = pci_resource_len(dev, 0);
+	info->mem[0].memtype = UIO_MEM_PORT;
+
+	info->version = "0.0.1";
+	info->irq = dev->irq;
+	info->irq_flags = IRQF_DISABLED | IRQF_SHARED;
+	info->handler = aectc_irq;
+
+	print_board_data(info);
+	ret = uio_register_device(&dev->dev, info);
+	if (ret)
+		goto out_unmap;
+
+	iowrite32(INT_ENABLE, info->mem[0].internal_addr + INT_ENABLE_ADDR);
+	iowrite8(INT_MASK_ALL, info->mem[0].internal_addr + INT_MASK_ADDR);
+	if (ioread8(info->mem[0].internal_addr + INTA_DRVR_ADDR)
+			& INTA_ENABLED_FLAG)
+		printk(KERN_INFO "aectc: interrupts successfully enabled\n");
+
+	pci_set_drvdata(dev, info);
+
+	return 0;
+
+out_unmap:
+	pci_iounmap(dev, info->mem[0].internal_addr);
+out_release:
+	pci_release_regions(dev);
+out_disable:
+	pci_disable_device(dev);
+out_free:
+	kfree(info);
+	return -ENODEV;
+}
+
+static void remove(struct pci_dev *dev)
+{
+	struct uio_info *info = pci_get_drvdata(dev);
+
+	/* disable interrupts */
+	iowrite8(INT_DISABLE, info->mem[0].internal_addr + INT_MASK_ADDR);
+	iowrite32(INT_DISABLE, info->mem[0].internal_addr + INT_ENABLE_ADDR);
+	/* read mailbox to ensure board drops irq */
+	ioread8(info->mem[0].internal_addr + MAILBOX);
+
+	uio_unregister_device(info);
+	pci_release_regions(dev);
+	pci_disable_device(dev);
+	pci_set_drvdata(dev, NULL);
+	iounmap(info->mem[0].internal_addr);
+
+	kfree(info);
+}
+
+static struct pci_driver pci_driver = {
+	.name = "aectc",
+	.id_table = ids,
+	.probe = probe,
+	.remove = remove,
+};
+
+static int __init aectc_init(void)
+{
+	return pci_register_driver(&pci_driver);
+}
+
+static void __exit aectc_exit(void)
+{
+	pci_unregister_driver(&pci_driver);
+}
+
+MODULE_LICENSE("GPL");
+
+module_init(aectc_init);
+module_exit(aectc_exit);

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

* Re: uio: add the uio_aec driver
  2009-01-20 20:47 uio: add the uio_aec driver Brandon Philips
@ 2009-01-20 21:48 ` Hans J. Koch
  2009-01-21 17:01   ` Brandon Philips
  2009-01-27 21:00   ` [PATCHv2] " Brandon Philips
  0 siblings, 2 replies; 7+ messages in thread
From: Hans J. Koch @ 2009-01-20 21:48 UTC (permalink / raw)
  To: Brandon Philips; +Cc: hjk, Greg KH, linux-kernel

On Tue, Jan 20, 2009 at 12:47:29PM -0800, Brandon Philips wrote:
> UIO driver for the Adrienne Electronics Corporation PCI time code
> device.
> 
> This device differs from other UIO devices since it uses I/O ports instead of
> memory mapped I/O. In order to make it possible for UIO to work with this
> device a utility, uioport, can be used to read and write the ports.

We just added a feature to UIO that allows you to pass information about
such I/O ports to userspace. In struct uio_info, there's now also a port[]
array for that purpose. In your case, you will probably want to set the
the porttype member of struct uio_port to UIO_PORT_X86.

The portio feature is available since .29-rc1, please use it.

Otherwise, your driver looks quite good, I'd say.
I've added a few remarks, though.

Thanks,
Hans

> 
> uioport is designed to be a setuid program and checks the permissions of
> the /dev/uio* node and if the user has write permissions it will use
> iopl and out*/in* to access the device.

What happens with PCI on PowerPC ?

> 
> [1] git clone git://ifup.org/philips/uioport.git
> 
> Signed-off-by: Brandon Philips <brandon@ifup.org>
> 
> ---
>  drivers/uio/Kconfig        |   18 ++++
>  drivers/uio/Makefile       |    1 
>  drivers/uio/uio_aec.c      |  177 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/uio_driver.h |    1 
>  4 files changed, 197 insertions(+)
> 
> Index: linux-2.6/drivers/uio/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/uio/Makefile
> +++ linux-2.6/drivers/uio/Makefile
> @@ -3,4 +3,5 @@ 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
> +obj-$(CONFIG_UIO_AEC)	+= uio_aec.o
>  obj-$(CONFIG_UIO_SERCOS3)	+= uio_sercos3.o
> Index: linux-2.6/include/linux/uio_driver.h
> ===================================================================
> --- linux-2.6.orig/include/linux/uio_driver.h
> +++ linux-2.6/include/linux/uio_driver.h
> @@ -91,5 +91,6 @@ extern void uio_event_notify(struct uio_
>  #define UIO_MEM_PHYS	1
>  #define UIO_MEM_LOGICAL	2
>  #define UIO_MEM_VIRTUAL 3
> +#define UIO_MEM_PORT	4

As I explained above, this is not needed anymore. The reason we don't want
this: It breaks generic userspace tools that rely on the fact that any
memory found underneath the maps/ directory in sysfs can be mapped.

In case of ports we don't have anything to be mapped, we simply want to
pass information to userspace. That justifies having a different sysfs
directory for it.

>  
>  #endif /* _LINUX_UIO_DRIVER_H_ */
> Index: linux-2.6/drivers/uio/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/uio/Kconfig
> +++ linux-2.6/drivers/uio/Kconfig
> @@ -58,6 +58,24 @@ config UIO_SMX
>  
>  	  If you compile this as a module, it will be called uio_smx.
>  
> +config UIO_AEC
> +	tristate "AEC video timestamp device"
> +	depends on PCI
> +	default n
> +	help
> +
> +	  UIO driver for the Adrienne Electronics Corporation PCI time
> +	  code device.
> +
> +	  This device differs from other UIO devices since it uses I/O
> +	  ports instead of memory mapped I/O. In order to make it
> +	  possible for UIO to work with this device a utility, uioport,
> +	  can be used to read and write the ports:
> +
> +	    git clone git://ifup.org/philips/uioport.git
> +
> +	  If you compile this as a module, it will be called uio_aec.
> +
>  config UIO_SERCOS3
>  	tristate "Automata Sercos III PCI card driver"
>  	default n
> Index: linux-2.6/drivers/uio/uio_aec.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/uio/uio_aec.c
> @@ -0,0 +1,177 @@
> +/*
> + * uio_aec.c -- simple driver for Adrienne Electronics Corp time code PCI device
> + *
> + * Copyright (C) 2008 Brandon Philips <brandon@ifup.org>
> + *
> + *   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.
> + *
> + *   This program is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *   GNU General Public License for more details.
> + *
> + *   You should have received a copy of the GNU General Public License along
> + *   with this program; if not, write to the Free Software Foundation, Inc., 59
> + *   Temple Place, Suite 330, Boston, MA 02111-1307, USA.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/cdev.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <asm/system.h>

please put this <asm/...> include under the <linux/...> includes, separated by
one blank line. Makes it easier to find. BTW, do you actually need this one?

> +#include <linux/uaccess.h>
> +#include <linux/uio_driver.h>
> +
> +#define PCI_VENDOR_ID_AEC 0xaecb
> +#define PCI_DEVICE_ID_AEC_VITCLTC 0x6250
> +
> +#define INT_ENABLE_ADDR		0xFC
> +#define INT_ENABLE		0x10
> +#define INT_DISABLE		0x0
> +
> +#define INT_MASK_ADDR		0x2E
> +#define INT_MASK_ALL		0x3F
> +
> +#define INTA_DRVR_ADDR		0xFE
> +#define INTA_ENABLED_FLAG	0x08
> +#define INTA_FLAG		0x01
> +
> +#define MAILBOX			0x0F
> +
> +static struct pci_device_id ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AEC, PCI_DEVICE_ID_AEC_VITCLTC), },
> +	{ 0, }
> +};
> +MODULE_DEVICE_TABLE(pci, ids);
> +
> +static irqreturn_t aectc_irq(int irq, struct uio_info *dev_info)
> +{
> +	void __iomem *int_flag = dev_info->mem[0].internal_addr
> +					+ INTA_DRVR_ADDR;
> +	unsigned char status = ioread8(int_flag);
> +
> +
> +	if ((status & INTA_ENABLED_FLAG) && (status & INTA_FLAG)) {
> +		/* application writes 0x00 to 0x2F to get next interrupt */
> +		status = ioread8(dev_info->mem[0].internal_addr + MAILBOX);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static void print_board_data(struct uio_info *i)
> +{
> +	printk(KERN_INFO "PCI-TC board vendor: %x%x number: %x%x"
> +		" revision: %c%c\n",
> +		ioread8(i->mem[0].internal_addr + 0x01),
> +		ioread8(i->mem[0].internal_addr + 0x00),
> +		ioread8(i->mem[0].internal_addr + 0x03),
> +		ioread8(i->mem[0].internal_addr + 0x02),
> +		ioread8(i->mem[0].internal_addr + 0x06),
> +		ioread8(i->mem[0].internal_addr + 0x07));
> +}
> +
> +static int __devinit probe(struct pci_dev *dev, const struct pci_device_id *id)
> +{
> +	struct uio_info *info;
> +	int ret;
> +
> +	info = kzalloc(sizeof(struct uio_info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	if (pci_enable_device(dev))
> +		goto out_free;
> +
> +	if (pci_request_regions(dev, "aectc"))
> +		goto out_disable;
> +
> +	info->name = "aectc";
> +	info->mem[0].addr = pci_resource_start(dev, 0);
> +	if (!info->mem[0].addr)
> +		goto out_release;
> +	info->mem[0].internal_addr = pci_iomap(dev, 0, 0);
> +	if (!info->mem[0].internal_addr)
> +		goto out_release;
> +	info->mem[0].size = pci_resource_len(dev, 0);
> +	info->mem[0].memtype = UIO_MEM_PORT;

port[] instead of mem[].

> +
> +	info->version = "0.0.1";
> +	info->irq = dev->irq;
> +	info->irq_flags = IRQF_DISABLED | IRQF_SHARED;
> +	info->handler = aectc_irq;
> +
> +	print_board_data(info);
> +	ret = uio_register_device(&dev->dev, info);
> +	if (ret)
> +		goto out_unmap;
> +
> +	iowrite32(INT_ENABLE, info->mem[0].internal_addr + INT_ENABLE_ADDR);
> +	iowrite8(INT_MASK_ALL, info->mem[0].internal_addr + INT_MASK_ADDR);
> +	if (ioread8(info->mem[0].internal_addr + INTA_DRVR_ADDR)
> +			& INTA_ENABLED_FLAG)
> +		printk(KERN_INFO "aectc: interrupts successfully enabled\n");

I'd find it better if you printed a message in case of failure...
And please use dev_err() etc. instead of printk().

BTW, if this test fails, are you sure you can continue and let probe()
succeed?

> +
> +	pci_set_drvdata(dev, info);
> +
> +	return 0;
> +
> +out_unmap:
> +	pci_iounmap(dev, info->mem[0].internal_addr);
> +out_release:
> +	pci_release_regions(dev);
> +out_disable:
> +	pci_disable_device(dev);
> +out_free:
> +	kfree(info);
> +	return -ENODEV;
> +}
> +
> +static void remove(struct pci_dev *dev)
> +{
> +	struct uio_info *info = pci_get_drvdata(dev);
> +
> +	/* disable interrupts */
> +	iowrite8(INT_DISABLE, info->mem[0].internal_addr + INT_MASK_ADDR);
> +	iowrite32(INT_DISABLE, info->mem[0].internal_addr + INT_ENABLE_ADDR);
> +	/* read mailbox to ensure board drops irq */
> +	ioread8(info->mem[0].internal_addr + MAILBOX);
> +
> +	uio_unregister_device(info);
> +	pci_release_regions(dev);
> +	pci_disable_device(dev);
> +	pci_set_drvdata(dev, NULL);
> +	iounmap(info->mem[0].internal_addr);
> +
> +	kfree(info);
> +}
> +
> +static struct pci_driver pci_driver = {
> +	.name = "aectc",
> +	.id_table = ids,
> +	.probe = probe,
> +	.remove = remove,
> +};
> +
> +static int __init aectc_init(void)
> +{
> +	return pci_register_driver(&pci_driver);
> +}
> +
> +static void __exit aectc_exit(void)
> +{
> +	pci_unregister_driver(&pci_driver);
> +}
> +
> +MODULE_LICENSE("GPL");
> +
> +module_init(aectc_init);
> +module_exit(aectc_exit);

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

* Re: uio: add the uio_aec driver
  2009-01-20 21:48 ` Hans J. Koch
@ 2009-01-21 17:01   ` Brandon Philips
  2009-01-21 20:06     ` Hans J. Koch
  2009-01-27 21:00   ` [PATCHv2] " Brandon Philips
  1 sibling, 1 reply; 7+ messages in thread
From: Brandon Philips @ 2009-01-21 17:01 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: Greg KH, linux-kernel

On 22:48 Tue 20 Jan 2009, Hans J. Koch wrote:
> On Tue, Jan 20, 2009 at 12:47:29PM -0800, Brandon Philips wrote:
> > UIO driver for the Adrienne Electronics Corporation PCI time code
> > device.
> > 
> > This device differs from other UIO devices since it uses I/O ports instead of
> > memory mapped I/O. In order to make it possible for UIO to work with this
> > device a utility, uioport, can be used to read and write the ports.
> 
> We just added a feature to UIO that allows you to pass information about
> such I/O ports to userspace. In struct uio_info, there's now also a port[]
> array for that purpose. In your case, you will probably want to set the
> the porttype member of struct uio_port to UIO_PORT_X86.
> 
> The portio feature is available since .29-rc1, please use it.

Sounds good. I will rework it along with your suggestions below.

> > uioport is designed to be a setuid program and checks the permissions of
> > the /dev/uio* node and if the user has write permissions it will use
> > iopl and out*/in* to access the device.
> 
> What happens with PCI on PowerPC ?

I don't know. I could use /dev/port but my concern is that the device
spec sheet specifies that some writes should be done with outb and
others with outl. /dev/port only uses outb. Do you know if outl is
different than four outb's to a device?

Cheers,

	Brandon

> > @@ -91,5 +91,6 @@ extern void uio_event_notify(struct uio_
> >  #define UIO_MEM_PHYS	1
> >  #define UIO_MEM_LOGICAL	2
> >  #define UIO_MEM_VIRTUAL 3
> > +#define UIO_MEM_PORT	4
> 
> As I explained above, this is not needed anymore. The reason we don't want
> this: It breaks generic userspace tools that rely on the fact that any
> memory found underneath the maps/ directory in sysfs can be mapped.
> 
> In case of ports we don't have anything to be mapped, we simply want to
> pass information to userspace. That justifies having a different sysfs
> directory for it.

ack.

> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/cdev.h>
> > +#include <linux/fs.h>
> > +#include <linux/io.h>
> > +#include <asm/system.h>
> 
> please put this <asm/...> include under the <linux/...> includes, separated by
> one blank line. Makes it easier to find. BTW, do you actually need this one?

ack.

> > +	iowrite32(INT_ENABLE, info->mem[0].internal_addr + INT_ENABLE_ADDR);
> > +	iowrite8(INT_MASK_ALL, info->mem[0].internal_addr + INT_MASK_ADDR);
> > +	if (ioread8(info->mem[0].internal_addr + INTA_DRVR_ADDR)
> > +			& INTA_ENABLED_FLAG)
> > +		printk(KERN_INFO "aectc: interrupts successfully enabled\n");
> 
> I'd find it better if you printed a message in case of failure...
> And please use dev_err() etc. instead of printk().

ack.

> BTW, if this test fails, are you sure you can continue and let probe()
> succeed?

Yes, if it fails the device still has a "polling" mode.

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

* Re: uio: add the uio_aec driver
  2009-01-21 17:01   ` Brandon Philips
@ 2009-01-21 20:06     ` Hans J. Koch
  0 siblings, 0 replies; 7+ messages in thread
From: Hans J. Koch @ 2009-01-21 20:06 UTC (permalink / raw)
  To: Brandon Philips; +Cc: Hans J. Koch, Greg KH, linux-kernel

On Wed, Jan 21, 2009 at 09:01:16AM -0800, Brandon Philips wrote:
> > The portio feature is available since .29-rc1, please use it.
> 
> Sounds good. I will rework it along with your suggestions below.

Fine. I had a look at their homepage, seems to be interesting hardware.

> 
> > > uioport is designed to be a setuid program and checks the permissions of
> > > the /dev/uio* node and if the user has write permissions it will use
> > > iopl and out*/in* to access the device.
> > 
> > What happens with PCI on PowerPC ?
> 
> I don't know. I could use /dev/port but my concern is that the device
> spec sheet specifies that some writes should be done with outb and
> others with outl. /dev/port only uses outb. Do you know if outl is
> different than four outb's to a device?

It is very probably different (4 WR pulses instead of one), the question
is if that causes different hardware behaviour. If there's a spec for it,
then you should definitly obey it.

What I had in mind was that those ioport regions cannot be mmapped on x86,
but it's possible on ppc, AFAIK.
Furthermore, you need to think about correct endianness handling.

Thanks,
Hans

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

* [PATCHv2] uio: add the uio_aec driver
  2009-01-20 21:48 ` Hans J. Koch
  2009-01-21 17:01   ` Brandon Philips
@ 2009-01-27 21:00   ` Brandon Philips
  2009-01-29 23:26     ` Hans J. Koch
  1 sibling, 1 reply; 7+ messages in thread
From: Brandon Philips @ 2009-01-27 21:00 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: Greg KH, linux-kernel

UIO driver for the Adrienne Electronics Corporation PCI time code
device.

This device differs from other UIO devices since it uses I/O ports instead of
memory mapped I/O. In order to make it possible for UIO to work with this
device a utility, uioport, can be used to read and write the ports.

uioport is designed to be a setuid program and checks the permissions of
the /dev/uio* node and if the user has write permissions it will use
iopl and out*/in* to access the device.

[1] git clone git://ifup.org/philips/uioport.git

Signed-off-by: Brandon Philips <brandon@ifup.org>

---
 drivers/uio/Kconfig   |   18 +++++
 drivers/uio/Makefile  |    1 
 drivers/uio/uio_aec.c |  175 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 194 insertions(+)

Index: linux-2.6/drivers/uio/Makefile
===================================================================
--- linux-2.6.orig/drivers/uio/Makefile
+++ linux-2.6/drivers/uio/Makefile
@@ -3,4 +3,5 @@ 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
+obj-$(CONFIG_UIO_AEC)	+= uio_aec.o
 obj-$(CONFIG_UIO_SERCOS3)	+= uio_sercos3.o
Index: linux-2.6/drivers/uio/Kconfig
===================================================================
--- linux-2.6.orig/drivers/uio/Kconfig
+++ linux-2.6/drivers/uio/Kconfig
@@ -58,6 +58,24 @@ config UIO_SMX
 
 	  If you compile this as a module, it will be called uio_smx.
 
+config UIO_AEC
+	tristate "AEC video timestamp device"
+	depends on PCI
+	default n
+	help
+
+	  UIO driver for the Adrienne Electronics Corporation PCI time
+	  code device.
+
+	  This device differs from other UIO devices since it uses I/O
+	  ports instead of memory mapped I/O. In order to make it
+	  possible for UIO to work with this device a utility, uioport,
+	  can be used to read and write the ports:
+
+	    git clone git://ifup.org/philips/uioport.git
+
+	  If you compile this as a module, it will be called uio_aec.
+
 config UIO_SERCOS3
 	tristate "Automata Sercos III PCI card driver"
 	default n
Index: linux-2.6/drivers/uio/uio_aec.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/uio/uio_aec.c
@@ -0,0 +1,175 @@
+/*
+ * uio_aec.c -- simple driver for Adrienne Electronics Corp time code PCI device
+ *
+ * Copyright (C) 2008 Brandon Philips <brandon@ifup.org>
+ *
+ *   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.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License along
+ *   with this program; if not, write to the Free Software Foundation, Inc., 59
+ *   Temple Place, Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/cdev.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+#include <linux/uio_driver.h>
+
+#define PCI_VENDOR_ID_AEC 0xaecb
+#define PCI_DEVICE_ID_AEC_VITCLTC 0x6250
+
+#define INT_ENABLE_ADDR		0xFC
+#define INT_ENABLE		0x10
+#define INT_DISABLE		0x0
+
+#define INT_MASK_ADDR		0x2E
+#define INT_MASK_ALL		0x3F
+
+#define INTA_DRVR_ADDR		0xFE
+#define INTA_ENABLED_FLAG	0x08
+#define INTA_FLAG		0x01
+
+#define MAILBOX			0x0F
+
+static struct pci_device_id ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_AEC, PCI_DEVICE_ID_AEC_VITCLTC), },
+	{ 0, }
+};
+MODULE_DEVICE_TABLE(pci, ids);
+
+static irqreturn_t aectc_irq(int irq, struct uio_info *dev_info)
+{
+	void __iomem *int_flag = dev_info->priv + INTA_DRVR_ADDR;
+	unsigned char status = ioread8(int_flag);
+
+
+	if ((status & INTA_ENABLED_FLAG) && (status & INTA_FLAG)) {
+		/* application writes 0x00 to 0x2F to get next interrupt */
+		status = ioread8(dev_info->priv + MAILBOX);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static void print_board_data(struct pci_dev *pdev, struct uio_info *i)
+{
+	dev_info(&pdev->dev, "PCI-TC board vendor: %x%x number: %x%x"
+		" revision: %c%c\n",
+		ioread8(i->priv + 0x01),
+		ioread8(i->priv + 0x00),
+		ioread8(i->priv + 0x03),
+		ioread8(i->priv + 0x02),
+		ioread8(i->priv + 0x06),
+		ioread8(i->priv + 0x07));
+}
+
+static int __devinit probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct uio_info *info;
+	int ret;
+
+	info = kzalloc(sizeof(struct uio_info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	if (pci_enable_device(pdev))
+		goto out_free;
+
+	if (pci_request_regions(pdev, "aectc"))
+		goto out_disable;
+
+	info->name = "aectc";
+	info->port[0].start = pci_resource_start(pdev, 0);
+	if (!info->port[0].start)
+		goto out_release;
+	info->priv = pci_iomap(pdev, 0, 0);
+	if (!info->priv)
+		goto out_release;
+	info->port[0].size = pci_resource_len(pdev, 0);
+	info->port[0].porttype = UIO_PORT_GPIO;
+
+	info->version = "0.0.1";
+	info->irq = pdev->irq;
+	info->irq_flags = IRQF_SHARED;
+	info->handler = aectc_irq;
+
+	print_board_data(pdev, info);
+	ret = uio_register_device(&pdev->dev, info);
+	if (ret)
+		goto out_unmap;
+
+	iowrite32(INT_ENABLE, info->priv + INT_ENABLE_ADDR);
+	iowrite8(INT_MASK_ALL, info->priv + INT_MASK_ADDR);
+	if (!(ioread8(info->priv + INTA_DRVR_ADDR)
+			& INTA_ENABLED_FLAG))
+		dev_err(&pdev->dev, "aectc: interrupts not enabled\n");
+
+	pci_set_drvdata(pdev, info);
+
+	return 0;
+
+out_unmap:
+	pci_iounmap(pdev, info->priv);
+out_release:
+	pci_release_regions(pdev);
+out_disable:
+	pci_disable_device(pdev);
+out_free:
+	kfree(info);
+	return -ENODEV;
+}
+
+static void remove(struct pci_dev *pdev)
+{
+	struct uio_info *info = pci_get_drvdata(pdev);
+
+	/* disable interrupts */
+	iowrite8(INT_DISABLE, info->priv + INT_MASK_ADDR);
+	iowrite32(INT_DISABLE, info->priv + INT_ENABLE_ADDR);
+	/* read mailbox to ensure board drops irq */
+	ioread8(info->priv + MAILBOX);
+
+	uio_unregister_device(info);
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	pci_set_drvdata(pdev, NULL);
+	iounmap(info->priv);
+
+	kfree(info);
+}
+
+static struct pci_driver pci_driver = {
+	.name = "aectc",
+	.id_table = ids,
+	.probe = probe,
+	.remove = remove,
+};
+
+static int __init aectc_init(void)
+{
+	return pci_register_driver(&pci_driver);
+}
+
+static void __exit aectc_exit(void)
+{
+	pci_unregister_driver(&pci_driver);
+}
+
+MODULE_LICENSE("GPL");
+
+module_init(aectc_init);
+module_exit(aectc_exit);

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

* Re: [PATCHv2] uio: add the uio_aec driver
  2009-01-27 21:00   ` [PATCHv2] " Brandon Philips
@ 2009-01-29 23:26     ` Hans J. Koch
  2009-02-04 23:07       ` Greg KH
  0 siblings, 1 reply; 7+ messages in thread
From: Hans J. Koch @ 2009-01-29 23:26 UTC (permalink / raw)
  To: Brandon Philips; +Cc: Hans J. Koch, Greg KH, linux-kernel

On Tue, Jan 27, 2009 at 01:00:04PM -0800, Brandon Philips wrote:
> UIO driver for the Adrienne Electronics Corporation PCI time code
> device.

Sorry for not reviewing this earlier, I was quite busy this week and didn't
work at my home office.
I know I'm a bit late, since Greg already added it to his tree, but I reviewed
it anyway. Nice piece of work. I've got no objections either.

> 
> This device differs from other UIO devices since it uses I/O ports instead of
> memory mapped I/O. In order to make it possible for UIO to work with this
> device a utility, uioport, can be used to read and write the ports.
> 
> uioport is designed to be a setuid program and checks the permissions of
> the /dev/uio* node and if the user has write permissions it will use
> iopl and out*/in* to access the device.
> 
> [1] git clone git://ifup.org/philips/uioport.git
> 
> Signed-off-by: Brandon Philips <brandon@ifup.org>

Signed-off-by: Hans J. Koch <hjk@linutronix.de>

> 
> ---
>  drivers/uio/Kconfig   |   18 +++++
>  drivers/uio/Makefile  |    1 
>  drivers/uio/uio_aec.c |  175 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 194 insertions(+)
> 
> Index: linux-2.6/drivers/uio/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/uio/Makefile
> +++ linux-2.6/drivers/uio/Makefile
> @@ -3,4 +3,5 @@ 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
> +obj-$(CONFIG_UIO_AEC)	+= uio_aec.o
>  obj-$(CONFIG_UIO_SERCOS3)	+= uio_sercos3.o
> Index: linux-2.6/drivers/uio/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/uio/Kconfig
> +++ linux-2.6/drivers/uio/Kconfig
> @@ -58,6 +58,24 @@ config UIO_SMX
>  
>  	  If you compile this as a module, it will be called uio_smx.
>  
> +config UIO_AEC
> +	tristate "AEC video timestamp device"
> +	depends on PCI
> +	default n
> +	help
> +
> +	  UIO driver for the Adrienne Electronics Corporation PCI time
> +	  code device.
> +
> +	  This device differs from other UIO devices since it uses I/O
> +	  ports instead of memory mapped I/O. In order to make it
> +	  possible for UIO to work with this device a utility, uioport,
> +	  can be used to read and write the ports:
> +
> +	    git clone git://ifup.org/philips/uioport.git
> +
> +	  If you compile this as a module, it will be called uio_aec.
> +
>  config UIO_SERCOS3
>  	tristate "Automata Sercos III PCI card driver"
>  	default n
> Index: linux-2.6/drivers/uio/uio_aec.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/uio/uio_aec.c
> @@ -0,0 +1,175 @@
> +/*
> + * uio_aec.c -- simple driver for Adrienne Electronics Corp time code PCI device
> + *
> + * Copyright (C) 2008 Brandon Philips <brandon@ifup.org>
> + *
> + *   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.
> + *
> + *   This program is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *   GNU General Public License for more details.
> + *
> + *   You should have received a copy of the GNU General Public License along
> + *   with this program; if not, write to the Free Software Foundation, Inc., 59
> + *   Temple Place, Suite 330, Boston, MA 02111-1307, USA.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/cdev.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +#include <linux/uio_driver.h>
> +
> +#define PCI_VENDOR_ID_AEC 0xaecb
> +#define PCI_DEVICE_ID_AEC_VITCLTC 0x6250
> +
> +#define INT_ENABLE_ADDR		0xFC
> +#define INT_ENABLE		0x10
> +#define INT_DISABLE		0x0
> +
> +#define INT_MASK_ADDR		0x2E
> +#define INT_MASK_ALL		0x3F
> +
> +#define INTA_DRVR_ADDR		0xFE
> +#define INTA_ENABLED_FLAG	0x08
> +#define INTA_FLAG		0x01
> +
> +#define MAILBOX			0x0F
> +
> +static struct pci_device_id ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AEC, PCI_DEVICE_ID_AEC_VITCLTC), },
> +	{ 0, }
> +};
> +MODULE_DEVICE_TABLE(pci, ids);
> +
> +static irqreturn_t aectc_irq(int irq, struct uio_info *dev_info)
> +{
> +	void __iomem *int_flag = dev_info->priv + INTA_DRVR_ADDR;
> +	unsigned char status = ioread8(int_flag);
> +
> +
> +	if ((status & INTA_ENABLED_FLAG) && (status & INTA_FLAG)) {
> +		/* application writes 0x00 to 0x2F to get next interrupt */
> +		status = ioread8(dev_info->priv + MAILBOX);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static void print_board_data(struct pci_dev *pdev, struct uio_info *i)
> +{
> +	dev_info(&pdev->dev, "PCI-TC board vendor: %x%x number: %x%x"
> +		" revision: %c%c\n",
> +		ioread8(i->priv + 0x01),
> +		ioread8(i->priv + 0x00),
> +		ioread8(i->priv + 0x03),
> +		ioread8(i->priv + 0x02),
> +		ioread8(i->priv + 0x06),
> +		ioread8(i->priv + 0x07));
> +}
> +
> +static int __devinit probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct uio_info *info;
> +	int ret;
> +
> +	info = kzalloc(sizeof(struct uio_info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	if (pci_enable_device(pdev))
> +		goto out_free;
> +
> +	if (pci_request_regions(pdev, "aectc"))
> +		goto out_disable;
> +
> +	info->name = "aectc";
> +	info->port[0].start = pci_resource_start(pdev, 0);
> +	if (!info->port[0].start)
> +		goto out_release;
> +	info->priv = pci_iomap(pdev, 0, 0);
> +	if (!info->priv)
> +		goto out_release;
> +	info->port[0].size = pci_resource_len(pdev, 0);
> +	info->port[0].porttype = UIO_PORT_GPIO;
> +
> +	info->version = "0.0.1";
> +	info->irq = pdev->irq;
> +	info->irq_flags = IRQF_SHARED;
> +	info->handler = aectc_irq;
> +
> +	print_board_data(pdev, info);
> +	ret = uio_register_device(&pdev->dev, info);
> +	if (ret)
> +		goto out_unmap;
> +
> +	iowrite32(INT_ENABLE, info->priv + INT_ENABLE_ADDR);
> +	iowrite8(INT_MASK_ALL, info->priv + INT_MASK_ADDR);
> +	if (!(ioread8(info->priv + INTA_DRVR_ADDR)
> +			& INTA_ENABLED_FLAG))
> +		dev_err(&pdev->dev, "aectc: interrupts not enabled\n");
> +
> +	pci_set_drvdata(pdev, info);
> +
> +	return 0;
> +
> +out_unmap:
> +	pci_iounmap(pdev, info->priv);
> +out_release:
> +	pci_release_regions(pdev);
> +out_disable:
> +	pci_disable_device(pdev);
> +out_free:
> +	kfree(info);
> +	return -ENODEV;
> +}
> +
> +static void remove(struct pci_dev *pdev)
> +{
> +	struct uio_info *info = pci_get_drvdata(pdev);
> +
> +	/* disable interrupts */
> +	iowrite8(INT_DISABLE, info->priv + INT_MASK_ADDR);
> +	iowrite32(INT_DISABLE, info->priv + INT_ENABLE_ADDR);
> +	/* read mailbox to ensure board drops irq */
> +	ioread8(info->priv + MAILBOX);
> +
> +	uio_unregister_device(info);
> +	pci_release_regions(pdev);
> +	pci_disable_device(pdev);
> +	pci_set_drvdata(pdev, NULL);
> +	iounmap(info->priv);
> +
> +	kfree(info);
> +}
> +
> +static struct pci_driver pci_driver = {
> +	.name = "aectc",
> +	.id_table = ids,
> +	.probe = probe,
> +	.remove = remove,
> +};
> +
> +static int __init aectc_init(void)
> +{
> +	return pci_register_driver(&pci_driver);
> +}
> +
> +static void __exit aectc_exit(void)
> +{
> +	pci_unregister_driver(&pci_driver);
> +}
> +
> +MODULE_LICENSE("GPL");
> +
> +module_init(aectc_init);
> +module_exit(aectc_exit);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [PATCHv2] uio: add the uio_aec driver
  2009-01-29 23:26     ` Hans J. Koch
@ 2009-02-04 23:07       ` Greg KH
  0 siblings, 0 replies; 7+ messages in thread
From: Greg KH @ 2009-02-04 23:07 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: Brandon Philips, Greg KH, linux-kernel

On Fri, Jan 30, 2009 at 12:26:17AM +0100, Hans J. Koch wrote:
> On Tue, Jan 27, 2009 at 01:00:04PM -0800, Brandon Philips wrote:
> > UIO driver for the Adrienne Electronics Corporation PCI time code
> > device.
> 
> Sorry for not reviewing this earlier, I was quite busy this week and didn't
> work at my home office.
> I know I'm a bit late, since Greg already added it to his tree, but I reviewed
> it anyway. Nice piece of work. I've got no objections either.
> 
> > 
> > This device differs from other UIO devices since it uses I/O ports instead of
> > memory mapped I/O. In order to make it possible for UIO to work with this
> > device a utility, uioport, can be used to read and write the ports.
> > 
> > uioport is designed to be a setuid program and checks the permissions of
> > the /dev/uio* node and if the user has write permissions it will use
> > iopl and out*/in* to access the device.
> > 
> > [1] git clone git://ifup.org/philips/uioport.git
> > 
> > Signed-off-by: Brandon Philips <brandon@ifup.org>
> 
> Signed-off-by: Hans J. Koch <hjk@linutronix.de>

Thanks for the review, I've added your signed-off-by to the patch in my
queue.

greg k-h

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

end of thread, other threads:[~2009-02-04 23:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-20 20:47 uio: add the uio_aec driver Brandon Philips
2009-01-20 21:48 ` Hans J. Koch
2009-01-21 17:01   ` Brandon Philips
2009-01-21 20:06     ` Hans J. Koch
2009-01-27 21:00   ` [PATCHv2] " Brandon Philips
2009-01-29 23:26     ` Hans J. Koch
2009-02-04 23:07       ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox