2007-08-01 07:24:37

by Matt Sealey

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection

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 <[email protected]>
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 <[email protected]>
>>
>> --- 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
> [email protected]
> https://ozlabs.org/mailman/listinfo/linuxppc-dev


2007-08-01 07:27:39

by Matt Sealey

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection


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 <[email protected]>
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 :)
>

2007-08-02 04:41:16

by Alan Curry

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection

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
[email protected]

2007-08-06 18:46:49

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection

>> 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

2007-08-06 21:39:53

by Matt Sealey

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection

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 <[email protected]>
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?
>

2007-08-06 21:58:39

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection

> 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

2007-08-07 04:16:34

by Alan Curry

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection

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.)

2007-08-07 16:20:35

by Matt Sealey

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection


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 <[email protected]>
Genesi, Manager, Developer Relations

2007-08-07 16:26:20

by Matt Sealey

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection


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 <[email protected]>
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.)
>

2007-08-09 16:31:16

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection

>> 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

2007-08-09 16:45:34

by Matt Sealey

[permalink] [raw]
Subject: Re: [PATCH] powerpc: Pegasos keyboard detection


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 <[email protected]>
Genesi, Manager, Developer Relations