Messages in this thread | | | Date | Mon, 14 Apr 2008 13:20:21 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH] Re: [PATCH 4/4 v2] [RFC] UIO: generic platform driver |
| |
Hello Russell,
> > > What's this drivers/uio/uio_pdrv.ko module, and why doesn't it appear > > > in the LKML archive of this thread? > > Don't know why lkml.org didn't link these. The start of the thread can > > be found at > > > > http://lkml.org/lkml/2008/4/10/110 > > Thanks. Well, tbh, I don't know which way to go on this. Each of the > suggested ways have their downsides. > > However: > > + pdata->clk = clk_get(&pdev->dev, DRIVER_NAME); > > seems wrong - "uio" as a clock name? What do you suggest? "uioclk"?
> + /* XXX: better use dev_dbg, but which device should I use? > + * info->uio_dev->dev isn't accessible here as struct uio_device > + * is opaque. > + */ > > why not store a copy of 'dev' in struct uio_platdata ? I don't like increasing the size of struct uio_platdata only for a debug helper. In most cases pr_debug (or dev_dbg) compiles to nothing. Maybe the easiest way is to remove that debug statement.
> + uiomem = &uioinfo->mem[0]; > + for (i = 0; i < pdev->num_resources; ++i) { > ... > + ++uiomem; > + } > > Who's to say there's pdev->num_resources entries in the 'mem' array? The code you skipped with ... from that loop includes
if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) { dev_warn(&pdev->dev, "..."); break; }
that should take care of that. In v2 of this patch the remaining entries are disabled by .size = 0.
> Shouldn't this loop also be limited to MAX_UIO_MAPS iterations (or > maybe complain if there's more than MAX_UIO_MAPS)? > > +/* XXX: I thought there already exists something like STRINGIFY, but I could not > + * find it ... > + */ > +#ifndef STRINGIFY > +#define STRINGIFY(x) __STRINGIFY_HELPER(x) > +#define __STRINGIFY_HELPER(x) #x > +#endif > > #include <linux/stringify.h> ? If you look at the v2 patch[1] I found that in the mean time. Thanks all the same.
Best regards and thanks for your review, Uwe
[1] http://lkml.org/lkml/2008/4/11/81 or http://thread.gmane.org/gmane.linux.kernel/665257
-- Uwe Kleine-König, Software Engineer Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962 -- 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/
| |