keyboard not found in 2.6.22.1

keyboard not found in 2.6.22.1

[Alan Curry]

When I boot 2.6.22.1 on my PegasosPPC, the PS/2 keyboard is not detected. It
works in 2.6.21.5. Both kernels have:
CONFIG_KEYBOARD_ATKBD=y
CONFIG_SERIO_I8042=y

The 2.6.21.5 kernel logs this stuff during boot:
  serio: i8042 KBD port at 0x60,0x64 irq 1
  serio: i8042 AUX port at 0x60,0x64 irq 12
  input: AT Translated Set 2 keyboard as /class/input/input0
  atkbd.c: keyboard reset failed on isa0060/serio1

(The "keyboard reset failed" has always been there and the keyboard has
always worked, it doesn't seem to indicate any actual problem)

The 2.6.22.1 doesn't log anything like those 4 lines. The only new thing that
shows up in 2.6.22.1 is
  console handover: boot [udbg0] -> real [tty0]

Aside from those lines, there's an exact line-for-line correspondence between
the boot logs.

I can log in over the network while running the broken kernel, but I'm not
sure what information I should be gathering once I'm in there.

In my attempt to understand the workings of input/serio/i8042.c, the only
result so far is that I noticed an unimportant typo in the 2.6.22 patch
("readinng" in the comment above i8042_toggle_aux).

[PATCH] powerpc: Pegasos keyboard detection

[Alan Curry]

As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse controller
on the PegasosPPC. This is because of a feature/bug in the OF device tree:
the "device_type" attribute is an empty string instead of "8042" as the
kernel expects. This patch (against 2.6.22.1) adds a secondary detection
which looks for a device whose *name* is "8042" if there is no device whose
*type* is "8042".

Signed-off-by: Alan Curry <pacman@world.std.com>

--- arch/powerpc/kernel/setup-common.c.orig	2007-07-24 19:04:17.000000000 -0500
+++ arch/powerpc/kernel/setup-common.c	2007-07-24 19:06:36.000000000 -0500
@@ -487,6 +487,10 @@ int check_legacy_ioport(unsigned long ba
 	switch(base_port) {
 	case I8042_DATA_REG:
 		np = of_find_node_by_type(NULL, "8042");
+		/* Pegasos has no device_type on its 8042 node, look for the
+		 * name instead */
+		if (!np)
+			np = of_find_node_by_name(NULL, "8042");
 		break;
 	case FDC_BASE: /* FDC1 */
 		np = of_find_node_by_type(NULL, "fdc");

Re: [PATCH] powerpc: Pegasos keyboard detection

[Benjamin Herrenschmidt]

On Tue, 2007-07-24 at 21:28 -0400, Alan Curry wrote:
> As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse controller
> on the PegasosPPC. This is because of a feature/bug in the OF device tree:
> the "device_type" attribute is an empty string instead of "8042" as the
> kernel expects. This patch (against 2.6.22.1) adds a secondary detection
> which looks for a device whose *name* is "8042" if there is no device whose
> *type* is "8042".
> 
> Signed-off-by: Alan Curry <pacman@world.std.com>

Doesn't it have something in "compatible" instead ? that would be the
way to go.

Ben.

Re: [PATCH] powerpc: Pegasos keyboard detection

[Alan Curry]

Benjamin Herrenschmidt writes the following:
>
>Doesn't it have something in "compatible" instead ? that would be the
>way to go.
>

Assuming that would be represented as a file named "compatible" in the
directory /proc/device-tree/*/*/8042@i60 ... no, it doesn't have one of
those.

For the amusement of all, I offer the entire device-tree as presented in
/proc by my running 2.6.22.1 kernel. Available for a limited time from
http://world.std.com/~pacman/pegasosII_proc_device-tree.tar.gz

-- 
Alan Curry
pacman@world.std.com

Re: [PATCH] powerpc: Pegasos keyboard detection

[Benjamin Herrenschmidt]

On Tue, 2007-07-24 at 21:28 -0400, Alan Curry wrote:
> As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse controller
> on the PegasosPPC. This is because of a feature/bug in the OF device tree:
> the "device_type" attribute is an empty string instead of "8042" as the
> kernel expects. This patch (against 2.6.22.1) adds a secondary detection
> which looks for a device whose *name* is "8042" if there is no device whose
> *type* is "8042".
> 
> Signed-off-by: Alan Curry <pacman@world.std.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Note, if there's a volunteer, we could probably turn that code into a
nice table lookup.

Cheers,
Ben.

> --- arch/powerpc/kernel/setup-common.c.orig	2007-07-24 19:04:17.000000000 -0500
> +++ arch/powerpc/kernel/setup-common.c	2007-07-24 19:06:36.000000000 -0500
> @@ -487,6 +487,10 @@ int check_legacy_ioport(unsigned long ba
>  	switch(base_port) {
>  	case I8042_DATA_REG:
>  		np = of_find_node_by_type(NULL, "8042");
> +		/* Pegasos has no device_type on its 8042 node, look for the
> +		 * name instead */
> +		if (!np)
> +			np = of_find_node_by_name(NULL, "8042");
>  		break;
>  	case FDC_BASE: /* FDC1 */
>  		np = of_find_node_by_type(NULL, "fdc");
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Pegasos keyboard detection

[Andrew Morton]

On Wed, 25 Jul 2007 17:12:45 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Tue, 2007-07-24 at 21:28 -0400, Alan Curry wrote:
> > As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse controller
> > on the PegasosPPC. This is because of a feature/bug in the OF device tree:
> > the "device_type" attribute is an empty string instead of "8042" as the
> > kernel expects. This patch (against 2.6.22.1) adds a secondary detection
> > which looks for a device whose *name* is "8042" if there is no device whose
> > *type* is "8042".
> > 
> > Signed-off-by: Alan Curry <pacman@world.std.com>
> 
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Note, if there's a volunteer, we could probably turn that code into a
> nice table lookup.

Did this get merged, or otherwise fixed?  Even though the code in there has
changed quite a bit, it looks to my untrained eye like the fix is still
applicable?


From: Alan Curry <pacman@TheWorld.com>

As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse
controller on the PegasosPPC.  This is because of a feature/bug in the OF
device tree: the "device_type" attribute is an empty string instead of
"8042" as the kernel expects.  This patch (against 2.6.22.1) adds a
secondary detection which looks for a device whose *name* is "8042" if
there is no device whose *type* is "8042".

Signed-off-by: Alan Curry <pacman@world.std.com>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/powerpc/kernel/setup-common.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff -puN /dev/null /dev/null
diff -puN arch/powerpc/kernel/setup-common.c~powerpc-pegasos-keyboard-detection arch/powerpc/kernel/setup-common.c
--- a/arch/powerpc/kernel/setup-common.c~powerpc-pegasos-keyboard-detection
+++ a/arch/powerpc/kernel/setup-common.c
@@ -496,6 +496,12 @@ int check_legacy_ioport(unsigned long ba
 			break;
 		}
 		np = of_find_node_by_type(NULL, "8042");
+		/*
+		 * Pegasos has no device_type on its 8042 node, look for the
+		 * name instead
+		 */
+		if (!np)
+			np = of_find_node_by_name(NULL, "8042");
 		break;
 	case FDC_BASE: /* FDC1 */
 		np = of_find_node_by_type(NULL, "fdc");
_


And ALan says that 2.6.22 is bust, but this patch no won't apply there so
if we want to fix 2.6.22.x then Alan's original patch would be needed.

Re: [PATCH] powerpc: Pegasos keyboard detection

[Benjamin Herrenschmidt]

On Thu, 2007-07-26 at 18:48 -0700, Andrew Morton wrote:
> On Wed, 25 Jul 2007 17:12:45 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > On Tue, 2007-07-24 at 21:28 -0400, Alan Curry wrote:
> > > As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse controller
> > > on the PegasosPPC. This is because of a feature/bug in the OF device tree:
> > > the "device_type" attribute is an empty string instead of "8042" as the
> > > kernel expects. This patch (against 2.6.22.1) adds a secondary detection
> > > which looks for a device whose *name* is "8042" if there is no device whose
> > > *type* is "8042".
> > > 
> > > Signed-off-by: Alan Curry <pacman@world.std.com>
> > 
> > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > 
> > Note, if there's a volunteer, we could probably turn that code into a
> > nice table lookup.
> 
> Did this get merged, or otherwise fixed?  Even though the code in there has
> changed quite a bit, it looks to my untrained eye like the fix is still
> applicable?

Merged a fixed version:

f5d834fc34e61f1a40435981062000e5d2b2baa8

(In linus tree as of now)

Cheers,
Ben.

> 
> From: Alan Curry <pacman@TheWorld.com>
> 
> As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse
> controller on the PegasosPPC.  This is because of a feature/bug in the OF
> device tree: the "device_type" attribute is an empty string instead of
> "8042" as the kernel expects.  This patch (against 2.6.22.1) adds a
> secondary detection which looks for a device whose *name* is "8042" if
> there is no device whose *type* is "8042".
> 
> Signed-off-by: Alan Curry <pacman@world.std.com>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  arch/powerpc/kernel/setup-common.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff -puN /dev/null /dev/null
> diff -puN arch/powerpc/kernel/setup-common.c~powerpc-pegasos-keyboard-detection arch/powerpc/kernel/setup-common.c
> --- a/arch/powerpc/kernel/setup-common.c~powerpc-pegasos-keyboard-detection
> +++ a/arch/powerpc/kernel/setup-common.c
> @@ -496,6 +496,12 @@ int check_legacy_ioport(unsigned long ba
>  			break;
>  		}
>  		np = of_find_node_by_type(NULL, "8042");
> +		/*
> +		 * Pegasos has no device_type on its 8042 node, look for the
> +		 * name instead
> +		 */
> +		if (!np)
> +			np = of_find_node_by_name(NULL, "8042");
>  		break;
>  	case FDC_BASE: /* FDC1 */
>  		np = of_find_node_by_type(NULL, "fdc");
> _
> 
> 
> And ALan says that 2.6.22 is bust, but this patch no won't apply there so
> if we want to fix 2.6.22.x then Alan's original patch would be needed.
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] powerpc: Pegasos keyboard detection

[Andrew Morton]

On Fri, 27 Jul 2007 13:00:21 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Thu, 2007-07-26 at 18:48 -0700, Andrew Morton wrote:
> > On Wed, 25 Jul 2007 17:12:45 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > 
> > > On Tue, 2007-07-24 at 21:28 -0400, Alan Curry wrote:
> > > > As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse controller
> > > > on the PegasosPPC. This is because of a feature/bug in the OF device tree:
> > > > the "device_type" attribute is an empty string instead of "8042" as the
> > > > kernel expects. This patch (against 2.6.22.1) adds a secondary detection
> > > > which looks for a device whose *name* is "8042" if there is no device whose
> > > > *type* is "8042".
> > > > 
> > > > Signed-off-by: Alan Curry <pacman@world.std.com>
> > > 
> > > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > 
> > > Note, if there's a volunteer, we could probably turn that code into a
> > > nice table lookup.
> > 
> > Did this get merged, or otherwise fixed?  Even though the code in there has
> > changed quite a bit, it looks to my untrained eye like the fix is still
> > applicable?
> 
> Merged a fixed version:
> 
> f5d834fc34e61f1a40435981062000e5d2b2baa8
> 
> (In linus tree as of now)

I hope so.  Alan's patch looks rather different from what you have now:
fall back to of_find_node_by_name() if of_find_node_by_type() failed.

Re: [PATCH] powerpc: Pegasos keyboard detection

[Alan Curry]

Andrew Morton writes the following:
>> > 
>> > Did this get merged, or otherwise fixed?  Even though the code in there has
>> > changed quite a bit, it looks to my untrained eye like the fix is still
>> > applicable?
>> 
>> Merged a fixed version:
>> 
>> f5d834fc34e61f1a40435981062000e5d2b2baa8
>> 
>> (In linus tree as of now)
>
>I hope so.  Alan's patch looks rather different from what you have now:
>fall back to of_find_node_by_name() if of_find_node_by_type() failed.
>

Looking at it via gitweb, I think the commit mentioned above is actually the
same as what I submitted. The diff context lines are different because there
was a similar patch in between the 2.6.22 release and my patch, which
apparently fixed the same problem on a different type of machine. It looks
like these 2 workarounds will function independently of each other.

The first 8042-detection fix was:
db0dbae9d9680bce69fe1ce775709e60c8ee9c29

And mine is:
f5d834fc34e61f1a40435981062000e5d2b2baa8

-- 
Alan Curry
pacman@world.std.com

Re: [PATCH] powerpc: Pegasos keyboard detection

[Benjamin Herrenschmidt]

On Thu, 2007-07-26 at 21:22 -0700, Andrew Morton wrote:
> 
> > Merged a fixed version:
> > 
> > f5d834fc34e61f1a40435981062000e5d2b2baa8
> > 
> > (In linus tree as of now)
> 
> I hope so.  Alan's patch looks rather different from what you have
> now:
> fall back to of_find_node_by_name() if of_find_node_by_type() failed.

Which is just waht that commit does :-)

Ben.

Re: [PATCH] powerpc: Pegasos keyboard detection

[Segher Boessenkool]

> As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse 
> controller
> on the PegasosPPC. This is because of a feature/bug in the OF device 
> tree:
> the "device_type" attribute is an empty string instead of "8042" as the
> kernel expects. This patch (against 2.6.22.1) adds a secondary 
> detection
> which looks for a device whose *name* is "8042" if there is no device 
> whose
> *type* is "8042".
>
> Signed-off-by: Alan Curry <pacman@world.std.com>
>
> --- arch/powerpc/kernel/setup-common.c.orig	2007-07-24 
> 19:04:17.000000000 -0500
> +++ arch/powerpc/kernel/setup-common.c	2007-07-24 19:06:36.000000000 
> -0500
> @@ -487,6 +487,10 @@ int check_legacy_ioport(unsigned long ba
>  	switch(base_port) {
>  	case I8042_DATA_REG:
>  		np = of_find_node_by_type(NULL, "8042");
> +		/* Pegasos has no device_type on its 8042 node, look for the
> +		 * name instead */
> +		if (!np)
> +			np = of_find_node_by_name(NULL, "8042");

[I know it already got merged, I'm behind on mail, but anyway...]

Could board-specific quirks like this please always include a
check for that board?  Or, even better, do a fixup in the
bootwrapper.

In this case the workaround won't likely trigger on the wrong
boards, but "just a little bit" more dangerous workarounds
_will_, and the law of big numbers works against us...


Segher

Re: [PATCH] powerpc: Pegasos keyboard detection

[Matt Sealey]

Yeah please do a fixup for the boot wrapper.

Or, if you have trouble, go into the firmware and type "nvedit", add
these lines;

" /isa/8042" find-device
" 8042" encode-string device-type

(then ctrl-c to exit and nvstore to run it on next reboot. Try it without
the patch first, on the firmware console, just to be sure I got it right,
because I can't test it here)

You don't need to patch Linux at all. In fact for silly things like this
I would recommend against it :)

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations




Segher Boessenkool wrote:
>> As of 2.6.22 the kernel doesn't recognize the i8042 keyboard/mouse 
>> controller
>> on the PegasosPPC. This is because of a feature/bug in the OF device 
>> tree:
>> the "device_type" attribute is an empty string instead of "8042" as the
>> kernel expects. This patch (against 2.6.22.1) adds a secondary 
>> detection
>> which looks for a device whose *name* is "8042" if there is no device 
>> whose
>> *type* is "8042".
>>
>> Signed-off-by: Alan Curry <pacman@world.std.com>
>>
>> --- arch/powerpc/kernel/setup-common.c.orig	2007-07-24 
>> 19:04:17.000000000 -0500
>> +++ arch/powerpc/kernel/setup-common.c	2007-07-24 19:06:36.000000000 
>> -0500
>> @@ -487,6 +487,10 @@ int check_legacy_ioport(unsigned long ba
>>  	switch(base_port) {
>>  	case I8042_DATA_REG:
>>  		np = of_find_node_by_type(NULL, "8042");
>> +		/* Pegasos has no device_type on its 8042 node, look for the
>> +		 * name instead */
>> +		if (!np)
>> +			np = of_find_node_by_name(NULL, "8042");
> 
> [I know it already got merged, I'm behind on mail, but anyway...]
> 
> Could board-specific quirks like this please always include a
> check for that board?  Or, even better, do a fixup in the
> bootwrapper.
> 
> In this case the workaround won't likely trigger on the wrong
> boards, but "just a little bit" more dangerous workarounds
> _will_, and the law of big numbers works against us...
> 
> 
> Segher
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Pegasos keyboard detection

[Matt Sealey]

Just so you guys have it all in one pretty little package, these will remove
the need for the Pegasos IDE and ISA fixups in the prom_init.c too.

s" /pci@80000000/ide@C,1" find-device
d# 14 encode-int 0 encode-int
d# 15 encode-int 0 encode-int
encode+ encode+ encode+ s" interrupts" property
0x1018a encode-int s" class-code" property
device-end

s" /pci@80000000/isa@C" find-device
0x1 encode-int 0x0 encode-int 0x1006000 encode-int
0x0 encode-int 0x0 encode-int 0x1000 encode-int
encode+ encode+ encode+ encode+ encode+ s" ranges" property
device-end

If anyone wants to test and confirm the 8042 fix and then we can add
it to the end here.. we can unclutter the kernel.

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Matt Sealey wrote:
> Yeah please do a fixup for the boot wrapper.
> 
> Or, if you have trouble, go into the firmware and type "nvedit", add
> these lines;
> 
> " /isa/8042" find-device
> " 8042" encode-string device-type
> 
> (then ctrl-c to exit and nvstore to run it on next reboot. Try it without
> the patch first, on the firmware console, just to be sure I got it right,
> because I can't test it here)
> 
> You don't need to patch Linux at all. In fact for silly things like this
> I would recommend against it :)
> 

Re: [PATCH] powerpc: Pegasos keyboard detection

[Alan Curry]

Matt Sealey writes the following:
>
>Yeah please do a fixup for the boot wrapper.
>
>Or, if you have trouble, go into the firmware and type "nvedit", add
>these lines;
>
>" /isa/8042" find-device
>" 8042" encode-string device-type
>
>(then ctrl-c to exit and nvstore to run it on next reboot. Try it without
>the patch first, on the firmware console, just to be sure I got it right,
>because I can't test it here)

It works from the ok prompt but in the nvramrc it doesn't find the device.
(pci/isa nodes not created yet?)

But the larger point:

>
>You don't need to patch Linux at all. In fact for silly things like this
>I would recommend against it :)

If the workaround doesn't go into the kernel, everybody with affected
hardware has to individually find out about the bug (probably by experiencing
an annoying keyboardless boot) and fix it himself. Is that worth the
reduction in kernel clutter?

-- 
Alan Curry
pacman@world.std.com

Re: [PATCH] powerpc: Pegasos keyboard detection

[Segher Boessenkool]

>> You don't need to patch Linux at all. In fact for silly things like 
>> this
>> I would recommend against it :)
>
> If the workaround doesn't go into the kernel, everybody with affected
> hardware has to individually find out about the bug (probably by 
> experiencing
> an annoying keyboardless boot) and fix it himself.

That's one of the reasons why this should go into a bootwrapper.


Segher

Re: [PATCH] powerpc: Pegasos keyboard detection

[Matt Sealey]

Okay before you add to the nvramrc you also need to add probe-all to build the
device tree first; I assumed this was common knowledge.

probe-all
" /pci/isa/8042" find-device
" 8042" encode-string device-type
install-console
banner

That should do it. Without probe-all/install-console/banner in the nvramrc they
will be run by the boot process (this is documented in the IEEE 1275 specs)

Yeah it will break for people on their first boot but let's put a few things
across;

1) Pegasos has been discontinued. The number of people with this bug are in
the low thousands - if they upgrade to 2.6.22 at all.

2) The fix was in the wrong place anyway, if it was going to be done anywhere
at all it needs to be in arch/powerpc/kernel/prom_init.c:fixup_device_tree_chrp()
like the ISA ranges breakage (which is on Briq) and IDE IRQ misnumbering fix.
Not the keyboard platform driver.

3) In any case this should be something that is fixed in the firmware, as any
stalwart, stubborn Linux developer will rant at you about. If you can't get a
firmware update or it's not fixed, this is the best place to do it.

As for Segher, bootwrapper not such a good place as that's still mussing up
the kernel with these fixes. Let the boot loader do it for the OS, and don't
mess up the OS with device-tree fixups. After all it may not just be Linux
that stumbles on it. Why have the same patch in every OS?

With nvramrc, the fix is done for EVERY operating system from firmware upwards.
The semi-official Genesi line of support and what I have been told by the board
designer is if you need to fix something in the device tree, that is what nvramrc
is for, and that is why Open Firmware runs Forth scripts.

I have a fixup script for Pegasos and one for Efika which I may publish at some
point in the very near future. We may ship a small patch for Marcin Kurek's
"BootCreator" (http://tbs-software.com/morgoth/projects.html) which includes
all the stuff. We may write our own binary bootloader.. I am waiting for the
result of the new firmware feature requests before we waste time on stuff
bplan is silently fixing.

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Alan Curry wrote:
> Matt Sealey writes the following:
>> Yeah please do a fixup for the boot wrapper.
>>
>> Or, if you have trouble, go into the firmware and type "nvedit", add
>> these lines;
>>
>> " /isa/8042" find-device
>> " 8042" encode-string device-type
>>
>> (then ctrl-c to exit and nvstore to run it on next reboot. Try it without
>> the patch first, on the firmware console, just to be sure I got it right,
>> because I can't test it here)
> 
> It works from the ok prompt but in the nvramrc it doesn't find the device.
> (pci/isa nodes not created yet?)
> 
> But the larger point:
> 
>> You don't need to patch Linux at all. In fact for silly things like this
>> I would recommend against it :)
> 
> If the workaround doesn't go into the kernel, everybody with affected
> hardware has to individually find out about the bug (probably by experiencing
> an annoying keyboardless boot) and fix it himself. Is that worth the
> reduction in kernel clutter?
> 

Re: [PATCH] powerpc: Pegasos keyboard detection

[Segher Boessenkool]

> 2) The fix was in the wrong place anyway, if it was going to be done 
> anywhere
> at all it needs to be in 
> arch/powerpc/kernel/prom_init.c:fixup_device_tree_chrp()
> like the ISA ranges breakage (which is on Briq) and IDE IRQ 
> misnumbering fix.
> Not the keyboard platform driver.

Yeah.  In the bootwrapper.

> 3) In any case this should be something that is fixed in the firmware, 
> as any
> stalwart, stubborn Linux developer will rant at you about.

Sure, if you *can* get a fix for the firmware.  Until every
user has this update, fixing it in the kernel wrapper helps
users.

> As for Segher, bootwrapper not such a good place as that's still 
> mussing up
> the kernel with these fixes. Let the boot loader do it for the OS, and 
> don't
> mess up the OS with device-tree fixups.

Sure, the bootloader can do it too, but then we need to fix
every bootloader that's used with Linux on this platform.
Maybe that's just one, that would make things simple :-)

> After all it may not just be Linux
> that stumbles on it. Why have the same patch in every OS?

This is just pragmatics: Linux needs the workaround -> Linux
implements the workaround.  Sure it is not a _proper_ fix, but
a correctly implemented workaround "fixes" it for all users,
forever.

> With nvramrc, the fix is done for EVERY operating system from firmware 
> upwards.

But it's something every user has to do separately.

> The semi-official Genesi line of support and what I have been told by 
> the board
> designer is if you need to fix something in the device tree, that is 
> what nvramrc
> is for, and that is why Open Firmware runs Forth scripts.

That's hardly the only reason.  But yeah, that's one way to
implement the workaround, but _we_ (the Linux community) cannot
do it like that (easily) for all users.


Segher

Re: [PATCH] powerpc: Pegasos keyboard detection

[Matt Sealey]

Segher Boessenkool wrote:
> 
> That's hardly the only reason.  But yeah, that's one way to
> implement the workaround, but _we_ (the Linux community) cannot
> do it like that (easily) for all users.

But you're the guy who told us our firmware sucks and we should fix our
firmware rather than clutter Linux with too many fixups. There are about
200 lines of code required to bring the Efika device tree up to the
Linux "specification" of a 5200B device tree, which will never make it
into code. Pegasos is ostensibly the same way.

Linux is already a bad enough moving target, and none of these fixes help
other operating systems or developers, if we only patch Linux, and only
say, you must run the latest Linux kernel version, and the latest U-Boot,
and the latest FDT binary, and encourage users to upgrade it all regardless
of your worries.

So, there are two opinions here;

1) the reports as we had when Efika was released and continually levied
against Pegasos firmware, that the firmware is broken and must be fixed
to comply, and no fixes will be considered because "bplan sucks and must
fix it"

2) As long as the patches are 2 lines big, you will allow them in, because
it is too much for a user to update firmware or run a script to boot?

Would you guys rather we shipped a boot script that ran the OS, fixed
all these issues in-place in-firmware, so Linux did not have to have these
workarounds, or are you accepting patches now? Because I can write those
200 lines of code to work around Efika device tree mistakes you yourself
complained about at Christmas last year..

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Re: [PATCH] powerpc: Pegasos keyboard detection

[Segher Boessenkool]

>> That's hardly the only reason.  But yeah, that's one way to
>> implement the workaround, but _we_ (the Linux community) cannot
>> do it like that (easily) for all users.
>
> But you're the guy who told us our firmware sucks and we should fix our
> firmware

Yes, and?  You _should_ fix your firmware, it is buggy after all.
Esp. back then as it wasn't shipping yet.

> rather than clutter Linux with too many fixups.

Also, putting fixups in the wrapper is a wholly different thing from
putting fixups deep inside the kernel code proper.

> Linux is already a bad enough moving target, and none of these fixes 
> help
> other operating systems or developers, if we only patch Linux,

But that's not Linux' concern.  You might care, we don't.  Is
this so hard to understand?

> 1) the reports as we had when Efika was released and continually levied
> against Pegasos firmware, that the firmware is broken and must be fixed
> to comply, and no fixes will be considered because "bplan sucks and 
> must
> fix it"
>
> 2) As long as the patches are 2 lines big, you will allow them in, 
> because
> it is too much for a user to update firmware or run a script to boot?

Our only two concerns are what is best on technical grounds, and what
is best for our users.

> Would you guys rather we shipped a boot script that ran the OS, fixed
> all these issues in-place in-firmware, so Linux did not have to have 
> these
> workarounds,

Sure, if you can do that, that would be great.


Segher

Re: [PATCH] powerpc: Pegasos keyboard detection

[Matt Sealey]

Segher Boessenkool wrote:
>> Would you guys rather we shipped a boot script that ran the OS, fixed
>> all these issues in-place in-firmware, so Linux did not have to have 
>> these
>> workarounds,
> 
> Sure, if you can do that, that would be great.

Right, so don't accept that keyboard fix, and we will work on the script
instead.

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Re: [PATCH] powerpc: Pegasos keyboard detection

[Alan Curry]

I'm almost sorry I spoke up...

Matt Sealey writes the following:
>
>Okay before you add to the nvramrc you also need to add probe-all to build the
>device tree first; I assumed this was common knowledge.

Maybe for experienced OpenFirmware developers; this is the first time I've
had to touch the stuff. So first I had to learn Forth. To patch the kernel
you only need to know C. (Of course I already knew C -- it's common
knowledge.)

Re: [PATCH] powerpc: Pegasos keyboard detection

[Matt Sealey]

Okay, you don't need to be an experienced Open Firmware developer.

In fact I know we have had experienced Open Firmware developers who have
said that our firmware sucks (some comment about "shitty German engineering",
I really did quit caring after that point) because they could not run probe-all
from the "ok" prompt.

In the first few pages of the OF spec, it very clearly states that this will
probably make your system explode.

So even experts do not know what they are doing. I don't expect users to.
But as an engineer, here, I think patching the device-type in the boot
wrapper (be it the chrp boot loader) is the wrong place, because we have
had real experts here (Ben, Segher etc.) complain that fixing the device
tree in the boot wrapper is no substitute for a correctly working
firmware.

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Alan Curry wrote:
> I'm almost sorry I spoke up...
> 
> Matt Sealey writes the following:
>> Okay before you add to the nvramrc you also need to add probe-all to build the
>> device tree first; I assumed this was common knowledge.
> 
> Maybe for experienced OpenFirmware developers; this is the first time I've
> had to touch the stuff. So first I had to learn Forth. To patch the kernel
> you only need to know C. (Of course I already knew C -- it's common
> knowledge.)
>