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@xxxxxxxxxx wrote:
> > > From: Helge Deller <deller@xxxxxx>
> > >
> > > 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@xxxxxx>
> > > Cc: <stable@xxxxxxxxxxxxxxx>
> > > ---
> > > 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
