[parisc-linux] suppress led task for KittyHawk's LCD

[parisc-linux] suppress led task for KittyHawk's LCD

[Guy Martin]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]


Hi All,

I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
After some investigation, it looked like the code updating the led status was filling the LCD.

I believe there is not led support on K class but I didn't checked.

The attached patch disable the led_task, which is updating the led status, for KittyHawk hardware.

Thanks for reviewing :)

  Guy


-- 
Guy Martin
Gentoo Linux - HPPA port Lead


[-- Attachment #2: no-led-on-kittyhawk.patch --]
[-- Type: text/x-patch, Size: 1510 bytes --]

diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c
index 8dac2ba..29cb1fe 100644
--- a/drivers/parisc/led.c
+++ b/drivers/parisc/led.c
@@ -56,6 +56,7 @@ #include <asm/uaccess.h>
    relatively large amount of CPU time, some of the calculations can be 
    turned off with the following variables (controlled via procfs) */
 
+static int no_led = 0;
 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;
@@ -65,7 +66,7 @@ static char lcd_text[32]          __read
 static char lcd_text_default[32]  __read_mostly;
 
 
-static struct workqueue_struct *led_wq;
+static struct workqueue_struct *led_wq = NULL;
 static void led_work_func(void *);
 static DECLARE_WORK(led_task, led_work_func, NULL);
 
@@ -134,6 +135,9 @@ static int start_task(void) 
 	/* Display the default text now */
 	if (led_type == LED_HASLCD) lcd_print( lcd_text_default );
 
+	/* There is no led support on KittyHawk */
+	if (no_led) return 0;
+
 	/* Create the work queue and queue the LED task */
 	led_wq = create_singlethread_workqueue("led_wq");	
 	queue_work(led_wq, &led_task);
@@ -695,6 +699,7 @@ int __init led_init(void)
 	case 0x58B:		/* KittyHawk DC2 100 (K200) */
 		printk(KERN_INFO "%s: KittyHawk-Machine (hversion 0x%x) found, "
 				"LED detection skipped.\n", __FILE__, CPU_HVERSION);
+		no_led = 1;	/* no led support on KittyHawk */
 		goto found;	/* use the preinitialized values of lcd_info */
 	}
 

[-- Attachment #3: Type: text/plain, Size: 169 bytes --]

_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Matthew Wilcox]

On Sun, Nov 12, 2006 at 01:45:35AM +0100, Guy Martin wrote:
> I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
> After some investigation, it looked like the code updating the led status was filling the LCD.
> 
> I believe there is not led support on K class but I didn't checked.
> 
> The attached patch disable the led_task, which is updating the led status, for KittyHawk hardware.

That's clearly the wrong solution.  The led_task is responsible for
updating both LEDs and LCDs.  It should be calling led_LCD_driver()
which should be putting the image of a beating heart on the LCD panel.

It would be great if you could do some more debugging and figure out
what's going on here.
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Guy Martin]

Hi Matthew,

As far as I understand the code, led_LCD_driver() is only in charge of updating the led status. The led being a true led or a char on the LCD.
I can't find any documentation stating that the LCD on the K410 supports those chars. It only displays CPU01234 depending on the number of CPU you have at boot time.
The update of the LCD text is done via lcd_print() which doesn't use led_LCD_driver(). It only uses a reference to it to check if the device is correctly initialized.

If you tell me that led_LCD_driver() is supposed to work and it should display the right leds character, then led_LCD_driver() is buggy and needs to be fixed. Currently each time it's called, a char of the LCD become black for each leds that need updating. It goes from left to right and starts from the bottom row and then occure at the top row a bit later. If you echo something to /proc/pdc/lcd, you'll see the text for a few and then being overwritten.


Also, I actually sent the wrong patch. With the current version of the driver, only the odd chars from the string you pass is written on the lcd.
You need to set the min_cmd_delay to 80t of fix this.

@@ -695,6 +699,8 @@ int __init led_init(void)
        case 0x58B:             /* KittyHawk DC2 100 (K200) */
                printk(KERN_INFO "%s: KittyHawk-Machine (hversion 0x%x) found, "
                                "LED detection skipped.\n", __FILE__, CPU_HVERSION);
+               lcd_info.min_cmd_delay = 80;
+               no_led = 1;     /* no led support on KittyHawk */
                goto found;     /* use the preinitialized values of lcd_info */
        }
 

Regards,
  Guy



On Sat, 11 Nov 2006 19:07:38 -0700
Matthew Wilcox <matthew@wil.cx> wrote:

> On Sun, Nov 12, 2006 at 01:45:35AM +0100, Guy Martin wrote:
> > I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
> > After some investigation, it looked like the code updating the led status was filling the LCD.
> > 
> > I believe there is not led support on K class but I didn't checked.
> > 
> > The attached patch disable the led_task, which is updating the led status, for KittyHawk hardware.
> 
> That's clearly the wrong solution.  The led_task is responsible for
> updating both LEDs and LCDs.  It should be calling led_LCD_driver()
> which should be putting the image of a beating heart on the LCD panel.
> 
> It would be great if you could do some more debugging and figure out
> what's going on here.

_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Helge Deller]

Hi Guy,

On Sunday 12 November 2006 01:45, Guy Martin wrote:
> I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
> After some investigation, it looked like the code updating the led status was filling the LCD.
....

instead of adding:
+               no_led = 1;     /* no led support on KittyHawk */
                goto found;     /* use the preinitialized values of lcd_info */

did you tried to comment out the "goto found" ?

Helge
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Guy Martin]

Hi Helge,

This can't work. If you don't call register_led_driver(), led_func_ptr and led_type stay NULL and lcd_print() never prints anything.
You can change lcd_print() not to check led_func_ptr but I fear something may break if the device is not initialized :)

Maybe we could use static int initialized defined in register_led_driver and make it global to check if the device is ready or not.

  Guy


On Sun, 12 Nov 2006 15:04:38 +0100
Helge Deller <deller@gmx.de> wrote:

> Hi Guy,
> 
> On Sunday 12 November 2006 01:45, Guy Martin wrote:
> > I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
> > After some investigation, it looked like the code updating the led status was filling the LCD.
> ....
> 
> instead of adding:
> +               no_led = 1;     /* no led support on KittyHawk */
>                 goto found;     /* use the preinitialized values of lcd_info */
> 
> did you tried to comment out the "goto found" ?
> 
> Helge

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Helge Deller]

Hi Guy,

I think you misunderstood me.
My idea was that you check what your machine's BIOS (PDC) returns for pdc_chassis_info().

So, if you would remove this code from led_init() you may test the results without this "workaround for KittyHawk":
        /* Work around the buggy PDC of KittyHawk-machines */
        switch (CPU_HVERSION) {
        case 0x580:             /* KittyHawk DC2-100 (K100) */
        case 0x581:             /* KittyHawk DC3-120 (K210) */
        case 0x582:             /* KittyHawk DC3 100 (K400) */
        case 0x583:             /* KittyHawk DC3 120 (K410) */
        case 0x58B:             /* KittyHawk DC2 100 (K200) */
                printk(KERN_INFO "%s: KittyHawk-Machine (hversion 0x%x) found, "
                                "LED detection skipped.\n", __FILE__, CPU_HVERSION);
                goto found;     /* use the preinitialized values of lcd_info */
        }

Helge


On Sunday 12 November 2006 15:50, Guy Martin wrote:
> 
> Hi Helge,
> 
> This can't work. If you don't call register_led_driver(), led_func_ptr and led_type stay NULL and lcd_print() never prints anything.
> You can change lcd_print() not to check led_func_ptr but I fear something may break if the device is not initialized :)
> 
> Maybe we could use static int initialized defined in register_led_driver and make it global to check if the device is ready or not.
> 
>   Guy
> 
> 
> On Sun, 12 Nov 2006 15:04:38 +0100
> Helge Deller <deller@gmx.de> wrote:
> 
> > Hi Guy,
> > 
> > On Sunday 12 November 2006 01:45, Guy Martin wrote:
> > > I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
> > > After some investigation, it looked like the code updating the led status was filling the LCD.
> > ....
> > 
> > instead of adding:
> > +               no_led = 1;     /* no led support on KittyHawk */
> >                 goto found;     /* use the preinitialized values of lcd_info */
> > 
> > did you tried to comment out the "goto found" ?
> > 
> > Helge
> 
> 
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Guy Martin]

Hi Helge,

Sorry for the missunderstanding.

It says this :
[42949382.500000] LED display at ffd0c000 registered

And nothing can be seen on the lcd since it's not detected and disabled.
/proc/pdc/lcd doesn't exists.

Do you know if this lcd is supposed to display the leds character ?


  Guy


On Sun, 12 Nov 2006 16:05:07 +0100
Helge Deller <deller@gmx.de> wrote:

> Hi Guy,
> 
> I think you misunderstood me.
> My idea was that you check what your machine's BIOS (PDC) returns for pdc_chassis_info().
> 
> So, if you would remove this code from led_init() you may test the results without this "workaround for KittyHawk":
>         /* Work around the buggy PDC of KittyHawk-machines */
>         switch (CPU_HVERSION) {
>         case 0x580:             /* KittyHawk DC2-100 (K100) */
>         case 0x581:             /* KittyHawk DC3-120 (K210) */
>         case 0x582:             /* KittyHawk DC3 100 (K400) */
>         case 0x583:             /* KittyHawk DC3 120 (K410) */
>         case 0x58B:             /* KittyHawk DC2 100 (K200) */
>                 printk(KERN_INFO "%s: KittyHawk-Machine (hversion 0x%x) found, "
>                                 "LED detection skipped.\n", __FILE__, CPU_HVERSION);
>                 goto found;     /* use the preinitialized values of lcd_info */
>         }
> 
> Helge
> 
> 
> On Sunday 12 November 2006 15:50, Guy Martin wrote:
> > 
> > Hi Helge,
> > 
> > This can't work. If you don't call register_led_driver(), led_func_ptr and led_type stay NULL and lcd_print() never prints anything.
> > You can change lcd_print() not to check led_func_ptr but I fear something may break if the device is not initialized :)
> > 
> > Maybe we could use static int initialized defined in register_led_driver and make it global to check if the device is ready or not.
> > 
> >   Guy
> > 
> > 
> > On Sun, 12 Nov 2006 15:04:38 +0100
> > Helge Deller <deller@gmx.de> wrote:
> > 
> > > Hi Guy,
> > > 
> > > On Sunday 12 November 2006 01:45, Guy Martin wrote:
> > > > I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
> > > > After some investigation, it looked like the code updating the led status was filling the LCD.
> > > ....
> > > 
> > > instead of adding:
> > > +               no_led = 1;     /* no led support on KittyHawk */
> > >                 goto found;     /* use the preinitialized values of lcd_info */
> > > 
> > > did you tried to comment out the "goto found" ?
> > > 
> > > Helge
> > 
> > 

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Helge Deller]

Hi Guy,

I'm not sure if the LCD display on KittyHawks is supposed to print the activity symbols.
I think the best thing is to try it with HPUX. If HP-UX works, then something needs to be fixed.

Maybe HP-UX only uses the LCD display to print the LCD text (/proc/pdc/lcd) as well ?
In this case you should try again to disable the led status symbols for KittyHawk machines only.

Helge

On Monday 13 November 2006 22:54, Guy Martin wrote:
> Sorry for the missunderstanding.
> 
> It says this :
> [42949382.500000] LED display at ffd0c000 registered
> 
> And nothing can be seen on the lcd since it's not detected and disabled.
> /proc/pdc/lcd doesn't exists.
> 
> Do you know if this lcd is supposed to display the leds character ?
> 
> 
>   Guy
> 
> 
> On Sun, 12 Nov 2006 16:05:07 +0100
> Helge Deller <deller@gmx.de> wrote:
> 
> > Hi Guy,
> > 
> > I think you misunderstood me.
> > My idea was that you check what your machine's BIOS (PDC) returns for pdc_chassis_info().
> > 
> > So, if you would remove this code from led_init() you may test the results without this "workaround for KittyHawk":
> >         /* Work around the buggy PDC of KittyHawk-machines */
> >         switch (CPU_HVERSION) {
> >         case 0x580:             /* KittyHawk DC2-100 (K100) */
> >         case 0x581:             /* KittyHawk DC3-120 (K210) */
> >         case 0x582:             /* KittyHawk DC3 100 (K400) */
> >         case 0x583:             /* KittyHawk DC3 120 (K410) */
> >         case 0x58B:             /* KittyHawk DC2 100 (K200) */
> >                 printk(KERN_INFO "%s: KittyHawk-Machine (hversion 0x%x) found, "
> >                                 "LED detection skipped.\n", __FILE__, CPU_HVERSION);
> >                 goto found;     /* use the preinitialized values of lcd_info */
> >         }
> > 
> > Helge
> > 
> > 
> > On Sunday 12 November 2006 15:50, Guy Martin wrote:
> > > 
> > > Hi Helge,
> > > 
> > > This can't work. If you don't call register_led_driver(), led_func_ptr and led_type stay NULL and lcd_print() never prints anything.
> > > You can change lcd_print() not to check led_func_ptr but I fear something may break if the device is not initialized :)
> > > 
> > > Maybe we could use static int initialized defined in register_led_driver and make it global to check if the device is ready or not.
> > > 
> > >   Guy
> > > 
> > > 
> > > On Sun, 12 Nov 2006 15:04:38 +0100
> > > Helge Deller <deller@gmx.de> wrote:
> > > 
> > > > Hi Guy,
> > > > 
> > > > On Sunday 12 November 2006 01:45, Guy Martin wrote:
> > > > > I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
> > > > > After some investigation, it looked like the code updating the led status was filling the LCD.
> > > > ....
> > > > 
> > > > instead of adding:
> > > > +               no_led = 1;     /* no led support on KittyHawk */
> > > >                 goto found;     /* use the preinitialized values of lcd_info */
> > > > 
> > > > did you tried to comment out the "goto found" ?
> > > > 
> > > > Helge
> > > 
> > > 
> 
> 
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Guy Martin]

Helge,

I installed HP-UX on that box and it only displayed PCD state codes as linux does when you select only CONFIG_PDC_CHASSIS.
On that point, I think that CONFIG_PDC_CHASSIS and CONFIG_CHASSIS_LCD_LED should be mutually exclusive.
When you select CONFIG_CHASSIS_LCD_LED, you don't see the chassis state anymore but only the text from /proc/pdc/lcd.

How am I supposed to enable something like CONFIG_CHASSIS_LCD_LED in HP-UX and display random text ?


  Guy


On Thu, 16 Nov 2006 21:55:49 +0100
Helge Deller <deller@gmx.de> wrote:

> Hi Guy,
> 
> I'm not sure if the LCD display on KittyHawks is supposed to print the activity symbols.
> I think the best thing is to try it with HPUX. If HP-UX works, then something needs to be fixed.
> 
> Maybe HP-UX only uses the LCD display to print the LCD text (/proc/pdc/lcd) as well ?
> In this case you should try again to disable the led status symbols for KittyHawk machines only.
> 
> Helge
> 
> On Monday 13 November 2006 22:54, Guy Martin wrote:
> > Sorry for the missunderstanding.
> > 
> > It says this :
> > [42949382.500000] LED display at ffd0c000 registered
> > 
> > And nothing can be seen on the lcd since it's not detected and disabled.
> > /proc/pdc/lcd doesn't exists.
> > 
> > Do you know if this lcd is supposed to display the leds character ?
> > 
> > 
> >   Guy
> > 
> > 
> > On Sun, 12 Nov 2006 16:05:07 +0100
> > Helge Deller <deller@gmx.de> wrote:
> > 
> > > Hi Guy,
> > > 
> > > I think you misunderstood me.
> > > My idea was that you check what your machine's BIOS (PDC) returns for pdc_chassis_info().
> > > 
> > > So, if you would remove this code from led_init() you may test the results without this "workaround for KittyHawk":
> > >         /* Work around the buggy PDC of KittyHawk-machines */
> > >         switch (CPU_HVERSION) {
> > >         case 0x580:             /* KittyHawk DC2-100 (K100) */
> > >         case 0x581:             /* KittyHawk DC3-120 (K210) */
> > >         case 0x582:             /* KittyHawk DC3 100 (K400) */
> > >         case 0x583:             /* KittyHawk DC3 120 (K410) */
> > >         case 0x58B:             /* KittyHawk DC2 100 (K200) */
> > >                 printk(KERN_INFO "%s: KittyHawk-Machine (hversion 0x%x) found, "
> > >                                 "LED detection skipped.\n", __FILE__, CPU_HVERSION);
> > >                 goto found;     /* use the preinitialized values of lcd_info */
> > >         }
> > > 
> > > Helge
> > > 
> > > 
> > > On Sunday 12 November 2006 15:50, Guy Martin wrote:
> > > > 
> > > > Hi Helge,
> > > > 
> > > > This can't work. If you don't call register_led_driver(), led_func_ptr and led_type stay NULL and lcd_print() never prints anything.
> > > > You can change lcd_print() not to check led_func_ptr but I fear something may break if the device is not initialized :)
> > > > 
> > > > Maybe we could use static int initialized defined in register_led_driver and make it global to check if the device is ready or not.
> > > > 
> > > >   Guy
> > > > 
> > > > 
> > > > On Sun, 12 Nov 2006 15:04:38 +0100
> > > > Helge Deller <deller@gmx.de> wrote:
> > > > 
> > > > > Hi Guy,
> > > > > 
> > > > > On Sunday 12 November 2006 01:45, Guy Martin wrote:
> > > > > > I've noticed that the LCD on my K410 is filled at a constant rate with black characters.
> > > > > > After some investigation, it looked like the code updating the led status was filling the LCD.
> > > > > ....
> > > > > 
> > > > > instead of adding:
> > > > > +               no_led = 1;     /* no led support on KittyHawk */
> > > > >                 goto found;     /* use the preinitialized values of lcd_info */
> > > > > 
> > > > > did you tried to comment out the "goto found" ?
> > > > > 
> > > > > Helge
> > > > 
> > > > 
> > 
> > 

Re: [parisc-linux] suppress led task for KittyHawk's LCD

[Thibaut VARENE]

On 11/16/06, Guy Martin <gmsoft@tuxicoman.be> wrote:
>
> Helge,
>
> I installed HP-UX on that box and it only displayed PCD state codes as linux does when you select only CONFIG_PDC_CHASSIS.
> On that point, I think that CONFIG_PDC_CHASSIS and CONFIG_CHASSIS_LCD_LED should be mutually exclusive.

Certainly not.

> When you select CONFIG_CHASSIS_LCD_LED, you don't see the chassis state anymore but only the text from /proc/pdc/lcd.

Wrong. You see whichever sent the last event. If the system is working
normaly you see the proc text, if it panic'd, you'll see a FLT
message. Besides, CONFIG_CHASSIS_LCD_LED drives the heartbeat, network
and disk symbols on the lcd as you know, so because there's a
"conflict" on the upper part of the lcd doesn't mean that you want to
make those mutually exclusive anyway.

I've coded this that way on purpose, and the wiki and config help are
(imho) explicit enough as to what will happen when you select both.
Selecting both does have a purpose.

T-Bone

-- 
Thibaut VARENE
http://www.parisc-linux.org/~varenet/
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux