| Message ID | 20171110145731.921271-1-arnd@arndb.de |
|---|---|
| State | Accepted |
| Commit | b9f5fb1800d8a4a3bc6cd3152c5f3d252986cf79 |
| Headers | show |
| Series | cramfs: fix MTD dependency | expand |
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
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
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
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
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
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
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 > >
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
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(+) -- 2.9.0