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@xxxxxxxxxx> wrote:
> > > On Fri, 10 Nov 2017, Arnd Bergmann wrote:
> > >
> > >> On Fri, Nov 10, 2017 at 5:35 PM, Nicolas Pitre <nicolas.pitre@xxxxxxxxxx> 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@xxxxxxxx>
> > >> >> ---
> > >> >>  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