| Message ID | 20180709200945.30116-5-boris.brezillon@bootlin.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: rawnand: Improve compile-test coverage | expand |
On Mon, 9 Jul 2018 22:09:25 +0200 Boris Brezillon <boris.brezillon@bootlin.com> wrote: > It just makes NAND maintainers' life easier by allowing them to > compile-test this driver without having ARCH_S3C24XX or ARCH_S3C64XX > enabled. > > We add a dependency on HAS_IOMEM to make sure the driver compiles > correctly, and a dependency on !IA64 because the {read,write}s{bwl}() > accessors are not defined for this architecture. I see that SPARC does not define those accessors either. So I guess we should add depends on !SPARC. Arnd, any other way to know when the platform implements {read,write}s{bwl}() accessors? > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/mtd/nand/raw/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig > index 1488ad0a0199..e01b3da726c0 100644 > --- a/drivers/mtd/nand/raw/Kconfig > +++ b/drivers/mtd/nand/raw/Kconfig > @@ -119,7 +119,8 @@ config MTD_NAND_AU1550 > > config MTD_NAND_S3C2410 > tristate "NAND Flash support for Samsung S3C SoCs" > - depends on ARCH_S3C24XX || ARCH_S3C64XX > + depends on ARCH_S3C24XX || ARCH_S3C64XX || COMPILE_TEST > + depends on HAS_IOMEM && !IA64 > help > This enables the NAND flash controller on the S3C24xx and S3C64xx > SoCs
On Wed, Jul 11, 2018 at 1:16 PM, Boris Brezillon <boris.brezillon@bootlin.com> wrote: > On Mon, 9 Jul 2018 22:09:25 +0200 > Boris Brezillon <boris.brezillon@bootlin.com> wrote: > >> It just makes NAND maintainers' life easier by allowing them to >> compile-test this driver without having ARCH_S3C24XX or ARCH_S3C64XX >> enabled. >> >> We add a dependency on HAS_IOMEM to make sure the driver compiles >> correctly, and a dependency on !IA64 because the {read,write}s{bwl}() >> accessors are not defined for this architecture. > > I see that SPARC does not define those accessors either. So I guess we > should add depends on !SPARC. > > Arnd, any other way to know when the platform implements > {read,write}s{bwl}() accessors? I'd just consider that a bug, and send a patch to fix sparc64 if it's broken. sparc32 appears to have these, and when Thierry sent the patch to implement them everywhere[1], he said that he tested sparc64 as well, so either something regressed since then, or his testing was incomplete. Either way, the correct answer IMHO would be to make it work rather than to add infrastructure around the broken configurations. Arnd [1] https://lwn.net/Articles/604819/
On Wed, 11 Jul 2018 13:27:53 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > On Wed, Jul 11, 2018 at 1:16 PM, Boris Brezillon > <boris.brezillon@bootlin.com> wrote: > > On Mon, 9 Jul 2018 22:09:25 +0200 > > Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > > >> It just makes NAND maintainers' life easier by allowing them to > >> compile-test this driver without having ARCH_S3C24XX or ARCH_S3C64XX > >> enabled. > >> > >> We add a dependency on HAS_IOMEM to make sure the driver compiles > >> correctly, and a dependency on !IA64 because the {read,write}s{bwl}() > >> accessors are not defined for this architecture. > > > > I see that SPARC does not define those accessors either. So I guess we > > should add depends on !SPARC. > > > > Arnd, any other way to know when the platform implements > > {read,write}s{bwl}() accessors? > > I'd just consider that a bug, and send a patch to fix sparc64 if it's broken. > sparc32 appears to have these, and when Thierry sent the patch > to implement them everywhere[1], he said that he tested sparc64 as > well, so either something regressed since then, or his testing > was incomplete. Either way, the correct answer IMHO would be to > make it work rather than to add infrastructure around the broken > configurations. I guess the same goes for IA64 then.
On Wed, Jul 11, 2018 at 1:32 PM, Boris Brezillon <boris.brezillon@bootlin.com> wrote: > On Wed, 11 Jul 2018 13:27:53 +0200 > Arnd Bergmann <arnd@arndb.de> wrote: > >> On Wed, Jul 11, 2018 at 1:16 PM, Boris Brezillon >> <boris.brezillon@bootlin.com> wrote: >> > On Mon, 9 Jul 2018 22:09:25 +0200 >> > Boris Brezillon <boris.brezillon@bootlin.com> wrote: >> > >> >> It just makes NAND maintainers' life easier by allowing them to >> >> compile-test this driver without having ARCH_S3C24XX or ARCH_S3C64XX >> >> enabled. >> >> >> >> We add a dependency on HAS_IOMEM to make sure the driver compiles >> >> correctly, and a dependency on !IA64 because the {read,write}s{bwl}() >> >> accessors are not defined for this architecture. >> > >> > I see that SPARC does not define those accessors either. So I guess we >> > should add depends on !SPARC. >> > >> > Arnd, any other way to know when the platform implements >> > {read,write}s{bwl}() accessors? >> >> I'd just consider that a bug, and send a patch to fix sparc64 if it's broken. >> sparc32 appears to have these, and when Thierry sent the patch >> to implement them everywhere[1], he said that he tested sparc64 as >> well, so either something regressed since then, or his testing >> was incomplete. Either way, the correct answer IMHO would be to >> make it work rather than to add infrastructure around the broken >> configurations. > > I guess the same goes for IA64 then. Right. FWIW, I just tried it out and sent the respective arch patches. Arnd
On Wed, 11 Jul 2018 14:09:19 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > On Wed, Jul 11, 2018 at 1:32 PM, Boris Brezillon > <boris.brezillon@bootlin.com> wrote: > > On Wed, 11 Jul 2018 13:27:53 +0200 > > Arnd Bergmann <arnd@arndb.de> wrote: > > > >> On Wed, Jul 11, 2018 at 1:16 PM, Boris Brezillon > >> <boris.brezillon@bootlin.com> wrote: > >> > On Mon, 9 Jul 2018 22:09:25 +0200 > >> > Boris Brezillon <boris.brezillon@bootlin.com> wrote: > >> > > >> >> It just makes NAND maintainers' life easier by allowing them to > >> >> compile-test this driver without having ARCH_S3C24XX or ARCH_S3C64XX > >> >> enabled. > >> >> > >> >> We add a dependency on HAS_IOMEM to make sure the driver compiles > >> >> correctly, and a dependency on !IA64 because the {read,write}s{bwl}() > >> >> accessors are not defined for this architecture. > >> > > >> > I see that SPARC does not define those accessors either. So I guess we > >> > should add depends on !SPARC. > >> > > >> > Arnd, any other way to know when the platform implements > >> > {read,write}s{bwl}() accessors? > >> > >> I'd just consider that a bug, and send a patch to fix sparc64 if it's broken. > >> sparc32 appears to have these, and when Thierry sent the patch > >> to implement them everywhere[1], he said that he tested sparc64 as > >> well, so either something regressed since then, or his testing > >> was incomplete. Either way, the correct answer IMHO would be to > >> make it work rather than to add infrastructure around the broken > >> configurations. > > > > I guess the same goes for IA64 then. > > Right. FWIW, I just tried it out and sent the respective arch patches. Cool! Thanks for doing that.
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig index 1488ad0a0199..e01b3da726c0 100644 --- a/drivers/mtd/nand/raw/Kconfig +++ b/drivers/mtd/nand/raw/Kconfig @@ -119,7 +119,8 @@ config MTD_NAND_AU1550 config MTD_NAND_S3C2410 tristate "NAND Flash support for Samsung S3C SoCs" - depends on ARCH_S3C24XX || ARCH_S3C64XX + depends on ARCH_S3C24XX || ARCH_S3C64XX || COMPILE_TEST + depends on HAS_IOMEM && !IA64 help This enables the NAND flash controller on the S3C24xx and S3C64xx SoCs
It just makes NAND maintainers' life easier by allowing them to compile-test this driver without having ARCH_S3C24XX or ARCH_S3C64XX enabled. We add a dependency on HAS_IOMEM to make sure the driver compiles correctly, and a dependency on !IA64 because the {read,write}s{bwl}() accessors are not defined for this architecture. Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- drivers/mtd/nand/raw/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)