[PATCH 0/2] mtd: parsers: bcm63xx: simplify CFE detection

[PATCH 0/2] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] MIPS: BCM63xx: add helper function to detect CFE

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Miquel Raynal]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Florian Fainelli]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Miquel Raynal]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Miquel Raynal]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Miquel Raynal]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3] mtd: parsers: bcm63xx: simplify CFE detection

[Miquel Raynal]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4] mtd: parsers: bcm63xx: simplify CFE detection

[Álvaro Fernández Rojas]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4] mtd: parsers: bcm63xx: simplify CFE detection

[Florian Fainelli]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4] mtd: parsers: bcm63xx: simplify CFE detection

[Miquel Raynal]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4] mtd: parsers: bcm63xx: simplify CFE detection

[Guenter Roeck]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4] mtd: parsers: bcm63xx: simplify CFE detection

[Naresh Kamboju]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4] mtd: parsers: bcm63xx: simplify CFE detection

[Guenter Roeck]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4] mtd: parsers: bcm63xx: simplify CFE detection

[Miquel Raynal]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4] mtd: parsers: bcm63xx: simplify CFE detection

[Florian Fainelli]

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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel