[PATCH] olpc: fix endian bug in openfirmware workaround

[PATCH] olpc: fix endian bug in openfirmware workaround

[Harvey Harrison]

Boardrev is always treated as a u32 everywhere else, no reason to
byteswap the 0xc2 value.  The only use is to print out if it is
a prerelease board, the test being:

(olpc_platform_info.boardrev & 0xf) < 8

Which is currently always true as be32_to_cpu(0xc2) & 0xf = 0
but I doubt that was the intention here.  The consequences of the bug
are pretty minor though (incorrect boardrev displayed in dmesg when
ofw support not configured)

Also annotate the temporary used to read the boardrev in the ofw
case.

The confusion was noticed by sparse:
arch/x86/kernel/olpc.c:206:32: warning: cast to restricted __be32

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 arch/x86/kernel/olpc.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/olpc.c b/arch/x86/kernel/olpc.c
index 3e66722..7a13fac 100644
--- a/arch/x86/kernel/olpc.c
+++ b/arch/x86/kernel/olpc.c
@@ -190,12 +190,12 @@ EXPORT_SYMBOL_GPL(olpc_ec_cmd);
 static void __init platform_detect(void)
 {
 	size_t propsize;
-	u32 rev;
+	__be32 rev;
 
 	if (ofw("getprop", 4, 1, NULL, "board-revision-int", &rev, 4,
 			&propsize) || propsize != 4) {
 		printk(KERN_ERR "ofw: getprop call failed!\n");
-		rev = 0;
+		rev = cpu_to_be32(0);
 	}
 	olpc_platform_info.boardrev = be32_to_cpu(rev);
 }
@@ -203,7 +203,7 @@ static void __init platform_detect(void)
 static void __init platform_detect(void)
 {
 	/* stopgap until OFW support is added to the kernel */
-	olpc_platform_info.boardrev = be32_to_cpu(0xc2);
+	olpc_platform_info.boardrev = 0xc2;
 }
 #endif
 
-- 
1.6.0.2.471.g47a76

Re: [PATCH] olpc: fix endian bug in openfirmware workaround

[Ingo Molnar]

* Harvey Harrison <harvey.harrison@gmail.com> wrote:

> Boardrev is always treated as a u32 everywhere else, no reason to 
> byteswap the 0xc2 value.  The only use is to print out if it is a 
> prerelease board, the test being:
> 
> (olpc_platform_info.boardrev & 0xf) < 8
> 
> Which is currently always true as be32_to_cpu(0xc2) & 0xf = 0
> but I doubt that was the intention here.  The consequences of the bug
> are pretty minor though (incorrect boardrev displayed in dmesg when
> ofw support not configured)
> 
> Also annotate the temporary used to read the boardrev in the ofw
> case.
> 
> The confusion was noticed by sparse:
> arch/x86/kernel/olpc.c:206:32: warning: cast to restricted __be32
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>

applied to tip/x86/sparse-fixes, thanks Harvey!

	Ingo

[PATCH] olpc: fix model detection without OFW

[Chris Ball]

Harvey's endianness patch (e51a1ac2dfca9ad869471e88f828281db7e810c0)
breaks model comparison on OLPC; the value 0xc2 needs to be scaled
up by olpc_board().  The pre-patch version was wrong, but accidentally
worked anyway (big-endian 0xc2 is big enough to satisfy all other
board revisions, but little endian 0xc2 is not).

Signed-off-by: Chris Ball <cjb@laptop.org>
---
 arch/x86/kernel/olpc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/olpc.c b/arch/x86/kernel/olpc.c
index 7a13fac..4006c52 100644
--- a/arch/x86/kernel/olpc.c
+++ b/arch/x86/kernel/olpc.c
@@ -203,7 +203,7 @@ static void __init platform_detect(void)
 static void __init platform_detect(void)
 {
 	/* stopgap until OFW support is added to the kernel */
-	olpc_platform_info.boardrev = 0xc2;
+	olpc_platform_info.boardrev = olpc_board(0xc2);
 }
 #endif
 
-- 
1.6.1.3

Re: [PATCH] olpc: fix model detection without OFW

[Andres Salomon]

As discussed on IRC, this has the Dilinger Seal Of Approval (tm).

Acked-by: Andres Salomon <dilinger@debian.org>



On Fri, 13 Feb 2009 20:56:18 -0500
Chris Ball <cjb@laptop.org> wrote:

> Harvey's endianness patch (e51a1ac2dfca9ad869471e88f828281db7e810c0)
> breaks model comparison on OLPC; the value 0xc2 needs to be scaled
> up by olpc_board().  The pre-patch version was wrong, but accidentally
> worked anyway (big-endian 0xc2 is big enough to satisfy all other
> board revisions, but little endian 0xc2 is not).
> 
> Signed-off-by: Chris Ball <cjb@laptop.org>
> ---
>  arch/x86/kernel/olpc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/olpc.c b/arch/x86/kernel/olpc.c
> index 7a13fac..4006c52 100644
> --- a/arch/x86/kernel/olpc.c
> +++ b/arch/x86/kernel/olpc.c
> @@ -203,7 +203,7 @@ static void __init platform_detect(void)
>  static void __init platform_detect(void)
>  {
>  	/* stopgap until OFW support is added to the kernel */
> -	olpc_platform_info.boardrev = 0xc2;
> +	olpc_platform_info.boardrev = olpc_board(0xc2);
>  }
>  #endif
>  

Re: [PATCH] olpc: fix model detection without OFW

[Ingo Molnar]

* Andres Salomon <dilinger@queued.net> wrote:

> As discussed on IRC, this has the Dilinger Seal Of Approval (tm).
> 
> Acked-by: Andres Salomon <dilinger@debian.org>

> On Fri, 13 Feb 2009 20:56:18 -0500
> Chris Ball <cjb@laptop.org> wrote:
> 
> > Harvey's endianness patch (e51a1ac2dfca9ad869471e88f828281db7e810c0)

> > breaks model comparison on OLPC; the value 0xc2 needs to be scaled
> > up by olpc_board().  The pre-patch version was wrong, but accidentally
> > worked anyway (big-endian 0xc2 is big enough to satisfy all other
> > board revisions, but little endian 0xc2 is not).
> > 
> > Signed-off-by: Chris Ball <cjb@laptop.org>

Applied to tip:x86/urgent [for v2.6.29], thanks guys!

Did i get the impact-line below right?

	Ingo

---------------->
>From 307250cda516547c0b0fe70dc3a3626bd82820cc Mon Sep 17 00:00:00 2001
From: Chris Ball <cjb@laptop.org>
Date: Fri, 13 Feb 2009 20:56:18 -0500
Subject: [PATCH] x86, olpc: fix model detection without OFW

Impact: make certain OLPC-board revision based quirks [mouse, sound] work properly

Harvey's endianness patch (e51a1ac2dfca9ad869471e88f828281db7e810c0)
breaks model comparison on OLPC; the value 0xc2 needs to be scaled
up by olpc_board().  The pre-patch version was wrong, but accidentally
worked anyway (big-endian 0xc2 is big enough to satisfy all other
board revisions, but little endian 0xc2 is not).

Signed-off-by: Chris Ball <cjb@laptop.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Acked-by: Andres Salomon <dilinger@queued.net>
Cc: Harvey Harrison <harvey.harrison@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/olpc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/olpc.c b/arch/x86/kernel/olpc.c
index 7a13fac..4006c52 100644
--- a/arch/x86/kernel/olpc.c
+++ b/arch/x86/kernel/olpc.c
@@ -203,7 +203,7 @@ static void __init platform_detect(void)
 static void __init platform_detect(void)
 {
 	/* stopgap until OFW support is added to the kernel */
-	olpc_platform_info.boardrev = 0xc2;
+	olpc_platform_info.boardrev = olpc_board(0xc2);
 }
 #endif
 

Re: [PATCH] olpc: fix model detection without OFW

[Andres Salomon]

On Sat, 14 Feb 2009 22:43:12 +0100
Ingo Molnar <mingo@elte.hu> wrote:
[...]
> 
> Applied to tip:x86/urgent [for v2.6.29], thanks guys!
> 
> Did i get the impact-line below right?
> 
> 	Ingo
> 

More like -

Impact: garbled display, laptop is unusable

;)

The DCON detection code determines whether there's a DCON attached
based upon the model (any XO model >= B2 is assumed to have a DCON).
The LXFB driver determines which mode to use based upon whether or not
it thinks there's a DCON attached, and the DCON/LXFB can't deal w/
standard modes.

So, the result is that LXFB attempts to use some random standard mode
rather than the DCON-required 1200x900, and the display is unusable.



> ---------------->
> From 307250cda516547c0b0fe70dc3a3626bd82820cc Mon Sep 17 00:00:00 2001
> From: Chris Ball <cjb@laptop.org>
> Date: Fri, 13 Feb 2009 20:56:18 -0500
> Subject: [PATCH] x86, olpc: fix model detection without OFW
> 
> Impact: make certain OLPC-board revision based quirks [mouse, sound]
> work properly
> 
> Harvey's endianness patch (e51a1ac2dfca9ad869471e88f828281db7e810c0)
> breaks model comparison on OLPC; the value 0xc2 needs to be scaled
> up by olpc_board().  The pre-patch version was wrong, but accidentally
> worked anyway (big-endian 0xc2 is big enough to satisfy all other
> board revisions, but little endian 0xc2 is not).
> 
> Signed-off-by: Chris Ball <cjb@laptop.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Acked-by: Andres Salomon <dilinger@queued.net>
> Cc: Harvey Harrison <harvey.harrison@gmail.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/kernel/olpc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/olpc.c b/arch/x86/kernel/olpc.c
> index 7a13fac..4006c52 100644
> --- a/arch/x86/kernel/olpc.c
> +++ b/arch/x86/kernel/olpc.c
> @@ -203,7 +203,7 @@ static void __init platform_detect(void)
>  static void __init platform_detect(void)
>  {
>  	/* stopgap until OFW support is added to the kernel */
> -	olpc_platform_info.boardrev = 0xc2;
> +	olpc_platform_info.boardrev = olpc_board(0xc2);
>  }
>  #endif
>  

Re: [PATCH] olpc: fix model detection without OFW

[Ingo Molnar]

* Andres Salomon <dilinger@queued.net> wrote:

> On Sat, 14 Feb 2009 22:43:12 +0100
> Ingo Molnar <mingo@elte.hu> wrote:
> [...]
> > 
> > Applied to tip:x86/urgent [for v2.6.29], thanks guys!
> > 
> > Did i get the impact-line below right?
> > 
> > 	Ingo
> > 
> 
> More like -
> 
> Impact: garbled display, laptop is unusable
> 
> ;)

ah, okay :-) Serious regression that warrants urgent upstream routing.

> The DCON detection code determines whether there's a DCON attached
> based upon the model (any XO model >= B2 is assumed to have a DCON).
> The LXFB driver determines which mode to use based upon whether or not
> it thinks there's a DCON attached, and the DCON/LXFB can't deal w/
> standard modes.
> 
> So, the result is that LXFB attempts to use some random standard mode
> rather than the DCON-required 1200x900, and the display is unusable.

Where did my git grepping skills go wrong?

I git-grepped the code and this is the scope i found:

olpc_platform_info.boardrev is used for:

 - a printk
 - olpc_board_at_least() which is used in:
    - drivers/input/mouse/hgpk.c:                mouse driver quirk
    - sound/pci/cs5535audio/cs5535audio_olpc.c:  audio quirk

aha. I missed this roundabout impact:

    - arch/x86/kernel/olpc.c:                    OLPC_F_DCON flag
      - which is used in olpc_has_dcon(), which is used in:
        - drivers/video/geode/gxfb_core.c:       GX modes db
                                                 [ouch if this goes wrong!]
        - drivers/video/geode/lxfb_core.c:       ditto

So i guess the mouse and audio quirk point was correct too, but the
major impact is the graphics mode array mismatch and the resulting
nonsensical mode setting, right?

	Ingo

Re: [PATCH] olpc: fix model detection without OFW

[Andres Salomon]

On Sat, 14 Feb 2009 23:14:29 +0100
Ingo Molnar <mingo@elte.hu> wrote:
[...]
> Where did my git grepping skills go wrong?
> 
> I git-grepped the code and this is the scope i found:
> 
> olpc_platform_info.boardrev is used for:
> 
>  - a printk
>  - olpc_board_at_least() which is used in:
>     - drivers/input/mouse/hgpk.c:                mouse driver quirk
>     - sound/pci/cs5535audio/cs5535audio_olpc.c:  audio quirk
> 
> aha. I missed this roundabout impact:
> 
>     - arch/x86/kernel/olpc.c:                    OLPC_F_DCON flag
>       - which is used in olpc_has_dcon(), which is used in:
>         - drivers/video/geode/gxfb_core.c:       GX modes db
>                                                  [ouch if this goes
> wrong!]
>         - drivers/video/geode/lxfb_core.c:       ditto
> 
> So i guess the mouse and audio quirk point was correct too, but the
> major impact is the graphics mode array mismatch and the resulting
> nonsensical mode setting, right?
> 
> 	Ingo


Yep.