[patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Adrian Bunk]

On Thu, Nov 18, 2004 at 02:15:38AM -0800, Andrew Morton wrote:
>...
> Changes since 2.6.10-rc2-mm1:
>...
>  bk-mtd.patch
>...

Ths causes the following compile error:

<--  snip  -->

...
  CC      drivers/mtd/chips/cfi_probe.o
In file included from drivers/mtd/chips/cfi_probe.c:18:
include/linux/mtd/xip.h:77:2: #error "missing IRQ and timer primitives for XIP MTD support"
{standard input}: Assembler messages:
{standard input}:5: Warning: ignoring changed section attributes for .data
make[3]: *** [drivers/mtd/chips/cfi_probe.o] Error 1

<--  snip  -->


Let's put the dependencies from the #error into the Kconfig file:


Signed-off-by: Adrian Bunk <bunk@fs.tum.de>

--- linux-2.6.10-rc2-mm2-full/drivers/mtd/chips/Kconfig.old	2004-11-18 16:35:40.000000000 +0100
+++ linux-2.6.10-rc2-mm2-full/drivers/mtd/chips/Kconfig	2004-11-18 16:38:37.000000000 +0100
@@ -274,7 +274,7 @@
 
 config MTD_XIP
 	bool "XIP aware MTD support"
-	depends on !SMP && MTD_CFI_INTELEXT && EXPERIMENTAL
+	depends on !SMP && MTD_CFI_INTELEXT && (ARCH_SA1100 || ARCH_PXA) && EXPERIMENTAL
 	default y if XIP_KERNEL
 	help
 	  This allows MTD support to work with flash memory which is also

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[David Woodhouse]

On Thu, 2004-11-18 at 16:41 +0100, Adrian Bunk wrote:
> Let's put the dependencies from the #error into the Kconfig file:

Looks sane to me. Nico?

-- 
dwmw2

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Nicolas Pitre]

On Thu, 18 Nov 2004, David Woodhouse wrote:

> On Thu, 2004-11-18 at 16:41 +0100, Adrian Bunk wrote:
> > Let's put the dependencies from the #error into the Kconfig file:
> 
> Looks sane to me. Nico?

And why is the current arrangement actually a problem?


Nicolas

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Russell King]

On Thu, Nov 18, 2004 at 11:34:56AM -0500, Nicolas Pitre wrote:
> On Thu, 18 Nov 2004, David Woodhouse wrote:
> 
> > On Thu, 2004-11-18 at 16:41 +0100, Adrian Bunk wrote:
> > > Let's put the dependencies from the #error into the Kconfig file:
> > 
> > Looks sane to me. Nico?
> 
> And why is the current arrangement actually a problem?

because it prevents building with, eg, make allyesconfig

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Adrian Bunk]

On Thu, Nov 18, 2004 at 11:34:56AM -0500, Nicolas Pitre wrote:
> On Thu, 18 Nov 2004, David Woodhouse wrote:
> 
> > On Thu, 2004-11-18 at 16:41 +0100, Adrian Bunk wrote:
> > > Let's put the dependencies from the #error into the Kconfig file:
> > 
> > Looks sane to me. Nico?
> 
> And why is the current arrangement actually a problem?

If you are able to select an option, it should also compile (and work).

At least on i386, this is usually true for every single option.

> Nicolas

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Nicolas Pitre]

On Thu, 18 Nov 2004, Adrian Bunk wrote:

> On Thu, Nov 18, 2004 at 11:34:56AM -0500, Nicolas Pitre wrote:
> > On Thu, 18 Nov 2004, David Woodhouse wrote:
> > 
> > > On Thu, 2004-11-18 at 16:41 +0100, Adrian Bunk wrote:
> > > > Let's put the dependencies from the #error into the Kconfig file:
> > > 
> > > Looks sane to me. Nico?
> > 
> > And why is the current arrangement actually a problem?
> 
> If you are able to select an option, it should also compile (and work).
> 
> At least on i386, this is usually true for every single option.

Fine.  I thought the #error would encourage people to add the missing 
bits to that file.  No?  ;-)

Can we make it conditional on CONFIG_XIP_KERNEL instead?
It would be less messy IMHO.


Nicolas

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Adrian Bunk]

On Thu, Nov 18, 2004 at 05:31:32PM -0500, Nicolas Pitre wrote:
>...
> Can we make it conditional on CONFIG_XIP_KERNEL instead?
> It would be less messy IMHO.

I copied the dependency from the #ifdef before the #error.

The #error should either go or be the same than the Kconfig dependency.

> Nicolas

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Nicolas Pitre]

On Fri, 19 Nov 2004, Adrian Bunk wrote:

> On Thu, Nov 18, 2004 at 05:31:32PM -0500, Nicolas Pitre wrote:
> >...
> > Can we make it conditional on CONFIG_XIP_KERNEL instead?
> > It would be less messy IMHO.
> 
> I copied the dependency from the #ifdef before the #error.
> 
> The #error should either go or be the same than the Kconfig dependency.

And on what basis?  This just doesn't make sense.

CONFIG_MTD_XIP is there to be compatible with kernels which are made 
XIP.  This currently means _all_ ARM flavours the kernel currently 
supports.  Yet there is only SA11x0 and PXA2xx which have proper MTD_XIP 
primitives ence the #error.

My position is therefore that the CONFIG_MTD_XIP should depend on 
CONFIG_XIP_KERNEL since this is what it is for, and the #error stay as 
is.  If ever you make x86 kernel XIPable you'll need to add the missing 
bits guarded by the #error anyway.

And no, allyesconfig makes little sense on ARM as it has been discussed 
on lkml before.


Nicolas

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Adrian Bunk]

On Thu, Nov 18, 2004 at 08:58:26PM -0500, Nicolas Pitre wrote:
> On Fri, 19 Nov 2004, Adrian Bunk wrote:
> 
> > On Thu, Nov 18, 2004 at 05:31:32PM -0500, Nicolas Pitre wrote:
> > >...
> > > Can we make it conditional on CONFIG_XIP_KERNEL instead?
> > > It would be less messy IMHO.
> > 
> > I copied the dependency from the #ifdef before the #error.
> > 
> > The #error should either go or be the same than the Kconfig dependency.
> 
> And on what basis?  This just doesn't make sense.
> 
> CONFIG_MTD_XIP is there to be compatible with kernels which are made 
> XIP.  This currently means _all_ ARM flavours the kernel currently 
> supports.  Yet there is only SA11x0 and PXA2xx which have proper MTD_XIP 
> primitives ence the #error.
> 
> My position is therefore that the CONFIG_MTD_XIP should depend on 
> CONFIG_XIP_KERNEL since this is what it is for, and the #error stay as 
> is.  If ever you make x86 kernel XIPable you'll need to add the missing 
> bits guarded by the #error anyway.
> 
> And no, allyesconfig makes little sense on ARM as it has been discussed 
> on lkml before.

I'm not talking about allyesconfig.

The Kconfig file should express all dependencies of a driver.
If a driver doesn't compile, it should not be selectable - and not 
#error at compile time.

Rethinking it, perhaps the following expresses the dependencies best:

  depends on ... && XIP_KERNEL && (ARCH_SA1100 || ARCH_PXA || BROKEN)

This would push the #error as a dependency on BROKEN to the Kconfig 
file.

> Nicolas

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Nicolas Pitre]

On Fri, 19 Nov 2004, Adrian Bunk wrote:

> On Thu, Nov 18, 2004 at 08:58:26PM -0500, Nicolas Pitre wrote:
> > On Fri, 19 Nov 2004, Adrian Bunk wrote:
> > 
> > > The #error should either go or be the same than the Kconfig dependency.
> > 
> > And on what basis?  This just doesn't make sense.
> > 
> > CONFIG_MTD_XIP is there to be compatible with kernels which are made 
> > XIP.  This currently means _all_ ARM flavours the kernel currently 
> > supports.  Yet there is only SA11x0 and PXA2xx which have proper MTD_XIP 
> > primitives ence the #error.
> > 
> > My position is therefore that the CONFIG_MTD_XIP should depend on 
> > CONFIG_XIP_KERNEL since this is what it is for, and the #error stay as 
> > is.  If ever you make x86 kernel XIPable you'll need to add the missing 
> > bits guarded by the #error anyway.
> > 
> > And no, allyesconfig makes little sense on ARM as it has been discussed 
> > on lkml before.
> 
> I'm not talking about allyesconfig.
> 
> The Kconfig file should express all dependencies of a driver.
> If a driver doesn't compile, it should not be selectable - and not 
> #error at compile time.

Absolutely!

So please would you just ask Andrew to apply the following patch and be 
happy?  Thank you.

--- ./drivers/mtd/chips/Kconfig.orig	Fri Nov 19 11:25:45 2004
+++ ./drivers/mtd/chips/Kconfig	Fri Nov 19 11:28:08 2004
@@ -274,8 +274,7 @@
 
 config MTD_XIP
 	bool "XIP aware MTD support"
-	depends on !SMP && MTD_CFI_INTELEXT && EXPERIMENTAL
-	default y if XIP_KERNEL
+	depends on XIP_KERNEL && !SMP && MTD_CFI_INTELEXT && EXPERIMENTAL
 	help
 	  This allows MTD support to work with flash memory which is also
 	  used for XIP purposes.  If you're not sure what this is all about


Nicolas

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Adrian Bunk]

On Fri, Nov 19, 2004 at 11:35:26AM -0500, Nicolas Pitre wrote:
> On Fri, 19 Nov 2004, Adrian Bunk wrote:
> 
> >...
> > The Kconfig file should express all dependencies of a driver.
> > If a driver doesn't compile, it should not be selectable - and not 
> > #error at compile time.
> 
> Absolutely!

Good that we agree.  :-)

> So please would you just ask Andrew to apply the following patch and be 
> happy?  Thank you.

A slightly improved patch is below.

> --- ./drivers/mtd/chips/Kconfig.orig	Fri Nov 19 11:25:45 2004
> +++ ./drivers/mtd/chips/Kconfig	Fri Nov 19 11:28:08 2004
> @@ -274,8 +274,7 @@
>  
>  config MTD_XIP
>  	bool "XIP aware MTD support"
> -	depends on !SMP && MTD_CFI_INTELEXT && EXPERIMENTAL
> -	default y if XIP_KERNEL
> +	depends on XIP_KERNEL && !SMP && MTD_CFI_INTELEXT && EXPERIMENTAL
>  	help
>  	  This allows MTD support to work with flash memory which is also
>  	  used for XIP purposes.  If you're not sure what this is all about
> 
> 
> Nicolas


cu
Adrian

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.10-rc2-mm2-full/drivers/mtd/chips/Kconfig.old	2004-11-18 16:35:40.000000000 +0100
+++ linux-2.6.10-rc2-mm2-full/drivers/mtd/chips/Kconfig	2004-11-21 20:54:43.000000000 +0100
@@ -274,8 +274,8 @@
 
 config MTD_XIP
 	bool "XIP aware MTD support"
-	depends on !SMP && MTD_CFI_INTELEXT && EXPERIMENTAL
-	default y if XIP_KERNEL
+	depends on XIP_KERNEL && !SMP && MTD_CFI_INTELEXT && EXPERIMENTAL && (ARCH_SA1100 || ARCH_PXA || BROKEN)
+	default y
 	help
 	  This allows MTD support to work with flash memory which is also
 	  used for XIP purposes.  If you're not sure what this is all about

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[Nicolas Pitre]

On Sun, 21 Nov 2004, Adrian Bunk wrote:

> On Fri, Nov 19, 2004 at 11:35:26AM -0500, Nicolas Pitre wrote:
> 
> > On Fri, 19 Nov 2004, Adrian Bunk wrote:
> > 
> > > The Kconfig file should express all dependencies of a driver.
> > 
> > Absolutely!
> 
> Good that we agree.  :-)

On this very point only.

> > So please would you just ask Andrew to apply the following patch and be 
> > happy?  Thank you.
> 
> A slightly improved patch is below.

But I continue to disagree with your proposed patch.
And I'll bet that you will continue to disagree with mine.

Can we let the MTD maintainer arbitrate on this?


Nicolas

Re: [patch] 2.6.10-rc2-mm2: MTD_XIP dependencies

[David Woodhouse]

On Sun, 2004-11-21 at 19:38 -0500, Nicolas Pitre wrote:
> But I continue to disagree with your proposed patch.
> And I'll bet that you will continue to disagree with mine.
> 
> Can we let the MTD maintainer arbitrate on this?

Personally I prefer the #error. People with platforms which _don't_ yet
provide the underlying primitives which the XIP code needs will see the
option, turn it on and work out what they need to do. Otherwise, they'll
continue to be unaware that it even exists. 

I consider that to be more important than the case of someone who turns
it on when they don't actually want it.

-- 
dwmw2