[PATCH 001/001] PMAC HD runtime blinking control

Hi,

I've finally spend the time to mend the patch for control of
the HD led blinking at runtime. This is a patch against
2.6.15

The sysfs entry is attached to the PCI device and to the
MACIO device if available. I think that was what Ben was
asking for:

lauren-ph:/sys# find . | grep blink
./module/ide_core/parameters/noblink
./devices/pci0001:10/0001:10:17.0/blinking_led
./devices/pci0001:10/0001:10:17.0/0.80000000:mac-io/0.0001f000:ata-4/blinking_led


To activate blinking:
echo 1 > ./devices/pci0001:10/0001:10:17.0/blinking_led

To deactivate it:
echo 0 > ./devices/pci0001:10/0001:10:17.0/blinking_led

There is also a boot time parameter idecore:noblink to
deactivate the blinking at this stage.


signed-off-by: Cedric Pradalier <redacted>
---

--- drivers/ide/ppc/pmac.c.orig	2006-01-03

13:21:10.000000000 +1000 +++ drivers/ide/ppc/pmac.c
2006-01-22 11:51:11.000000000 +1000 @@ -36,6 +36,11 @@
 #include <linux/pmu.h>
 #include <linux/scatterlist.h>
 
+#ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
+#include <linux/device.h>
+#include <asm/of_device.h>
+#endif
+
 #include <asm/prom.h>
 #include <asm/io.h>
 #include <asm/dbdma.h>

@@ -427,6 +432,15 @@ static void pmac_ide_kauai_selectproc

(id 
 #ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
 
+MODULE_AUTHOR("Paul Mackerras & Ben. Herrenschmidt");
+MODULE_DESCRIPTION("Support for IDE interfaces on
PowerMacs"); +MODULE_LICENSE("GPL");
+
+static int blinking_led = 1;
+module_param_named(noblink,blinking_led, invbool, 0666);
+MODULE_PARM_DESC(noblink,"Enable/Disable blinking led
[Default: enabled]"); +
+
 /* Set to 50ms minimum led-on time (also used to limit
frequency
  * of requests sent to the PMU
  */

@@ -437,8 +451,7 @@ static spinlock_t pmu_blink_lock;
 static unsigned long pmu_blink_stoptime;
 static int pmu_blink_ledstate;
 static struct timer_list pmu_blink_timer;
-static int pmu_ide_blink_enabled;
-
+static int pmu_ide_blink_enabled = 0;
 
 static void
 pmu_hd_blink_timeout(unsigned long data)

@@ -468,6 +481,8 @@ static void
 pmu_hd_kick_blink(void *data, int rw)
 {
 	unsigned long flags;
+	if (!blinking_led)
+		return;
 	
 	pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME;
 	wmb();

@@ -483,8 +498,28 @@ pmu_hd_kick_blink(void *data, int rw)
 	spin_unlock_irqrestore(&pmu_blink_lock, flags);
 }
 
+static ssize_t show_blinkingled_activity(struct device

*dev, struct device_attribute *attr, char *buf)\ +{  
+	return sprintf(buf, "%c\n", blinking_led?'1':'0'); 
+}
+
+static ssize_t set_blinkingled_activity(struct device
*dev, struct device_attribute *attr, 
+		const char *buf, size_t count)
+{
+	int blink;
+	if (sscanf (buf, "%d", &blink) != 1)
+		return -EINVAL;
+	blinking_led = (blink != 0);
+	printk(KERN_INFO "pmac blinking led initialized
(blink %s)\n",
+			blinking_led?"enabled":"disabled");
+	return count;
+}
+
+static DEVICE_ATTR (blinking_led, S_IRUGO | S_IWUSR, 
+		show_blinkingled_activity,
set_blinkingled_activity); +
 static int
-pmu_hd_blink_init(void)
+pmu_hd_blink_init(pmac_ide_hwif_t *pmif, ide_hwif_t *hwif)
 {
 	struct device_node *dt;
 	const char *model;

@@ -516,6 +551,13 @@ pmu_hd_blink_init(void)
 	init_timer(&pmu_blink_timer);
 	pmu_blink_timer.function = pmu_hd_blink_timeout;
 
+	device_create_file (&hwif->pci_dev->dev,

&dev_attr_blinking_led);
+	if (pmif->mdev != NULL)
+		device_create_file
(&pmif->mdev->ofdev.dev, &dev_attr_blinking_led);
+	
+	printk(KERN_INFO "pmac blinking led initialized
(blink %s)\n",
+			blinking_led?"enabled":"disabled");
+
 	return 1;
 }
 

@@ -1271,7 +1313,7 @@ static int
 pmac_ide_setup_device(pmac_ide_hwif_t *pmif, ide_hwif_t

*hwif) {
 	struct device_node *np = pmif->node;
-	int *bidp, i;
+	int *bidp;
 
 	pmif->cable_80 = 0;
 	pmif->broken_dma = pmif->broken_dma_warn = 0;

@@ -1375,7 +1417,7 @@ pmac_ide_setup_device(pmac_ide_hwif_t

*p hwif->speedproc = pmac_ide_tune_chipset;
 
 #ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
-	pmu_ide_blink_enabled = pmu_hd_blink_init();
+	pmu_ide_blink_enabled = pmu_hd_blink_init
(pmif,hwif); 
 	if (pmu_ide_blink_enabled)
 		hwif->led_act = pmu_hd_kick_blink;

@@ -1476,6 +1518,7 @@ pmac_ide_macio_attach(struct

macio_dev * #endif /* CONFIG_BLK_DEV_IDEDMA_PMAC */
 	dev_set_drvdata(&mdev->ofdev.dev, hwif);
 
+	printk(KERN_INFO "pmac: using macio interface");
 	rc = pmac_ide_setup_device(pmif, hwif);
 	if (rc != 0) {
 		/* The inteface is released to the common
IDE layer */ @@ -1584,6 +1627,7 @@ pmac_ide_pci_attach
(struct pci_dev *pdev 
 	pci_set_drvdata(pdev, hwif);
 
+	printk(KERN_INFO "pmac: using PCI interface");
 	rc = pmac_ide_setup_device(pmif, hwif);
 	if (rc != 0) {
 		/* The inteface is released to the common
IDE layer */

On Sun, 2006-01-22 at 12:19 +1000, Cedric Pradalier wrote:

Hi,

I've finally spend the time to mend the patch for control of
the HD led blinking at runtime. This is a patch against
2.6.15

The sysfs entry is attached to the PCI device and to the
MACIO device if available. I think that was what Ben was
asking for:

Heh, nice :) Almost ! It would be better if it was attached to the sysfs
node of the ide interface, you can find it in the HWIF array after
probe, but I won't be too much of a pain about that for now :)

Ben.

According to Benjamin Herrenschmidt, on Sun, 22 Jan 2006
13:49:53 +1100, 

On Sun, 2006-01-22 at 12:19 +1000, Cedric Pradalier wrote:

quoted

Hi,

I've finally spend the time to mend the patch for control of
the HD led blinking at runtime. This is a patch against
2.6.15

The sysfs entry is attached to the PCI device and to the
MACIO device if available. I think that was what Ben was
asking for:

Heh, nice :) Almost ! It would be better if it was attached to the sysfs
node of the ide interface, you can find it in the HWIF array after
probe, but I won't be too much of a pain about that for now :)

Yep, now I remember, that was what you where asking for.
Can you give me an idea of the sysfs path you're expecting?
Is it in 
bus/ide/..., 
bus/pci/drivers/ide-pmac,
bus/pci/drivers/ide-disk
devices/pci0001:10/0001:10:17.0/0.80000000:mac-io/0.0001f000:ata-4/ide0

I would guess on the latter. 
For now I'm using the pmif->mdev->ofdev.dev as attachment
device, which brings me into 0.0001f000:ata-4
What in pmif can brings me to the ide0? pmif->node? Or one
of its siblings?

In ide_hwif_t, I'm using the pci_dev. Where in the pci_dev
can I find ide0? or is it somewhere else in ide_hwif_t?

For the HWIF array, I guess you're talking about ide_hwifs.
How do I identify the interface itself there?

Hmm, plenty of questions. I hope at least one is relevant
for this problem...

Thanks
--
Cedric

On Sun, 2006-01-22 at 18:11 +1000, Cedric Pradalier wrote:

In ide_hwif_t, I'm using the pci_dev. Where in the pci_dev
can I find ide0? or is it somewhere else in ide_hwif_t?

Yah, I think it's in there (in ide_hwif_t)

For the HWIF array, I guess you're talking about ide_hwifs.
How do I identify the interface itself there?

well, depends what you are working from...  pmif has an index into it
for example...

Hmm, plenty of questions. I hope at least one is relevant
for this problem...

Thanks
--
Cedric

On Sunday 22 January 2006 03:19, Cedric Pradalier wrote:

Hi,

I've finally spend the time to mend the patch for control of
the HD led blinking at runtime. This is a patch against
2.6.15

I wonder if it's also possible to disable blinking when laptop sleeps?

=2D-=20
Arkadiusz Mi=B6kiewicz                    PLD/Linux Team
http://www.t17.ds.pwr.wroc.pl/~misiek/  http://ftp.pld-linux.org/

On Sun, 2006-01-22 at 12:56 +0100, Arkadiusz Miskiewicz wrote:

On Sunday 22 January 2006 03:19, Cedric Pradalier wrote:

quoted

Hi,

I've finally spend the time to mend the patch for control of
the HD led blinking at runtime. This is a patch against
2.6.15

I wonder if it's also possible to disable blinking when laptop sleeps?

Not that I know.. this is entirely done by the PMU.

Ben.

According to Benjamin Herrenschmidt, on Sun, 22 Jan 2006
13:49:53 +1100, 

On Sun, 2006-01-22 at 12:19 +1000, Cedric Pradalier wrote:

quoted

Hi,

I've finally spend the time to mend the patch for control of
the HD led blinking at runtime. This is a patch against
2.6.15

The sysfs entry is attached to the PCI device and to the
MACIO device if available. I think that was what Ben was
asking for:

Heh, nice :) Almost ! It would be better if it was attached to the sysfs
node of the ide interface, you can find it in the HWIF array after
probe, but I won't be too much of a pain about that for now :)

Ben.

OK, I think now I got it.

pradalie@lauren-ph:~$ find /sys/ | grep blink 
/sys/devices/pci0001:10/0001:10:17.0/0.80000000:mac-io/0.0001f000:ata-4/ide0/blinking_led

The key I could not understand was that hwif->gendev is
only initialised in the probe. So I had to move the
device creation after that.

Currently, it is blinking by default. Should it be that
way? I guess so, since it is activated by a kernel config
option. It is easy to change if required.

Anyway, here is the updated patch.

signed-off-by: Cedric Pradalier <redacted>
---

--- drivers/ide/ppc/pmac.c.orig	2006-01-03 13:21:10.000000000 +1000
+++ drivers/ide/ppc/pmac.c	2006-01-23 23:32:18.000000000 +1000

@@ -36,6 +36,11 @@
 #include <linux/pmu.h>
 #include <linux/scatterlist.h>
 
+#ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
+#include <linux/device.h>
+#include <asm/of_device.h>
+#endif
+
 #include <asm/prom.h>
 #include <asm/io.h>
 #include <asm/dbdma.h>

@@ -427,6 +432,15 @@ static void pmac_ide_kauai_selectproc(id
 
 #ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
 
+MODULE_AUTHOR("Paul Mackerras & Ben. Herrenschmidt");
+MODULE_DESCRIPTION("Support for IDE interfaces on PowerMacs");
+MODULE_LICENSE("GPL");
+
+static int blinking_led = 1;
+module_param_named(noblink,blinking_led, invbool, 0666);
+MODULE_PARM_DESC(noblink,"Enable/Disable blinking led [Default: enabled]");
+
+
 /* Set to 50ms minimum led-on time (also used to limit frequency
  * of requests sent to the PMU
  */

@@ -437,8 +451,7 @@ static spinlock_t pmu_blink_lock;
 static unsigned long pmu_blink_stoptime;
 static int pmu_blink_ledstate;
 static struct timer_list pmu_blink_timer;
-static int pmu_ide_blink_enabled;
-
+static int pmu_ide_blink_enabled = 0;
 
 static void
 pmu_hd_blink_timeout(unsigned long data)

@@ -468,6 +481,8 @@ static void
 pmu_hd_kick_blink(void *data, int rw)
 {
 	unsigned long flags;
+	if (!blinking_led)
+		return;
 	
 	pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME;
 	wmb();

@@ -483,6 +498,26 @@ pmu_hd_kick_blink(void *data, int rw)
 	spin_unlock_irqrestore(&pmu_blink_lock, flags);
 }
 
+static ssize_t show_blinkingled_activity(struct device *dev, struct device_attribute *attr, char *buf)\
+{  
+	return sprintf(buf, "%c\n", blinking_led?'1':'0'); 
+}
+
+static ssize_t set_blinkingled_activity(struct device *dev, struct device_attribute *attr, 
+		const char *buf, size_t count)
+{
+	int blink;
+	if (sscanf (buf, "%d", &blink) != 1)
+		return -EINVAL;
+	blinking_led = (blink != 0);
+	printk(KERN_INFO "pmac blinking led initialized (blink %s)\n",
+			blinking_led?"enabled":"disabled");
+	return count;
+}
+
+static DEVICE_ATTR (blinking_led, S_IRUGO | S_IWUSR, 
+		show_blinkingled_activity, set_blinkingled_activity);
+
 static int
 pmu_hd_blink_init(void)
 {

@@ -516,6 +551,9 @@ pmu_hd_blink_init(void)
 	init_timer(&pmu_blink_timer);
 	pmu_blink_timer.function = pmu_hd_blink_timeout;
 
+	printk(KERN_INFO "pmac blinking led initialized (blink %s)\n",
+			blinking_led?"enabled":"disabled");
+
 	return 1;
 }
 

@@ -1271,7 +1309,7 @@ static int
 pmac_ide_setup_device(pmac_ide_hwif_t *pmif, ide_hwif_t *hwif)
 {
 	struct device_node *np = pmif->node;
-	int *bidp, i;
+	int *bidp;
 
 	pmif->cable_80 = 0;
 	pmif->broken_dma = pmif->broken_dma_warn = 0;

@@ -1401,6 +1439,12 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 	/* We probe the hwif now */
 	probe_hwif_init(hwif);
 
+#ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
+	/* We wait till here to have the gendev initialized in hwif */
+	device_create_file (&hwif->gendev, &dev_attr_blinking_led);
+#endif
+
+
 	return 0;
 }
 

The key I could not understand was that hwif->gendev is
only initialised in the probe. So I had to move the
device creation after that.

Currently, it is blinking by default. Should it be that
way? I guess so, since it is activated by a kernel config
option. It is easy to change if required.

Yes. In fact, by enabled default for ATA disks and by disabled for ATAPI
would make sense...

Also, we should think a bit about the file name... "blinking_led" isn't
terrific for something that will end up in a non-ppc specific location.
Or maybe on the contrary it's good ... what about "activity_led"
rather ?

According to Benjamin Herrenschmidt, on Tue, 24 Jan 2006
00:47:16 +1100, 

quoted

The key I could not understand was that hwif->gendev is
only initialised in the probe. So I had to move the
device creation after that.

Currently, it is blinking by default. Should it be that
way? I guess so, since it is activated by a kernel config
option. It is easy to change if required.

Yes. In fact, by enabled default for ATA disks and by disabled for ATAPI
would make sense...

How do I tell the difference?. There is a 'kind' in pmif,
and also a atapi_dma flag in hwif. Which is more sensible?

Also, we should think a bit about the file name... "blinking_led" isn't
terrific for something that will end up in a non-ppc specific location.
Or maybe on the contrary it's good ... what about "activity_led"
rather ?

I'm open to any suggestion. I'll wait a bit to see if
someone else has a comment, then I'll change to
"activity_led". 

--
Cedric

On Tue, 2006-01-24 at 07:31 +1000, Cedric Pradalier wrote:

According to Benjamin Herrenschmidt, on Tue, 24 Jan 2006
00:47:16 +1100, 

quoted

quoted

The key I could not understand was that hwif->gendev is
only initialised in the probe. So I had to move the
device creation after that.

Currently, it is blinking by default. Should it be that
way? I guess so, since it is activated by a kernel config
option. It is easy to change if required.

Yes. In fact, by enabled default for ATA disks and by disabled for ATAPI
would make sense...

How do I tell the difference?. There is a 'kind' in pmif,
and also a atapi_dma flag in hwif. Which is more sensible?

You need to check drive->media ... Which is a bit annoying since it mans
it's per drive, not per-HWIF... you may want to move your sysfs entry
down one more level :) (Each HWIF has an array of 2 drives, though check
drive->present before expecting a useful struct device there)

quoted

Also, we should think a bit about the file name... "blinking_led" isn't
terrific for something that will end up in a non-ppc specific location.
Or maybe on the contrary it's good ... what about "activity_led"
rather ?

I'm open to any suggestion. I'll wait a bit to see if
someone else has a comment, then I'll change to
"activity_led". 

--
Cedric

According to Benjamin Herrenschmidt, on Tue, 24 Jan 2006
10:10:26 +1100, 

On Tue, 2006-01-24 at 07:31 +1000, Cedric Pradalier wrote:

quoted

quoted

Yes. In fact, by enabled default for ATA disks and by disabled for ATAPI
would make sense...

How do I tell the difference?. There is a 'kind' in pmif,
and also a atapi_dma flag in hwif. Which is more sensible?

You need to check drive->media ... Which is a bit annoying since it mans
it's per drive, not per-HWIF... you may want to move your sysfs entry
down one more level :) (Each HWIF has an array of 2 drives, though check
drive->present before expecting a useful struct device there)

Well in this case, I'll let this part waiting for now. 
I don't think there will be enough real users of this minor
thing to justify putting any more efforts for now.

I think the practical solution is to disable the led by
default. The few user who will want it will be able to add
either ide_core.blink to their boot parameter or make a
small init.d script to put 1 into the sysfs entry.

That's the way I've implemented it, and  I've changed the
name into activity_led.

pradalier@localhost:~$ find /sys/ -name activity_led
/sys/devices/pci0001:10/0001:10:17.0/0.80000000:mac-io/0.0001f000:ata-4/ide0/activity_led

This is the updated version of the patch. I, for one, will
consider it stable now.

signed-off-by: Cedric Pradalier <redacted>
---

--- drivers/ide/ppc/pmac.c.orig	2006-01-03 13:21:10.000000000 +1000
+++ drivers/ide/ppc/pmac.c	2006-01-24 21:18:36.000000000 +1000

@@ -36,6 +36,11 @@
 #include <linux/pmu.h>
 #include <linux/scatterlist.h>
 
+#ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
+#include <linux/device.h>
+#include <asm/of_device.h>
+#endif
+
 #include <asm/prom.h>
 #include <asm/io.h>
 #include <asm/dbdma.h>

@@ -427,6 +432,15 @@ static void pmac_ide_kauai_selectproc(id
 
 #ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
 
+MODULE_AUTHOR("Paul Mackerras & Ben. Herrenschmidt");
+MODULE_DESCRIPTION("Support for IDE interfaces on PowerMacs");
+MODULE_LICENSE("GPL");
+
+static int activity_led = 0;
+module_param_named(blink,activity_led, bool, 0666);
+MODULE_PARM_DESC(blink,"Enable/Disable activity led [Default: disabled]");
+
+
 /* Set to 50ms minimum led-on time (also used to limit frequency
  * of requests sent to the PMU
  */

@@ -437,8 +451,7 @@ static spinlock_t pmu_blink_lock;
 static unsigned long pmu_blink_stoptime;
 static int pmu_blink_ledstate;
 static struct timer_list pmu_blink_timer;
-static int pmu_ide_blink_enabled;
-
+static int pmu_ide_blink_enabled = 0;
 
 static void
 pmu_hd_blink_timeout(unsigned long data)

@@ -468,6 +481,8 @@ static void
 pmu_hd_kick_blink(void *data, int rw)
 {
 	unsigned long flags;
+	if (!activity_led)
+		return;
 	
 	pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME;
 	wmb();

@@ -483,6 +498,26 @@ pmu_hd_kick_blink(void *data, int rw)
 	spin_unlock_irqrestore(&pmu_blink_lock, flags);
 }
 
+static ssize_t show_activity_led_enable(struct device *dev, struct device_attribute *attr, char *buf)\
+{  
+	return sprintf(buf, "%c\n", activity_led?'1':'0'); 
+}
+
+static ssize_t set_activity_led_enable(struct device *dev, struct device_attribute *attr, 
+		const char *buf, size_t count)
+{
+	int blink=0;
+	if (sscanf (buf, "%d", &blink) != 1)
+		return -EINVAL;
+	activity_led = (blink != 0);
+	printk(KERN_INFO "pmac blinking led initialized (blink %s)\n",
+			activity_led?"enabled":"disabled");
+	return count;
+}
+
+static DEVICE_ATTR (activity_led, S_IRUGO | S_IWUSR, 
+		show_activity_led_enable, set_activity_led_enable);
+
 static int
 pmu_hd_blink_init(void)
 {

@@ -516,6 +551,9 @@ pmu_hd_blink_init(void)
 	init_timer(&pmu_blink_timer);
 	pmu_blink_timer.function = pmu_hd_blink_timeout;
 
+	printk(KERN_INFO "pmac blinking led initialized (blink %s)\n",
+			activity_led?"enabled":"disabled");
+
 	return 1;
 }
 

@@ -1271,7 +1309,7 @@ static int
 pmac_ide_setup_device(pmac_ide_hwif_t *pmif, ide_hwif_t *hwif)
 {
 	struct device_node *np = pmif->node;
-	int *bidp, i;
+	int *bidp;
 
 	pmif->cable_80 = 0;
 	pmif->broken_dma = pmif->broken_dma_warn = 0;

@@ -1401,6 +1439,12 @@ pmac_ide_setup_device(pmac_ide_hwif_t *p
 	/* We probe the hwif now */
 	probe_hwif_init(hwif);
 
+#ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
+	/* We wait till here to have the gendev initialized in hwif */
+	device_create_file (&hwif->gendev, &dev_attr_activity_led);
+#endif
+
+
 	return 0;
 }
 

On Tue, 2006-01-24 at 22:25 +1000, Cedric Pradalier wrote:

According to Benjamin Herrenschmidt, on Tue, 24 Jan 2006
10:10:26 +1100, 

quoted

On Tue, 2006-01-24 at 07:31 +1000, Cedric Pradalier wrote:

quoted

quoted

Yes. In fact, by enabled default for ATA disks and by disabled for ATAPI
would make sense...

How do I tell the difference?. There is a 'kind' in pmif,
and also a atapi_dma flag in hwif. Which is more sensible?

You need to check drive->media ... Which is a bit annoying since it mans
it's per drive, not per-HWIF... you may want to move your sysfs entry
down one more level :) (Each HWIF has an array of 2 drives, though check
drive->present before expecting a useful struct device there)

Well in this case, I'll let this part waiting for now. 
I don't think there will be enough real users of this minor
thing to justify putting any more efforts for now.

I think the practical solution is to disable the led by
default. The few user who will want it will be able to add
either ide_core.blink to their boot parameter or make a
small init.d script to put 1 into the sysfs entry.

That's the way I've implemented it, and  I've changed the
name into activity_led.

I'll have a look into making it work he way I want :) Then I'll submit
it.

Thanks,
Ben.

Cedric Pradalier [off-list ref] writes:

No need to zero-initialize a static variable.

Same.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

According to Andreas Schwab, on Wed, 25 Jan 2006 12:06:44
+0100, 

Cedric Pradalier [off-list ref] writes:

quoted

@@ -427,6 +432,15 @@ static void pmac_ide_kauai_selectproc(id
 
 #ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK
 
+MODULE_AUTHOR("Paul Mackerras & Ben. Herrenschmidt");
+MODULE_DESCRIPTION("Support for IDE interfaces on PowerMacs");
+MODULE_LICENSE("GPL");
+
+static int activity_led = 0;

No need to zero-initialize a static variable.

On the other hand, the cost is minor for an increased
readability and a behavior which does not rely on any 
assumption on the compiler (or even the C specification).

I tend to favor readability first in my coding.

Cheers
--
Cedric

Cedric Pradalier [off-list ref] writes:

I tend to favor readability first in my coding.

linux-kernel@ has a different opinion.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."