[PATCH 0/3] fix NVRAM partition size if larger than expected

[PATCH 0/3] fix NVRAM partition size if larger than expected

[Jonas Gorski]

Some device vendors use a larger nvram size than expected. While Broadcom
has defined it as 64K max, devices with 128K have been seen in the wild.
Luckily they properly set the nvram's PSI size to the correct value, so
we can use that to size the nvram partion.

Yes it's a bit confusing as there are two nvrams, one with a fixed layout
with in the bootloader, with the size information about the other.

Since 2 of 3 patches are for the mtd tree, this patchset should go there
(but it applies to both l2-mtd and mips-next fine).

Jonas Gorski (3):
  MTD: bcm63xxpart: use size macro for CFE block size
  MIPS: BCM63XX: export PSI size from nvram
  MTD: bcm63xxpart: use nvram for PSI size

 arch/mips/bcm63xx/nvram.c                          |   11 +++++++++++
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h |    2 ++
 drivers/mtd/bcm63xxpart.c                          |    9 ++++++---
 3 files changed, 19 insertions(+), 3 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] MTD: bcm63xxpart: use size macro for CFE block size

[Jonas Gorski]

Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 drivers/mtd/bcm63xxpart.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index 6eeb84c..fc3d7d0 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -27,6 +27,7 @@
 #include <linux/crc32.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/sizes.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/mtd/mtd.h>
@@ -37,7 +38,7 @@
 
 #define BCM63XX_EXTENDED_SIZE	0xBFC00000	/* Extended flash address */
 
-#define BCM63XX_CFE_BLOCK_SIZE	0x10000		/* always at least 64KiB */
+#define BCM63XX_CFE_BLOCK_SIZE	SZ_64K		/* always at least 64KiB */
 
 #define BCM63XX_CFE_MAGIC_OFFSET 0x4e0
 
-- 
1.7.10.4

[PATCH 2/3] MIPS: BCM63XX: export PSI size from nvram

[Jonas Gorski]

Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 arch/mips/bcm63xx/nvram.c                          |   11 +++++++++++
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h |    2 ++
 2 files changed, 13 insertions(+)

diff --git a/arch/mips/bcm63xx/nvram.c b/arch/mips/bcm63xx/nvram.c
index 6206116..44af608 100644
--- a/arch/mips/bcm63xx/nvram.c
+++ b/arch/mips/bcm63xx/nvram.c
@@ -35,6 +35,8 @@ struct bcm963xx_nvram {
 	u32	checksum_high;
 };
 
+#define BCM63XX_DEFAULT_PSI_SIZE	64
+
 static struct bcm963xx_nvram nvram;
 static int mac_addr_used;
 
@@ -105,3 +107,12 @@ int bcm63xx_nvram_get_mac_address(u8 *mac)
 	return 0;
 }
 EXPORT_SYMBOL(bcm63xx_nvram_get_mac_address);
+
+int bcm63xx_nvram_get_psi_size(void)
+{
+	if (nvram.psi_size > 0)
+		return nvram.psi_size;
+
+	return BCM63XX_DEFAULT_PSI_SIZE;
+}
+EXPORT_SYMBOL(bcm63xx_nvram_get_psi_size);
diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
index 62d6a3b..d76a486 100644
--- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
+++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
@@ -32,4 +32,6 @@ u8 *bcm63xx_nvram_get_name(void);
  */
 int bcm63xx_nvram_get_mac_address(u8 *mac);
 
+int bcm63xx_nvram_get_psi_size(void);
+
 #endif /* BCM63XX_NVRAM_H */
-- 
1.7.10.4

[PATCH 3/3] MTD: bcm63xxpart: use nvram for PSI size

[Jonas Gorski]

Read out the SPI size from nvram instead of defaulting to 64K - some
vendors actually use values larger than the "max" value of 64.

Signed-off-by: Jonas Gorski <jogo@openwrt.org>
---
 drivers/mtd/bcm63xxpart.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index fc3d7d0..5c81390 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -4,7 +4,7 @@
  * Copyright © 2006-2008  Florian Fainelli <florian@openwrt.org>
  *			  Mike Albon <malbon@openwrt.org>
  * Copyright © 2009-2010  Daniel Dickinson <openwrt@cshore.neomailbox.net>
- * Copyright © 2011-2012  Jonas Gorski <jonas.gorski@gmail.com>
+ * Copyright © 2011-2013  Jonas Gorski <jonas.gorski@gmail.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -33,6 +33,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 
+#include <asm/mach-bcm63xx/bcm63xx_nvram.h>
 #include <asm/mach-bcm63xx/bcm963xx_tag.h>
 #include <asm/mach-bcm63xx/board_bcm963xx.h>
 
@@ -91,7 +92,8 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 			      BCM63XX_CFE_BLOCK_SIZE);
 
 	cfelen = cfe_erasesize;
-	nvramlen = cfe_erasesize;
+	nvramlen = bcm63xx_nvram_get_psi_size() * SZ_1K;
+	nvramlen = roundup(nvramlen, cfe_erasesize);
 
 	/* Allocate memory for buffer */
 	buf = vmalloc(sizeof(struct bcm_tag));
-- 
1.7.10.4

Re: [PATCH 2/3] MIPS: BCM63XX: export PSI size from nvram

[Florian Fainelli]

Hello Jonas,

A few minor comments below:

Le samedi 23 mars 2013 14:07:48, Jonas Gorski a écrit :
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
>  arch/mips/bcm63xx/nvram.c                          |   11 +++++++++++
>  arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h |    2 ++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/arch/mips/bcm63xx/nvram.c b/arch/mips/bcm63xx/nvram.c
> index 6206116..44af608 100644
> --- a/arch/mips/bcm63xx/nvram.c
> +++ b/arch/mips/bcm63xx/nvram.c
> @@ -35,6 +35,8 @@ struct bcm963xx_nvram {
>  	u32	checksum_high;
>  };
> 
> +#define BCM63XX_DEFAULT_PSI_SIZE	64
> +
>  static struct bcm963xx_nvram nvram;
>  static int mac_addr_used;
> 
> @@ -105,3 +107,12 @@ int bcm63xx_nvram_get_mac_address(u8 *mac)
>  	return 0;
>  }
>  EXPORT_SYMBOL(bcm63xx_nvram_get_mac_address);
> +
> +int bcm63xx_nvram_get_psi_size(void)
> +{
> +	if (nvram.psi_size > 0)
> +		return nvram.psi_size;
> +
> +	return BCM63XX_DEFAULT_PSI_SIZE;

I wonder if it would not be better to return a size in KBytes here instead of 
a multiple of KBytes?

> +}
> +EXPORT_SYMBOL(bcm63xx_nvram_get_psi_size);
> diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
> b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h index
> 62d6a3b..d76a486 100644
> --- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
> +++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
> @@ -32,4 +32,6 @@ u8 *bcm63xx_nvram_get_name(void);
>   */
>  int bcm63xx_nvram_get_mac_address(u8 *mac);
> 
> +int bcm63xx_nvram_get_psi_size(void);

You could use an unsigned int here.
-- 
Florian

Re: [PATCH 0/3] fix NVRAM partition size if larger than expected

[Florian Fainelli]

Le samedi 23 mars 2013 14:07:46, Jonas Gorski a écrit :
> Some device vendors use a larger nvram size than expected. While Broadcom
> has defined it as 64K max, devices with 128K have been seen in the wild.
> Luckily they properly set the nvram's PSI size to the correct value, so
> we can use that to size the nvram partion.
> 
> Yes it's a bit confusing as there are two nvrams, one with a fixed layout
> with in the bootloader, with the size information about the other.
> 
> Since 2 of 3 patches are for the mtd tree, this patchset should go there
> (but it applies to both l2-mtd and mips-next fine).

Besides the minor nitpicks from patch 2, this serie looks good:

Acked-by: Florian Fainelli <florian@openwrt.org>

Thanks!
-- 
Florian

Re: [PATCH 0/3] fix NVRAM partition size if larger than expected

[Artem Bityutskiy]

On Sat, 2013-03-23 at 14:07 +0100, Jonas Gorski wrote:
> Some device vendors use a larger nvram size than expected. While Broadcom
> has defined it as 64K max, devices with 128K have been seen in the wild.
> Luckily they properly set the nvram's PSI size to the correct value, so
> we can use that to size the nvram partion.
> 
> Yes it's a bit confusing as there are two nvrams, one with a fixed layout
> with in the bootloader, with the size information about the other.
> 
> Since 2 of 3 patches are for the mtd tree, this patchset should go there
> (but it applies to both l2-mtd and mips-next fine).

Pushed all 3 to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 2/3] MIPS: BCM63XX: export PSI size from nvram

[Artem Bityutskiy]

On Sat, 2013-03-23 at 14:07 +0100, Jonas Gorski wrote:
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
>  arch/mips/bcm63xx/nvram.c                          |   11 +++++++++++
>  arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h |    2 ++
>  2 files changed, 13 insertions(+)

Acks from MIPS folks would be nice to have, but I pushed this patch to
the l2-mtd.git tree, thanks!

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 2/3] MIPS: BCM63XX: export PSI size from nvram

[Jonas Gorski]

On Fri, May 10, 2013 at 2:36 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Sat, 2013-03-23 at 14:07 +0100, Jonas Gorski wrote:
>> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
>> ---
>>  arch/mips/bcm63xx/nvram.c                          |   11 +++++++++++
>>  arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h |    2 ++
>>  2 files changed, 13 insertions(+)
>
> Acks from MIPS folks would be nice to have, but I pushed this patch to
> the l2-mtd.git tree, thanks!

I had expected Florian's valid comment from preventing this series
from going in, but if you pushed it already then I will fix the return
type problem  that Florian pointed out in a separate patch (or rather
add add some range check for nvram.psi_size). Luckily it is a
theoretical issue only, as I haven't seen a device yet with an invalid
value.


Regards
Jonas

Re: [PATCH 2/3] MIPS: BCM63XX: export PSI size from nvram

[Florian Fainelli]

Le 12/05/2013 12:48, Jonas Gorski a écrit :
> On Fri, May 10, 2013 at 2:36 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>> On Sat, 2013-03-23 at 14:07 +0100, Jonas Gorski wrote:
>>> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
>>> ---
>>>   arch/mips/bcm63xx/nvram.c                          |   11 +++++++++++
>>>   arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h |    2 ++
>>>   2 files changed, 13 insertions(+)
>>
>> Acks from MIPS folks would be nice to have, but I pushed this patch to
>> the l2-mtd.git tree, thanks!
>
> I had expected Florian's valid comment from preventing this series
> from going in, but if you pushed it already then I will fix the return
> type problem  that Florian pointed out in a separate patch (or rather
> add add some range check for nvram.psi_size). Luckily it is a
> theoretical issue only, as I haven't seen a device yet with an invalid
> value.

Right, but this is no blocker from my perspective. As about the MIPS 
folks, Maxime, Kevin, Jonas and myself have been the "historical" 
contributors to the MIPS BCM63XX port, so I would consider Jonas to be 
authoritave here for these paches. John and Ralf usually do not comment 
unless the see something bad.
--
Florian