[PATCH] cramfs: fix MTD dependency

[PATCH] cramfs: fix MTD dependency

[Arnd Bergmann]

With CONFIG_MTD=m and CONFIG_CRAMFS=y, we now get a link failure:

fs/cramfs/inode.o: In function `cramfs_mount':
inode.c:(.text+0x220): undefined reference to `mount_mtd'
fs/cramfs/inode.o: In function `cramfs_mtd_fill_super':
inode.c:(.text+0x6d8): undefined reference to `mtd_point'
inode.c:(.text+0xae4): undefined reference to `mtd_unpoint'

This adds a more specific Kconfig dependency to avoid the
broken configuration. Alternatively we could make CRAMFS
itself depend on "MTD || !MTD" with a similar result.

Fixes: 99c18ce580c6 ("cramfs: direct memory access support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/cramfs/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
index f937082f3244..58e2fe40b2a0 100644
--- a/fs/cramfs/Kconfig
+++ b/fs/cramfs/Kconfig
@@ -34,6 +34,7 @@ config CRAMFS_BLOCKDEV
 config CRAMFS_MTD
 	bool "Support CramFs image directly mapped in physical memory"
 	depends on CRAMFS && MTD
+	depends on CRAMFS=m || MTD=y
 	default y if !CRAMFS_BLOCKDEV
 	help
 	  This option allows the CramFs driver to load data directly from
-- 
2.9.0

Re: [PATCH] cramfs: fix MTD dependency

[Nicolas Pitre]

On Fri, 10 Nov 2017, Arnd Bergmann wrote:

> With CONFIG_MTD=m and CONFIG_CRAMFS=y, we now get a link failure:
> 
> fs/cramfs/inode.o: In function `cramfs_mount':
> inode.c:(.text+0x220): undefined reference to `mount_mtd'
> fs/cramfs/inode.o: In function `cramfs_mtd_fill_super':
> inode.c:(.text+0x6d8): undefined reference to `mtd_point'
> inode.c:(.text+0xae4): undefined reference to `mtd_unpoint'
> 
> This adds a more specific Kconfig dependency to avoid the
> broken configuration. Alternatively we could make CRAMFS
> itself depend on "MTD || !MTD" with a similar result.
> 
> Fixes: 99c18ce580c6 ("cramfs: direct memory access support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/cramfs/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> index f937082f3244..58e2fe40b2a0 100644
> --- a/fs/cramfs/Kconfig
> +++ b/fs/cramfs/Kconfig
> @@ -34,6 +34,7 @@ config CRAMFS_BLOCKDEV
>  config CRAMFS_MTD
>  	bool "Support CramFs image directly mapped in physical memory"
>  	depends on CRAMFS && MTD
> +	depends on CRAMFS=m || MTD=y

I think the following is better:

diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
index f937082f32..a00740c668 100644
--- a/fs/cramfs/Kconfig
+++ b/fs/cramfs/Kconfig
@@ -33,7 +33,7 @@ config CRAMFS_BLOCKDEV
 
 config CRAMFS_MTD
 	bool "Support CramFs image directly mapped in physical memory"
-	depends on CRAMFS && MTD
+	depends on CRAMFS && (CRAMFS <= MTD)
 	default y if !CRAMFS_BLOCKDEV
 	help
 	  This option allows the CramFs driver to load data directly from

Nicolas

Re: [PATCH] cramfs: fix MTD dependency

[Arnd Bergmann]

On Fri, Nov 10, 2017 at 5:35 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Fri, 10 Nov 2017, Arnd Bergmann wrote:
>
>> With CONFIG_MTD=m and CONFIG_CRAMFS=y, we now get a link failure:
>>
>> fs/cramfs/inode.o: In function `cramfs_mount':
>> inode.c:(.text+0x220): undefined reference to `mount_mtd'
>> fs/cramfs/inode.o: In function `cramfs_mtd_fill_super':
>> inode.c:(.text+0x6d8): undefined reference to `mtd_point'
>> inode.c:(.text+0xae4): undefined reference to `mtd_unpoint'
>>
>> This adds a more specific Kconfig dependency to avoid the
>> broken configuration. Alternatively we could make CRAMFS
>> itself depend on "MTD || !MTD" with a similar result.
>>
>> Fixes: 99c18ce580c6 ("cramfs: direct memory access support")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  fs/cramfs/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
>> index f937082f3244..58e2fe40b2a0 100644
>> --- a/fs/cramfs/Kconfig
>> +++ b/fs/cramfs/Kconfig
>> @@ -34,6 +34,7 @@ config CRAMFS_BLOCKDEV
>>  config CRAMFS_MTD
>>       bool "Support CramFs image directly mapped in physical memory"
>>       depends on CRAMFS && MTD
>> +     depends on CRAMFS=m || MTD=y
>
> I think the following is better:
>
> diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> index f937082f32..a00740c668 100644
> --- a/fs/cramfs/Kconfig
> +++ b/fs/cramfs/Kconfig
> @@ -33,7 +33,7 @@ config CRAMFS_BLOCKDEV
>
>  config CRAMFS_MTD
>         bool "Support CramFs image directly mapped in physical memory"
> -       depends on CRAMFS && MTD
> +       depends on CRAMFS && (CRAMFS <= MTD)
>         default y if !CRAMFS_BLOCKDEV
>         help
>           This option allows the CramFs driver to load data directly from

I've never seen that syntax, what does it mean?

       Arnd

Re: [PATCH] cramfs: fix MTD dependency

[Nicolas Pitre]

On Fri, 10 Nov 2017, Arnd Bergmann wrote:

> On Fri, Nov 10, 2017 at 5:35 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Fri, 10 Nov 2017, Arnd Bergmann wrote:
> >
> >> With CONFIG_MTD=m and CONFIG_CRAMFS=y, we now get a link failure:
> >>
> >> fs/cramfs/inode.o: In function `cramfs_mount':
> >> inode.c:(.text+0x220): undefined reference to `mount_mtd'
> >> fs/cramfs/inode.o: In function `cramfs_mtd_fill_super':
> >> inode.c:(.text+0x6d8): undefined reference to `mtd_point'
> >> inode.c:(.text+0xae4): undefined reference to `mtd_unpoint'
> >>
> >> This adds a more specific Kconfig dependency to avoid the
> >> broken configuration. Alternatively we could make CRAMFS
> >> itself depend on "MTD || !MTD" with a similar result.
> >>
> >> Fixes: 99c18ce580c6 ("cramfs: direct memory access support")
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >>  fs/cramfs/Kconfig | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> >> index f937082f3244..58e2fe40b2a0 100644
> >> --- a/fs/cramfs/Kconfig
> >> +++ b/fs/cramfs/Kconfig
> >> @@ -34,6 +34,7 @@ config CRAMFS_BLOCKDEV
> >>  config CRAMFS_MTD
> >>       bool "Support CramFs image directly mapped in physical memory"
> >>       depends on CRAMFS && MTD
> >> +     depends on CRAMFS=m || MTD=y
> >
> > I think the following is better:
> >
> > diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> > index f937082f32..a00740c668 100644
> > --- a/fs/cramfs/Kconfig
> > +++ b/fs/cramfs/Kconfig
> > @@ -33,7 +33,7 @@ config CRAMFS_BLOCKDEV
> >
> >  config CRAMFS_MTD
> >         bool "Support CramFs image directly mapped in physical memory"
> > -       depends on CRAMFS && MTD
> > +       depends on CRAMFS && (CRAMFS <= MTD)
> >         default y if !CRAMFS_BLOCKDEV
> >         help
> >           This option allows the CramFs driver to load data directly from
> 
> I've never seen that syntax, what does it mean?

In the Kconfig language: n < m < y. Therefore (m < y) is true and
(y < m) is false.


Nicolas

Re: [PATCH] cramfs: fix MTD dependency

[Arnd Bergmann]

On Fri, Nov 10, 2017 at 6:01 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Fri, 10 Nov 2017, Arnd Bergmann wrote:
>
>> On Fri, Nov 10, 2017 at 5:35 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>> > On Fri, 10 Nov 2017, Arnd Bergmann wrote:
>> >
>> >> With CONFIG_MTD=m and CONFIG_CRAMFS=y, we now get a link failure:
>> >>
>> >> fs/cramfs/inode.o: In function `cramfs_mount':
>> >> inode.c:(.text+0x220): undefined reference to `mount_mtd'
>> >> fs/cramfs/inode.o: In function `cramfs_mtd_fill_super':
>> >> inode.c:(.text+0x6d8): undefined reference to `mtd_point'
>> >> inode.c:(.text+0xae4): undefined reference to `mtd_unpoint'
>> >>
>> >> This adds a more specific Kconfig dependency to avoid the
>> >> broken configuration. Alternatively we could make CRAMFS
>> >> itself depend on "MTD || !MTD" with a similar result.
>> >>
>> >> Fixes: 99c18ce580c6 ("cramfs: direct memory access support")
>> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> >> ---
>> >>  fs/cramfs/Kconfig | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
>> >> index f937082f3244..58e2fe40b2a0 100644
>> >> --- a/fs/cramfs/Kconfig
>> >> +++ b/fs/cramfs/Kconfig
>> >> @@ -34,6 +34,7 @@ config CRAMFS_BLOCKDEV
>> >>  config CRAMFS_MTD
>> >>       bool "Support CramFs image directly mapped in physical memory"
>> >>       depends on CRAMFS && MTD
>> >> +     depends on CRAMFS=m || MTD=y
>> >
>> > I think the following is better:
>> >
>> > diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
>> > index f937082f32..a00740c668 100644
>> > --- a/fs/cramfs/Kconfig
>> > +++ b/fs/cramfs/Kconfig
>> > @@ -33,7 +33,7 @@ config CRAMFS_BLOCKDEV
>> >
>> >  config CRAMFS_MTD
>> >         bool "Support CramFs image directly mapped in physical memory"
>> > -       depends on CRAMFS && MTD
>> > +       depends on CRAMFS && (CRAMFS <= MTD)
>> >         default y if !CRAMFS_BLOCKDEV
>> >         help
>> >           This option allows the CramFs driver to load data directly from
>>
>> I've never seen that syntax, what does it mean?
>
> In the Kconfig language: n < m < y. Therefore (m < y) is true and
> (y < m) is false.

I see. However, since I didn't recognize that syntax, and it doesn't seem
to be used much, I would still prefer my original suggestion, which
I think is more common.

      Arnd

Re: [PATCH] cramfs: fix MTD dependency

[Nicolas Pitre]

On Wed, 15 Nov 2017, Arnd Bergmann wrote:

> On Fri, Nov 10, 2017 at 6:01 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Fri, 10 Nov 2017, Arnd Bergmann wrote:
> >
> >> On Fri, Nov 10, 2017 at 5:35 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >> > On Fri, 10 Nov 2017, Arnd Bergmann wrote:
> >> >
> >> >> With CONFIG_MTD=m and CONFIG_CRAMFS=y, we now get a link failure:
> >> >>
> >> >> fs/cramfs/inode.o: In function `cramfs_mount':
> >> >> inode.c:(.text+0x220): undefined reference to `mount_mtd'
> >> >> fs/cramfs/inode.o: In function `cramfs_mtd_fill_super':
> >> >> inode.c:(.text+0x6d8): undefined reference to `mtd_point'
> >> >> inode.c:(.text+0xae4): undefined reference to `mtd_unpoint'
> >> >>
> >> >> This adds a more specific Kconfig dependency to avoid the
> >> >> broken configuration. Alternatively we could make CRAMFS
> >> >> itself depend on "MTD || !MTD" with a similar result.
> >> >>
> >> >> Fixes: 99c18ce580c6 ("cramfs: direct memory access support")
> >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> >> ---
> >> >>  fs/cramfs/Kconfig | 1 +
> >> >>  1 file changed, 1 insertion(+)
> >> >>
> >> >> diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> >> >> index f937082f3244..58e2fe40b2a0 100644
> >> >> --- a/fs/cramfs/Kconfig
> >> >> +++ b/fs/cramfs/Kconfig
> >> >> @@ -34,6 +34,7 @@ config CRAMFS_BLOCKDEV
> >> >>  config CRAMFS_MTD
> >> >>       bool "Support CramFs image directly mapped in physical memory"
> >> >>       depends on CRAMFS && MTD
> >> >> +     depends on CRAMFS=m || MTD=y
> >> >
> >> > I think the following is better:
> >> >
> >> > diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> >> > index f937082f32..a00740c668 100644
> >> > --- a/fs/cramfs/Kconfig
> >> > +++ b/fs/cramfs/Kconfig
> >> > @@ -33,7 +33,7 @@ config CRAMFS_BLOCKDEV
> >> >
> >> >  config CRAMFS_MTD
> >> >         bool "Support CramFs image directly mapped in physical memory"
> >> > -       depends on CRAMFS && MTD
> >> > +       depends on CRAMFS && (CRAMFS <= MTD)
> >> >         default y if !CRAMFS_BLOCKDEV
> >> >         help
> >> >           This option allows the CramFs driver to load data directly from
> >>
> >> I've never seen that syntax, what does it mean?
> >
> > In the Kconfig language: n < m < y. Therefore (m < y) is true and
> > (y < m) is false.
> 
> I see. However, since I didn't recognize that syntax, and it doesn't seem
> to be used much, I would still prefer my original suggestion, which
> I think is more common.

How can a "better" way become common enough for you to use it if you 
don't contribute to make it more common?


Nicolas

Re: [PATCH] cramfs: fix MTD dependency

[Nicolas Pitre]

On Wed, 15 Nov 2017, Nicolas Pitre wrote:

> On Wed, 15 Nov 2017, Arnd Bergmann wrote:
> 
> > On Fri, Nov 10, 2017 at 6:01 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > On Fri, 10 Nov 2017, Arnd Bergmann wrote:
> > >
> > >> On Fri, Nov 10, 2017 at 5:35 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > >> > On Fri, 10 Nov 2017, Arnd Bergmann wrote:
> > >> >
> > >> >> With CONFIG_MTD=m and CONFIG_CRAMFS=y, we now get a link failure:
> > >> >>
> > >> >> fs/cramfs/inode.o: In function `cramfs_mount':
> > >> >> inode.c:(.text+0x220): undefined reference to `mount_mtd'
> > >> >> fs/cramfs/inode.o: In function `cramfs_mtd_fill_super':
> > >> >> inode.c:(.text+0x6d8): undefined reference to `mtd_point'
> > >> >> inode.c:(.text+0xae4): undefined reference to `mtd_unpoint'
> > >> >>
> > >> >> This adds a more specific Kconfig dependency to avoid the
> > >> >> broken configuration. Alternatively we could make CRAMFS
> > >> >> itself depend on "MTD || !MTD" with a similar result.
> > >> >>
> > >> >> Fixes: 99c18ce580c6 ("cramfs: direct memory access support")
> > >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > >> >> ---
> > >> >>  fs/cramfs/Kconfig | 1 +
> > >> >>  1 file changed, 1 insertion(+)
> > >> >>
> > >> >> diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> > >> >> index f937082f3244..58e2fe40b2a0 100644
> > >> >> --- a/fs/cramfs/Kconfig
> > >> >> +++ b/fs/cramfs/Kconfig
> > >> >> @@ -34,6 +34,7 @@ config CRAMFS_BLOCKDEV
> > >> >>  config CRAMFS_MTD
> > >> >>       bool "Support CramFs image directly mapped in physical memory"
> > >> >>       depends on CRAMFS && MTD
> > >> >> +     depends on CRAMFS=m || MTD=y
> > >> >
> > >> > I think the following is better:
> > >> >
> > >> > diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> > >> > index f937082f32..a00740c668 100644
> > >> > --- a/fs/cramfs/Kconfig
> > >> > +++ b/fs/cramfs/Kconfig
> > >> > @@ -33,7 +33,7 @@ config CRAMFS_BLOCKDEV
> > >> >
> > >> >  config CRAMFS_MTD
> > >> >         bool "Support CramFs image directly mapped in physical memory"
> > >> > -       depends on CRAMFS && MTD
> > >> > +       depends on CRAMFS && (CRAMFS <= MTD)
> > >> >         default y if !CRAMFS_BLOCKDEV
> > >> >         help
> > >> >           This option allows the CramFs driver to load data directly from
> > >>
> > >> I've never seen that syntax, what does it mean?
> > >
> > > In the Kconfig language: n < m < y. Therefore (m < y) is true and
> > > (y < m) is false.
> > 
> > I see. However, since I didn't recognize that syntax, and it doesn't seem
> > to be used much, I would still prefer my original suggestion, which
> > I think is more common.
> 
> How can a "better" way become common enough for you to use it if you 
> don't contribute to make it more common?

FYI, this ability was introduced in commit 31847b67bec0 i.e. Linux v4.2.
That's relatively recent and might explain why it has not been used 
much so far... and the fact that no one updated 
Documentation/kbuild/kconfig-language.txt to match.

However, while documenting it, I wanted to test the actual expression
result and stumbled on a problem. The comparison for bool/tristate 
symbols is done as if those were plain strings i.e. "m" < "n" < "y".
I've produced a patch to fix this.

Nicolas

Re: [PATCH] cramfs: fix MTD dependency

[Nicolas Pitre]

On Fri, 10 Nov 2017, Arnd Bergmann wrote:

> With CONFIG_MTD=m and CONFIG_CRAMFS=y, we now get a link failure:
> 
> fs/cramfs/inode.o: In function `cramfs_mount':
> inode.c:(.text+0x220): undefined reference to `mount_mtd'
> fs/cramfs/inode.o: In function `cramfs_mtd_fill_super':
> inode.c:(.text+0x6d8): undefined reference to `mtd_point'
> inode.c:(.text+0xae4): undefined reference to `mtd_unpoint'
> 
> This adds a more specific Kconfig dependency to avoid the
> broken configuration. Alternatively we could make CRAMFS
> itself depend on "MTD || !MTD" with a similar result.
> 
> Fixes: 99c18ce580c6 ("cramfs: direct memory access support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Well, my alternative fix depends on a kconfig fix which has not received 
any feedback yet. So let's do the obvious for now to cut on the build 
breakage reports.  Please could you push your patch upstream?

Acked-by- Nicolas Pitre <nico@linaro.org>

> ---
>  fs/cramfs/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/cramfs/Kconfig b/fs/cramfs/Kconfig
> index f937082f3244..58e2fe40b2a0 100644
> --- a/fs/cramfs/Kconfig
> +++ b/fs/cramfs/Kconfig
> @@ -34,6 +34,7 @@ config CRAMFS_BLOCKDEV
>  config CRAMFS_MTD
>  	bool "Support CramFs image directly mapped in physical memory"
>  	depends on CRAMFS && MTD
> +	depends on CRAMFS=m || MTD=y
>  	default y if !CRAMFS_BLOCKDEV
>  	help
>  	  This option allows the CramFs driver to load data directly from
> -- 
> 2.9.0
> 
>