[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20070720164823.95608e43.akpm@linux-foundation.org>
Date: Fri, 20 Jul 2007 16:48:23 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: olof@...om.net (Olof Johansson)
Cc: Christoph Hellwig <hch@...radead.org>,
linux-kernel@...r.kernel.org, linux-pcmcia@...ts.infradead.org,
linuxppc-dev@...abs.org, miltonm@....com
Subject: Re: [PATCH v3] pcmcia: CompactFlash driver for PA Semi Electra
boards
On Thu, 5 Jul 2007 09:49:14 -0500
olof@...om.net (Olof Johansson) wrote:
> Driver for the CompactFlash slot on the PA Semi Electra eval board. It's
> a simple device sitting on localbus, with interrupts and detect/voltage
> control over GPIO.
>
> The driver is implemented as an of_platform driver, and adds localbus
> as a bus being probed by the of_platform framework.
>
>
> Signed-off-by: Olof Johansson <olof@...om.net>
>
> ---
>
> On Mon, Jun 25, 2007 at 03:43:41PM -0500, olof wrote:
>
> > The ifdef is needed since for CONFIG_PCMCIA=n builds, the bus notifier
> > isn't available. I wanted to do the bus notifier registration explicitly
> > before the of_platform bus probe to avoid later surprises due to reordered
> > initcalls in case it was split up in it's own initcall.
> >
> > I could add the code under ifdef as well, but it didn't seem too
> > critical. Once the second major board comes along I'll probably move it
> > out to a per-board file, there's no real need for it just yet.
>
> Alright, turns out I still need to declare the extern bus type, which would mean
> two #ifdefs in one function. Moving it out instead.
>
> I've addressed Milton's comments as well.
>
> Who's maintaining PCMCIA? MAINTAINERS only lists a mailing list, no person. Seems
> weird for a component that's marked as maintained.
Dominik Brodowski. He's having a bit of downtime at present (exams, I
think). He expects to return. Meanwhile, cc'ing me usually has some
effect.
>
> ...
>
> +static const char driver_name[] = "electra-cf";
>
> ...
>
> +static struct of_device_id electra_cf_match[] =
> +{
> + {
> + .compatible = "electra-cf",
> + },
> + {},
> +};
Could have reused driver_name[] here, if that was appropriate.
> +static struct of_platform_driver electra_cf_driver =
> +{
> + .name = (char *)driver_name,
ug. But it's not your fault - we should have always made it const.
> --- mainline.orig/arch/powerpc/platforms/pasemi/setup.c
> +++ mainline/arch/powerpc/platforms/pasemi/setup.c
I never know who maintains random-scruffy-ppc code like this. From a peek
in the git-whatchanged output, it appears to be yourself.
Have a few little fixies:
--- a/drivers/pcmcia/electra_cf.c~pcmcia-compactflash-driver-for-pa-semi-electra-boards-fix
+++ a/drivers/pcmcia/electra_cf.c
@@ -201,9 +201,7 @@ static int __devinit electra_cf_probe(st
if (!cf)
return -ENOMEM;
- init_timer(&cf->timer);
- cf->timer.function = electra_cf_timer;
- cf->timer.data = (unsigned long) cf;
+ setup_timer(&cf->timer, electra_cf_timer, (unsigned long)cf);
cf->irq = NO_IRQ;
cf->ofdev = ofdev;
@@ -340,16 +338,14 @@ static int __devexit electra_cf_remove(s
return 0;
}
-static struct of_device_id electra_cf_match[] =
-{
+static struct of_device_id electra_cf_match[] = {
{
.compatible = "electra-cf",
},
{},
};
-static struct of_platform_driver electra_cf_driver =
-{
+static struct of_platform_driver electra_cf_driver = {
.name = (char *)driver_name,
.match_table = electra_cf_match,
.probe = electra_cf_probe,
@@ -371,4 +367,3 @@ module_exit(electra_cf_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR ("Olof Johansson <olof@...om.net>");
MODULE_DESCRIPTION("PA Semi Electra CF driver");
-
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists