From: Helge Deller deller@gmx.de
Older PA-RISC machines have LEDs which show the disk- and LAN-activity. The computation is done in software and takes quite some time, e.g. on a J6500 this may take up to 60% time of one CPU if the machine is loaded via network traffic.
Since most people don't care about the LEDs, start with LEDs disabled and just show a CPU heartbeat LED. The disk and LAN LEDs can be turned on manually via /proc/pdc/led.
Signed-off-by: Helge Deller deller@gmx.de Cc: stable@vger.kernel.org --- drivers/parisc/led.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c index 8bdc5e043831..765f19608f60 100644 --- a/drivers/parisc/led.c +++ b/drivers/parisc/led.c @@ -56,8 +56,8 @@ static int led_type __read_mostly = -1; static unsigned char lastleds; /* LED state from most recent update */ static unsigned int led_heartbeat __read_mostly = 1; -static unsigned int led_diskio __read_mostly = 1; -static unsigned int led_lanrxtx __read_mostly = 1; +static unsigned int led_diskio __read_mostly; +static unsigned int led_lanrxtx __read_mostly; static char lcd_text[32] __read_mostly; static char lcd_text_default[32] __read_mostly; static int lcd_no_led_support __read_mostly = 0; /* KittyHawk doesn't support LED on its LCD */ @@ -589,6 +589,9 @@ int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long d return 1; } + pr_info("LED: Enable disk and LAN activity LEDs " + "via /proc/pdc/led\n"); + /* mark the LCD/LED driver now as initialized and * register to the reboot notifier chain */ initialized++;
On Fri, Aug 25, 2023 at 08:09:26PM +0200, deller@kernel.org wrote:
From: Helge Deller deller@gmx.de
Older PA-RISC machines have LEDs which show the disk- and LAN-activity. The computation is done in software and takes quite some time, e.g. on a J6500 this may take up to 60% time of one CPU if the machine is loaded via network traffic.
Since most people don't care about the LEDs, start with LEDs disabled and just show a CPU heartbeat LED. The disk and LAN LEDs can be turned on manually via /proc/pdc/led.
Signed-off-by: Helge Deller deller@gmx.de Cc: stable@vger.kernel.org
drivers/parisc/led.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c index 8bdc5e043831..765f19608f60 100644 --- a/drivers/parisc/led.c +++ b/drivers/parisc/led.c @@ -56,8 +56,8 @@ static int led_type __read_mostly = -1; static unsigned char lastleds; /* LED state from most recent update */ static unsigned int led_heartbeat __read_mostly = 1; -static unsigned int led_diskio __read_mostly = 1; -static unsigned int led_lanrxtx __read_mostly = 1; +static unsigned int led_diskio __read_mostly; +static unsigned int led_lanrxtx __read_mostly; static char lcd_text[32] __read_mostly; static char lcd_text_default[32] __read_mostly; static int lcd_no_led_support __read_mostly = 0; /* KittyHawk doesn't support LED on its LCD */ @@ -589,6 +589,9 @@ int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long d return 1; }
- pr_info("LED: Enable disk and LAN activity LEDs "
"via /proc/pdc/led\n");
When drivers are working properly, they should be quiet. Who is going to see this message?
I don't even understand it, are you saying that you now need to go enable the led through proc? And why are leds in proc, I thought they had a real class for them? Why not use that instead?
And finally, you shouldn't split strings across lines :)
thanks,
greg k-h
On 8/26/23 09:34, Greg KH wrote:
On Fri, Aug 25, 2023 at 08:09:26PM +0200, deller@kernel.org wrote:
From: Helge Deller deller@gmx.de
Older PA-RISC machines have LEDs which show the disk- and LAN-activity. The computation is done in software and takes quite some time, e.g. on a J6500 this may take up to 60% time of one CPU if the machine is loaded via network traffic.
Since most people don't care about the LEDs, start with LEDs disabled and just show a CPU heartbeat LED. The disk and LAN LEDs can be turned on manually via /proc/pdc/led.
Signed-off-by: Helge Deller deller@gmx.de Cc: stable@vger.kernel.org
drivers/parisc/led.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c index 8bdc5e043831..765f19608f60 100644 --- a/drivers/parisc/led.c +++ b/drivers/parisc/led.c @@ -56,8 +56,8 @@ static int led_type __read_mostly = -1; static unsigned char lastleds; /* LED state from most recent update */ static unsigned int led_heartbeat __read_mostly = 1; -static unsigned int led_diskio __read_mostly = 1; -static unsigned int led_lanrxtx __read_mostly = 1; +static unsigned int led_diskio __read_mostly; +static unsigned int led_lanrxtx __read_mostly; static char lcd_text[32] __read_mostly; static char lcd_text_default[32] __read_mostly; static int lcd_no_led_support __read_mostly = 0; /* KittyHawk doesn't support LED on its LCD */ @@ -589,6 +589,9 @@ int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long d return 1; }
- pr_info("LED: Enable disk and LAN activity LEDs "
"via /proc/pdc/led\n");When drivers are working properly, they should be quiet. Who is going to see this message?
That patch shouldn't have gone to stable@ yet... git-send-patch just pulled the CC in and I didn't noticed. So, please don't apply it yet.
Anyway, yes, I can drop this info.
I don't even understand it, are you saying that you now need to go enable the led through proc? And why are leds in proc, I thought they had a real class for them? Why not use that instead?
This is an old driver from the very beginning, and I don't want to change much in it for old kernels other than to reduce the CPU overhead it generates.
Additionally I've started a rewrite of the driver (in my for-next tree), and I did look at the led class and led trigger classes. If it fits, I'll convert to those, but at least for hdd activity it seems only IDE/ATA activity is monitored and SCSI activity is missing. Anyway, this is still work-in-progress...
And finally, you shouldn't split strings across lines :)
Ok.
Thanks! Helge
On Sat, Aug 26, 2023 at 09:54:55AM +0200, Helge Deller wrote:
On 8/26/23 09:34, Greg KH wrote:
On Fri, Aug 25, 2023 at 08:09:26PM +0200, deller@kernel.org wrote:
From: Helge Deller deller@gmx.de
Older PA-RISC machines have LEDs which show the disk- and LAN-activity. The computation is done in software and takes quite some time, e.g. on a J6500 this may take up to 60% time of one CPU if the machine is loaded via network traffic.
Since most people don't care about the LEDs, start with LEDs disabled and just show a CPU heartbeat LED. The disk and LAN LEDs can be turned on manually via /proc/pdc/led.
Signed-off-by: Helge Deller deller@gmx.de Cc: stable@vger.kernel.org
drivers/parisc/led.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c index 8bdc5e043831..765f19608f60 100644 --- a/drivers/parisc/led.c +++ b/drivers/parisc/led.c @@ -56,8 +56,8 @@ static int led_type __read_mostly = -1; static unsigned char lastleds; /* LED state from most recent update */ static unsigned int led_heartbeat __read_mostly = 1; -static unsigned int led_diskio __read_mostly = 1; -static unsigned int led_lanrxtx __read_mostly = 1; +static unsigned int led_diskio __read_mostly; +static unsigned int led_lanrxtx __read_mostly; static char lcd_text[32] __read_mostly; static char lcd_text_default[32] __read_mostly; static int lcd_no_led_support __read_mostly = 0; /* KittyHawk doesn't support LED on its LCD */ @@ -589,6 +589,9 @@ int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long d return 1; }
- pr_info("LED: Enable disk and LAN activity LEDs "
"via /proc/pdc/led\n");When drivers are working properly, they should be quiet. Who is going to see this message?
That patch shouldn't have gone to stable@ yet... git-send-patch just pulled the CC in and I didn't noticed. So, please don't apply it yet.
I will not, and it's fine to cc: stable@ on stuff that is still making it's way into the kernel tree. It gives us a heads-up on stuff, AND sometimes it gives you an additional free review :)
I don't even understand it, are you saying that you now need to go enable the led through proc? And why are leds in proc, I thought they had a real class for them? Why not use that instead?
This is an old driver from the very beginning, and I don't want to change much in it for old kernels other than to reduce the CPU overhead it generates.
Ah, makes sense, that is a crazy amount of cpu time for a blinking led.
How about just default to it off (like the first chunk you have here), which will go to stable trees, and then a rewrite to use the proper LED api? Or not, your call, I doubt many people actually have this hardware to care about it...
thanks,
greg k-h
On 8/26/23 09:58, Greg KH wrote:
On Sat, Aug 26, 2023 at 09:54:55AM +0200, Helge Deller wrote:
On 8/26/23 09:34, Greg KH wrote:
On Fri, Aug 25, 2023 at 08:09:26PM +0200, deller@kernel.org wrote:
From: Helge Deller deller@gmx.de
Older PA-RISC machines have LEDs which show the disk- and LAN-activity. The computation is done in software and takes quite some time, e.g. on a J6500 this may take up to 60% time of one CPU if the machine is loaded via network traffic.
Since most people don't care about the LEDs, start with LEDs disabled and just show a CPU heartbeat LED. The disk and LAN LEDs can be turned on manually via /proc/pdc/led.
Signed-off-by: Helge Deller deller@gmx.de Cc: stable@vger.kernel.org
drivers/parisc/led.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c index 8bdc5e043831..765f19608f60 100644 --- a/drivers/parisc/led.c +++ b/drivers/parisc/led.c @@ -56,8 +56,8 @@ static int led_type __read_mostly = -1; static unsigned char lastleds; /* LED state from most recent update */ static unsigned int led_heartbeat __read_mostly = 1; -static unsigned int led_diskio __read_mostly = 1; -static unsigned int led_lanrxtx __read_mostly = 1; +static unsigned int led_diskio __read_mostly; +static unsigned int led_lanrxtx __read_mostly; static char lcd_text[32] __read_mostly; static char lcd_text_default[32] __read_mostly; static int lcd_no_led_support __read_mostly = 0; /* KittyHawk doesn't support LED on its LCD */ @@ -589,6 +589,9 @@ int __init register_led_driver(int model, unsigned long cmd_reg, unsigned long d return 1; }
- pr_info("LED: Enable disk and LAN activity LEDs "
"via /proc/pdc/led\n");When drivers are working properly, they should be quiet. Who is going to see this message?
That patch shouldn't have gone to stable@ yet... git-send-patch just pulled the CC in and I didn't noticed. So, please don't apply it yet.
I will not, and it's fine to cc: stable@ on stuff that is still making it's way into the kernel tree. It gives us a heads-up on stuff, AND sometimes it gives you an additional free review :)
Yes, thanks! :-)
I don't even understand it, are you saying that you now need to go enable the led through proc? And why are leds in proc, I thought they had a real class for them? Why not use that instead?
This is an old driver from the very beginning, and I don't want to change much in it for old kernels other than to reduce the CPU overhead it generates.
Ah, makes sense, that is a crazy amount of cpu time for a blinking led.
How about just default to it off (like the first chunk you have here), which will go to stable trees,
Yes, that was the idea of this patch. I'll drop the pr_info() next time.
and then a rewrite to use the proper LED api?
Yes.
Helge
linux-stable-mirror@lists.linaro.org
-
deller＠kernel.org -
Greg KH -
Helge Deller