linux-next: Tree for June 9

Hi all,

Changes since 20100608:

My fixes tree contains:
v4l-dvb: update gfp/slab.h includes
arm: update gfp/slab.h includes
davinci: update gfp/slab.h includes
ocfs2: update gfp/slab.h includes
acpi: update gfp/slab.h includes

Quoted 9 lines

Changes since 20100608:

My fixes tree contains:
v4l-dvb: update gfp/slab.h includes
arm: update gfp/slab.h includes
davinci: update gfp/slab.h includes
ocfs2: update gfp/slab.h includes
acpi: update gfp/slab.h includes

on x86_64 or i386, CONFIG_OF_DEVICE is not enabled:

drivers/net/niu.c:9700: warning: 'struct of_device' declared inside parameter list drivers/net/niu.c:9700: warning: its scope is only this definition or declaration, which is probably not what you want drivers/net/niu.c:9716: warning: assignment from incompatible pointer type

From: Randy Dunlap <[email protected]>
Date: Wed, 9 Jun 2010 10:36:57 -0700

Quoted 13 lines · On Wed, 9 Jun 2010 13:34:43 +1000 Stephen Rothwell wrote:

Quoted 9 lines

Changes since 20100608:

My fixes tree contains:
v4l-dvb: update gfp/slab.h includes
arm: update gfp/slab.h includes
davinci: update gfp/slab.h includes
ocfs2: update gfp/slab.h includes
acpi: update gfp/slab.h includes

Quoted 5 lines

on x86_64 or i386, CONFIG_OF_DEVICE is not enabled:

drivers/net/niu.c:9700: warning: 'struct of_device' declared inside parameter list drivers/net/niu.c:9700: warning: its scope is only this definition or declaration, which is probably not what you want drivers/net/niu.c:9716: warning: assignment from incompatible pointer type

Hmmm, I'm confused why this never happened before :-)

We conditionalize linux/of_device.h inclusion with CONFIG_SPARC64, yet we unconditionally use "struct of_device *" pointers in the driver with no such ifdef protection.

Even if we unconditionally included linux/of_device.h, that file does nothing unless CONFIG_OF_DEVICE is defined so it should have always produced these warnings since I can't see from where else it could have gotten even a "struct of_device;" somewhere.

Do you have any idea Randy? Pease try analyze this further so we can fix it properly.

From: Randy Dunlap <[email protected]>

Drivers may use struct of_device (struct platform_device), even when CONFIG_OF_DEVICE is not enabled, so minimally provide that struct for that kconfig case.

Signed-off-by Randy Dunlap <[email protected]>

Cc Grant Likely <[email protected]>

Cc Dave Miller <[email protected]>

---
 include/linux/of_device.h |    4 ++++

 1 file changed, 4 insertions(+)

From: Randy Dunlap <[email protected]>

The niu driver uses struct of_device when built on any arch, not only SPARC64, so always #include <linux/of_device.h>.

drivers/net/niu.c:9700: warning: 'struct of_device' declared inside parameter list drivers/net/niu.c:9700: warning: its scope is only this definition or declaration, which is probably not what you want drivers/net/niu.c:9716: warning: assignment from incompatible pointer type

Signed-off-by Randy Dunlap <[email protected]>

Cc Grant Likely <[email protected]>

Cc Dave Miller <[email protected]>

---
 drivers/net/niu.c |    3 ---

 1 file changed, 3 deletions(-)

drivers/net/niu.c +0 -3

@@ -28,10 +28,7 @@
  #include <linux/slab.h>
 
  #include <linux/io.h>
-
-#ifdef CONFIG_SPARC64
  #include <linux/of_device.h>
-#endif
 
  #include "niu.h"
 

Quoted 9 lines

From: Randy Dunlap <[email protected]>

The niu driver uses struct of_device when built on any arch, not only SPARC64, so always #include <linux/of_device.h>.

drivers/net/niu.c:9700: warning: 'struct of_device' declared inside parameter list drivers/net/niu.c:9700: warning: its scope is only this definition or declaration, which is probably not what you want drivers/net/niu.c:9716: warning: assignment from incompatible pointer type

Quoted 5 lines

Signed-off-by Randy Dunlap <[email protected]>

Cc Grant Likely <[email protected]>

Cc Dave Miller <[email protected]>

---
 drivers/net/niu.c |    3 ---

Looks okay to me (but I haven't build tested it yet). The bulk of of_device.h is compiled out when CONFIG_OF_DEVICE is not selected.

David, are you okay with me taking this via my tree as it depends on Randy's other patch?

drivers/input/keyboard/Kconfig config KEYBOARD_ATARI · line 73

Quoted 6 lines · Randy Dunlap

+++ linux-next-20100609/drivers/input/keyboard/Kconfig

drivers/input/keyboard/Kconfig +1 -0

@@ -70,9 +70,10 @@ config KEYBOARD_ATARI
 
 config KEYBOARD_ATKBD
        tristate "AT keyboard" if EMBEDDED || !X86
+       depends on !X86 || (X86 && !X86_MRST)

Should it be simply 'depends on !X86_MRST' and then we could kill '!X86_MRST' conditionals in selects?

drivers/input/keyboard/Kconfig config KEYBOARD_ATARI · line 79

Quoted 8 lines

drivers/input/keyboard/Kconfig +1 -1

        default y
        select SERIO
-       select SERIO_LIBPS2
+       select SERIO_LIBPS2 if !X86_MRST
        select SERIO_I8042 if X86 && !X86_MRST
        select SERIO_GSCPS2 if GSC
        help

Thanks.

Quoted 4 lines · Dmitry Torokhov

Quoted 3 lines · Randy Dunlap

+++ linux-next-20100609/drivers/input/keyboard/Kconfig

(quoted diff) +0 -0

@@ -70,9 +70,10 @@ config KEYBOARD_ATARI
 

Quoted 6 lines

Review config KEYBOARD_ATARI · line 73

Quoted 3 lines

(quoted diff) +1 -0

 config KEYBOARD_ATKBD
        tristate "AT keyboard" if EMBEDDED || !X86
+       depends on !X86 || (X86 && !X86_MRST)

Should it be simply 'depends on !X86_MRST' and then we could kill '!X86_MRST' conditionals in selects?

Duh, that sounds good, yes.

Quoted 12 lines · Randy Dunlap

Quoted 10 lines · Dmitry Torokhov

drivers/input/keyboard/Kconfig config KEYBOARD_ATARI · line 73

Quoted 6 lines · Randy Dunlap

+++ linux-next-20100609/drivers/input/keyboard/Kconfig

drivers/input/keyboard/Kconfig +1 -0

@@ -70,9 +70,10 @@ config KEYBOARD_ATARI
 
 config KEYBOARD_ATKBD
        tristate "AT keyboard" if EMBEDDED || !X86
+       depends on !X86 || (X86 && !X86_MRST)

Should it be simply 'depends on !X86_MRST' and then we could kill '!X86_MRST' conditionals in selects?

Quoted 1 line

Duh, that sounds good, yes.

Actually, I do not think this is a correct approach. While Moorestown does not have i8042 theoretically it is possible to add AT-style keyboard by other means (however unlikely it is) so we should not be disabling it.

We should, however, disallow i8042 from being selected. Could you please tell me if the patch below works for you?

Quoted 12 lines · Randy Dunlap

Quoted 10 lines · Dmitry Torokhov

drivers/input/keyboard/Kconfig config KEYBOARD_ATARI · line 73

Quoted 6 lines · Randy Dunlap

+++ linux-next-20100609/drivers/input/keyboard/Kconfig

drivers/input/keyboard/Kconfig +1 -0

@@ -70,9 +70,10 @@ config KEYBOARD_ATARI
 
 config KEYBOARD_ATKBD
        tristate "AT keyboard" if EMBEDDED || !X86
+       depends on !X86 || (X86 && !X86_MRST)

Should it be simply 'depends on !X86_MRST' and then we could kill '!X86_MRST' conditionals in selects?

Quoted 1 line

Duh, that sounds good, yes.

Actually, I do not think this is a correct approach. While Moorestown does not have i8042 theoretically it is possible to add AT-style keyboard by other means (however unlikely it is) so we should not be disabling it.

We should, however, disallow i8042 from being selected. Could you please tell me if the patch below works for you?

Quoted 17 lines · Dmitry Torokhov

Quoted 7 lines · Randy Dunlap

Quoted 6 lines · Dmitry Torokhov

Quoted 5 lines · Randy Dunlap

+++ linux-next-20100609/drivers/input/keyboard/Kconfig

drivers/input/keyboard/Kconfig +0 -0

@@ -70,9 +70,10 @@ config KEYBOARD_ATARI
 
 config KEYBOARD_ATKBD
 

Quoted 8 lines

Quoted 6 lines

Review config KEYBOARD_ATARI · line 75

Quoted 3 lines

(quoted diff) +1 -0

        tristate "AT keyboard" if EMBEDDED || !X86
 
+       depends on !X86 || (X86 && !X86_MRST)

Should it be simply 'depends on !X86_MRST' and then we could kill '!X86_MRST' conditionals in selects?

Duh, that sounds good, yes.

Quoted 7 lines

Actually, I do not think this is a correct approach. While Moorestown does not have i8042 theoretically it is possible to add AT-style keyboard by other means (however unlikely it is) so we should not be disabling it.

We should, however, disallow i8042 from being selected. Could you please tell me if the patch below works for you?

Dmitry,

This patch (in current mainline git) causes a problem when X86_MRST is enabled. CONFIG_SERIO_I8042 is no longer enabled when X86_MRST is enabled, and X86_MRST could be enabled when someone is trying to build a generic kernel.

Quoted 26 lines · Randy Dunlap

Quoted 17 lines · Dmitry Torokhov

Quoted 7 lines · Randy Dunlap

Quoted 6 lines · Dmitry Torokhov

Quoted 5 lines · Randy Dunlap

+++ linux-next-20100609/drivers/input/keyboard/Kconfig

drivers/input/keyboard/Kconfig +0 -0

@@ -70,9 +70,10 @@ config KEYBOARD_ATARI
 
 config KEYBOARD_ATKBD
 

Quoted 8 lines

Quoted 6 lines

Review config KEYBOARD_ATARI · line 75

Quoted 3 lines

(quoted diff) +1 -0

        tristate "AT keyboard" if EMBEDDED || !X86
 
+       depends on !X86 || (X86 && !X86_MRST)

Should it be simply 'depends on !X86_MRST' and then we could kill '!X86_MRST' conditionals in selects?

Duh, that sounds good, yes.

Quoted 7 lines

Actually, I do not think this is a correct approach. While Moorestown does not have i8042 theoretically it is possible to add AT-style keyboard by other means (however unlikely it is) so we should not be disabling it.

We should, however, disallow i8042 from being selected. Could you please tell me if the patch below works for you?

Quoted 6 lines

Dmitry,

This patch (in current mainline git) causes a problem when X86_MRST is enabled. CONFIG_SERIO_I8042 is no longer enabled when X86_MRST is enabled, and X86_MRST could be enabled when someone is trying to build a generic kernel.

Randy,

Looking at arch/x86/kernel/mrst.c I am not sure if generic kernel will work well witbh X86_MRST set. However if we supposed to support Moorestown on generic images then, instead of playing with Kconfig, we need to adjust i8042_platform_init() to abort on Moorestown.

-----Original Message-----
From: Randy Dunlap [mailto:[email protected]]
Sent: Monday, June 28, 2010 1:24 PM
To: Dmitry Torokhov
Cc: Stephen Rothwell; Pan, Jacob jun; [email protected]; LKML; [email protected]; akpm; [email protected]
Subject: Re: problem: Re: [PATCH] input: fixup X86_MRST selects

Quoted 12 lines · Dmitry Torokhov

Quoted 11 lines · Randy Dunlap

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines · Randy Dunlap

Quoted 8 lines · Dmitry Torokhov

Quoted 7 lines · Randy Dunlap

+++ linux-next-20100609/drivers/input/keyboard/Kconfig

(quoted diff) +0 -0

@@ -70,9 +70,10 @@ config KEYBOARD_ATARI
 
 config KEYBOARD_ATKBD
 
        tristate "AT keyboard" if EMBEDDED || !X86
 

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Review config KEYBOARD_ATARI · line 75

Quoted 1 line

(quoted diff) +1 -0

+       depends on !X86 || (X86 && !X86_MRST)

Should it be simply 'depends on !X86_MRST' and then we could kill '!X86_MRST' conditionals in selects?

Duh, that sounds good, yes.

Actually, I do not think this is a correct approach. While

Moorestown

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

does not have i8042 theoretically it is possible to add AT-style keyboard by other means (however unlikely it is) so we should not be disabling it.

We should, however, disallow i8042 from being selected. Could you

please

Quoted 5 lines

Quoted 5 lines

Quoted 1 line

tell me if the patch below works for you?

Dmitry,

This patch (in current mainline git) causes a problem when X86_MRST

is

Quoted 1 line

Quoted 1 line

enabled. CONFIG_SERIO_I8042 is no longer enabled when X86_MRST is

enabled,

Quoted 1 line

Quoted 1 line

and X86_MRST could be enabled when someone is trying to build a

generic

Quoted 6 lines

Quoted 1 line

kernel.

Randy,

Looking at arch/x86/kernel/mrst.c I am not sure if generic kernel will work well witbh X86_MRST set. However if we supposed to support

Moorestown

Quoted 1 line

on generic images then, instead of playing with Kconfig, we need to

adjust

Quoted 1 line

i8042_platform_init() to abort on Moorestown.

OK. Until something else is done, you might want to revert this patch and just live with the build errors (that caused me to send the original patch).

Quoted 1 line

We do intend to maintain binary compatibility between generic kernel and Moorestown. I guess the challenge is not having enumeration of i8042 pass to the driver. Do you prefer abort i8042_platform_init() based on #define CONFIG_X86_MRST? It is no safe to probe HW on Moorestown, unfortunately.

-----Original Message-----
From: Dmitry Torokhov [mailto:[email protected]]
Sent: Monday, June 28, 2010 3:45 PM
To: Pan, Jacob jun
Cc: Randy Dunlap; Stephen Rothwell; [email protected]; LKML; [email protected]; akpm; [email protected]; H. Peter Anvin
Subject: Re: problem: Re: [PATCH] input: fixup X86_MRST selects

Quoted 2 lines · Jacob jun

Quoted 1 line

We do intend to maintain binary compatibility between generic kernel

and Moorestown.

Quoted 1 line

I guess the challenge is not having enumeration of i8042 pass to the

driver. Do you

Quoted 1 line

prefer abort i8042_platform_init() based on #define CONFIG_X86_MRST?

It is no safe

Quoted 1 line

to probe HW on Moorestown, unfortunately.

Any check based on CONFIG_X86_MRST means that kernel is not generic. We'd need a runtime check (but not necessarily one that bangs ports). Is there something in processor flags, or DMI, or similar that woudl allow i8042 to see that it runs on Moorestown?

--
Dmitry

There is no DMI support yet in MRST FW. We have a new x86 HW subarch ID in boot_param, then we use it to select x86_init abstractions. Both boot_param and x86_init are x86 arch specific so I guess we can use them in 8042 driver under CONFIG_X86. Not sure if it is possible to move x86 i8042_platform_init under x86_init (the x86 part).

Quoted 6 lines · Jacob jun

There is no DMI support yet in MRST FW. We have a new x86 HW subarch ID in boot_param, then we use it to select x86_init abstractions. Both boot_param and x86_init are x86 arch specific so I guess we can use them in 8042 driver under CONFIG_X86. Not sure if it is possible to move x86 i8042_platform_init under x86_init (the x86 part).

Moving i8042_platform_init() into platform code is quite invasive, how about we to the following?

Thanks.