Re: [PATCH] UIO: Implement a UIO interface for the SMX Cryptengine

Ben Nizette
2008-03-12T09:49:03Z

On Tue, 2008-03-11 at 09:15 +0100, Hans-Jürgen Koch wrote:
> Am Tue, 11 Mar 2008 15:57:10 +1100
> schrieb Ben Nizette <bn@niasdigital.com>:
> > 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.

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>
> > +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?
>
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>
> > +
> > +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.