Re: Hard Drive light
According to Benjamin Herrenschmidt, on Mon, 24 Jan 2005 09:19:13 +1100, Hi ben, Did you have time to have a look at that ? As far as I can see, I actually attached to the only struct device in the hwif. So it might be what you wanted. No, sorry I've been a bit too busy. Please, re-send me your latest patch. No need to be sorry, this is in no way a first priority matter :) There it is. Cedric --- pmac.c.old 2004-12-05 23:58:25.0 +1100 +++ pmac.c 2005-01-23 21:20:08.0 +1100 @@ -35,6 +35,11 @@ #include linux/adb.h #include linux/pmu.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 @@ -372,6 +377,15 @@ #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 */ @@ -382,8 +396,7 @@ 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) @@ -413,6 +426,8 @@ pmu_hd_kick_blink(void *data, int rw) { unsigned long flags; + if (!blinking_led) + return; pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME; wmb(); @@ -428,8 +443,28 @@ spin_unlock_irqrestore(pmu_blink_lock, flags); } +static ssize_t show_blinkingled_activity(struct device *dev, char *buf)\ +{ + return sprintf(buf, %c\n, blinking_led?'1':'0'); +} + +static ssize_t set_blinkingled_activity(struct device *dev, + 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(struct device * dev) { struct device_node *dt; const char *model; @@ -458,6 +493,11 @@ init_timer(pmu_blink_timer); pmu_blink_timer.function = pmu_hd_blink_timeout; + device_create_file (dev, dev_attr_blinking_led); + + printk(KERN_INFO pmac blinking led initialized (blink %s)\n, + blinking_led?enabled:disabled); + return 1; } @@ -1229,13 +1269,6 @@ hwif-selectproc = pmac_ide_selectproc; hwif-speedproc = pmac_ide_tune_chipset; -#ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK - pmu_ide_blink_enabled = pmu_hd_blink_init(); - - if (pmu_ide_blink_enabled) - hwif-led_act = pmu_hd_kick_blink; -#endif - printk(KERN_INFO ide%d: Found Apple %s controller, bus ID %d%s, irq %d\n, hwif-index, model_name[pmif-kind], pmif-aapl_bus_id, pmif-mediabay ? (mediabay) : , hwif-irq); @@ -1253,6 +1286,13 @@ /* We probe the hwif now */ probe_hwif_init(hwif); +#ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK + pmu_ide_blink_enabled = pmu_hd_blink_init(hwif-gendev.parent); + + if (pmu_ide_blink_enabled) + hwif-led_act = pmu_hd_kick_blink; +#endif + /* The code IDE code will have set hwif-present if we have devices attached, * if we don't, the discard the interface except if we are on a media bay slot @@ -1342,6 +1382,7 @@ #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 */ @@ -1454,6 +1495,7 @@ 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 */
Re: Hard Drive light
According to Benjamin Herrenschmidt, on Thu, 23 Dec 2004 14:57:03 +0100, Thanks for your help. I could not find how to attach the sys entry to the ide interface, but now, it is attached to the bus owning the IDE, either macio or pci according to which 'attach' function is called. On my ibook, it is macio and I have no way to test if the pci entry works fine. At the end of pmac_ide_setup_device(), after probe_hwif_init() (and after the test for !present) you have the fully probed hwif which should contain a device structure you can attach to, though it's not clear how to hook a cleanup function... but then, you probably don't need one neither, modularity isn't quite supported on ide pmac. Avoid attaching anything to the mediabay tho, that will cause more problems as the media bay is sort-of hotplug. In accord with popular demand, the blink by default is ON, and it can be disabled with boot parameter ide_core.noblink, or through the /sys/bus/(macio|pci)/*/blinking_led interface. With the christmas break, I will not be able to work on that for a couple of week. So if there is still modifications to do, they will have to wait, (or I don't mind if someone else handle them :). Ok, well, I may do my own changes, depends on how much time I can spare. Ben. Hi ben, Did you have time to have a look at that ? As far as I can see, I actually attached to the only struct device in the hwif. So it might be what you wanted. Cedric -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: Hard Drive light
According to Benjamin Herrenschmidt, on Mon, 20 Dec 2004 17:00:43 +0100, I chose macio since I had the struct device pointer easily accessible in pmac.c. Not all laptops have the IDE under macio though ... some have it under a PCI node. It should be per-controller ideally... (some machines have both the macio one and the PCI one). I don't think so. Module parameters do not depend of the module name (as far as I understand from the doc). What I guess is that ide-pmac module is initialized too early to have correct boot parameters. For instance, I tried to had a dummy module_init function, and it is called much later than ide_pmac_setup_device (where blinking is initialized). Anybody can confirm or infirm this guess ? When a module is built-in, it's parameters are passed in the form modulename.paramname=value Furthermore I've looked at the way framebuffers manage boot params, and these module seems to be parsing the comand line themselves too. Framebuffers use a setup function, not a module param. Ben. Hi all, Thanks for your help. I could not find how to attach the sys entry to the ide interface, but now, it is attached to the bus owning the IDE, either macio or pci according to which 'attach' function is called. On my ibook, it is macio and I have no way to test if the pci entry works fine. In accord with popular demand, the blink by default is ON, and it can be disabled with boot parameter ide_core.noblink, or through the /sys/bus/(macio|pci)/*/blinking_led interface. With the christmas break, I will not be able to work on that for a couple of week. So if there is still modifications to do, they will have to wait, (or I don't mind if someone else handle them :). -- Cedric Pradalier --- pmac.c 2004-12-23 23:00:49.184102112 +1100 +++ pmac.c.old 2004-12-05 23:58:25.0 +1100 @@ -35,11 +35,6 @@ #include linux/adb.h #include linux/pmu.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 @@ -377,15 +372,6 @@ #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 = 0; -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 */ @@ -396,7 +382,8 @@ static unsigned long pmu_blink_stoptime; static int pmu_blink_ledstate; static struct timer_list pmu_blink_timer; -static int pmu_ide_blink_enabled = 0; +static int pmu_ide_blink_enabled; + static void pmu_hd_blink_timeout(unsigned long data) @@ -426,8 +413,6 @@ pmu_hd_kick_blink(void *data, int rw) { unsigned long flags; - if (!blinking_led) - return; pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME; wmb(); @@ -443,28 +428,8 @@ spin_unlock_irqrestore(pmu_blink_lock, flags); } -static ssize_t show_blinkingled_activity(struct device *dev, char *buf)\ -{ - return sprintf(buf, %c\n, blinking_led?'1':'0'); -} - -static ssize_t set_blinkingled_activity(struct device *dev, - 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(struct device * dev) +pmu_hd_blink_init(void) { struct device_node *dt; const char *model; @@ -493,11 +458,6 @@ init_timer(pmu_blink_timer); pmu_blink_timer.function = pmu_hd_blink_timeout; - device_create_file (dev, dev_attr_blinking_led); - - printk(KERN_INFO pmac blinking led initialized (blink %s)\n, - blinking_led?enabled:disabled); - return 1; } @@ -1270,7 +1230,7 @@ hwif-speedproc = pmac_ide_tune_chipset; #ifdef CONFIG_BLK_DEV_IDE_PMAC_BLINK - pmu_ide_blink_enabled = pmu_hd_blink_init(hwif-gendev.parent); + pmu_ide_blink_enabled = pmu_hd_blink_init(); if (pmu_ide_blink_enabled) hwif-led_act = pmu_hd_kick_blink; @@ -1382,7 +1342,6 @@ #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 */ @@ -1495,7 +1454,6 @@ pci_set_drvdata(pdev, hwif); - printk(KERN_INFO pmac: using PCI
Re: Hard Drive light
Thanks for your help. I could not find how to attach the sys entry to the ide interface, but now, it is attached to the bus owning the IDE, either macio or pci according to which 'attach' function is called. On my ibook, it is macio and I have no way to test if the pci entry works fine. At the end of pmac_ide_setup_device(), after probe_hwif_init() (and after the test for !present) you have the fully probed hwif which should contain a device structure you can attach to, though it's not clear how to hook a cleanup function... but then, you probably don't need one neither, modularity isn't quite supported on ide pmac. Avoid attaching anything to the mediabay tho, that will cause more problems as the media bay is sort-of hotplug. In accord with popular demand, the blink by default is ON, and it can be disabled with boot parameter ide_core.noblink, or through the /sys/bus/(macio|pci)/*/blinking_led interface. With the christmas break, I will not be able to work on that for a couple of week. So if there is still modifications to do, they will have to wait, (or I don't mind if someone else handle them :). Ok, well, I may do my own changes, depends on how much time I can spare. Ben.
Re: Hard Drive light
On Tue, 2004-12-21 at 14:21 +0100, Colin Leroy wrote: On 22 Dec 2004 at 00h12, Cedric Pradalier wrote: Hi, No, not at this point. Beside, i'd like to have it enabled by default, not disabled by default :) I don't really agree with that. Blinking led is not really relevant for default user. I would say that if someone feels geek enough ;) to want it, it is good to make it available. On the other hand, I would not bother unconcerned people with this blinking. Having it enabled by default will let people know that it exists. I know too many Mac users that badly want a LED to show them when it's working and I should wait... Heh, in fact, I regulary have OS X users asking me for a similar feature in Darwin :) Ben.
Re: Hard Drive light
On Tue, 2004-12-21 at 14:21 +0100, Colin Leroy wrote: On 22 Dec 2004 at 00h12, Cedric Pradalier wrote: Hi, No, not at this point. Beside, i'd like to have it enabled by default, not disabled by default :) I don't really agree with that. Blinking led is not really relevant for default user. I would say that if someone feels geek enough ;) to want it, it is good to make it available. On the other hand, I would not bother unconcerned people with this blinking. Having it enabled by default will let people know that it exists. I know too many Mac users that badly want a LED to show them when it's working and I should wait... Oh, and to add to this ... I prefer having it ON by default so users avoid moving their laptop around while the HD is busy... Ben.
Re: Hard Drive light
On 22 Dec 2004 at 17h12, Benjamin Herrenschmidt wrote: Hi, Having it enabled by default will let people know that it exists. I know too many Mac users that badly want a LED to show them when it's working and I should wait... Oh, and to add to this ... I prefer having it ON by default so users avoid moving their laptop around while the HD is busy... Is it dangerous? A metallic chair failed on my ibook G4 a few monthes ago (don't ask why it was on the floor...), it only fucked a bit of plastic at the bottom of the screen - not the screen, not the keyboard, not the HD, nothing! and I was pretty impressed by the fact i didn't destroy the whole thing. -- Colin
Re: Hard Drive light
On Wed, Dec 22, 2004 at 05:12:22PM +0100, Colin Leroy wrote: On 22 Dec 2004 at 17h12, Benjamin Herrenschmidt wrote: Oh, and to add to this ... I prefer having it ON by default so users avoid moving their laptop around while the HD is busy... Is it dangerous? It will always be a bit more dangerous to hit the laptop while the disks are spinning and the heads are over the platter than when they are parked and the disk still. J -- Jesus Climent info:www.pumuki.org Unix SysAdm|Linux User #66350|Debian Developer|2.4.28|Helsinki Finland GPG: 1024D/86946D69 BB64 2339 1CAA 7064 E429 7E18 66FC 1D7F 8694 6D69 And then he ran into my knife... he ran into my knife ten times. --June (Chicago)
Re: Hard Drive light
According to Benjamin Herrenschmidt, on Mon, 20 Dec 2004 17:01:40 +0100, On Mon, 2004-12-20 at 14:04 +0100, Sven Luther wrote: I was able to parse kernel command line parameters in the pci setup code (chrp_pci.c) which i believe happens *well* before what you are trying to do. It was some time back though, but was rather straigthforward. The trick was effectively this damn module name. The parameter will be named ide_core.blinking_led Thanks a lot for all your hints. I'm learning a lot these days... Cedric and Ben, please tell me when this is ready for inclusion in the debian powerpc kernels :) No, not at this point. Beside, i'd like to have it enabled by default, not disabled by default :) I don't really agree with that. Blinking led is not really relevant for default user. I would say that if someone feels geek enough ;) to want it, it is good to make it available. On the other hand, I would not bother unconcerned people with this blinking. Anybody against or in favor of this argument ? Cheers -- Cedric Pradalier
Re: Hard Drive light
On 22 Dec 2004 at 00h12, Cedric Pradalier wrote: Hi, No, not at this point. Beside, i'd like to have it enabled by default, not disabled by default :) I don't really agree with that. Blinking led is not really relevant for default user. I would say that if someone feels geek enough ;) to want it, it is good to make it available. On the other hand, I would not bother unconcerned people with this blinking. Having it enabled by default will let people know that it exists. I know too many Mac users that badly want a LED to show them when it's working and I should wait... my 2¢ :) -- Colin
Re: Hard Drive light
You mean in /sys/bus/macio/drivers/ide-pmac ? That's the driver entry, but the devices themselves have entries as well, I'd rather put a per-device switch in them (either through the actual device path, either PCI or macio, or the ide interface proper). The main reason is that I could not find how to add my sysfs in this directory since it is not created in the ide-pmac.c file (still a lot to learn :)). So I convinced myself that a platform device was not so irrelevant. It's totally irrelevant actually :) Just give me an hint or an example and I'll change that. You can add you attributes to the driver itself, it contains the necessary kobject etc... or you can add them to the device it matches against (either a macio_dev or a pci_dev) ... Other thing strange to me is that I have to use the saved command line to know if there is a boot time argument. According to the comments around MODULE_PARM, a module parameter should be enough. I tried that, but the boot parameter never had any influence on the parameter. You must have typed it wrong ... One problem with ide-pmac though is that the module name is confusing since the file name itself is just pmac. I'm not too familiar with the module params thing tho, it may be possible to override the module name. Thanks for your remark. -- Benjamin Herrenschmidt [EMAIL PROTECTED]
Re: [Patch #3] Re: Hard Drive light
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Cedric Pradalier wrote: | | You're right. So this is a version with the strings updated. | Please excuse my english... | | Cheers, | | a stupid question: how i shall apply this patch? patch -p1 patch is right? thanks ciao -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.4 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFBxo9dmQRKGuVp5FMRAmS9AJ0SOVJqWA2VVa3/Av5S6EuRMHhRqgCeLY31 OSsg5yTt3mWUkBEglwrTulg= =Mj9I -END PGP SIGNATURE-
Re: [Patch #3] Re: Hard Drive light
According to marco giusti, on Mon, 20 Dec 2004 09:37:49 +0100, -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Cedric Pradalier wrote: | | You're right. So this is a version with the strings updated. | Please excuse my english... | | Cheers, | | a stupid question: how i shall apply this patch? patch -p1 patch is right? thanks I would say cd drivers/ide/ppc patch -p0 patch Notice that there will be some improvement soon, according to Benh's mail. Cheers -- Cedric Pradalier
Re: Hard Drive light
According to Benjamin Herrenschmidt, on Mon, 20 Dec 2004 07:59:49 +0100, You mean in /sys/bus/macio/drivers/ide-pmac ? That's the driver entry, but the devices themselves have entries as well, I'd rather put a per-device switch in them (either through the actual device path, either PCI or macio, or the ide interface proper). The main reason is that I could not find how to add my sysfs in this directory since it is not created in the ide-pmac.c file (still a lot to learn :)). So I convinced myself that a platform device was not so irrelevant. It's totally irrelevant actually :) Ok I believe you. It is now attached to the macio device /sys/bus/macio/devices/0.0001f000\:ata-4/blinking_led Is this what you suggested? I chose macio since I had the struct device pointer easily accessible in pmac.c. Other thing strange to me is that I have to use the saved command line to know if there is a boot time argument. According to the comments around MODULE_PARM, a module parameter should be enough. I tried that, but the boot parameter never had any influence on the parameter. You must have typed it wrong ... One problem with ide-pmac though is that the module name is confusing since the file name itself is just pmac. I'm not too familiar with the module params thing tho, it may be possible to override the module name. I don't think so. Module parameters do not depend of the module name (as far as I understand from the doc). What I guess is that ide-pmac module is initialized too early to have correct boot parameters. For instance, I tried to had a dummy module_init function, and it is called much later than ide_pmac_setup_device (where blinking is initialized). Anybody can confirm or infirm this guess ? Furthermore I've looked at the way framebuffers manage boot params, and these module seems to be parsing the comand line themselves too. -- Cedric Pradalier --- pmac.c.old 2004-12-05 23:58:25.0 +1100 +++ pmac.c 2004-12-20 21:40:43.976967000 +1100 @@ -35,6 +35,11 @@ #include linux/adb.h #include linux/pmu.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 @@ -381,9 +386,9 @@ static spinlock_t pmu_blink_lock; static unsigned long pmu_blink_stoptime; static int pmu_blink_ledstate; +static int blinking_led = 0; 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) @@ -413,6 +418,8 @@ pmu_hd_kick_blink(void *data, int rw) { unsigned long flags; + if (!blinking_led) + return; pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME; wmb(); @@ -428,9 +435,30 @@ spin_unlock_irqrestore(pmu_blink_lock, flags); } +static ssize_t show_blinkingled_activity(struct device *dev, char *buf)\ +{ + return sprintf(buf, %c\n, blinking_led?'1':'0'); +} + +static ssize_t set_blinkingled_activity(struct device *dev, + 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 has been %sactivated\n, + blinking_led?:dis); + 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(struct device * dev) { + extern char saved_command_line[]; struct device_node *dt; const char *model; @@ -458,6 +486,12 @@ init_timer(pmu_blink_timer); pmu_blink_timer.function = pmu_hd_blink_timeout; + device_create_file (dev, dev_attr_blinking_led); + + blinking_led = (strstr(saved_command_line,blinking_led) != NULL); + printk(KERN_INFO pmac blinking led initialized (blink %sactivated)\n, + blinking_led?:dis); + return 1; } @@ -1230,7 +1264,7 @@ 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-mdev-ofdev.dev)); if (pmu_ide_blink_enabled) hwif-led_act = pmu_hd_kick_blink;
Re: Hard Drive light
On Mon, Dec 20, 2004 at 10:04:03PM +1100, Cedric Pradalier wrote: According to Benjamin Herrenschmidt, on Mon, 20 Dec 2004 07:59:49 +0100, You mean in /sys/bus/macio/drivers/ide-pmac ? That's the driver entry, but the devices themselves have entries as well, I'd rather put a per-device switch in them (either through the actual device path, either PCI or macio, or the ide interface proper). The main reason is that I could not find how to add my sysfs in this directory since it is not created in the ide-pmac.c file (still a lot to learn :)). So I convinced myself that a platform device was not so irrelevant. It's totally irrelevant actually :) Ok I believe you. It is now attached to the macio device /sys/bus/macio/devices/0.0001f000\:ata-4/blinking_led Is this what you suggested? I chose macio since I had the struct device pointer easily accessible in pmac.c. Other thing strange to me is that I have to use the saved command line to know if there is a boot time argument. According to the comments around MODULE_PARM, a module parameter should be enough. I tried that, but the boot parameter never had any influence on the parameter. You must have typed it wrong ... One problem with ide-pmac though is that the module name is confusing since the file name itself is just pmac. I'm not too familiar with the module params thing tho, it may be possible to override the module name. I don't think so. Module parameters do not depend of the module name (as far as I understand from the doc). What I guess is that ide-pmac module is initialized too early to have correct boot parameters. For instance, I tried to had a dummy module_init function, and it is called much later than ide_pmac_setup_device (where blinking is initialized). Anybody can confirm or infirm this guess ? I was able to parse kernel command line parameters in the pci setup code (chrp_pci.c) which i believe happens *well* before what you are trying to do. It was some time back though, but was rather straigthforward. Cedric and Ben, please tell me when this is ready for inclusion in the debian powerpc kernels :) Friendly, Sven Luther
Re: Hard Drive light
I chose macio since I had the struct device pointer easily accessible in pmac.c. Not all laptops have the IDE under macio though ... some have it under a PCI node. It should be per-controller ideally... (some machines have both the macio one and the PCI one). I don't think so. Module parameters do not depend of the module name (as far as I understand from the doc). What I guess is that ide-pmac module is initialized too early to have correct boot parameters. For instance, I tried to had a dummy module_init function, and it is called much later than ide_pmac_setup_device (where blinking is initialized). Anybody can confirm or infirm this guess ? When a module is built-in, it's parameters are passed in the form modulename.paramname=value Furthermore I've looked at the way framebuffers manage boot params, and these module seems to be parsing the comand line themselves too. Framebuffers use a setup function, not a module param. Ben.
Re: Hard Drive light
On Mon, 2004-12-20 at 14:04 +0100, Sven Luther wrote: I was able to parse kernel command line parameters in the pci setup code (chrp_pci.c) which i believe happens *well* before what you are trying to do. It was some time back though, but was rather straigthforward. Cedric and Ben, please tell me when this is ready for inclusion in the debian powerpc kernels :) No, not at this point. Beside, i'd like to have it enabled by default, not disabled by default :) Ben.
Re: Hard Drive light
On Mon, Dec 20, 2004 at 05:01:40PM +0100, Benjamin Herrenschmidt wrote: On Mon, 2004-12-20 at 14:04 +0100, Sven Luther wrote: I was able to parse kernel command line parameters in the pci setup code (chrp_pci.c) which i believe happens *well* before what you are trying to do. It was some time back though, but was rather straigthforward. Cedric and Ben, please tell me when this is ready for inclusion in the debian powerpc kernels :) No, not at this point. Beside, i'd like to have it enabled by default, not disabled by default :) Yeah, noticed that, which is why is said when and not if ;) Friendly, Sven Luther
Re: Hard Drive light
I attach a very small patch doing that. This a patch only for drivers/ide/ppc/pmac.c file, with respect to a pure kernel 2.6.9 from kernel.org It adds an entry in /sys/devices/ide-pmac/blinking_led echo 1 /sys/devices/ide-pmac/blinking_led activates the led echo 0 /sys/devices/ide-pmac/blinking_led disactivates it It starts disactivated. BTW.. looked at the patch a bit more, why do you create a platform device just for that ? The mac-io IDE already has it's own sysfs entry Ben.
Re: Hard Drive light
According to Benjamin Herrenschmidt, on Sun, 19 Dec 2004 15:48:07 +0100, I attach a very small patch doing that. This a patch only for drivers/ide/ppc/pmac.c file, with respect to a pure kernel 2.6.9 from kernel.org It adds an entry in /sys/devices/ide-pmac/blinking_led echo 1 /sys/devices/ide-pmac/blinking_led activates the led echo 0 /sys/devices/ide-pmac/blinking_led disactivates it It starts disactivated. BTW.. looked at the patch a bit more, why do you create a platform device just for that ? The mac-io IDE already has it's own sysfs entry You mean in /sys/bus/macio/drivers/ide-pmac ? The main reason is that I could not find how to add my sysfs in this directory since it is not created in the ide-pmac.c file (still a lot to learn :)). So I convinced myself that a platform device was not so irrelevant. Just give me an hint or an example and I'll change that. Other thing strange to me is that I have to use the saved command line to know if there is a boot time argument. According to the comments around MODULE_PARM, a module parameter should be enough. I tried that, but the boot parameter never had any influence on the parameter. Thanks for your remark. -- Cedric Pradalier
[Patch #3] Re: Hard Drive light
According to David Pye, on Mon, 6 Dec 2004 20:46:15 +, On Monday 06 December 2004 13:31, Cedric Pradalier wrote: + printk (KERN_INFO pmac blinking led has been %sactivated\n, + blinking_led?:dis); You never get an easy life around here ;) I think that should be de, not dis :) Though would %s, blinking_led?enabled:disabled sound better? [...] You're right. So this is a version with the strings updated. Please excuse my english... Cheers, -- Cedric Pradalier --- pmac.c.orig 2004-12-06 00:27:50.0 +1100 +++ pmac.c 2004-12-07 00:03:44.0 +1100 @@ -35,6 +35,11 @@ #include linux/adb.h #include linux/pmu.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 @@ -381,9 +386,9 @@ static spinlock_t pmu_blink_lock; static unsigned long pmu_blink_stoptime; static int pmu_blink_ledstate; +static int blinking_led = 0; 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) @@ -413,6 +418,8 @@ pmu_hd_kick_blink(void *data, int rw) { unsigned long flags; + if (!blinking_led) + return; pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME; wmb(); @@ -428,9 +435,32 @@ spin_unlock_irqrestore(pmu_blink_lock, flags); } +static ssize_t show_blinkingled_activity(struct device *dev, char *buf)\ +{ + return sprintf(buf, %c\n, blinking_led?'1':'0'); +} + +static ssize_t set_blinkingled_activity(struct device *dev, + 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 has been %sabled\n, + blinking_led?en:dis); + return count; +} + +static DEVICE_ATTR (blinking_led, S_IRUGO | S_IWUSR, + show_blinkingled_activity, set_blinkingled_activity); + static int pmu_hd_blink_init(void) { + extern char saved_command_line[]; + struct device_node *np; + static struct of_device *of_hd_dev; struct device_node *dt; const char *model; @@ -458,6 +488,16 @@ init_timer(pmu_blink_timer); pmu_blink_timer.function = pmu_hd_blink_timeout; + np = of_find_node_by_name (NULL, disk); + if (!np) return 1; + of_hd_dev = of_platform_device_create (np, ide-pmac); + if (!of_hd_dev) return 1; + device_create_file (of_hd_dev-dev, dev_attr_blinking_led); + + blinking_led = (strstr(saved_command_line,blinking_led) != NULL); + printk(KERN_INFO pmac blinking led initialized (blink %sabled)\n, + blinking_led?en:dis); + return 1; }
Re: [Patch #3] Re: Hard Drive light
On 07 Dec 2004 at 23h12, Cedric Pradalier wrote: Hi, + return sprintf(buf, %c\n, blinking_led?'1':'0'); Really, this is useless and ugly :) -- Colin
[Patch] Re: Hard Drive light
According to Jesus Climent, on Mon, 6 Dec 2004 01:44:17 +0100, On Mon, Dec 06, 2004 at 12:28:24AM +1100, Cedric Pradalier wrote: I attach a very small patch doing that. This a patch only for drivers/ide/ppc/pmac.c file, with respect to a pure kernel 2.6.9 from kernel.org It adds an entry in /sys/devices/ide-pmac/blinking_led echo 1 /sys/devices/ide-pmac/blinking_led activates the led echo 0 /sys/devices/ide-pmac/blinking_led disactivates it It starts disactivated. Can it be configurable? I would like to see it from the very start of the kernel run. Actually, it is not possible to activate it at the very start. You have to wait the ide-pmac initialisation, that is one or two seconds after boot ;o) In this version of the patch, if your boot your kernel with the argument blinking_led (append=blinking_led in yaboot.conf), the led blinks. The /sys interface stays the same. I've used a more standard way of managing boolean... ;o) -- Cedric Pradalier --- pmac.c.orig 2004-12-06 00:27:50.0 +1100 +++ pmac.c 2004-12-07 00:03:44.0 +1100 @@ -35,6 +35,11 @@ #include linux/adb.h #include linux/pmu.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 @@ -381,9 +386,9 @@ static spinlock_t pmu_blink_lock; static unsigned long pmu_blink_stoptime; static int pmu_blink_ledstate; +static int blinking_led = 0; 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) @@ -413,6 +418,8 @@ pmu_hd_kick_blink(void *data, int rw) { unsigned long flags; + if (!blinking_led) + return; pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME; wmb(); @@ -428,9 +435,32 @@ spin_unlock_irqrestore(pmu_blink_lock, flags); } +static ssize_t show_blinkingled_activity(struct device *dev, char *buf)\ +{ + return sprintf(buf, %c\n, blinking_led?'1':'0'); +} + +static ssize_t set_blinkingled_activity(struct device *dev, + 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 has been %sactivated\n, + blinking_led?:dis); + return count; +} + +static DEVICE_ATTR (blinking_led, S_IRUGO | S_IWUSR, + show_blinkingled_activity, set_blinkingled_activity); + static int pmu_hd_blink_init(void) { + extern char saved_command_line[]; + struct device_node *np; + static struct of_device *of_hd_dev; struct device_node *dt; const char *model; @@ -458,6 +488,16 @@ init_timer(pmu_blink_timer); pmu_blink_timer.function = pmu_hd_blink_timeout; + np = of_find_node_by_name (NULL, disk); + if (!np) return 1; + of_hd_dev = of_platform_device_create (np, ide-pmac); + if (!of_hd_dev) return 1; + device_create_file (of_hd_dev-dev, dev_attr_blinking_led); + + blinking_led = (strstr(saved_command_line,blinking_led) != NULL); + printk(KERN_INFO pmac blinking led initialized (blink %sactivated)\n, + blinking_led?:dis); + return 1; }
Re: Hard Drive light
According to Benjamin Herrenschmidt, on Sat, 04 Dec 2004 09:22:36 +1100, On Fri, 2004-12-03 at 10:53 +0100, Jens Schmalzing wrote: Hi, Benjamin Herrenschmidt writes: Any reason why it's disabled ? When it was off, people asked for it to be on. When it was on, others asked for it to be off. Eventually, we even got a bug report and then turned it off for good and put an entry into the changelog. Oh well, I'd gladly accept a patch adding some entry in sysfs to control the behaviour :) Ben. I attach a very small patch doing that. This a patch only for drivers/ide/ppc/pmac.c file, with respect to a pure kernel 2.6.9 from kernel.org It adds an entry in /sys/devices/ide-pmac/blinking_led echo 1 /sys/devices/ide-pmac/blinking_led activates the led echo 0 /sys/devices/ide-pmac/blinking_led disactivates it It starts disactivated. I could not find any place to put the code for removing the sysfs entry. Anyway I guess, this file cannot be compiled as a module, and so cannot be removed. So this should not be a problem. Regards, -- Cedric Pradalier --- pmac.c.old 2004-12-05 23:58:25.274167448 +1100 +++ pmac.c 2004-12-05 23:55:29.458895432 +1100 @@ -35,6 +35,11 @@ #include linux/adb.h #include linux/pmu.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 @@ -381,8 +386,9 @@ static spinlock_t pmu_blink_lock; static unsigned long pmu_blink_stoptime; static int pmu_blink_ledstate; +static int pmu_blink_led_activated = 0; static struct timer_list pmu_blink_timer; -static int pmu_ide_blink_enabled; +static int pmu_ide_blink_enabled = 0; static void @@ -413,6 +419,8 @@ pmu_hd_kick_blink(void *data, int rw) { unsigned long flags; + if (!pmu_blink_led_activated) + return; pmu_blink_stoptime = jiffies + PMU_HD_BLINK_TIME; wmb(); @@ -428,9 +436,32 @@ spin_unlock_irqrestore(pmu_blink_lock, flags); } +static ssize_t show_blinkingled_activity(struct device *dev, char *buf)\ +{ + return sprintf(buf, %c\n, pmu_blink_led_activated?'1':'0'); +} + +static ssize_t set_blinkingled_activity(struct device *dev, + const char *buf, size_t count) +{ + int newact; + if (sscanf (buf, %d, newact) != 1) + return -EINVAL; + pmu_blink_led_activated = newact?1:0; + printk (KERN_INFO + pmac blinking led has been %sactivated\n, + pmu_blink_led_activated?:dis); + return count; +} + +static DEVICE_ATTR (blinking_led, S_IRUGO | S_IWUSR, + show_blinkingled_activity, set_blinkingled_activity); + static int pmu_hd_blink_init(void) { + struct device_node *np; + static struct of_device *of_hd_dev; struct device_node *dt; const char *model; @@ -458,6 +489,21 @@ init_timer(pmu_blink_timer); pmu_blink_timer.function = pmu_hd_blink_timeout; + np = of_find_node_by_name (NULL, disk); + if (!np) return 1; + of_hd_dev = of_platform_device_create (np, ide-pmac); + if (!of_hd_dev) return 1; + device_create_file (of_hd_dev-dev, dev_attr_blinking_led); + + /** Should be used to remove sysfs entry ** +* Don't know where though... * + if (of_hd_dev) { + device_remove_file (of_hd_dev-dev, dev_attr_blinking_led); + of_device_unregister (of_hd_dev); + } +* */ + + return 1; }
Re: Hard Drive light
On 06 Dec 2004 at 00h12, Cedric Pradalier wrote: Hi Cedric, + return sprintf(buf, %c\n, pmu_blink_led_activated?'1':'0'); why not simply sprintf(buf, %d\n, pmu_blink_led_activated) ? + pmu_blink_led_activated = newact?1:0; pmu_blink_led_activated = (newact != 0); is enough and more nice imho. + device_create_file (of_hd_dev-dev, dev_attr_blinking_led); where is dev_attr_blinking_led defined ? -- Colin http://ayttm.sf.net/ : Chat with your friends via msn, aim, yahoo, and more
Re: Hard Drive light
According to Colin Leroy, on Sun, 5 Dec 2004 14:42:33 +0100, On 06 Dec 2004 at 00h12, Cedric Pradalier wrote: Hi Cedric, +return sprintf(buf, %c\n, pmu_blink_led_activated?'1':'0'); why not simply sprintf(buf, %d\n, pmu_blink_led_activated) ? I like to ensure that whatever the implementation of this boolean, the textual output stays the same. +pmu_blink_led_activated = newact?1:0; pmu_blink_led_activated = (newact != 0); is enough and more nice imho. I wanted to have either 1 or 0 in this boolean, not whatever int sent in the buffer. +device_create_file (of_hd_dev-dev, dev_attr_blinking_led); where is dev_attr_blinking_led defined ? +static DEVICE_ATTR (blinking_led, S_IRUGO | S_IWUSR, + show_blinkingled_activity, set_blinkingled_activity); Note that the patch is made from a compiled and tested pmac.c ;o) -- Cedric Pradalier Research Engineer CSIRO - ICT Centre
Re: Hard Drive light
On Sunday 05 December 2004 20:44, Cedric Pradalier wrote: +pmu_blink_led_activated = newact?1:0; pmu_blink_led_activated = (newact != 0); is enough and more nice imho. I wanted to have either 1 or 0 in this boolean, not whatever int sent in the buffer. Hi, (newact != 0) returns either: 0: false - newact is zero. 1: true - newact is not zero. ie, obviously, it's a true/false comparison. So, that means pmu_blink_led_activated will be either 1 or 0, not whatever value newact contains, surely? David -- -BEGIN GEEK CODE BLOCK- Version: 3.12 GCS d- s-: a-- C++ UL P L+++ E--- W++ N+ o+ K- w--- O M V- PS+ PE+ Y+ PGP t 5- X+ R- tv+ b+ DI++ D+ G+ e++ h--- r++ y++ --END GEEK CODE BLOCK--
Re: Hard Drive light
On Mon, Dec 06, 2004 at 12:28:24AM +1100, Cedric Pradalier wrote: I attach a very small patch doing that. This a patch only for drivers/ide/ppc/pmac.c file, with respect to a pure kernel 2.6.9 from kernel.org It adds an entry in /sys/devices/ide-pmac/blinking_led echo 1 /sys/devices/ide-pmac/blinking_led activates the led echo 0 /sys/devices/ide-pmac/blinking_led disactivates it It starts disactivated. Can it be configurable? I would like to see it from the very start of the kernel run. -- Jesus Climent info:www.pumuki.org Unix SysAdm|Linux User #66350|Debian Developer|2.4.27|Helsinki Finland GPG: 1024D/86946D69 BB64 2339 1CAA 7064 E429 7E18 66FC 1D7F 8694 6D69 Allow myself to introduce... myself. --Austin Powers (Austin Powers: International Man of Mystery)
Re: Hard Drive light
Hi, Benjamin Herrenschmidt writes: Any reason why it's disabled ? When it was off, people asked for it to be on. When it was on, others asked for it to be off. Eventually, we even got a bug report and then turned it off for good and put an entry into the changelog. Regards, Jens. -- J'qbpbe, le m'en fquz pe j'qbpbe! Le veux aimeb et mqubib panz je pézqbpbe je djuz tqtaj!
Re: Hard Drive light
According to Jens Schmalzing, on 03 Dec 2004 10:53:31 +0100, Hi, Benjamin Herrenschmidt writes: Any reason why it's disabled ? When it was off, people asked for it to be on. When it was on, others asked for it to be off. Eventually, we even got a bug report and then turned it off for good and put an entry into the changelog. Regards, Jens. So I think it would make sense to make this a dynamic feature. I can do it, but I would like to be blessed by the gurus before. Which interface would you suggest ? /proc, /sys, sysctl or ioctl ? Just tell me, I'll implement the preferred one. Regards -- Cedric Pradalier
Re: Hard Drive light
On Thu, Dec 02, 2004 at 08:04:48AM +0100, Jens Schmalzing wrote: Hi, Andrea Giusto writes: nice feature: the suspend light located on the display opening button (I own a PB12'') acted as a Hard Drive light, just as on my old 586! Do we have something like that in Debian kernels? It is disabled in the official kernel. You can roll your own kernel of course, which in Debian is very easy thanks to kernel-package, and include the feature. Well, actually it was enabled previously, but some (2) people complained, and it was removed again. There was some discussion of making the feature configurable either with a boot param switch (easier way) or with a /proc entry, but i don't know what it did give. And don't remember who was working on it, a french guy, if i remember well. Friendly, Sven Luther
Re: Hard Drive light
On Fri, 2004-12-03 at 10:53 +0100, Jens Schmalzing wrote: Hi, Benjamin Herrenschmidt writes: Any reason why it's disabled ? When it was off, people asked for it to be on. When it was on, others asked for it to be off. Eventually, we even got a bug report and then turned it off for good and put an entry into the changelog. Oh well, I'd gladly accept a patch adding some entry in sysfs to control the behaviour :) Ben.
Re: Hard Drive light
Hi, Andrea Giusto writes: nice feature: the suspend light located on the display opening button (I own a PB12'') acted as a Hard Drive light, just as on my old 586! Do we have something like that in Debian kernels? It is disabled in the official kernel. You can roll your own kernel of course, which in Debian is very easy thanks to kernel-package, and include the feature. Regards, Jens. -- J'qbpbe, le m'en fquz pe j'qbpbe! Le veux aimeb et mqubib panz je pézqbpbe je djuz tqtaj!
Re: Hard Drive light
According to Jens Schmalzing, on 02 Dec 2004 08:04:48 +0100, Hi, Andrea Giusto writes: nice feature: the suspend light located on the display opening button (I own a PB12'') acted as a Hard Drive light, just as on my old 586! Do we have something like that in Debian kernels? It is disabled in the official kernel. You can roll your own kernel of course, which in Debian is very easy thanks to kernel-package, and include the feature. Regards, Jens. I'm just wondering if it could be made activable with a sysctl or through a /sys entry ? There have been several people asking for it when it was removed, or asking for removal when it was included. I should be able to make this kind of patch if needed. Regards -- Cedric Pradalier
Re: Hard Drive light
On Thu, Dec 02, 2004 at 06:41:39PM +1100, Cedric Pradalier wrote: There have been several people asking for it when it was removed, or asking for removal when it was included. I should be able to make this kind of patch if needed. That would be great, if such an interface existed... Sometimes is anoying to have the light blinking (dark rooms, for example). -- Jesus Climent info:www.pumuki.org Unix SysAdm|Linux User #66350|Debian Developer|2.4.27|Helsinki Finland GPG: 1024D/86946D69 BB64 2339 1CAA 7064 E429 7E18 66FC 1D7F 8694 6D69 That's thirty minutes away. I'll be there in ten. --Wolf (Pulp Fiction)
Re: Hard Drive light
On 02 Dec 2004 at 17h12, Jesus Climent wrote: Hi, There have been several people asking for it when it was removed, or asking for removal when it was included. I should be able to make this kind of patch if needed. That would be great, if such an interface existed... Sometimes is anoying to have the light blinking (dark rooms, for example). Or dvd-watching session :) -- Colin
Re: Hard Drive light
On Thu, 2004-12-02 at 08:04 +0100, Jens Schmalzing wrote: Hi, Andrea Giusto writes: nice feature: the suspend light located on the display opening button (I own a PB12'') acted as a Hard Drive light, just as on my old 586! Do we have something like that in Debian kernels? It is disabled in the official kernel. You can roll your own kernel of course, which in Debian is very easy thanks to kernel-package, and include the feature. Any reason why it's disabled ? Ben.
Re: Hard Drive light
On Dec 03 2004, Benjamin Herrenschmidt wrote: On Thu, 2004-12-02 at 08:04 +0100, Jens Schmalzing wrote: It is disabled in the official kernel. You can roll your own kernel of course, which in Debian is very easy thanks to kernel-package, and include the feature. Any reason why it's disabled ? When I first saw that feature, I thought it was great. But after 5 minutes of use, I thought that I was going to have an epileptic attack. :-) (In)sanely yours, Rogério Brito. -- Learn to quote e-mails decently at: http://pub.tsn.dk/how-to-quote.php http://learn.to/quote http://www.xs4all.nl/~sbpoley/toppost.htm
Hard Drive light
Before switching to Debian I was a Gentoo user, and I got a kernel (2.6.7 or .8 I don't remember exactly) that had a nice feature: the suspend light located on the display opening button (I own a PB12'') acted as a Hard Drive light, just as on my old 586! Do we have something like that in Debian kernels?