Re: [PATCH v3] pcmcia: CompactFlash driver for PA Semi Electra boards

On Thu, 5 Jul 2007 09:49:14 -0500
olof@lixom.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 <redacted>

---

On Mon, Jun 25, 2007 at 03:43:41PM -0500, olof wrote:

quoted

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.

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@lixom.net>");
 MODULE_DESCRIPTION("PA Semi Electra CF driver");
-

_