Instead of trying to parse CFE version string, which is customized by some vendors, let's just check that "CFE1" was passed on argument 3. Álvaro Fernández Rojas (2): MIPS: BCM63xx: add helper function to detect CFE mtd: parsers: bcm63xx: simplify CFE detection .../include/asm/mach-bcm63xx/bcm63xx_board.h | 6 ++++ drivers/mtd/parsers/bcm63xxpart.c | 28 ++----------------- 2 files changed, 9 insertions(+), 25 deletions(-) -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
CFE passes argument 3 as "CFE1". Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> --- arch/mips/include/asm/mach-bcm63xx/bcm63xx_board.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_board.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_board.h index 1d19a726f86c..5af07796e8c7 100644 --- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_board.h +++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_board.h @@ -2,6 +2,8 @@ #ifndef BCM63XX_BOARD_H_ #define BCM63XX_BOARD_H_ +#include <asm/bootinfo.h> + const char *board_get_name(void); void board_prom_init(void); @@ -10,4 +12,8 @@ void board_setup(void); int board_register_devices(void); +static inline bool bcm63xx_is_cfe_present(void) { + return fw_arg3 == 0x43464531; +} + #endif /* ! BCM63XX_BOARD_H_ */ -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Instead of trying to parse CFE version string, which is customized by some vendors, let's just check that "CFE1" was passed on argument 3. Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> --- drivers/mtd/parsers/bcm63xxpart.c | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c index 78f90c6c18fd..06b69e905a42 100644 --- a/drivers/mtd/parsers/bcm63xxpart.c +++ b/drivers/mtd/parsers/bcm63xxpart.c @@ -22,6 +22,8 @@ #include <linux/mtd/partitions.h> #include <linux/of.h> +#include <asm/mach-bcm63xx/bcm63xx_board.h> + #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 @@ -32,30 +34,6 @@ #define STR_NULL_TERMINATE(x) \ do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) -static int bcm63xx_detect_cfe(struct mtd_info *master) -{ - char buf[9]; - int ret; - size_t retlen; - - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, - (void *)buf); - buf[retlen] = 0; - - if (ret) - return ret; - - if (strncmp("cfe-v", buf, 5) == 0) - return 0; - - /* very old CFE's do not have the cfe-v string, so check for magic */ - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, - (void *)buf); - buf[retlen] = 0; - - return strncmp("CFE1CFE1", buf, 8); -} - static int bcm63xx_read_nvram(struct mtd_info *master, struct bcm963xx_nvram *nvram) { @@ -138,7 +116,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, struct bcm963xx_nvram *nvram = NULL; int ret; - if (bcm63xx_detect_cfe(master)) + if (!bcm63xx_is_cfe_present()) return -EINVAL; nvram = vzalloc(sizeof(*nvram)); -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Instead of trying to parse CFE version string, which is customized by some vendors, let's just check that "CFE1" was passed on argument 3. Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> --- v2: use CFE_EPTSEAL definition and avoid using an additional funtion. drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c index 78f90c6c18fd..493a75b2f266 100644 --- a/drivers/mtd/parsers/bcm63xxpart.c +++ b/drivers/mtd/parsers/bcm63xxpart.c @@ -22,6 +22,9 @@ #include <linux/mtd/partitions.h> #include <linux/of.h> +#include <asm/bootinfo.h> +#include <asm/fw/cfe/cfe_api.h> + #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 @@ -32,30 +35,6 @@ #define STR_NULL_TERMINATE(x) \ do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) -static int bcm63xx_detect_cfe(struct mtd_info *master) -{ - char buf[9]; - int ret; - size_t retlen; - - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, - (void *)buf); - buf[retlen] = 0; - - if (ret) - return ret; - - if (strncmp("cfe-v", buf, 5) == 0) - return 0; - - /* very old CFE's do not have the cfe-v string, so check for magic */ - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, - (void *)buf); - buf[retlen] = 0; - - return strncmp("CFE1CFE1", buf, 8); -} - static int bcm63xx_read_nvram(struct mtd_info *master, struct bcm963xx_nvram *nvram) { @@ -138,7 +117,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, struct bcm963xx_nvram *nvram = NULL; int ret; - if (bcm63xx_detect_cfe(master)) + if (fw_arg3 != CFE_EPTSEAL) return -EINVAL; nvram = vzalloc(sizeof(*nvram)); -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Álvaro, Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 18:06:49 +0200: > Instead of trying to parse CFE version string, which is customized by some > vendors, let's just check that "CFE1" was passed on argument 3. > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > --- > v2: use CFE_EPTSEAL definition and avoid using an additional funtion. > > drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- > 1 file changed, 4 insertions(+), 25 deletions(-) > > diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c > index 78f90c6c18fd..493a75b2f266 100644 > --- a/drivers/mtd/parsers/bcm63xxpart.c > +++ b/drivers/mtd/parsers/bcm63xxpart.c > @@ -22,6 +22,9 @@ > #include <linux/mtd/partitions.h> > #include <linux/of.h> > > +#include <asm/bootinfo.h> > +#include <asm/fw/cfe/cfe_api.h> Are you sure both includes are needed? I don't think it is a good habit to include asm/ headers, are you sure there is not another header doing it just fine? > + > #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ > > #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 > @@ -32,30 +35,6 @@ > #define STR_NULL_TERMINATE(x) \ > do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) > > -static int bcm63xx_detect_cfe(struct mtd_info *master) > -{ > - char buf[9]; > - int ret; > - size_t retlen; > - > - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, > - (void *)buf); > - buf[retlen] = 0; > - > - if (ret) > - return ret; > - > - if (strncmp("cfe-v", buf, 5) == 0) > - return 0; > - > - /* very old CFE's do not have the cfe-v string, so check for magic */ > - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, > - (void *)buf); > - buf[retlen] = 0; > - > - return strncmp("CFE1CFE1", buf, 8); > -} > - > static int bcm63xx_read_nvram(struct mtd_info *master, > struct bcm963xx_nvram *nvram) > { > @@ -138,7 +117,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, > struct bcm963xx_nvram *nvram = NULL; > int ret; > > - if (bcm63xx_detect_cfe(master)) > + if (fw_arg3 != CFE_EPTSEAL) > return -EINVAL; > > nvram = vzalloc(sizeof(*nvram)); ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Miquel, > El 11 jun 2020, a las 9:55, Miquel Raynal <miquel.raynal@bootlin.com> escribió: > > Hi Álvaro, > > Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 > 18:06:49 +0200: > >> Instead of trying to parse CFE version string, which is customized by some >> vendors, let's just check that "CFE1" was passed on argument 3. >> >> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> >> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> >> --- >> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. >> >> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- >> 1 file changed, 4 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c >> index 78f90c6c18fd..493a75b2f266 100644 >> --- a/drivers/mtd/parsers/bcm63xxpart.c >> +++ b/drivers/mtd/parsers/bcm63xxpart.c >> @@ -22,6 +22,9 @@ >> #include <linux/mtd/partitions.h> >> #include <linux/of.h> >> >> +#include <asm/bootinfo.h> >> +#include <asm/fw/cfe/cfe_api.h> > > Are you sure both includes are needed? asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed for CFE_EPTSEAL. > > I don't think it is a good habit to include asm/ headers, are you sure > there is not another header doing it just fine? Both are needed unless you want to add another definition of CFE_EPTSEAL value. There are currently two CFE magic definitions, the one in asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 > >> + >> #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ >> >> #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 >> @@ -32,30 +35,6 @@ >> #define STR_NULL_TERMINATE(x) \ >> do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) >> >> -static int bcm63xx_detect_cfe(struct mtd_info *master) >> -{ >> - char buf[9]; >> - int ret; >> - size_t retlen; >> - >> - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, >> - (void *)buf); >> - buf[retlen] = 0; >> - >> - if (ret) >> - return ret; >> - >> - if (strncmp("cfe-v", buf, 5) == 0) >> - return 0; >> - >> - /* very old CFE's do not have the cfe-v string, so check for magic */ >> - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, >> - (void *)buf); >> - buf[retlen] = 0; >> - >> - return strncmp("CFE1CFE1", buf, 8); >> -} >> - >> static int bcm63xx_read_nvram(struct mtd_info *master, >> struct bcm963xx_nvram *nvram) >> { >> @@ -138,7 +117,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, >> struct bcm963xx_nvram *nvram = NULL; >> int ret; >> >> - if (bcm63xx_detect_cfe(master)) >> + if (fw_arg3 != CFE_EPTSEAL) >> return -EINVAL; >> >> nvram = vzalloc(sizeof(*nvram)); Best regards, Álvaro. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On 6/11/2020 8:16 AM, Álvaro Fernández Rojas wrote: > Hi Miquel, > >> El 11 jun 2020, a las 9:55, Miquel Raynal <miquel.raynal@bootlin.com> escribió: >> >> Hi Álvaro, >> >> Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 >> 18:06:49 +0200: >> >>> Instead of trying to parse CFE version string, which is customized by some >>> vendors, let's just check that "CFE1" was passed on argument 3. >>> >>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> >>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> >>> --- >>> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. >>> >>> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- >>> 1 file changed, 4 insertions(+), 25 deletions(-) >>> >>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c >>> index 78f90c6c18fd..493a75b2f266 100644 >>> --- a/drivers/mtd/parsers/bcm63xxpart.c >>> +++ b/drivers/mtd/parsers/bcm63xxpart.c >>> @@ -22,6 +22,9 @@ >>> #include <linux/mtd/partitions.h> >>> #include <linux/of.h> >>> >>> +#include <asm/bootinfo.h> >>> +#include <asm/fw/cfe/cfe_api.h> >> >> Are you sure both includes are needed? > > asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed for CFE_EPTSEAL. > >> >> I don't think it is a good habit to include asm/ headers, are you sure >> there is not another header doing it just fine? > > Both are needed unless you want to add another definition of CFE_EPTSEAL value. > There are currently two CFE magic definitions, the one in asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: > https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 > https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 The caveat with that approach is that this reduces the compilation surface to MIPS and BMIPS_GENERIC and BCM63XX only, which is a bit small. If we could move the CFE definitions to a shared header, and consolidate the value used by bcm47xxpart.c as well, that would allow us to build the bcm63xxpart.c file with COMPILE_TEST on other architectures. This does not really have functional value, but for maintainers like Miquel, it allows them to quickly test their entire drivers/mtd/ directory. -- Florian ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Florian Fainelli <f.fainelli@gmail.com> wrote on Thu, 11 Jun 2020 08:42:42 -0700: > On 6/11/2020 8:16 AM, Álvaro Fernández Rojas wrote: > > Hi Miquel, > > > >> El 11 jun 2020, a las 9:55, Miquel Raynal <miquel.raynal@bootlin.com> escribió: > >> > >> Hi Álvaro, > >> > >> Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 > >> 18:06:49 +0200: > >> > >>> Instead of trying to parse CFE version string, which is customized by some > >>> vendors, let's just check that "CFE1" was passed on argument 3. > >>> > >>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > >>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > >>> --- > >>> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. > >>> > >>> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- > >>> 1 file changed, 4 insertions(+), 25 deletions(-) > >>> > >>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c > >>> index 78f90c6c18fd..493a75b2f266 100644 > >>> --- a/drivers/mtd/parsers/bcm63xxpart.c > >>> +++ b/drivers/mtd/parsers/bcm63xxpart.c > >>> @@ -22,6 +22,9 @@ > >>> #include <linux/mtd/partitions.h> > >>> #include <linux/of.h> > >>> > >>> +#include <asm/bootinfo.h> > >>> +#include <asm/fw/cfe/cfe_api.h> > >> > >> Are you sure both includes are needed? > > > > asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed for CFE_EPTSEAL. > > > >> > >> I don't think it is a good habit to include asm/ headers, are you sure > >> there is not another header doing it just fine? > > > > Both are needed unless you want to add another definition of CFE_EPTSEAL value. > > There are currently two CFE magic definitions, the one in asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: > > https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 > > https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 > > The caveat with that approach is that this reduces the compilation > surface to MIPS and BMIPS_GENERIC and BCM63XX only, which is a bit > small. If we could move the CFE definitions to a shared header, and > consolidate the value used by bcm47xxpart.c as well, that would allow us > to build the bcm63xxpart.c file with COMPILE_TEST on other > architectures. This does not really have functional value, but for > maintainers like Miquel, it allows them to quickly test their entire > drivers/mtd/ directory. Absolutely! ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Florian, > El 11 jun 2020, a las 17:42, Florian Fainelli <f.fainelli@gmail.com> escribió: > > > > On 6/11/2020 8:16 AM, Álvaro Fernández Rojas wrote: >> Hi Miquel, >> >>> El 11 jun 2020, a las 9:55, Miquel Raynal <miquel.raynal@bootlin.com> escribió: >>> >>> Hi Álvaro, >>> >>> Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 >>> 18:06:49 +0200: >>> >>>> Instead of trying to parse CFE version string, which is customized by some >>>> vendors, let's just check that "CFE1" was passed on argument 3. >>>> >>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> >>>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> >>>> --- >>>> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. >>>> >>>> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- >>>> 1 file changed, 4 insertions(+), 25 deletions(-) >>>> >>>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c >>>> index 78f90c6c18fd..493a75b2f266 100644 >>>> --- a/drivers/mtd/parsers/bcm63xxpart.c >>>> +++ b/drivers/mtd/parsers/bcm63xxpart.c >>>> @@ -22,6 +22,9 @@ >>>> #include <linux/mtd/partitions.h> >>>> #include <linux/of.h> >>>> >>>> +#include <asm/bootinfo.h> >>>> +#include <asm/fw/cfe/cfe_api.h> >>> >>> Are you sure both includes are needed? >> >> asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed for CFE_EPTSEAL. >> >>> >>> I don't think it is a good habit to include asm/ headers, are you sure >>> there is not another header doing it just fine? >> >> Both are needed unless you want to add another definition of CFE_EPTSEAL value. >> There are currently two CFE magic definitions, the one in asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: >> https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 >> https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 > > The caveat with that approach is that this reduces the compilation > surface to MIPS and BMIPS_GENERIC and BCM63XX only, which is a bit > small. If we could move the CFE definitions to a shared header, and > consolidate the value used by bcm47xxpart.c as well, that would allow us > to build the bcm63xxpart.c file with COMPILE_TEST on other > architectures. This does not really have functional value, but for > maintainers like Miquel, it allows them to quickly test their entire > drivers/mtd/ directory. I don’t think fw_arg3 available on non mips archs, is it? I’m happy to move it to a shared header (which would be a good location for this?), but if I’m right it would still be restricted to MIPS. > -- > Florian ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Álvaro, Álvaro Fernández Rojas <noltari@gmail.com> wrote on Thu, 11 Jun 2020 18:14:20 +0200: > Hi Florian, > > > El 11 jun 2020, a las 17:42, Florian Fainelli <f.fainelli@gmail.com> escribió: > > > > > > > > On 6/11/2020 8:16 AM, Álvaro Fernández Rojas wrote: > >> Hi Miquel, > >> > >>> El 11 jun 2020, a las 9:55, Miquel Raynal <miquel.raynal@bootlin.com> escribió: > >>> > >>> Hi Álvaro, > >>> > >>> Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 > >>> 18:06:49 +0200: > >>> > >>>> Instead of trying to parse CFE version string, which is customized by some > >>>> vendors, let's just check that "CFE1" was passed on argument 3. > >>>> > >>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > >>>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > >>>> --- > >>>> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. > >>>> > >>>> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- > >>>> 1 file changed, 4 insertions(+), 25 deletions(-) > >>>> > >>>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c > >>>> index 78f90c6c18fd..493a75b2f266 100644 > >>>> --- a/drivers/mtd/parsers/bcm63xxpart.c > >>>> +++ b/drivers/mtd/parsers/bcm63xxpart.c > >>>> @@ -22,6 +22,9 @@ > >>>> #include <linux/mtd/partitions.h> > >>>> #include <linux/of.h> > >>>> > >>>> +#include <asm/bootinfo.h> > >>>> +#include <asm/fw/cfe/cfe_api.h> > >>> > >>> Are you sure both includes are needed? > >> > >> asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed for CFE_EPTSEAL. > >> > >>> > >>> I don't think it is a good habit to include asm/ headers, are you sure > >>> there is not another header doing it just fine? > >> > >> Both are needed unless you want to add another definition of CFE_EPTSEAL value. > >> There are currently two CFE magic definitions, the one in asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: > >> https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 > >> https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 > > > > The caveat with that approach is that this reduces the compilation > > surface to MIPS and BMIPS_GENERIC and BCM63XX only, which is a bit > > small. If we could move the CFE definitions to a shared header, and > > consolidate the value used by bcm47xxpart.c as well, that would allow us > > to build the bcm63xxpart.c file with COMPILE_TEST on other > > architectures. This does not really have functional value, but for > > maintainers like Miquel, it allows them to quickly test their entire > > drivers/mtd/ directory. > > I don’t think fw_arg3 available on non mips archs, is it? > I’m happy to move it to a shared header (which would be a good location for this?), but if I’m right it would still be restricted to MIPS. Restricting a definition to MIPS, even if it makes sense for you is very limiting for me. I need to be able to build as much drivers as I can from my laptop and verify they at least compile correctly. If I need a MIPS toolchain, an ARC toolchain, a PowerPC, an ARM, an ARM64 and whatever other funky toolchain to do just that in X steps: it's very painful. We have been adding COMPILE_TEST dependencies on as much drivers as we could and we want to continue moving forward. Using such include would need to drop the COMPILE_TEST condition from Kconfig and this is not something I am willing to do. Thanks for your understanding :) Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Miquèl, > El 12 jun 2020, a las 9:02, Miquel Raynal <miquel.raynal@bootlin.com> escribió: > > Hi Álvaro, > > Álvaro Fernández Rojas <noltari@gmail.com> wrote on Thu, 11 Jun 2020 > 18:14:20 +0200: > >> Hi Florian, >> >>> El 11 jun 2020, a las 17:42, Florian Fainelli <f.fainelli@gmail.com> escribió: >>> >>> >>> >>> On 6/11/2020 8:16 AM, Álvaro Fernández Rojas wrote: >>>> Hi Miquel, >>>> >>>>> El 11 jun 2020, a las 9:55, Miquel Raynal <miquel.raynal@bootlin.com> escribió: >>>>> >>>>> Hi Álvaro, >>>>> >>>>> Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 >>>>> 18:06:49 +0200: >>>>> >>>>>> Instead of trying to parse CFE version string, which is customized by some >>>>>> vendors, let's just check that "CFE1" was passed on argument 3. >>>>>> >>>>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> >>>>>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> >>>>>> --- >>>>>> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. >>>>>> >>>>>> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- >>>>>> 1 file changed, 4 insertions(+), 25 deletions(-) >>>>>> >>>>>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c >>>>>> index 78f90c6c18fd..493a75b2f266 100644 >>>>>> --- a/drivers/mtd/parsers/bcm63xxpart.c >>>>>> +++ b/drivers/mtd/parsers/bcm63xxpart.c >>>>>> @@ -22,6 +22,9 @@ >>>>>> #include <linux/mtd/partitions.h> >>>>>> #include <linux/of.h> >>>>>> >>>>>> +#include <asm/bootinfo.h> >>>>>> +#include <asm/fw/cfe/cfe_api.h> >>>>> >>>>> Are you sure both includes are needed? >>>> >>>> asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed for CFE_EPTSEAL. >>>> >>>>> >>>>> I don't think it is a good habit to include asm/ headers, are you sure >>>>> there is not another header doing it just fine? >>>> >>>> Both are needed unless you want to add another definition of CFE_EPTSEAL value. >>>> There are currently two CFE magic definitions, the one in asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: >>>> https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 >>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 >>> >>> The caveat with that approach is that this reduces the compilation >>> surface to MIPS and BMIPS_GENERIC and BCM63XX only, which is a bit >>> small. If we could move the CFE definitions to a shared header, and >>> consolidate the value used by bcm47xxpart.c as well, that would allow us >>> to build the bcm63xxpart.c file with COMPILE_TEST on other >>> architectures. This does not really have functional value, but for >>> maintainers like Miquel, it allows them to quickly test their entire >>> drivers/mtd/ directory. >> >> I don’t think fw_arg3 available on non mips archs, is it? >> I’m happy to move it to a shared header (which would be a good location for this?), but if I’m right it would still be restricted to MIPS. > > Restricting a definition to MIPS, even if it makes sense for you is > very limiting for me. I need to be able to build as much drivers as I > can from my laptop and verify they at least compile correctly. If I need > a MIPS toolchain, an ARC toolchain, a PowerPC, an ARM, an ARM64 and > whatever other funky toolchain to do just that in X steps: it's very > painful. We have been adding COMPILE_TEST dependencies on as much > drivers as we could and we want to continue moving forward. Using such > include would need to drop the COMPILE_TEST condition from Kconfig and > this is not something I am willing to do. I totally understand and agree with your point, but I still think that there could be a solution which would be valid for both of us. > > Thanks for your understanding :) The current way of detecting CFE isn’t the proper one. Thank you for understanding that too. > Miquèl Best regards, Álvaro. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Álvaro, Álvaro Fernández Rojas <noltari@gmail.com> wrote on Fri, 12 Jun 2020 09:30:27 +0200: > Hi Miquèl, > > > El 12 jun 2020, a las 9:02, Miquel Raynal <miquel.raynal@bootlin.com> escribió: > > > > Hi Álvaro, > > > > Álvaro Fernández Rojas <noltari@gmail.com> wrote on Thu, 11 Jun 2020 > > 18:14:20 +0200: > > > >> Hi Florian, > >> > >>> El 11 jun 2020, a las 17:42, Florian Fainelli <f.fainelli@gmail.com> escribió: > >>> > >>> > >>> > >>> On 6/11/2020 8:16 AM, Álvaro Fernández Rojas wrote: > >>>> Hi Miquel, > >>>> > >>>>> El 11 jun 2020, a las 9:55, Miquel Raynal <miquel.raynal@bootlin.com> escribió: > >>>>> > >>>>> Hi Álvaro, > >>>>> > >>>>> Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 > >>>>> 18:06:49 +0200: > >>>>> > >>>>>> Instead of trying to parse CFE version string, which is customized by some > >>>>>> vendors, let's just check that "CFE1" was passed on argument 3. > >>>>>> > >>>>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > >>>>>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > >>>>>> --- > >>>>>> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. > >>>>>> > >>>>>> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- > >>>>>> 1 file changed, 4 insertions(+), 25 deletions(-) > >>>>>> > >>>>>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c > >>>>>> index 78f90c6c18fd..493a75b2f266 100644 > >>>>>> --- a/drivers/mtd/parsers/bcm63xxpart.c > >>>>>> +++ b/drivers/mtd/parsers/bcm63xxpart.c > >>>>>> @@ -22,6 +22,9 @@ > >>>>>> #include <linux/mtd/partitions.h> > >>>>>> #include <linux/of.h> > >>>>>> > >>>>>> +#include <asm/bootinfo.h> > >>>>>> +#include <asm/fw/cfe/cfe_api.h> > >>>>> > >>>>> Are you sure both includes are needed? > >>>> > >>>> asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed for CFE_EPTSEAL. > >>>> > >>>>> > >>>>> I don't think it is a good habit to include asm/ headers, are you sure > >>>>> there is not another header doing it just fine? > >>>> > >>>> Both are needed unless you want to add another definition of CFE_EPTSEAL value. > >>>> There are currently two CFE magic definitions, the one in asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: > >>>> https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 > >>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 > >>> > >>> The caveat with that approach is that this reduces the compilation > >>> surface to MIPS and BMIPS_GENERIC and BCM63XX only, which is a bit > >>> small. If we could move the CFE definitions to a shared header, and > >>> consolidate the value used by bcm47xxpart.c as well, that would allow us > >>> to build the bcm63xxpart.c file with COMPILE_TEST on other > >>> architectures. This does not really have functional value, but for > >>> maintainers like Miquel, it allows them to quickly test their entire > >>> drivers/mtd/ directory. > >> > >> I don’t think fw_arg3 available on non mips archs, is it? > >> I’m happy to move it to a shared header (which would be a good location for this?), but if I’m right it would still be restricted to MIPS. > > > > Restricting a definition to MIPS, even if it makes sense for you is > > very limiting for me. I need to be able to build as much drivers as I > > can from my laptop and verify they at least compile correctly. If I need > > a MIPS toolchain, an ARC toolchain, a PowerPC, an ARM, an ARM64 and > > whatever other funky toolchain to do just that in X steps: it's very > > painful. We have been adding COMPILE_TEST dependencies on as much > > drivers as we could and we want to continue moving forward. Using such > > include would need to drop the COMPILE_TEST condition from Kconfig and > > this is not something I am willing to do. > > I totally understand and agree with your point, but I still think that there could be a solution which would be valid for both of us. What do you suggest? > > > > > Thanks for your understanding :) > > The current way of detecting CFE isn’t the proper one. > Thank you for understanding that too. Of course, I'm not saying I don't want this change, I'm just saying we should find another way to handle it, the below idea is totally fine by me. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Instead of trying to parse CFE version string, which is customized by some vendors, let's just check that "CFE1" was passed on argument 3. Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> --- v3: keep COMPILE_TEST compatibility by adding a new function that only checks fw_arg3 when CONFIG_MIPS is defined. v2: use CFE_EPTSEAL definition and avoid using an additional function. drivers/mtd/parsers/bcm63xxpart.c | 34 +++++++++++-------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c index 78f90c6c18fd..c514c04789af 100644 --- a/drivers/mtd/parsers/bcm63xxpart.c +++ b/drivers/mtd/parsers/bcm63xxpart.c @@ -22,6 +22,11 @@ #include <linux/mtd/partitions.h> #include <linux/of.h> +#ifdef CONFIG_MIPS +#include <asm/bootinfo.h> +#include <asm/fw/cfe/cfe_api.h> +#endif /* CONFIG_MIPS */ + #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 @@ -32,28 +37,13 @@ #define STR_NULL_TERMINATE(x) \ do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) -static int bcm63xx_detect_cfe(struct mtd_info *master) +static inline int bcm63xx_detect_cfe(void) { - char buf[9]; - int ret; - size_t retlen; - - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, - (void *)buf); - buf[retlen] = 0; - - if (ret) - return ret; - - if (strncmp("cfe-v", buf, 5) == 0) - return 0; - - /* very old CFE's do not have the cfe-v string, so check for magic */ - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, - (void *)buf); - buf[retlen] = 0; - - return strncmp("CFE1CFE1", buf, 8); +#ifdef CONFIG_MIPS + return (fw_arg3 == CFE_EPTSEAL); +#else + return 0; +#endif /* CONFIG_MIPS */ } static int bcm63xx_read_nvram(struct mtd_info *master, @@ -138,7 +128,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, struct bcm963xx_nvram *nvram = NULL; int ret; - if (bcm63xx_detect_cfe(master)) + if (!bcm63xx_detect_cfe()) return -EINVAL; nvram = vzalloc(sizeof(*nvram)); -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Miquèl, > El 12 jun 2020, a las 9:33, Miquel Raynal <miquel.raynal@bootlin.com> escribió: > > Hi Álvaro, > > Álvaro Fernández Rojas <noltari@gmail.com> wrote on Fri, 12 Jun 2020 > 09:30:27 +0200: > >> Hi Miquèl, >> >>> El 12 jun 2020, a las 9:02, Miquel Raynal <miquel.raynal@bootlin.com> escribió: >>> >>> Hi Álvaro, >>> >>> Álvaro Fernández Rojas <noltari@gmail.com> wrote on Thu, 11 Jun 2020 >>> 18:14:20 +0200: >>> >>>> Hi Florian, >>>> >>>>> El 11 jun 2020, a las 17:42, Florian Fainelli <f.fainelli@gmail.com> escribió: >>>>> >>>>> >>>>> >>>>> On 6/11/2020 8:16 AM, Álvaro Fernández Rojas wrote: >>>>>> Hi Miquel, >>>>>> >>>>>>> El 11 jun 2020, a las 9:55, Miquel Raynal <miquel.raynal@bootlin.com> escribió: >>>>>>> >>>>>>> Hi Álvaro, >>>>>>> >>>>>>> Álvaro Fernández Rojas <noltari@gmail.com> wrote on Mon, 8 Jun 2020 >>>>>>> 18:06:49 +0200: >>>>>>> >>>>>>>> Instead of trying to parse CFE version string, which is customized by some >>>>>>>> vendors, let's just check that "CFE1" was passed on argument 3. >>>>>>>> >>>>>>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> >>>>>>>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> >>>>>>>> --- >>>>>>>> v2: use CFE_EPTSEAL definition and avoid using an additional funtion. >>>>>>>> >>>>>>>> drivers/mtd/parsers/bcm63xxpart.c | 29 ++++------------------------- >>>>>>>> 1 file changed, 4 insertions(+), 25 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c >>>>>>>> index 78f90c6c18fd..493a75b2f266 100644 >>>>>>>> --- a/drivers/mtd/parsers/bcm63xxpart.c >>>>>>>> +++ b/drivers/mtd/parsers/bcm63xxpart.c >>>>>>>> @@ -22,6 +22,9 @@ >>>>>>>> #include <linux/mtd/partitions.h> >>>>>>>> #include <linux/of.h> >>>>>>>> >>>>>>>> +#include <asm/bootinfo.h> >>>>>>>> +#include <asm/fw/cfe/cfe_api.h> >>>>>>> >>>>>>> Are you sure both includes are needed? >>>>>> >>>>>> asm/bootinfo.h is needed for fw_arg3 and asm/fw/cfe/cfe_api.h is needed for CFE_EPTSEAL. >>>>>> >>>>>>> >>>>>>> I don't think it is a good habit to include asm/ headers, are you sure >>>>>>> there is not another header doing it just fine? >>>>>> >>>>>> Both are needed unless you want to add another definition of CFE_EPTSEAL value. >>>>>> There are currently two CFE magic definitions, the one in asm/fw/cfe/cfe_api.h and another one in bcm47xxpart.c: >>>>>> https://github.com/torvalds/linux/blob/master/arch/mips/include/asm/fw/cfe/cfe_api.h#L28 >>>>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/parsers/bcm47xxpart.c#L33 >>>>> >>>>> The caveat with that approach is that this reduces the compilation >>>>> surface to MIPS and BMIPS_GENERIC and BCM63XX only, which is a bit >>>>> small. If we could move the CFE definitions to a shared header, and >>>>> consolidate the value used by bcm47xxpart.c as well, that would allow us >>>>> to build the bcm63xxpart.c file with COMPILE_TEST on other >>>>> architectures. This does not really have functional value, but for >>>>> maintainers like Miquel, it allows them to quickly test their entire >>>>> drivers/mtd/ directory. >>>> >>>> I don’t think fw_arg3 available on non mips archs, is it? >>>> I’m happy to move it to a shared header (which would be a good location for this?), but if I’m right it would still be restricted to MIPS. >>> >>> Restricting a definition to MIPS, even if it makes sense for you is >>> very limiting for me. I need to be able to build as much drivers as I >>> can from my laptop and verify they at least compile correctly. If I need >>> a MIPS toolchain, an ARC toolchain, a PowerPC, an ARM, an ARM64 and >>> whatever other funky toolchain to do just that in X steps: it's very >>> painful. We have been adding COMPILE_TEST dependencies on as much >>> drivers as we could and we want to continue moving forward. Using such >>> include would need to drop the COMPILE_TEST condition from Kconfig and >>> this is not something I am willing to do. >> >> I totally understand and agree with your point, but I still think that there could be a solution which would be valid for both of us. > > What do you suggest? I’ve just sent v3 with my suggestion. If this isn’t valid then I’m out of ideas... > >> >>> >>> Thanks for your understanding :) >> >> The current way of detecting CFE isn’t the proper one. >> Thank you for understanding that too. > > Of course, I'm not saying I don't want this change, I'm just saying we > should find another way to handle it, the below idea is totally fine by > me. > > > Thanks, > Miquèl Regards, Álvaro. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Álvaro, Álvaro Fernández Rojas <noltari@gmail.com> wrote on Fri, 12 Jun 2020 09:35:49 +0200: > Instead of trying to parse CFE version string, which is customized by some > vendors, let's just check that "CFE1" was passed on argument 3. > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > --- > v3: keep COMPILE_TEST compatibility by adding a new function that only checks > fw_arg3 when CONFIG_MIPS is defined. > v2: use CFE_EPTSEAL definition and avoid using an additional function. > > drivers/mtd/parsers/bcm63xxpart.c | 34 +++++++++++-------------------- > 1 file changed, 12 insertions(+), 22 deletions(-) > > diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c > index 78f90c6c18fd..c514c04789af 100644 > --- a/drivers/mtd/parsers/bcm63xxpart.c > +++ b/drivers/mtd/parsers/bcm63xxpart.c > @@ -22,6 +22,11 @@ > #include <linux/mtd/partitions.h> > #include <linux/of.h> > > +#ifdef CONFIG_MIPS > +#include <asm/bootinfo.h> > +#include <asm/fw/cfe/cfe_api.h> > +#endif /* CONFIG_MIPS */ > + > #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ > > #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 > @@ -32,28 +37,13 @@ > #define STR_NULL_TERMINATE(x) \ > do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) > > -static int bcm63xx_detect_cfe(struct mtd_info *master) > +static inline int bcm63xx_detect_cfe(void) > { > - char buf[9]; > - int ret; > - size_t retlen; > - > - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, > - (void *)buf); > - buf[retlen] = 0; > - > - if (ret) > - return ret; > - > - if (strncmp("cfe-v", buf, 5) == 0) > - return 0; > - > - /* very old CFE's do not have the cfe-v string, so check for magic */ > - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, > - (void *)buf); > - buf[retlen] = 0; > - > - return strncmp("CFE1CFE1", buf, 8); > +#ifdef CONFIG_MIPS > + return (fw_arg3 == CFE_EPTSEAL); > +#else > + return 0; > +#endif /* CONFIG_MIPS */ What about: ret = 0; #ifdef CONFIG_MIPS ret = (fw_arg3 == CFE_EPTSEAL) #endif return ret; This is for shortening the conditional part. > } > > static int bcm63xx_read_nvram(struct mtd_info *master, > @@ -138,7 +128,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, > struct bcm963xx_nvram *nvram = NULL; > int ret; > > - if (bcm63xx_detect_cfe(master)) > + if (!bcm63xx_detect_cfe()) > return -EINVAL; > > nvram = vzalloc(sizeof(*nvram)); Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Instead of trying to parse CFE version string, which is customized by some vendors, let's just check that "CFE1" was passed on argument 3. Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> --- v4: shorten conditional compilation part as suggested by Miquèl. v3: keep COMPILE_TEST compatibility by adding a new function that only checks fw_arg3 when CONFIG_MIPS is defined. v2: use CFE_EPTSEAL definition and avoid using an additional function. drivers/mtd/parsers/bcm63xxpart.c | 32 ++++++++++++------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c index 78f90c6c18fd..b15bdadaedb5 100644 --- a/drivers/mtd/parsers/bcm63xxpart.c +++ b/drivers/mtd/parsers/bcm63xxpart.c @@ -22,6 +22,11 @@ #include <linux/mtd/partitions.h> #include <linux/of.h> +#ifdef CONFIG_MIPS +#include <asm/bootinfo.h> +#include <asm/fw/cfe/cfe_api.h> +#endif /* CONFIG_MIPS */ + #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 @@ -32,28 +37,15 @@ #define STR_NULL_TERMINATE(x) \ do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) -static int bcm63xx_detect_cfe(struct mtd_info *master) +static inline int bcm63xx_detect_cfe(void) { - char buf[9]; - int ret; - size_t retlen; + int ret = 0; - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, - (void *)buf); - buf[retlen] = 0; +#ifdef CONFIG_MIPS + ret = (fw_arg3 == CFE_EPTSEAL); +#endif /* CONFIG_MIPS */ - if (ret) - return ret; - - if (strncmp("cfe-v", buf, 5) == 0) - return 0; - - /* very old CFE's do not have the cfe-v string, so check for magic */ - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, - (void *)buf); - buf[retlen] = 0; - - return strncmp("CFE1CFE1", buf, 8); + return ret; } static int bcm63xx_read_nvram(struct mtd_info *master, @@ -138,7 +130,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, struct bcm963xx_nvram *nvram = NULL; int ret; - if (bcm63xx_detect_cfe(master)) + if (!bcm63xx_detect_cfe()) return -EINVAL; nvram = vzalloc(sizeof(*nvram)); -- 2.27.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On 6/15/2020 2:17 AM, Álvaro Fernández Rojas wrote: > Instead of trying to parse CFE version string, which is customized by some > vendors, let's just check that "CFE1" was passed on argument 3. > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Mon, 2020-06-15 at 09:17:40 UTC, =?utf-8?q?=C3=81lvaro_Fern=C3=A1ndez_Rojas?= wrote: > Instead of trying to parse CFE version string, which is customized by some > vendors, let's just check that "CFE1" was passed on argument 3. > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks. Miquel ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Mon, Jun 15, 2020 at 11:17:40AM +0200, Álvaro Fernández Rojas wrote: > Instead of trying to parse CFE version string, which is customized by some > vendors, let's just check that "CFE1" was passed on argument 3. > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> mips:allmodconfig: ERROR: modpost: "fw_arg3" [drivers/mtd/parsers/bcm63xxpart.ko] undefined! This is not surprising, since fw_arg3 is not exported. Guenter > --- > v4: shorten conditional compilation part as suggested by Miquèl. > v3: keep COMPILE_TEST compatibility by adding a new function that only checks > fw_arg3 when CONFIG_MIPS is defined. > v2: use CFE_EPTSEAL definition and avoid using an additional function. > > drivers/mtd/parsers/bcm63xxpart.c | 32 ++++++++++++------------------- > 1 file changed, 12 insertions(+), 20 deletions(-) > > diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c > index 78f90c6c18fd..b15bdadaedb5 100644 > --- a/drivers/mtd/parsers/bcm63xxpart.c > +++ b/drivers/mtd/parsers/bcm63xxpart.c > @@ -22,6 +22,11 @@ > #include <linux/mtd/partitions.h> > #include <linux/of.h> > > +#ifdef CONFIG_MIPS > +#include <asm/bootinfo.h> > +#include <asm/fw/cfe/cfe_api.h> > +#endif /* CONFIG_MIPS */ > + > #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ > > #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 > @@ -32,28 +37,15 @@ > #define STR_NULL_TERMINATE(x) \ > do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) > > -static int bcm63xx_detect_cfe(struct mtd_info *master) > +static inline int bcm63xx_detect_cfe(void) > { > - char buf[9]; > - int ret; > - size_t retlen; > + int ret = 0; > > - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, > - (void *)buf); > - buf[retlen] = 0; > +#ifdef CONFIG_MIPS > + ret = (fw_arg3 == CFE_EPTSEAL); > +#endif /* CONFIG_MIPS */ > > - if (ret) > - return ret; > - > - if (strncmp("cfe-v", buf, 5) == 0) > - return 0; > - > - /* very old CFE's do not have the cfe-v string, so check for magic */ > - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, > - (void *)buf); > - buf[retlen] = 0; > - > - return strncmp("CFE1CFE1", buf, 8); > + return ret; > } > > static int bcm63xx_read_nvram(struct mtd_info *master, > @@ -138,7 +130,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, > struct bcm963xx_nvram *nvram = NULL; > int ret; > > - if (bcm63xx_detect_cfe(master)) > + if (!bcm63xx_detect_cfe()) > return -EINVAL; > > nvram = vzalloc(sizeof(*nvram)); ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Fri, 14 Aug 2020 at 14:26, Guenter Roeck <linux@roeck-us.net> wrote: > > On Mon, Jun 15, 2020 at 11:17:40AM +0200, Álvaro Fernández Rojas wrote: > > Instead of trying to parse CFE version string, which is customized by some > > vendors, let's just check that "CFE1" was passed on argument 3. > > > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > We still see mips: allmodconfig build failure on Linus tree make -sk KBUILD_BUILD_USER=TuxBuild -C/linux ARCH=mips CROSS_COMPILE=mips-linux-gnu- HOSTCC=gcc CC="sccache mips-linux-gnu-gcc" O=build allmodconfig make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=mips CROSS_COMPILE=mips-linux-gnu- HOSTCC=gcc CC="sccache mips-linux-gnu-gcc" O=build > mips:allmodconfig: > > ERROR: modpost: "fw_arg3" [drivers/mtd/parsers/bcm63xxpart.ko] undefined! ERROR: modpost: "fw_arg3" [drivers/mtd/parsers/bcm63xxpart.ko] undefined! Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> Full build link, https://builds.tuxbuild.com/Sm59_9tjFMpIvT27qf5kNA/build.log > > This is not surprising, since fw_arg3 is not exported. > > Guenter > > > --- > > v4: shorten conditional compilation part as suggested by Miquèl. > > v3: keep COMPILE_TEST compatibility by adding a new function that only checks > > fw_arg3 when CONFIG_MIPS is defined. > > v2: use CFE_EPTSEAL definition and avoid using an additional function. > > > > drivers/mtd/parsers/bcm63xxpart.c | 32 ++++++++++++------------------- > > 1 file changed, 12 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c > > index 78f90c6c18fd..b15bdadaedb5 100644 > > --- a/drivers/mtd/parsers/bcm63xxpart.c > > +++ b/drivers/mtd/parsers/bcm63xxpart.c > > @@ -22,6 +22,11 @@ > > #include <linux/mtd/partitions.h> > > #include <linux/of.h> > > > > +#ifdef CONFIG_MIPS > > +#include <asm/bootinfo.h> > > +#include <asm/fw/cfe/cfe_api.h> > > +#endif /* CONFIG_MIPS */ > > + > > #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ > > > > #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 > > @@ -32,28 +37,15 @@ > > #define STR_NULL_TERMINATE(x) \ > > do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) > > > > -static int bcm63xx_detect_cfe(struct mtd_info *master) > > +static inline int bcm63xx_detect_cfe(void) > > { > > - char buf[9]; > > - int ret; > > - size_t retlen; > > + int ret = 0; > > > > - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, > > - (void *)buf); > > - buf[retlen] = 0; > > +#ifdef CONFIG_MIPS > > + ret = (fw_arg3 == CFE_EPTSEAL); > > +#endif /* CONFIG_MIPS */ > > > > - if (ret) > > - return ret; > > - > > - if (strncmp("cfe-v", buf, 5) == 0) > > - return 0; > > - > > - /* very old CFE's do not have the cfe-v string, so check for magic */ > > - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, > > - (void *)buf); > > - buf[retlen] = 0; > > - > > - return strncmp("CFE1CFE1", buf, 8); > > + return ret; > > } > > > > static int bcm63xx_read_nvram(struct mtd_info *master, > > @@ -138,7 +130,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, > > struct bcm963xx_nvram *nvram = NULL; > > int ret; > > > > - if (bcm63xx_detect_cfe(master)) > > + if (!bcm63xx_detect_cfe()) > > return -EINVAL; > > > > nvram = vzalloc(sizeof(*nvram)); ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On 9/21/20 8:18 PM, Naresh Kamboju wrote: > On Fri, 14 Aug 2020 at 14:26, Guenter Roeck <linux@roeck-us.net> wrote: >> >> On Mon, Jun 15, 2020 at 11:17:40AM +0200, Álvaro Fernández Rojas wrote: >>> Instead of trying to parse CFE version string, which is customized by some >>> vendors, let's just check that "CFE1" was passed on argument 3. >>> >>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> >>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> >>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> >> > > We still see mips: allmodconfig build failure on Linus tree > Yes, same here. Guenter > make -sk KBUILD_BUILD_USER=TuxBuild -C/linux ARCH=mips > CROSS_COMPILE=mips-linux-gnu- HOSTCC=gcc CC="sccache > mips-linux-gnu-gcc" O=build allmodconfig > > make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=mips > CROSS_COMPILE=mips-linux-gnu- HOSTCC=gcc CC="sccache > mips-linux-gnu-gcc" O=build > > >> mips:allmodconfig: >> >> ERROR: modpost: "fw_arg3" [drivers/mtd/parsers/bcm63xxpart.ko] undefined! > > ERROR: modpost: "fw_arg3" [drivers/mtd/parsers/bcm63xxpart.ko] undefined! > > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > > Full build link, > https://builds.tuxbuild.com/Sm59_9tjFMpIvT27qf5kNA/build.log > >> >> This is not surprising, since fw_arg3 is not exported. >> >> Guenter >> >>> --- >>> v4: shorten conditional compilation part as suggested by Miquèl. >>> v3: keep COMPILE_TEST compatibility by adding a new function that only checks >>> fw_arg3 when CONFIG_MIPS is defined. >>> v2: use CFE_EPTSEAL definition and avoid using an additional function. >>> >>> drivers/mtd/parsers/bcm63xxpart.c | 32 ++++++++++++------------------- >>> 1 file changed, 12 insertions(+), 20 deletions(-) >>> >>> diff --git a/drivers/mtd/parsers/bcm63xxpart.c b/drivers/mtd/parsers/bcm63xxpart.c >>> index 78f90c6c18fd..b15bdadaedb5 100644 >>> --- a/drivers/mtd/parsers/bcm63xxpart.c >>> +++ b/drivers/mtd/parsers/bcm63xxpart.c >>> @@ -22,6 +22,11 @@ >>> #include <linux/mtd/partitions.h> >>> #include <linux/of.h> >>> >>> +#ifdef CONFIG_MIPS >>> +#include <asm/bootinfo.h> >>> +#include <asm/fw/cfe/cfe_api.h> >>> +#endif /* CONFIG_MIPS */ >>> + >>> #define BCM963XX_CFE_BLOCK_SIZE SZ_64K /* always at least 64KiB */ >>> >>> #define BCM963XX_CFE_MAGIC_OFFSET 0x4e0 >>> @@ -32,28 +37,15 @@ >>> #define STR_NULL_TERMINATE(x) \ >>> do { char *_str = (x); _str[sizeof(x) - 1] = 0; } while (0) >>> >>> -static int bcm63xx_detect_cfe(struct mtd_info *master) >>> +static inline int bcm63xx_detect_cfe(void) >>> { >>> - char buf[9]; >>> - int ret; >>> - size_t retlen; >>> + int ret = 0; >>> >>> - ret = mtd_read(master, BCM963XX_CFE_VERSION_OFFSET, 5, &retlen, >>> - (void *)buf); >>> - buf[retlen] = 0; >>> +#ifdef CONFIG_MIPS >>> + ret = (fw_arg3 == CFE_EPTSEAL); >>> +#endif /* CONFIG_MIPS */ >>> >>> - if (ret) >>> - return ret; >>> - >>> - if (strncmp("cfe-v", buf, 5) == 0) >>> - return 0; >>> - >>> - /* very old CFE's do not have the cfe-v string, so check for magic */ >>> - ret = mtd_read(master, BCM963XX_CFE_MAGIC_OFFSET, 8, &retlen, >>> - (void *)buf); >>> - buf[retlen] = 0; >>> - >>> - return strncmp("CFE1CFE1", buf, 8); >>> + return ret; >>> } >>> >>> static int bcm63xx_read_nvram(struct mtd_info *master, >>> @@ -138,7 +130,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, >>> struct bcm963xx_nvram *nvram = NULL; >>> int ret; >>> >>> - if (bcm63xx_detect_cfe(master)) >>> + if (!bcm63xx_detect_cfe()) >>> return -EINVAL; >>> >>> nvram = vzalloc(sizeof(*nvram)); ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hello, Guenter Roeck <linux@roeck-us.net> wrote on Mon, 21 Sep 2020 20:26:19 -0700: > On 9/21/20 8:18 PM, Naresh Kamboju wrote: > > On Fri, 14 Aug 2020 at 14:26, Guenter Roeck <linux@roeck-us.net> wrote: > >> > >> On Mon, Jun 15, 2020 at 11:17:40AM +0200, Álvaro Fernández Rojas wrote: > >>> Instead of trying to parse CFE version string, which is customized by some > >>> vendors, let's just check that "CFE1" was passed on argument 3. > >>> > >>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > >>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > >>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > >> > > > > We still see mips: allmodconfig build failure on Linus tree > > > > Yes, same here. Perhaps this check should be done by an exported helper so that we do not blindly export the variable? Alvaro, Jonas, can one of you try to address this issue please? Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On 9/28/2020 7:16 AM, Miquel Raynal wrote: > Hello, > > Guenter Roeck <linux@roeck-us.net> wrote on Mon, 21 Sep 2020 20:26:19 > -0700: > >> On 9/21/20 8:18 PM, Naresh Kamboju wrote: >>> On Fri, 14 Aug 2020 at 14:26, Guenter Roeck <linux@roeck-us.net> wrote: >>>> >>>> On Mon, Jun 15, 2020 at 11:17:40AM +0200, Álvaro Fernández Rojas wrote: >>>>> Instead of trying to parse CFE version string, which is customized by some >>>>> vendors, let's just check that "CFE1" was passed on argument 3. >>>>> >>>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> >>>>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> >>>>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> >>>> >>> >>> We still see mips: allmodconfig build failure on Linus tree >>> >> >> Yes, same here. > > Perhaps this check should be done by an exported helper so that we do > not blindly export the variable? > > Alvaro, Jonas, can one of you try to address this issue please? We should probably just make the parser 'bool' there is no much point building this as a module, it will not improve compile time coverage, and it will most likely not help with the kernel image size on the platforms that require the parser to be there anyway. -- Florian ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
With commit 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE detection"), we generate a reference to fw_arg3 which is the fourth firmware/command line argument on MIPS platforms. That symbol is not exported and would cause a linking failure. The parser is typically necessary to boot a BCM63xx-based system anyway so having it be part of the kernel image makes sense, therefore make it 'bool' instead of 'tristate'. Fixes: 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE detection") Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/mtd/parsers/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/parsers/Kconfig b/drivers/mtd/parsers/Kconfig index f98363c9b363..e72354322f62 100644 --- a/drivers/mtd/parsers/Kconfig +++ b/drivers/mtd/parsers/Kconfig @@ -12,7 +12,7 @@ config MTD_BCM47XX_PARTS boards. config MTD_BCM63XX_PARTS - tristate "BCM63XX CFE partitioning parser" + bool "BCM63XX CFE partitioning parser" depends on BCM63XX || BMIPS_GENERIC || COMPILE_TEST select CRC32 select MTD_PARSER_IMAGETAG -- 2.25.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Tue, 2020-09-29 at 17:27:21 UTC, Florian Fainelli wrote: > With commit 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE > detection"), we generate a reference to fw_arg3 which is the fourth > firmware/command line argument on MIPS platforms. That symbol is not > exported and would cause a linking failure. > > The parser is typically necessary to boot a BCM63xx-based system anyway > so having it be part of the kernel image makes sense, therefore make it > 'bool' instead of 'tristate'. > > Fixes: 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE detection") > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks. Miquel ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Tue, Sep 29, 2020 at 10:27:21AM -0700, Florian Fainelli wrote: > With commit 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE > detection"), we generate a reference to fw_arg3 which is the fourth > firmware/command line argument on MIPS platforms. That symbol is not > exported and would cause a linking failure. > > The parser is typically necessary to boot a BCM63xx-based system anyway > so having it be part of the kernel image makes sense, therefore make it > 'bool' instead of 'tristate'. > > Fixes: 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE detection") > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> What happened with this patch ? The build failure is still seen in mainline and in next-20201009. Guenter > --- > drivers/mtd/parsers/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/parsers/Kconfig b/drivers/mtd/parsers/Kconfig > index f98363c9b363..e72354322f62 100644 > --- a/drivers/mtd/parsers/Kconfig > +++ b/drivers/mtd/parsers/Kconfig > @@ -12,7 +12,7 @@ config MTD_BCM47XX_PARTS > boards. > > config MTD_BCM63XX_PARTS > - tristate "BCM63XX CFE partitioning parser" > + bool "BCM63XX CFE partitioning parser" > depends on BCM63XX || BMIPS_GENERIC || COMPILE_TEST > select CRC32 > select MTD_PARSER_IMAGETAG > -- > 2.25.1 > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Hi Guenter, Guenter Roeck <linux@roeck-us.net> wrote on Sun, 11 Oct 2020 07:14:47 -0700: > On Tue, Sep 29, 2020 at 10:27:21AM -0700, Florian Fainelli wrote: > > With commit 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE > > detection"), we generate a reference to fw_arg3 which is the fourth > > firmware/command line argument on MIPS platforms. That symbol is not > > exported and would cause a linking failure. > > > > The parser is typically necessary to boot a BCM63xx-based system anyway > > so having it be part of the kernel image makes sense, therefore make it > > 'bool' instead of 'tristate'. > > > > Fixes: 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE detection") > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > > What happened with this patch ? The build failure is still seen in mainline > and in next-20201009. It has been applied on mtd/next: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=mtd/next (I don't remember when though) ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Mon, Oct 12, 2020 at 09:04:20AM +0200, Miquel Raynal wrote: > Hi Guenter, > > Guenter Roeck <linux@roeck-us.net> wrote on Sun, 11 Oct 2020 07:14:47 > -0700: > > > On Tue, Sep 29, 2020 at 10:27:21AM -0700, Florian Fainelli wrote: > > > With commit 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE > > > detection"), we generate a reference to fw_arg3 which is the fourth > > > firmware/command line argument on MIPS platforms. That symbol is not > > > exported and would cause a linking failure. > > > > > > The parser is typically necessary to boot a BCM63xx-based system anyway > > > so having it be part of the kernel image makes sense, therefore make it > > > 'bool' instead of 'tristate'. > > > > > > Fixes: 91e81150d388 ("mtd: parsers: bcm63xx: simplify CFE detection") > > > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > > > > What happened with this patch ? The build failure is still seen in mainline > > and in next-20201009. > > It has been applied on mtd/next: > https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=mtd/next > (I don't remember when though) Hmm, lets see ... Ah, I see: mips:allmodconfig now fails to build in -next for a different reason. Error log: In file included from <command-line>: arch/mips/mm/init.c: In function 'mem_init': include/linux/compiler_types.h:319:38: error: call to '__compiletime_assert_331' declared with attribute error: BUILD_BUG_ON failed: IS_ENABLED(CONFIG_32BIT) && (_PFN_SHIFT > PAGE_SHIFT) I'll send a separate report on that (if it wasn't reported before). Anyway, any reason for not applying this fix to mainline, ie why the mips:allmodconfig build failure was not fixed in v5.9 ? Guenter ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/