lkml.org 
[lkml]   [2008]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3] UIO: Implement a UIO interface for the SMX Cryptengine
    On Thu, Mar 13, 2008 at 08:35:40PM +1100, Ben Nizette wrote:
    > diff --git a/drivers/uio/uio_smx.c b/drivers/uio/uio_smx.c
    > new file mode 100644
    > index 0000000..fda8b8b
    > --- /dev/null
    > +++ b/drivers/uio/uio_smx.c
    > @@ -0,0 +1,118 @@
    > +/*
    > + * UIO SMX Cryptengine driver.
    > + *
    > + * (C) 2008 Nias Digital P/L <bn@niasdigital.com>
    > + *
    > + * 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/device.h>
    > +#include <linux/module.h>
    > +#include <linux/platform_device.h>
    > +#include <linux/uio_driver.h>
    > +
    > +#include <asm/io.h>
    > +
    linux/io.h is preferable here.

    > +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].size = regs->end - regs->start + 1;
    > + info->mem[0].internal_addr = ioremap(regs->start, info->mem[0].size);
    > + if (!info->mem[0].internal_addr)
    > + goto out_free;
    > +
    > + info->mem[0].memtype = UIO_MEM_PHYS;
    > +
    > + info->name = "smx-ce";
    > + info->version = "0.03";

    You may as well use DRV_NAME and DRV_VERSION, then you can toss that over
    to MODULE_VERSION() if you're so inclined. It's generally nice to keep
    modinfo happy.

    > + info->irq = platform_get_irq(dev, 0);
    > + info->irq_flags = IRQF_SHARED;
    > + info->handler = smx_handler;
    > +
    platform_get_irq() can fail, which you don't presently handle (though I
    guess uio_register_device() will error out if you hand off -ENXIO as your
    IRQ anyways, so it's not technically an issue). That's a pretty minor
    issue, though, but might be something you wish to fix up if you are
    planning on using this as an example driver for others.

    > + 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;
    > +}
    > +
    Your error paths are also pretty quiet. A dev_err() or so in the few
    failure cases you do have might not be a terrible idea.

    uio_register_device() also has quite a few different error cases, so it's
    at least worth preserving the error value and returning that back,
    rather than always returning -ENODEV.


    \
     
     \ /
      Last update: 2008-03-13 11:09    [W:7.611 / U:1.664 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site