UIO: Implement a UIO interface for the SMX Cryptengine
PATCH 1/1 UIO: Implement a UIO interface for the SMX Cryptengine
This patch implements a UIO interface for the SMX Cryptengine.
The Cryptengine found on the Nias Digital SMX board is best suited for a UIO interface. It is not wired in to the cryptographic API as the engine handles it's own keys, algorithms, everything. All that we know about is that if there's room in the buffer, you can write data to it and when there's data ready, you read it out again.
There isn't necessarily even any direct correlation between data going in and data coming out again, the engine may consume or generate data all on its own.
This driver is for proprietary hardware but we're always told to submit the drivers anyway; here you are. :-)
---
MAINTAINERS +5 -0
@@ -3554,6 +3554,11 @@ M: [email protected] L: [email protected] S: Maintained +SMX UIO Interface +P: Ben Nizette +M: [email protected] +S: Maintained + SOFTMAC LAYER (IEEE 802.11) P: Daniel Drake M: [email protected]
drivers/uio/Kconfig +8 -0
@@ -26,4 +26,12 @@ config UIO_CIF To compile this driver as a module, choose M here: the module will be called uio_cif. +config UIO_SMX + tristate "SMX cryptengine UIO interface" + depends on UIO + default n + help + Userspace IO interface to the Cryptography engine found on the + Nias Digital SMX boards. If you compile this as a module, it + will be called uio_smx. endmenu
drivers/uio/Makefile +1 -0
@@ -1,2 +1,3 @@ obj-$(CONFIG_UIO) += uio.o obj-$(CONFIG_UIO_CIF) += uio_cif.o +obj-$(CONFIG_UIO_SMX) += uio_smx.o
drivers/uio/uio_smx.c new file +52 -0
52 lines not on this page · raw message
regs->start);
+ if (!info->mem[0].internal_addr)
+ goto out_free;
+
+ info->mem[0].size = regs->end - regs->start;
+ info->mem[0].memtype = UIO_MEM_PHYS;
+
+ info->name = "smx-ce";
+ info->version = "0.03";
+ info->irq = platform_get_irq(dev, 0);
+ info->irq_flags = 0;
+ info->handler = smx_handler;
+
+ platform_set_drvdata(dev, info);
+
+ if (uio_register_device(&dev->dev, info))
+ goto out_unmap;
+
+ return 0;
+out_unmap:
+ iounmap(info->mem[0].internal_addr);
+out_free:
+ kfree(info);
+
+ return -ENODEV;
+}
+
+static int __devexit smx_ce_remove(struct platform_device *dev) +{
+ struct uio_info *info = platform_get_drvdata(dev);
+
+ uio_unregister_device(info);
+ platform_set_drvdata(dev, NULL);
+ iounmap(info->mem[0].internal_addr);
+
+ kfree (info);
+
+ return 0;
+}
+
+static struct platform_driver smx_ce_driver = {
+ .probe = smx_ce_probe,
+ .remove = __devexit_p(smx_ce_remove),
+ .driver = {
+ .name = "smx-ce",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init smx_ce_init_module(void)
+{
+ return platform_driver_register(&smx_ce_driver);
+}
+module_init(smx_ce_init_module);
+
+static void __exit smx_ce_exit_module(void)
+{
+ platform_driver_unregister(&smx_ce_driver);
+}
+module_exit(smx_ce_exit_module);
+
+MODULE_LICENSE("GPL V2");
+MODULE_AUTHOR("Ben Nizette <[email protected]>");
Am Tue, 11 Mar 2008 15:57:10 +1100
schrieb Ben Nizette <[email protected]>:
Hi Ben,
Quoted 8 lines
This patch implements a UIO interface for the SMX Cryptengine.
The Cryptengine found on the Nias Digital SMX board is best suited for a UIO interface. It is not wired in to the cryptographic API as the engine handles it's own keys, algorithms, everything. All that we know about is that if there's room in the buffer, you can write data to it and when there's data ready, you read it out again.
Quoted 6 lines
There isn't necessarily even any direct correlation between data going in and data coming out again, the engine may consume or generate data all on its own.
This driver is for proprietary hardware but we're always told to submit the drivers anyway; here you are. :-)
I've got no problems with that, but I'd like to know if that board can be bought somewhere (the niasdigital.com homepage doesn't mention it) and if and under which license the userspace part of the driver is available. The homepage says you're in favor of the GPL, so if it can be checked out or downloaded somewhere, please mention it, maybe in the Kconfig help text. If the userspace part is proprietary, please mention that, too.
There are some minor problems with your patch, see below.
Note: testing your patches with checkpatch.pl would help...
Quoted 2 lines
Signed-off-by Ben Nizette <[email protected]>
that should be signed-off-by (lower-case)
Quoted 34 lines
---
MAINTAINERS +5
-0@@ -3554,6 +3554,11 @@ M: [email protected] L: [email protected] S: Maintained +SMX UIO Interface +P: Ben Nizette +M: [email protected] +S: Maintained + SOFTMAC LAYER (IEEE 802.11) P: Daniel Drake M: [email protected]drivers/uio/Kconfig +0
-0@@ -26,4 +26,12 @@ config UIO_CIF To compile this driver as a module, choose M here: themodule will be called
uio_cif.drivers/uio/Kconfig +6
-0+config UIO_SMX + tristate "SMX cryptengine UIO interface" + depends on UIO + default n + help + Userspace IO interface to the Cryptography engine found onthe
drivers/uio/Kconfig +1
-0+ Nias Digital SMX boards. If you compile this as a module,it
drivers/uio/Kconfig config UIO_CIF · line 36
Quoted 1 line
drivers/uio/Kconfig +1
-0+ will be called uio_smx.
Please give a hint that a userspace part is needed, and how it can be obtained.
Quoted 40 lines
endmenu
drivers/uio/Makefile +1
-0@@ -1,2 +1,3 @@ obj-$(CONFIG_UIO) += uio.o obj-$(CONFIG_UIO_CIF) += uio_cif.o +obj-$(CONFIG_UIO_SMX) += uio_smx.odrivers/uio/uio_smx.c new file +6
-0@@ -0,0 +1,114 @@ +/* +* UIO SMX Cryptengine driver. +* +* (C) 2008 Nias Digital P/L <[email protected]> +* +* 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.
+ *
+ */drivers/uio/uio_smx.c new file +14
-0+ +#include <linux/device.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/uio_driver.h> + +#include <asm/io.h> + +#define SMX_CSR 0x00000000 +#define SMX_EnD 0x00000001 +#define SMX_RUN 0x00000002 +#define SMX_DRDY 0x00000004 +#define SMX_ERR 0x00000008 +
drivers/uio/uio_smx.c line 34
Quoted 10 lines
drivers/uio/uio_smx.c new file +10
-0+static irqreturn_t smx_handler (int irq,struct uio_info *dev_info) +{ +void __iomem *csr = dev_info->mem[0 ].internal_addr +SMX_CSR ; + +u32 status =ioread32 (csr); + +/* Disable interrupt */ +iowrite8 (status & ~SMX_DRDY , csr); +return IRQ_HANDLED ; +}
You don't support shared interrupts. Is that intentional?
Quoted 18 lines
drivers/uio/uio_smx.c new file +18
-0+ +static int __devinit smx_ce_probe (struct platform_device *dev) +{ +struct uio_info *info; +struct resource *regs; + + info =kzalloc (sizeof (struct uio_info ),GFP_KERNEL ); +if (!info) +return -ENOMEM ; + + regs =platform_get_resource (dev,IORESOURCE_MEM ,0 ); +if (!regs) + goto out_free; + + info->mem[0 ].addr = regs->start; +if (!info->mem[0 ].addr) + goto out_free; + info->mem[0].internal_addr = ioremap(regs->start, regs->end -
Quoted 1 line
regs->start);
Word wrap - please fix your mail client
Quoted 25 lines
drivers/uio/uio_smx.c new file +25
-0+if (!info->mem[0 ].internal_addr) + goto out_free; + + info->mem[0 ].size = regs->end - regs->start; + info->mem[0 ].memtype =UIO_MEM_PHYS ; + + info->name ="smx-ce" ; + info->version ="0.03" ; + info->irq =platform_get_irq (dev,0 ); + info->irq_flags =0 ; + info->handler = smx_handler; + +platform_set_drvdata (dev, info); + +if (uio_register_device (&dev->dev, info)) + goto out_unmap; + +return 0 ; +out_unmap: +iounmap (info->mem[0 ].internal_addr); +out_free: +kfree (info); + +return -ENODEV ; +}
drivers/uio/uio_smx.c
Quoted 10 lines
drivers/uio/uio_smx.c new file +10
-0+ +static int __devexit smx_ce_remove(struct platform_device *dev) +{ +struct uio_info *info =platform_get_drvdata (dev); + +uio_unregister_device (info); +platform_set_drvdata (dev,NULL ); +iounmap (info->mem[0 ].internal_addr); + +kfree (info);
No space before parenthesis
Quoted 16 lines
drivers/uio/uio_smx.c new file +16
-0+ +return 0 ; +} + +static struct platform_driver smx_ce_driver = { + .probe = smx_ce_probe, + .remove =__devexit_p (smx_ce_remove), + .driver = { + .name ="smx-ce" , + .owner =THIS_MODULE , + }, +}; + +static int __init smx_ce_init_module (void ) +{ +return platform_driver_register (&smx_ce_driver);
drivers/uio/uio_smx.c
Quoted 10 lines
drivers/uio/uio_smx.c new file +10
-0+} +module_init (smx_ce_init_module); + +static void __exit smx_ce_exit_module(void) +{ +platform_driver_unregister (&smx_ce_driver); +} +module_exit (smx_ce_exit_module); + +MODULE_LICENSE ("GPL V2" );
this needs to be "GPL v2" (lowercase 'v'), otherwise it won't build the module (at least on 2.6.25-rc5).
Quoted 1 line
+MODULE_AUTHOR("Ben Nizette <[email protected]>");
Quoted 5 lines · Hans-Jürgen Koch
Am Tue, 11 Mar 2008 15:57:10 +1100
schrieb Ben Nizette <[email protected]>:Quoted 2 lines
This driver is for proprietary hardware but we're always told to submit the drivers anyway; here you are. :-)
Quoted 7 lines
I've got no problems with that, but I'd like to know if that board can be bought somewhere (the niasdigital.com homepage doesn't mention it) and if and under which license the userspace part of the driver is available. The homepage says you're in favor of the GPL, so if it can be checked out or downloaded somewhere, please mention it, maybe in the
Kconfighelp text. If the userspace part is proprietary, please mention that, too.
The userspace part will be released under the GPL along with the hardware itself, Q4 '08. Have changed the Kconfig text to reflect this.
Of course this means for the next few months I will be the only direct user of this driver, but I'm also submitting it in the hope that I can convince others that it's a good thing to do. I'm an active member of AVRFreaks, an AVR32 support forum, and I spend far too much of my life trying to convince people that polling the peripheral and mmap'ing /dev/mem is not a good way to go. Fingers crossed that this driver at least serves to help educate until the SMX hardware is released and it can serve it's main purpose :-)
<snip>
Quoted 4 lines
Quoted 4 lines
drivers/uio/uio_smx.c new file +4
-0+static irqreturn_t smx_handler (int irq,struct uio_info *dev_info) +{ +void __iomem *csr = dev_info->mem[0 ].internal_addr +SMX_CSR ; +
Quoted 9 lines
drivers/uio/uio_smx.c line 34
Quoted 6 lines
drivers/uio/uio_smx.c new file +6
-0+u32 status =ioread32 (csr); + +/* Disable interrupt */ +iowrite8 (status & ~SMX_DRDY , csr); +return IRQ_HANDLED ; +}You don't support shared interrupts. Is that intentional?
On the hardware we have there is no possibility that the interrupt is shared. That said, it's little effort to allow the sharing and makes the whole thing more future-proof, changed.
<snip>
Quoted 6 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ +MODULE_LICENSE("GPL V2");this needs to be "GPL v2" (lowercase 'v'), otherwise it won't build the module (at least on 2.6.25-rc5).
Ah yep, thanks.
Thanks for the review, shall send a rejigged patch presently.
--Ben.
Quoted 9 lines · Ben Nizette
drivers/uio/uio_smx.c new file +8
-0+ regs =platform_get_resource (dev,IORESOURCE_MEM ,0 ); +if (!regs) + goto out_free; + + info->mem[0 ].addr = regs->start; +if (!info->mem[0 ].addr) + goto out_free; + info->mem[0].internal_addr = ioremap(regs->start, regs->end -
regs->start);
Review
Quoted 6 lines
(quoted diff) +6
-0+ if (!info->mem[0].internal_addr) + goto out_free; + + info->mem[0].size = regs->end - regs->start; + info->mem[0].memtype = UIO_MEM_PHYS; +
You have an off-by-1 in the resource size. ioremap and struct resource are not equal in their expectations. You probably want to do something like:
info->mem[0].size = regs->end - regs->start + 1; info->mem[0].internal_addr = ioremap(regs->start, info->mem[0].size); ...
and make sure that your resource end doesn't overlap with a start address of an unrelated resource in your definition. /proc/iomem tends to be useful for this.
Quoted 9 lines · Paul Mundt
drivers/uio/uio_smx.c line 16
Quoted 4 lines · Ben Nizette
drivers/uio/uio_smx.c new file +4
-0+ + info->mem[0 ].size = regs->end - regs->start; + info->mem[0 ].memtype =UIO_MEM_PHYS ; +You have an off-by-1 in the resource size. ioremap and struct resource are not equal in their expectations. You probably want to do something like:
Quoted 4 lines
info->mem[0].size = regs->end - regs->start + 1; info->mem[0].internal_addr = ioremap(regs->start, info->mem[0].size); ...
You're right, thanks. I hadn't noticed since we don't actually access all the way to the end of the region in normal use.
Quoted 3 lines
and make sure that your resource end doesn't overlap with a start address of an unrelated resource in your definition.
/proc/iomemtends to be useful for this.
Yup. In our case this is fairly easy as the engine just takes up a whole Chip Select's worth of address space on an AVR32 AT32AP7000.
--Ben.