[PATCH linux-next v4 00/11] mtd: bcm63xxpart: Add NAND partitioning support

[PATCH linux-next v4 00/11] mtd: bcm63xxpart: Add NAND partitioning support

[Simon Arlott]

The BCM963xx NAND flash boards have a different handling of the
partition layout from the NOR flash boards. For NAND there are offsets
for the partitions in nvram. Both types of boards use the same CFE
bootloader, nvram format and image tag in their rootfs partitions.

This patch series:
1-4:  Creates separate header files for bcm963xx_nvram and bcm_tag structures
5:    Updates the bcm_tag field image_sequence
6:    Removes the dependency on mach-bcm63xx from the bcm63xxpart parser
7:    Removes unused mach-bcm63xx nvram function
8-10: Cleanup and move NOR flash layout to a separate function
11:   Add NAND flash layout support

Patches 1-2 tested on BCM63XX with a BCM963168 (NAND) board.
Patches 3-11 compile tested on BCM63XX.
Patches 6-11 tested on BMIPS with a BCM963168 (NAND) board.
---
v4: Move struct bcm_tag to include/linux/.

    Modify bcm63xx parser to read nvram from NOR flash and handle the
    NAND flash layout in the same parser.

v3: Fix includes/type names, add comments explaining the nvram struct.

    Use COMPILE_TEST.

    Ensure that strings read from flash are null terminated and validate
    bcm_tag integer values (this also moves reporting of rootfs sequence
    numbers to later on).

v2: Use external struct bcm963xx_nvram definition for bcm963268part.

    Removed support for the nand partition number field, it's not a
    standard Broadcom field (was added by MitraStar Technology Corp.).

-- 
Simon Arlott

Re: [PATCH linux-next v4 00/11] mtd: bcm63xxpart: Add NAND partitioning support

[Simon Arlott]

On 13/12/15 22:42, Simon Arlott wrote:
> The BCM963xx NAND flash boards have a different handling of the
> partition layout from the NOR flash boards. For NAND there are offsets
> for the partitions in nvram. Both types of boards use the same CFE
> bootloader, nvram format and image tag in their rootfs partitions.
> 
> This patch series:
> 1-4:  Creates separate header files for bcm963xx_nvram and bcm_tag structures
> 5:    Updates the bcm_tag field image_sequence
> 6:    Removes the dependency on mach-bcm63xx from the bcm63xxpart parser
> 7:    Removes unused mach-bcm63xx nvram function
> 8-10: Cleanup and move NOR flash layout to a separate function
> 11:   Add NAND flash layout support

These patches are also available on github against next-20151211:
https://github.com/lp0/linux/commits/20151213-bcm63xxpart-v4

(Yes I am aware that the summary line and description of the patches are
not suitable for merging.)

-- 
Simon Arlott

[PATCH linux-next v4 01/11] MIPS: bcm963xx: Add Broadcom BCM963xx board nvram data structure

[Simon Arlott]

Broadcom BCM963xx boards have multiple nvram variants across different
SoCs with additional checksum fields added whenever the size of the
nvram was extended.

Add this structure as a header file so that multiple drivers can use it.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: Move out of uapi.

    Add nand offset/size functions/macros.

    Add checksum calculation function.

v3: Fix includes/type names, add comments explaining the nvram struct.

v2: Use external struct bcm963xx_nvram definition for bcm963268part.

 MAINTAINERS                    |   1 +
 include/linux/bcm963xx_nvram.h | 112 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)
 create mode 100644 include/linux/bcm963xx_nvram.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 31fc0bf..ac7de1a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2391,6 +2391,7 @@ F:	arch/mips/kernel/*bmips*
 F:	arch/mips/boot/dts/brcm/bcm*.dts*
 F:	drivers/irqchip/irq-bcm7*
 F:	drivers/irqchip/irq-brcmstb*
+F:	include/linux/bcm963xx_nvram.h
 
 BROADCOM TG3 GIGABIT ETHERNET DRIVER
 M:	Prashant Sreedharan <prashant@broadcom.com>
diff --git a/include/linux/bcm963xx_nvram.h b/include/linux/bcm963xx_nvram.h
new file mode 100644
index 0000000..290c231
--- /dev/null
+++ b/include/linux/bcm963xx_nvram.h
@@ -0,0 +1,112 @@
+#ifndef __LINUX_BCM963XX_NVRAM_H__
+#define __LINUX_BCM963XX_NVRAM_H__
+
+#include <linux/crc32.h>
+#include <linux/if_ether.h>
+#include <linux/sizes.h>
+#include <linux/types.h>
+
+/*
+ * Broadcom BCM963xx SoC board nvram data structure.
+ *
+ * The nvram structure varies in size depending on the SoC board version. Use
+ * the appropriate minimum BCM963XX_NVRAM_*_SIZE define for the information
+ * you need instead of sizeof(struct bcm963xx_nvram) as this may change.
+ */
+
+#define BCM963XX_NVRAM_V4_SIZE		300
+#define BCM963XX_NVRAM_V5_SIZE		(1 * SZ_1K)
+
+#define BCM963XX_DEFAULT_PSI_SIZE	64
+
+enum bcm963xx_nvram_nand_part {
+	BCM963XX_NVRAM_NAND_PART_BOOT = 0,
+	BCM963XX_NVRAM_NAND_PART_ROOTFS_1,
+	BCM963XX_NVRAM_NAND_PART_ROOTFS_2,
+	BCM963XX_NVRAM_NAND_PART_DATA,
+	BCM963XX_NVRAM_NAND_PART_BBT,
+
+	__BCM963XX_NVRAM_NAND_NR_PARTS
+};
+
+struct bcm963xx_nvram {
+	u32	version;
+	char	bootline[256];
+	char	name[16];
+	u32	main_tp_number;
+	u32	psi_size;
+	u32	mac_addr_count;
+	u8	mac_addr_base[ETH_ALEN];
+	u8	__reserved1[2];
+	u32	checksum_v4;
+
+	u8	__reserved2[292];
+	u32	nand_part_offset[__BCM963XX_NVRAM_NAND_NR_PARTS];
+	u32	nand_part_size[__BCM963XX_NVRAM_NAND_NR_PARTS];
+	u8	__reserved3[388];
+	u32	checksum_v5;
+};
+
+#define BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, part) \
+	bcm963xx_nvram_nand_part_offset(nvram, BCM963XX_NVRAM_NAND_PART_ ##part)
+
+static inline u64 __pure bcm963xx_nvram_nand_part_offset(
+	const struct bcm963xx_nvram *nvram,
+	enum bcm963xx_nvram_nand_part part)
+{
+	return nvram->nand_part_offset[part] * SZ_1K;
+}
+
+#define BCM963XX_NVRAM_NAND_PART_SIZE(nvram, part) \
+	bcm963xx_nvram_nand_part_size(nvram, BCM963XX_NVRAM_NAND_PART_ ##part)
+
+static inline u64 __pure bcm963xx_nvram_nand_part_size(
+	const struct bcm963xx_nvram *nvram,
+	enum bcm963xx_nvram_nand_part part)
+{
+	return nvram->nand_part_size[part] * SZ_1K;
+}
+
+/*
+ * bcm963xx_nvram_checksum - Verify nvram checksum
+ *
+ * @nvram: pointer to full size nvram data structure
+ * @expected_out: optional pointer to store expected checksum value
+ * @actual_out: optional pointer to store actual checksum value
+ *
+ * Return: 0 if the checksum is valid, otherwise -EINVAL
+ */
+static int __maybe_unused bcm963xx_nvram_checksum(
+	const struct bcm963xx_nvram *nvram,
+	u32 *expected_out, u32 *actual_out)
+{
+	u32 expected, actual;
+	size_t len;
+
+	if (nvram->version <= 4) {
+		expected = nvram->checksum_v4;
+		len = BCM963XX_NVRAM_V4_SIZE - sizeof(u32);
+	} else {
+		expected = nvram->checksum_v5;
+		len = BCM963XX_NVRAM_V5_SIZE - sizeof(u32);
+	}
+
+	/*
+	 * Calculate the CRC32 value for the nvram with a checksum value
+	 * of 0 without modifying or copying the nvram by combining:
+	 * - The CRC32 of the nvram without the checksum value
+	 * - The CRC32 of a zero checksum value (which is also 0)
+	 */
+	actual = crc32_le_combine(
+		crc32_le(~0, (u8 *)nvram, len), 0, sizeof(u32));
+
+	if (expected_out)
+		*expected_out = expected;
+
+	if (actual_out)
+		*actual_out = actual;
+
+	return expected == actual ? 0 : -EINVAL;
+};
+
+#endif /* __LINUX_BCM963XX_NVRAM_H__ */
-- 
2.1.4

-- 
Simon Arlott

[PATCH linux-next v4 02/11] MIPS: bcm63xx: nvram: Use nvram structure definition from header file

[Simon Arlott]

Use the common definition of the nvram structure from the header file
include/linux/bcm963xx_nvram.h instead of maintaining a separate copy.

Read the version 5 size of nvram data from memory and then call the
new checksum verification function from the header file.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: Use checksum verification function from header file.

v3: No changes (reworded commit message).

v2: Use external struct bcm963xx_nvram definition for bcm963268part.

 arch/mips/bcm63xx/nvram.c | 35 +++--------------------------------
 1 file changed, 3 insertions(+), 32 deletions(-)

diff --git a/arch/mips/bcm63xx/nvram.c b/arch/mips/bcm63xx/nvram.c
index 4b50d40..05757ae 100644
--- a/arch/mips/bcm63xx/nvram.c
+++ b/arch/mips/bcm63xx/nvram.c
@@ -10,6 +10,7 @@
 
 #define pr_fmt(fmt) "bcm63xx_nvram: " fmt
 
+#include <linux/bcm963xx_nvram.h>
 #include <linux/init.h>
 #include <linux/crc32.h>
 #include <linux/export.h>
@@ -18,23 +19,6 @@
 
 #include <bcm63xx_nvram.h>
 
-/*
- * nvram structure
- */
-struct bcm963xx_nvram {
-	u32	version;
-	u8	reserved1[256];
-	u8	name[16];
-	u32	main_tp_number;
-	u32	psi_size;
-	u32	mac_addr_count;
-	u8	mac_addr_base[ETH_ALEN];
-	u8	reserved2[2];
-	u32	checksum_old;
-	u8	reserved3[720];
-	u32	checksum_high;
-};
-
 #define BCM63XX_DEFAULT_PSI_SIZE	64
 
 static struct bcm963xx_nvram nvram;
@@ -42,27 +26,14 @@ static int mac_addr_used;
 
 void __init bcm63xx_nvram_init(void *addr)
 {
-	unsigned int check_len;
 	u32 crc, expected_crc;
 	u8 hcs_mac_addr[ETH_ALEN] = { 0x00, 0x10, 0x18, 0xff, 0xff, 0xff };
 
 	/* extract nvram data */
-	memcpy(&nvram, addr, sizeof(nvram));
+	memcpy(&nvram, addr, BCM963XX_NVRAM_V5_SIZE);
 
 	/* check checksum before using data */
-	if (nvram.version <= 4) {
-		check_len = offsetof(struct bcm963xx_nvram, reserved3);
-		expected_crc = nvram.checksum_old;
-		nvram.checksum_old = 0;
-	} else {
-		check_len = sizeof(nvram);
-		expected_crc = nvram.checksum_high;
-		nvram.checksum_high = 0;
-	}
-
-	crc = crc32_le(~0, (u8 *)&nvram, check_len);
-
-	if (crc != expected_crc)
+	if (bcm963xx_nvram_checksum(&nvram, &expected_crc, &crc))
 		pr_warn("nvram checksum failed, contents may be invalid (expected %08x, got %08x)\n",
 			expected_crc, crc);
 
-- 
2.1.4

-- 
Simon Arlott

[PATCH linux-next v4 03/11] MIPS: bcm963xx: Move Broadcom BCM963xx image tag data structure

[Simon Arlott]

Move Broadcom BCM963xx image tag data structure to include/linux/
so that drivers outside of mach-bcm63xx can use it.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: New patch.

 MAINTAINERS                                       |  1 +
 arch/mips/include/asm/mach-bcm63xx/bcm963xx_tag.h | 96 ----------------------
 drivers/mtd/bcm63xxpart.c                         |  2 +-
 include/linux/bcm963xx_tag.h                      | 98 +++++++++++++++++++++++
 4 files changed, 100 insertions(+), 97 deletions(-)
 delete mode 100644 arch/mips/include/asm/mach-bcm63xx/bcm963xx_tag.h
 create mode 100644 include/linux/bcm963xx_tag.h

diff --git a/MAINTAINERS b/MAINTAINERS
index ac7de1a..b2ef403 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2392,6 +2392,7 @@ F:	arch/mips/boot/dts/brcm/bcm*.dts*
 F:	drivers/irqchip/irq-bcm7*
 F:	drivers/irqchip/irq-brcmstb*
 F:	include/linux/bcm963xx_nvram.h
+F:	include/linux/bcm963xx_tag.h
 
 BROADCOM TG3 GIGABIT ETHERNET DRIVER
 M:	Prashant Sreedharan <prashant@broadcom.com>
diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm963xx_tag.h b/arch/mips/include/asm/mach-bcm63xx/bcm963xx_tag.h
deleted file mode 100644
index 1e6b587..0000000
--- a/arch/mips/include/asm/mach-bcm63xx/bcm963xx_tag.h
+++ /dev/null
@@ -1,96 +0,0 @@
-#ifndef __BCM963XX_TAG_H
-#define __BCM963XX_TAG_H
-
-#define TAGVER_LEN		4	/* Length of Tag Version */
-#define TAGLAYOUT_LEN		4	/* Length of FlashLayoutVer */
-#define SIG1_LEN		20	/* Company Signature 1 Length */
-#define SIG2_LEN		14	/* Company Signature 2 Length */
-#define BOARDID_LEN		16	/* Length of BoardId */
-#define ENDIANFLAG_LEN		2	/* Endian Flag Length */
-#define CHIPID_LEN		6	/* Chip Id Length */
-#define IMAGE_LEN		10	/* Length of Length Field */
-#define ADDRESS_LEN		12	/* Length of Address field */
-#define DUALFLAG_LEN		2	/* Dual Image flag Length */
-#define INACTIVEFLAG_LEN	2	/* Inactie Flag Length */
-#define RSASIG_LEN		20	/* Length of RSA Signature in tag */
-#define TAGINFO1_LEN		30	/* Length of vendor information field1 in tag */
-#define FLASHLAYOUTVER_LEN	4	/* Length of Flash Layout Version String tag */
-#define TAGINFO2_LEN		16	/* Length of vendor information field2 in tag */
-#define ALTTAGINFO_LEN		54	/* Alternate length for vendor information; Pirelli */
-
-#define NUM_PIRELLI		2
-#define IMAGETAG_CRC_START	0xFFFFFFFF
-
-#define PIRELLI_BOARDS { \
-	"AGPF-S0", \
-	"DWV-S0", \
-}
-
-/*
- * The broadcom firmware assumes the rootfs starts the image,
- * therefore uses the rootfs start (flash_image_address)
- * to determine where to flash the image.  Since we have the kernel first
- * we have to give it the kernel address, but the crc uses the length
- * associated with this address (root_length), which is added to the kernel
- * length (kernel_length) to determine the length of image to flash and thus
- * needs to be rootfs + deadcode (jffs2 EOF marker)
-*/
-
-struct bcm_tag {
-	/* 0-3: Version of the image tag */
-	char tag_version[TAGVER_LEN];
-	/* 4-23: Company Line 1 */
-	char sig_1[SIG1_LEN];
-	/*  24-37: Company Line 2 */
-	char sig_2[SIG2_LEN];
-	/* 38-43: Chip this image is for */
-	char chip_id[CHIPID_LEN];
-	/* 44-59: Board name */
-	char board_id[BOARDID_LEN];
-	/* 60-61: Map endianness -- 1 BE 0 LE */
-	char big_endian[ENDIANFLAG_LEN];
-	/* 62-71: Total length of image */
-	char total_length[IMAGE_LEN];
-	/* 72-83: Address in memory of CFE */
-	char cfe__address[ADDRESS_LEN];
-	/* 84-93: Size of CFE */
-	char cfe_length[IMAGE_LEN];
-	/* 94-105: Address in memory of image start
-	 * (kernel for OpenWRT, rootfs for stock firmware)
-	 */
-	char flash_image_start[ADDRESS_LEN];
-	/* 106-115: Size of rootfs */
-	char root_length[IMAGE_LEN];
-	/* 116-127: Address in memory of kernel */
-	char kernel_address[ADDRESS_LEN];
-	/* 128-137: Size of kernel */
-	char kernel_length[IMAGE_LEN];
-	/* 138-139: Unused at the moment */
-	char dual_image[DUALFLAG_LEN];
-	/* 140-141: Unused at the moment */
-	char inactive_flag[INACTIVEFLAG_LEN];
-	/* 142-161: RSA Signature (not used; some vendors may use this) */
-	char rsa_signature[RSASIG_LEN];
-	/* 162-191: Compilation and related information (not used in OpenWrt) */
-	char information1[TAGINFO1_LEN];
-	/* 192-195: Version flash layout */
-	char flash_layout_ver[FLASHLAYOUTVER_LEN];
-	/* 196-199: kernel+rootfs CRC32 */
-	__u32 fskernel_crc;
-	/* 200-215: Unused except on Alice Gate where is is information */
-	char information2[TAGINFO2_LEN];
-	/* 216-219: CRC32 of image less imagetag (kernel for Alice Gate) */
-	__u32 image_crc;
-	/* 220-223: CRC32 of rootfs partition */
-	__u32 rootfs_crc;
-	/* 224-227: CRC32 of kernel partition */
-	__u32 kernel_crc;
-	/* 228-235: Unused at present */
-	char reserved1[8];
-	/* 236-239: CRC32 of header excluding last 20 bytes */
-	__u32 header_crc;
-	/* 240-255: Unused at present */
-	char reserved2[16];
-};
-
-#endif /* __BCM63XX_TAG_H */
diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index 4409369..b5bad1f 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -24,6 +24,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/bcm963xx_tag.h>
 #include <linux/crc32.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -34,7 +35,6 @@
 #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>
 
 #define BCM63XX_EXTENDED_SIZE	0xBFC00000	/* Extended flash address */
diff --git a/include/linux/bcm963xx_tag.h b/include/linux/bcm963xx_tag.h
new file mode 100644
index 0000000..f389dac
--- /dev/null
+++ b/include/linux/bcm963xx_tag.h
@@ -0,0 +1,98 @@
+#ifndef __LINUX_BCM963XX_TAG_H__
+#define __LINUX_BCM963XX_TAG_H__
+
+#include <linux/types.h>
+
+#define TAGVER_LEN		4	/* Length of Tag Version */
+#define TAGLAYOUT_LEN		4	/* Length of FlashLayoutVer */
+#define SIG1_LEN		20	/* Company Signature 1 Length */
+#define SIG2_LEN		14	/* Company Signature 2 Length */
+#define BOARDID_LEN		16	/* Length of BoardId */
+#define ENDIANFLAG_LEN		2	/* Endian Flag Length */
+#define CHIPID_LEN		6	/* Chip Id Length */
+#define IMAGE_LEN		10	/* Length of Length Field */
+#define ADDRESS_LEN		12	/* Length of Address field */
+#define DUALFLAG_LEN		2	/* Dual Image flag Length */
+#define INACTIVEFLAG_LEN	2	/* Inactie Flag Length */
+#define RSASIG_LEN		20	/* Length of RSA Signature in tag */
+#define TAGINFO1_LEN		30	/* Length of vendor information field1 in tag */
+#define FLASHLAYOUTVER_LEN	4	/* Length of Flash Layout Version String tag */
+#define TAGINFO2_LEN		16	/* Length of vendor information field2 in tag */
+#define ALTTAGINFO_LEN		54	/* Alternate length for vendor information; Pirelli */
+
+#define NUM_PIRELLI		2
+#define IMAGETAG_CRC_START	0xFFFFFFFF
+
+#define PIRELLI_BOARDS { \
+	"AGPF-S0", \
+	"DWV-S0", \
+}
+
+/*
+ * The broadcom firmware assumes the rootfs starts the image,
+ * therefore uses the rootfs start (flash_image_address)
+ * to determine where to flash the image.  Since we have the kernel first
+ * we have to give it the kernel address, but the crc uses the length
+ * associated with this address (root_length), which is added to the kernel
+ * length (kernel_length) to determine the length of image to flash and thus
+ * needs to be rootfs + deadcode (jffs2 EOF marker)
+*/
+
+struct bcm_tag {
+	/* 0-3: Version of the image tag */
+	char tag_version[TAGVER_LEN];
+	/* 4-23: Company Line 1 */
+	char sig_1[SIG1_LEN];
+	/*  24-37: Company Line 2 */
+	char sig_2[SIG2_LEN];
+	/* 38-43: Chip this image is for */
+	char chip_id[CHIPID_LEN];
+	/* 44-59: Board name */
+	char board_id[BOARDID_LEN];
+	/* 60-61: Map endianness -- 1 BE 0 LE */
+	char big_endian[ENDIANFLAG_LEN];
+	/* 62-71: Total length of image */
+	char total_length[IMAGE_LEN];
+	/* 72-83: Address in memory of CFE */
+	char cfe__address[ADDRESS_LEN];
+	/* 84-93: Size of CFE */
+	char cfe_length[IMAGE_LEN];
+	/* 94-105: Address in memory of image start
+	 * (kernel for OpenWRT, rootfs for stock firmware)
+	 */
+	char flash_image_start[ADDRESS_LEN];
+	/* 106-115: Size of rootfs */
+	char root_length[IMAGE_LEN];
+	/* 116-127: Address in memory of kernel */
+	char kernel_address[ADDRESS_LEN];
+	/* 128-137: Size of kernel */
+	char kernel_length[IMAGE_LEN];
+	/* 138-139: Unused at the moment */
+	char dual_image[DUALFLAG_LEN];
+	/* 140-141: Unused at the moment */
+	char inactive_flag[INACTIVEFLAG_LEN];
+	/* 142-161: RSA Signature (not used; some vendors may use this) */
+	char rsa_signature[RSASIG_LEN];
+	/* 162-191: Compilation and related information (not used in OpenWrt) */
+	char information1[TAGINFO1_LEN];
+	/* 192-195: Version flash layout */
+	char flash_layout_ver[FLASHLAYOUTVER_LEN];
+	/* 196-199: kernel+rootfs CRC32 */
+	__u32 fskernel_crc;
+	/* 200-215: Unused except on Alice Gate where is is information */
+	char information2[TAGINFO2_LEN];
+	/* 216-219: CRC32 of image less imagetag (kernel for Alice Gate) */
+	__u32 image_crc;
+	/* 220-223: CRC32 of rootfs partition */
+	__u32 rootfs_crc;
+	/* 224-227: CRC32 of kernel partition */
+	__u32 kernel_crc;
+	/* 228-235: Unused at present */
+	char reserved1[8];
+	/* 236-239: CRC32 of header excluding last 20 bytes */
+	__u32 header_crc;
+	/* 240-255: Unused at present */
+	char reserved2[16];
+};
+
+#endif /* __LINUX_BCM63XX_TAG_H__ */
-- 
2.1.4

-- 
Simon Arlott

[PATCH linux-next v4 04/11] MIPS: bcm963xx: Move extended flash address to bcm_tag header file

[Simon Arlott]

The extended flash address needs to be subtracted from bcm_tag flash
image offsets. Move this value to the bcm_tag header file.

Renamed define name to consistently use bcm963xx for flash layout
which should be considered a property of the board and not the SoC
(i.e. bcm63xx could theoretically be used on a board without CFE
or any flash).

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: New patch.

 drivers/mtd/bcm63xxpart.c    | 6 ++----
 include/linux/bcm963xx_tag.h | 5 +++++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index b5bad1f..cec3188 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -37,8 +37,6 @@
 #include <asm/mach-bcm63xx/bcm63xx_nvram.h>
 #include <asm/mach-bcm63xx/board_bcm963xx.h>
 
-#define BCM63XX_EXTENDED_SIZE	0xBFC00000	/* Extended flash address */
-
 #define BCM63XX_CFE_BLOCK_SIZE	SZ_64K		/* always at least 64KiB */
 
 #define BCM63XX_CFE_MAGIC_OFFSET 0x4e0
@@ -123,8 +121,8 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 		pr_info("CFE boot tag found with version %s and board type %s\n",
 			tagversion, boardid);
 
-		kerneladdr = kerneladdr - BCM63XX_EXTENDED_SIZE;
-		rootfsaddr = rootfsaddr - BCM63XX_EXTENDED_SIZE;
+		kerneladdr = kerneladdr - BCM963XX_EXTENDED_SIZE;
+		rootfsaddr = rootfsaddr - BCM963XX_EXTENDED_SIZE;
 		spareaddr = roundup(totallen, master->erasesize) + cfelen;
 
 		if (rootfsaddr < kerneladdr) {
diff --git a/include/linux/bcm963xx_tag.h b/include/linux/bcm963xx_tag.h
index f389dac..08e0133 100644
--- a/include/linux/bcm963xx_tag.h
+++ b/include/linux/bcm963xx_tag.h
@@ -28,6 +28,11 @@
 	"DWV-S0", \
 }
 
+/* Extended flash address, needs to be subtracted
+ * from bcm_tag flash image offsets.
+ */
+#define BCM963XX_EXTENDED_SIZE	0xBFC00000
+
 /*
  * The broadcom firmware assumes the rootfs starts the image,
  * therefore uses the rootfs start (flash_image_address)
-- 
2.1.4

-- 
Simon Arlott

[PATCH linux-next v4 05/11] MIPS: bcm963xx: Update bcm_tag field image_sequence

[Simon Arlott]

The "dual_image" and "inactive_flag" fields should be merged into a single
"image_sequence" field.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: New patch.

 include/linux/bcm963xx_tag.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/linux/bcm963xx_tag.h b/include/linux/bcm963xx_tag.h
index 08e0133..161c7b3 100644
--- a/include/linux/bcm963xx_tag.h
+++ b/include/linux/bcm963xx_tag.h
@@ -12,8 +12,7 @@
 #define CHIPID_LEN		6	/* Chip Id Length */
 #define IMAGE_LEN		10	/* Length of Length Field */
 #define ADDRESS_LEN		12	/* Length of Address field */
-#define DUALFLAG_LEN		2	/* Dual Image flag Length */
-#define INACTIVEFLAG_LEN	2	/* Inactie Flag Length */
+#define IMAGE_SEQUENCE_LEN	4	/* Image sequence Length */
 #define RSASIG_LEN		20	/* Length of RSA Signature in tag */
 #define TAGINFO1_LEN		30	/* Length of vendor information field1 in tag */
 #define FLASHLAYOUTVER_LEN	4	/* Length of Flash Layout Version String tag */
@@ -72,10 +71,10 @@ struct bcm_tag {
 	char kernel_address[ADDRESS_LEN];
 	/* 128-137: Size of kernel */
 	char kernel_length[IMAGE_LEN];
-	/* 138-139: Unused at the moment */
-	char dual_image[DUALFLAG_LEN];
-	/* 140-141: Unused at the moment */
-	char inactive_flag[INACTIVEFLAG_LEN];
+	/* 138-141: Image sequence number
+	 * (to be incremented when flashed with a new image)
+	 */
+	char image_sequence[IMAGE_SEQUENCE_LEN];
 	/* 142-161: RSA Signature (not used; some vendors may use this) */
 	char rsa_signature[RSASIG_LEN];
 	/* 162-191: Compilation and related information (not used in OpenWrt) */
-- 
2.1.4

-- 
Simon Arlott

[PATCH linux-next v4 06/11] mtd: bcm63xxpart: Remove dependency on mach-bcm63xx

[Simon Arlott]

Read nvram directly from flash instead of using the in-memory copy that
mach-bcm63xx has, to remove the dependency on mach-bcm63xx and allow the
parser to work on bmips too.

Rename remaining BCM63XX defines to BCM963XX as these are properties of
the flash layout on the board.

BCM963XX_DEFAULT_PSI_SIZE changes from SZ_64K to 64 because it will be
multiplied by SZ_1K later on.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: New patch.

 drivers/mtd/Kconfig       |  2 +-
 drivers/mtd/bcm63xxpart.c | 72 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 42cc953..e83a279 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -142,7 +142,7 @@ config MTD_AR7_PARTS
 
 config MTD_BCM63XX_PARTS
 	tristate "BCM63XX CFE partitioning support"
-	depends on BCM63XX
+	depends on BCM63XX || BMIPS_GENERIC || COMPILE_TEST
 	select CRC32
 	help
 	  This provides partions parsing for BCM63xx devices with CFE
diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index cec3188..1eea8b6 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -24,6 +24,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/bcm963xx_nvram.h>
 #include <linux/bcm963xx_tag.h>
 #include <linux/crc32.h>
 #include <linux/module.h>
@@ -34,12 +35,11 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 
-#include <asm/mach-bcm63xx/bcm63xx_nvram.h>
-#include <asm/mach-bcm63xx/board_bcm963xx.h>
+#define BCM963XX_CFE_BLOCK_SIZE		SZ_64K	/* always at least 64KiB */
 
-#define BCM63XX_CFE_BLOCK_SIZE	SZ_64K		/* always at least 64KiB */
-
-#define BCM63XX_CFE_MAGIC_OFFSET 0x4e0
+#define BCM963XX_CFE_MAGIC_OFFSET	0x4e0
+#define BCM963XX_CFE_VERSION_OFFSET	0x570
+#define BCM963XX_NVRAM_OFFSET		0x580
 
 static int bcm63xx_detect_cfe(struct mtd_info *master)
 {
@@ -58,20 +58,45 @@ static int bcm63xx_detect_cfe(struct mtd_info *master)
 		return 0;
 
 	/* very old CFE's do not have the cfe-v string, so check for magic */
-	ret = mtd_read(master, BCM63XX_CFE_MAGIC_OFFSET, 8, &retlen,
+	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)
+{
+	u32 actual_crc, expected_crc;
+	size_t retlen;
+	int ret;
+
+	/* extract nvram data */
+	ret = mtd_read(master, BCM963XX_NVRAM_OFFSET, BCM963XX_NVRAM_V5_SIZE,
+			&retlen, (void *)nvram);
+	if (ret)
+		return ret;
+
+	ret = bcm963xx_nvram_checksum(nvram, &expected_crc, &actual_crc);
+	if (ret)
+		pr_warn("nvram checksum failed, contents may be invalid (expected %08x, got %08x)\n",
+			expected_crc, actual_crc);
+
+	if (!nvram->psi_size)
+		nvram->psi_size = BCM963XX_DEFAULT_PSI_SIZE;
+
+	return 0;
+}
+
 static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 					const struct mtd_partition **pparts,
 					struct mtd_part_parser_data *data)
 {
 	/* CFE, NVRAM and global Linux are always present */
 	int nrparts = 3, curpart = 0;
-	struct bcm_tag *buf;
+	struct bcm963xx_nvram *nvram = NULL;
+	struct bcm_tag *buf = NULL;
 	struct mtd_partition *parts;
 	int ret;
 	size_t retlen;
@@ -86,25 +111,35 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 	if (bcm63xx_detect_cfe(master))
 		return -EINVAL;
 
+	nvram = vzalloc(sizeof(*nvram));
+	if (!nvram)
+		return -ENOMEM;
+
+	ret = bcm63xx_read_nvram(master, nvram);
+	if (ret)
+		goto out;
+
 	cfe_erasesize = max_t(uint32_t, master->erasesize,
-			      BCM63XX_CFE_BLOCK_SIZE);
+			      BCM963XX_CFE_BLOCK_SIZE);
 
 	cfelen = cfe_erasesize;
-	nvramlen = bcm63xx_nvram_get_psi_size() * SZ_1K;
+	nvramlen = nvram->psi_size * SZ_1K;
 	nvramlen = roundup(nvramlen, cfe_erasesize);
 
 	/* Allocate memory for buffer */
 	buf = vmalloc(sizeof(struct bcm_tag));
-	if (!buf)
-		return -ENOMEM;
+	if (!buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	/* Get the tag */
 	ret = mtd_read(master, cfelen, sizeof(struct bcm_tag), &retlen,
 		       (void *)buf);
 
 	if (retlen != sizeof(struct bcm_tag)) {
-		vfree(buf);
-		return -EIO;
+		ret = -EIO;
+		goto out;
 	}
 
 	computed_crc = crc32_le(IMAGETAG_CRC_START, (u8 *)buf,
@@ -154,8 +189,8 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 	/* Ask kernel for more memory */
 	parts = kzalloc(sizeof(*parts) * nrparts + 10 * nrparts, GFP_KERNEL);
 	if (!parts) {
-		vfree(buf);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
 	/* Start building partition list */
@@ -206,8 +241,15 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 		sparelen);
 
 	*pparts = parts;
+	ret = 0;
+
+out:
+	vfree(nvram);
 	vfree(buf);
 
+	if (ret)
+		return ret;
+
 	return nrparts;
 };
 
-- 
2.1.4

-- 
Simon Arlott

Re: [PATCH linux-next v4 06/11] mtd: bcm63xxpart: Remove dependency on mach-bcm63xx

[Brian Norris]

On Sun, Dec 13, 2015 at 10:49:26PM +0000, Simon Arlott wrote:
> Read nvram directly from flash instead of using the in-memory copy that
> mach-bcm63xx has, to remove the dependency on mach-bcm63xx and allow the
> parser to work on bmips too.
> 
> Rename remaining BCM63XX defines to BCM963XX as these are properties of
> the flash layout on the board.
> 
> BCM963XX_DEFAULT_PSI_SIZE changes from SZ_64K to 64 because it will be
> multiplied by SZ_1K later on.
> 
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> ---
> v4: New patch.

Pushed patches 6, 8, 9, and 10 to l2-mtd.git. I had comments on patch
11.

Thanks,
Brian

[PATCH linux-next v4 07/11] MIPS: bcm63xx: nvram: Remove unused bcm63xx_nvram_get_psi_size() function

[Simon Arlott]

Remove bcm63xx_nvram_get_psi_size() as it now has no users.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: New patch.

 arch/mips/bcm63xx/nvram.c                          | 11 -----------
 arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h |  2 --
 2 files changed, 13 deletions(-)

diff --git a/arch/mips/bcm63xx/nvram.c b/arch/mips/bcm63xx/nvram.c
index 05757ae..5f2bc1e 100644
--- a/arch/mips/bcm63xx/nvram.c
+++ b/arch/mips/bcm63xx/nvram.c
@@ -19,8 +19,6 @@
 
 #include <bcm63xx_nvram.h>
 
-#define BCM63XX_DEFAULT_PSI_SIZE	64
-
 static struct bcm963xx_nvram nvram;
 static int mac_addr_used;
 
@@ -87,12 +85,3 @@ 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 348df49..4e0b6bc 100644
--- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
+++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
@@ -30,6 +30,4 @@ 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 */
-- 
2.1.4

-- 
Simon Arlott

Re: [PATCH linux-next v4 07/11] MIPS: bcm63xx: nvram: Remove unused bcm63xx_nvram_get_psi_size() function

[Brian Norris]

On Sun, Dec 13, 2015 at 10:50:13PM +0000, Simon Arlott wrote:
> Remove bcm63xx_nvram_get_psi_size() as it now has no users.
> 
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> ---
> v4: New patch.

Ralf,

Please revert this and send it to Linus (or else, I can send it myself).
This is causing build failures, because I didn't take the rest of
Simon's series yet.

drivers/mtd/bcm63xxpart.c: In function 'bcm63xx_parse_cfe_partitions':
drivers/mtd/bcm63xxpart.c:93:2: error: implicit declaration of function 'bcm63xx_nvram_get_psi_size' [-Werror=implicit-function-declaration]

I will reply to the series if/when I accept any patches from it. I would
appreciate it if you would do the same.

Also...did this ever make it to linux-next? I'd think somebody's build
bots would have complained about the build failure.

Regards,
Brian

>  arch/mips/bcm63xx/nvram.c                          | 11 -----------
>  arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h |  2 --
>  2 files changed, 13 deletions(-)
> 
> diff --git a/arch/mips/bcm63xx/nvram.c b/arch/mips/bcm63xx/nvram.c
> index 05757ae..5f2bc1e 100644
> --- a/arch/mips/bcm63xx/nvram.c
> +++ b/arch/mips/bcm63xx/nvram.c
> @@ -19,8 +19,6 @@
>  
>  #include <bcm63xx_nvram.h>
>  
> -#define BCM63XX_DEFAULT_PSI_SIZE	64
> -
>  static struct bcm963xx_nvram nvram;
>  static int mac_addr_used;
>  
> @@ -87,12 +85,3 @@ 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 348df49..4e0b6bc 100644
> --- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
> +++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_nvram.h
> @@ -30,6 +30,4 @@ 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 */
> -- 
> 2.1.4
> 
> -- 
> Simon Arlott

Re: [PATCH linux-next v4 07/11] MIPS: bcm63xx: nvram: Remove unused bcm63xx_nvram_get_psi_size() function

[Brian Norris]

Hi Ralf,

On Tue, Jan 26, 2016 at 11:16:07AM -0800, Brian Norris wrote:
> On Sun, Dec 13, 2015 at 10:50:13PM +0000, Simon Arlott wrote:
> > Remove bcm63xx_nvram_get_psi_size() as it now has no users.
> > 
> > Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
> > ---
> > v4: New patch.
> 
> Ralf,
> 
> Please revert this and send it to Linus (or else, I can send it myself).
> This is causing build failures, because I didn't take the rest of
> Simon's series yet.
> 
> drivers/mtd/bcm63xxpart.c: In function 'bcm63xx_parse_cfe_partitions':
> drivers/mtd/bcm63xxpart.c:93:2: error: implicit declaration of function 'bcm63xx_nvram_get_psi_size' [-Werror=implicit-function-declaration]
> 
> I will reply to the series if/when I accept any patches from it. I would
> appreciate it if you would do the same.

I see that the revert made it to Linus. Thanks!

I've now pushed the dependent pieces to l2-mtd.git, and they should make
it to v4.6-rc1. So I guess it's fair game to re-apply this patch?
Although, if you don't merge in my MTD branch first, you'd still be
breaking bisectability... Maybe it's a post-rc1 candidate?

Brian

[PATCH linux-next v4 08/11] mtd: bcm63xxpart: Extract read of image tag to separate function

[Simon Arlott]

Extract image tag reading and CRC check to a separate function.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: New patch.

 drivers/mtd/bcm63xxpart.c | 62 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index 1eea8b6..eafbf52 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -41,6 +41,10 @@
 #define BCM963XX_CFE_VERSION_OFFSET	0x570
 #define BCM963XX_NVRAM_OFFSET		0x580
 
+/* Ensure strings read from flash structs are null terminated */
+#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];
@@ -89,6 +93,37 @@ static int bcm63xx_read_nvram(struct mtd_info *master,
 	return 0;
 }
 
+static int bcm63xx_read_image_tag(struct mtd_info *master, const char *name,
+	loff_t tag_offset, struct bcm_tag *buf)
+{
+	int ret;
+	size_t retlen;
+	u32 computed_crc;
+
+	ret = mtd_read(master, tag_offset, sizeof(*buf), &retlen, (void *)buf);
+	if (ret)
+		return ret;
+
+	if (retlen != sizeof(*buf))
+		return -EIO;
+
+	computed_crc = crc32_le(IMAGETAG_CRC_START, (u8 *)buf,
+				offsetof(struct bcm_tag, header_crc));
+	if (computed_crc == buf->header_crc) {
+		STR_NULL_TERMINATE(buf->board_id);
+		STR_NULL_TERMINATE(buf->tag_version);
+
+		pr_info("%s: CFE image tag found at 0x%llx with version %s, board type %s\n",
+			name, tag_offset, buf->tag_version, buf->board_id);
+
+		return 0;
+	}
+
+	pr_warn("%s: CFE image tag at 0x%llx CRC invalid (expected %08x, actual %08x)\n",
+		name, tag_offset, buf->header_crc, computed_crc);
+	return 1;
+}
+
 static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 					const struct mtd_partition **pparts,
 					struct mtd_part_parser_data *data)
@@ -99,13 +134,11 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 	struct bcm_tag *buf = NULL;
 	struct mtd_partition *parts;
 	int ret;
-	size_t retlen;
 	unsigned int rootfsaddr, kerneladdr, spareaddr;
 	unsigned int rootfslen, kernellen, sparelen, totallen;
 	unsigned int cfelen, nvramlen;
 	unsigned int cfe_erasesize;
 	int i;
-	u32 computed_crc;
 	bool rootfs_first = false;
 
 	if (bcm63xx_detect_cfe(master))
@@ -134,28 +167,13 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 	}
 
 	/* Get the tag */
-	ret = mtd_read(master, cfelen, sizeof(struct bcm_tag), &retlen,
-		       (void *)buf);
-
-	if (retlen != sizeof(struct bcm_tag)) {
-		ret = -EIO;
-		goto out;
-	}
-
-	computed_crc = crc32_le(IMAGETAG_CRC_START, (u8 *)buf,
-				offsetof(struct bcm_tag, header_crc));
-	if (computed_crc == buf->header_crc) {
-		char *boardid = &(buf->board_id[0]);
-		char *tagversion = &(buf->tag_version[0]);
-
+	ret = bcm63xx_read_image_tag(master, "rootfs", cfelen, buf);
+	if (!ret) {
 		sscanf(buf->flash_image_start, "%u", &rootfsaddr);
 		sscanf(buf->kernel_address, "%u", &kerneladdr);
 		sscanf(buf->kernel_length, "%u", &kernellen);
 		sscanf(buf->total_length, "%u", &totallen);
 
-		pr_info("CFE boot tag found with version %s and board type %s\n",
-			tagversion, boardid);
-
 		kerneladdr = kerneladdr - BCM963XX_EXTENDED_SIZE;
 		rootfsaddr = rootfsaddr - BCM963XX_EXTENDED_SIZE;
 		spareaddr = roundup(totallen, master->erasesize) + cfelen;
@@ -169,13 +187,13 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 			rootfsaddr = kerneladdr + kernellen;
 			rootfslen = spareaddr - rootfsaddr;
 		}
-	} else {
-		pr_warn("CFE boot tag CRC invalid (expected %08x, actual %08x)\n",
-			buf->header_crc, computed_crc);
+	} else if (ret > 0) {
 		kernellen = 0;
 		rootfslen = 0;
 		rootfsaddr = 0;
 		spareaddr = cfelen;
+	} else {
+		goto out;
 	}
 	sparelen = master->size - spareaddr - nvramlen;
 
-- 
2.1.4

-- 
Simon Arlott

[PATCH linux-next v4 09/11] mtd: bcm63xxpart: Null terminate and validate conversion of flash strings

[Simon Arlott]

Strings read from flash could be missing null termination characters, or
not contain valid integers.

Null terminate the strings and check for errors when converting them to
integers.

Also validate that the addresses are at least BCM963XX_EXTENDED_SIZE
because this will be subtracted from them.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: New patch.

 drivers/mtd/bcm63xxpart.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index eafbf52..41aa202 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -169,10 +169,39 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 	/* Get the tag */
 	ret = bcm63xx_read_image_tag(master, "rootfs", cfelen, buf);
 	if (!ret) {
-		sscanf(buf->flash_image_start, "%u", &rootfsaddr);
-		sscanf(buf->kernel_address, "%u", &kerneladdr);
-		sscanf(buf->kernel_length, "%u", &kernellen);
-		sscanf(buf->total_length, "%u", &totallen);
+		STR_NULL_TERMINATE(buf->flash_image_start);
+		if (kstrtouint(buf->flash_image_start, 10, &rootfsaddr) ||
+				rootfsaddr < BCM963XX_EXTENDED_SIZE) {
+			pr_err("invalid rootfs address: %*ph\n",
+				sizeof(buf->flash_image_start),
+				buf->flash_image_start);
+			goto invalid_tag;
+		}
+
+		STR_NULL_TERMINATE(buf->kernel_address);
+		if (kstrtouint(buf->kernel_address, 10, &kerneladdr) ||
+				kerneladdr < BCM963XX_EXTENDED_SIZE) {
+			pr_err("invalid kernel address: %*ph\n",
+				sizeof(buf->kernel_address),
+				buf->kernel_address);
+			goto invalid_tag;
+		}
+
+		STR_NULL_TERMINATE(buf->kernel_length);
+		if (kstrtouint(buf->kernel_length, 10, &kernellen)) {
+			pr_err("invalid kernel length: %*ph\n",
+				sizeof(buf->kernel_length),
+				buf->kernel_length);
+			goto invalid_tag;
+		}
+
+		STR_NULL_TERMINATE(buf->total_length);
+		if (kstrtouint(buf->total_length, 10, &totallen)) {
+			pr_err("invalid total length: %*ph\n",
+				sizeof(buf->total_length),
+				buf->total_length);
+			goto invalid_tag;
+		}
 
 		kerneladdr = kerneladdr - BCM963XX_EXTENDED_SIZE;
 		rootfsaddr = rootfsaddr - BCM963XX_EXTENDED_SIZE;
@@ -188,6 +217,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 			rootfslen = spareaddr - rootfsaddr;
 		}
 	} else if (ret > 0) {
+invalid_tag:
 		kernellen = 0;
 		rootfslen = 0;
 		rootfsaddr = 0;
-- 
2.1.4

-- 
Simon Arlott

[PATCH linux-next v4 10/11] mtd: bcm63xxpart: Move NOR flash layout to a separate function

[Simon Arlott]

Move the NOR flash layout to a separate function to allow the NAND flash
layout to be supported.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: New patch.

 drivers/mtd/bcm63xxpart.c | 54 ++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index 41aa202..26c38a1 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -124,13 +124,11 @@ static int bcm63xx_read_image_tag(struct mtd_info *master, const char *name,
 	return 1;
 }
 
-static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
-					const struct mtd_partition **pparts,
-					struct mtd_part_parser_data *data)
+static int bcm63xx_parse_cfe_nor_partitions(struct mtd_info *master,
+	const struct mtd_partition **pparts, struct bcm963xx_nvram *nvram)
 {
 	/* CFE, NVRAM and global Linux are always present */
 	int nrparts = 3, curpart = 0;
-	struct bcm963xx_nvram *nvram = NULL;
 	struct bcm_tag *buf = NULL;
 	struct mtd_partition *parts;
 	int ret;
@@ -141,17 +139,6 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 	int i;
 	bool rootfs_first = false;
 
-	if (bcm63xx_detect_cfe(master))
-		return -EINVAL;
-
-	nvram = vzalloc(sizeof(*nvram));
-	if (!nvram)
-		return -ENOMEM;
-
-	ret = bcm63xx_read_nvram(master, nvram);
-	if (ret)
-		goto out;
-
 	cfe_erasesize = max_t(uint32_t, master->erasesize,
 			      BCM963XX_CFE_BLOCK_SIZE);
 
@@ -159,12 +146,9 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 	nvramlen = nvram->psi_size * SZ_1K;
 	nvramlen = roundup(nvramlen, cfe_erasesize);
 
-	/* Allocate memory for buffer */
 	buf = vmalloc(sizeof(struct bcm_tag));
-	if (!buf) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!buf)
+		return -ENOMEM;
 
 	/* Get the tag */
 	ret = bcm63xx_read_image_tag(master, "rootfs", cfelen, buf);
@@ -234,7 +218,6 @@ invalid_tag:
 	if (kernellen > 0)
 		nrparts++;
 
-	/* Ask kernel for more memory */
 	parts = kzalloc(sizeof(*parts) * nrparts + 10 * nrparts, GFP_KERNEL);
 	if (!parts) {
 		ret = -ENOMEM;
@@ -292,13 +275,40 @@ invalid_tag:
 	ret = 0;
 
 out:
-	vfree(nvram);
 	vfree(buf);
 
 	if (ret)
 		return ret;
 
 	return nrparts;
+}
+
+static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
+					const struct mtd_partition **pparts,
+					struct mtd_part_parser_data *data)
+{
+	struct bcm963xx_nvram *nvram = NULL;
+	int ret;
+
+	if (bcm63xx_detect_cfe(master))
+		return -EINVAL;
+
+	nvram = vzalloc(sizeof(*nvram));
+	if (!nvram)
+		return -ENOMEM;
+
+	ret = bcm63xx_read_nvram(master, nvram);
+	if (ret)
+		goto out;
+
+	if (!mtd_type_is_nand(master))
+		ret = bcm63xx_parse_cfe_nor_partitions(master, pparts, nvram);
+	else
+		ret = -EINVAL;
+
+out:
+	vfree(nvram);
+	return ret;
 };
 
 static struct mtd_part_parser bcm63xx_cfe_parser = {
-- 
2.1.4

-- 
Simon Arlott

[PATCH linux-next v4 11/11] mtd: bcm63xxpart: Add NAND partitioning support

[Simon Arlott]

Add partitioning support for BCM963xx boards with NAND flash.

The following partitions are defined:
  "boot":           CFE and nvram data
  "rootfs":         Currently selected rootfs
  "data":           Configuration data
  "rootfs1_update": Container for the whole flash area used
                    for the first rootfs to allow it to be
                    updated
  "rootfs2_update": Container for the whole flash area used
                    for the second rootfs to allow it to be
                    updated
  "rootfs_other":   The other (not currently selected) rootfs

Example:
[    2.157094] nand: device found, Manufacturer ID: 0xc2, Chip ID: 0xf1
[    2.163796] nand: Macronix NAND 128MiB 3,3V 8-bit
[    2.168648] nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
[    2.176588] bcm6368_nand 10000200.nand: detected 128MiB total, 128KiB blocks, 2KiB pages, 16B OOB, 8-bit, Hamming ECC
[    2.189782] Bad block table found at page 65472, version 0x01
[    2.196910] Bad block table found at page 65408, version 0x01
[    2.204003] nand_read_bbt: bad block at 0x000007480000
[    2.225220] bcm63xxpart: rootfs1: CFE image tag found at 0x20000 with version 6, board type 963168VX
[    2.236188] bcm63xxpart: rootfs2: CFE image tag found at 0x4000000 with version 6, board type 963168VX
[    2.246165] bcm63xxpart: CFE bootline selected latest image rootfs1 (rootfs1_seq=2, rootfs2_seq=1)
[    2.255800] 6 bcm63xxpart partitions found on MTD device brcmnand.0
[    2.275360] Creating 6 MTD partitions on "brcmnand.0":
[    2.280804] 0x000000000000-0x000000020000 : "boot"
[    2.294609] 0x000000040000-0x000001120000 : "rootfs"
[    2.310078] 0x000007b00000-0x000007f00000 : "data"
[    2.324052] 0x000000020000-0x000003ac0000 : "rootfs1_update"
[    2.339190] 0x000004000000-0x000007ac0000 : "rootfs2_update"
[    2.354290] 0x000004020000-0x000005060000 : "rootfs_other"

The nvram contains the offset and size of the boot, rootfs1, rootfs2
and data partitions. The presence of CFE and nvram is already verified
by reading from the boot partition which is assumed to be at offset 0
and the NAND process aborts if the nvram read indicates that this is
not the case.

There is bcm_tag information at the start of each rootfs that is used
to determine which rootfs is newer and what its real offset/size is.

The CFE bootline is used to select a rootfs.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
v4: Reorganised functions based on earlier new patches in the series,
    no real logic changes other than having to check for nvram->version
    >= 6 within the nand function instead of the nvram read function.

    Renamed "curpart" to "i" because it allows the partition layout
    lines to be under 80 characters.

v3: Use COMPILE_TEST.

    Ensure that strings read from flash are null terminated and validate
    bcm_tag integer values (this also moves reporting of rootfs sequence
    numbers to later on).

v2: Use external struct bcm963xx_nvram definition for bcm963268part.

    Removed support for the nand partition number field, it's not a
    standard Broadcom field (was added by MitraStar Technology Corp.).

 drivers/mtd/bcm63xxpart.c | 196 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 190 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
index 26c38a1..4576b78 100644
--- a/drivers/mtd/bcm63xxpart.c
+++ b/drivers/mtd/bcm63xxpart.c
@@ -16,10 +16,9 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
+ * NAND flash layout derived from bcm963xx_4.12L.06B_consumer/bcmdrivers/opensource/char/board/bcm963xx/impl1/board.c,
+ *	bcm963xx_4.12L.06B_consumer/shared/opensource/include/bcm963xx/bcm_hwdefs.h:
+ * Copyright (c) 2002 Broadcom Corporation
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -124,6 +123,25 @@ static int bcm63xx_read_image_tag(struct mtd_info *master, const char *name,
 	return 1;
 }
 
+static bool bcm63xx_boot_latest(struct bcm963xx_nvram *nvram)
+{
+	char *p;
+
+	STR_NULL_TERMINATE(nvram->bootline);
+
+	/* Find previous image parameter "p" */
+	if (!strncmp(nvram->bootline, "p=", 2))
+		p = nvram->bootline;
+	else
+		p = strstr(nvram->bootline, " p=");
+
+	if (p == NULL)
+		return true;
+
+	p += 2;
+	return *p != '0';
+}
+
 static int bcm63xx_parse_cfe_nor_partitions(struct mtd_info *master,
 	const struct mtd_partition **pparts, struct bcm963xx_nvram *nvram)
 {
@@ -283,6 +301,171 @@ out:
 	return nrparts;
 }
 
+static bool bcm63xx_parse_nand_image_tag(struct mtd_info *master,
+	const char *name, loff_t tag_offset, u64 *rootfs_offset,
+	u64 *rootfs_size, unsigned int *rootfs_sequence)
+{
+	struct bcm_tag *buf;
+	int ret;
+	bool rootfs_ok = false;
+
+	*rootfs_offset = 0;
+	*rootfs_size = 0;
+	*rootfs_sequence = 0;
+
+	buf = vmalloc(sizeof(struct bcm_tag));
+	if (!buf)
+		goto out;
+
+	ret = bcm63xx_read_image_tag(master, name, tag_offset, buf);
+	if (!ret) {
+		/* Get rootfs offset and size from tag data */
+		STR_NULL_TERMINATE(buf->flash_image_start);
+		if (kstrtou64(buf->flash_image_start, 10, rootfs_offset) ||
+				*rootfs_offset < BCM963XX_EXTENDED_SIZE) {
+			pr_err("%s: invalid rootfs offset: %*ph\n", name,
+				sizeof(buf->flash_image_start),
+				buf->flash_image_start);
+			goto out;
+		}
+
+		STR_NULL_TERMINATE(buf->root_length);
+		if (kstrtou64(buf->root_length, 10, rootfs_size) ||
+				rootfs_size == 0) {
+			pr_err("%s: invalid rootfs size: %*ph\n", name,
+				sizeof(buf->root_length), buf->root_length);
+			goto out;
+		}
+
+		/* Adjust for flash offset */
+		*rootfs_offset -= BCM963XX_EXTENDED_SIZE;
+
+		/* Remove bcm_tag data from length */
+		*rootfs_size -= *rootfs_offset - tag_offset;
+
+		/* Get image sequence number to determine which one is newer */
+		STR_NULL_TERMINATE(buf->image_sequence);
+		if (kstrtouint(buf->image_sequence, 10, rootfs_sequence)) {
+			pr_err("%s: invalid rootfs sequence: %*ph\n", name,
+				sizeof(buf->image_sequence),
+				buf->image_sequence);
+			goto out;
+		}
+
+		rootfs_ok = true;
+	}
+
+out:
+	vfree(buf);
+	return rootfs_ok;
+}
+
+static int bcm63xx_parse_cfe_nand_partitions(struct mtd_info *master,
+	const struct mtd_partition **pparts,
+	struct bcm963xx_nvram *nvram)
+{
+	int nrparts, i;
+	struct mtd_partition *parts;
+	u64 rootfs1_off, rootfs1_size;
+	unsigned int rootfs1_seq;
+	u64 rootfs2_off, rootfs2_size;
+	unsigned int rootfs2_seq;
+	bool rootfs1, rootfs2;
+	bool use_first;
+
+	if (nvram->version < 6) {
+		pr_warn("nvram version %u not supported\n", nvram->version);
+		return -EINVAL;
+	}
+
+	/* We've just read the nvram from offset 0,
+	 * so it must be located there.
+	 */
+	if (BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, BOOT) != 0)
+		return -EINVAL;
+
+	/* Get the rootfs partition locations */
+	rootfs1 = bcm63xx_parse_nand_image_tag(master, "rootfs1",
+		BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, ROOTFS_1),
+		&rootfs1_off, &rootfs1_size, &rootfs1_seq);
+	rootfs2 = bcm63xx_parse_nand_image_tag(master, "rootfs2",
+		BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, ROOTFS_2),
+		&rootfs2_off, &rootfs2_size, &rootfs2_seq);
+
+	/* Determine primary rootfs partition */
+	if (rootfs1 && rootfs2) {
+		bool use_latest = bcm63xx_boot_latest(nvram);
+
+		/* Compare sequence numbers */
+		if (use_latest)
+			use_first = rootfs1_seq > rootfs2_seq;
+		else
+			use_first = rootfs1_seq < rootfs2_seq;
+
+		pr_info("CFE bootline selected %s image rootfs%u (rootfs1_seq=%u, rootfs2_seq=%u)\n",
+			use_latest ? "latest" : "previous",
+			use_first ? 1 : 2,
+			rootfs1_seq, rootfs2_seq);
+	} else {
+		use_first = rootfs1;
+	}
+
+	/* Partitions:
+	 * 1 boot
+	 * 2 rootfs
+	 * 3 data
+	 * 4 rootfs1_update
+	 * 5 rootfs2_update
+	 * 6 rootfs_other
+	 */
+	nrparts = 6;
+	i = 0;
+
+	parts = kcalloc(nrparts, sizeof(*parts), GFP_KERNEL);
+	if (!parts)
+		return -ENOMEM;
+
+	parts[i].name = "boot";
+	parts[i].offset = BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, BOOT);
+	parts[i].size = BCM963XX_NVRAM_NAND_PART_SIZE(nvram, BOOT);
+	i++;
+
+	/* Primary rootfs if either is available */
+	if (rootfs1 || rootfs2) {
+		parts[i].name = "rootfs";
+		parts[i].offset = use_first ? rootfs1_off : rootfs2_off;
+		parts[i].size = use_first ? rootfs1_size : rootfs2_size;
+		i++;
+	}
+
+	parts[i].name = "data";
+	parts[i].offset = BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, DATA);
+	parts[i].size = BCM963XX_NVRAM_NAND_PART_SIZE(nvram, DATA);
+	i++;
+
+	/* Full rootfs partitions for updates */
+	parts[i].name = "rootfs1_update";
+	parts[i].offset = BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, ROOTFS_1);
+	parts[i].size = BCM963XX_NVRAM_NAND_PART_SIZE(nvram, ROOTFS_1);
+	i++;
+
+	parts[i].name = "rootfs2_update";
+	parts[i].offset = BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, ROOTFS_2);
+	parts[i].size = BCM963XX_NVRAM_NAND_PART_SIZE(nvram, ROOTFS_2);
+	i++;
+
+	/* Other rootfs if both are available */
+	if (rootfs1 && rootfs2) {
+		parts[i].name = "rootfs_other";
+		parts[i].offset = use_first ? rootfs2_off : rootfs1_off;
+		parts[i].size = use_first ? rootfs2_size : rootfs1_size;
+		i++;
+	}
+
+	*pparts = parts;
+	return nrparts;
+}
+
 static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 					const struct mtd_partition **pparts,
 					struct mtd_part_parser_data *data)
@@ -304,7 +487,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
 	if (!mtd_type_is_nand(master))
 		ret = bcm63xx_parse_cfe_nor_partitions(master, pparts, nvram);
 	else
-		ret = -EINVAL;
+		ret = bcm63xx_parse_cfe_nand_partitions(master, pparts, nvram);
 
 out:
 	vfree(nvram);
@@ -321,5 +504,6 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Daniel Dickinson <openwrt@cshore.neomailbox.net>");
 MODULE_AUTHOR("Florian Fainelli <florian@openwrt.org>");
 MODULE_AUTHOR("Mike Albon <malbon@openwrt.org>");
-MODULE_AUTHOR("Jonas Gorski <jonas.gorski@gmail.com");
+MODULE_AUTHOR("Jonas Gorski <jonas.gorski@gmail.com>");
+MODULE_AUTHOR("Simon Arlott");
 MODULE_DESCRIPTION("MTD partitioning for BCM63XX CFE bootloaders");
-- 
2.1.4

-- 
Simon Arlott

Re: [PATCH linux-next v4 11/11] mtd: bcm63xxpart: Add NAND partitioning support

[Brian Norris]

Hi,

On Sun, Dec 13, 2015 at 10:53:13PM +0000, Simon Arlott wrote:
> Add partitioning support for BCM963xx boards with NAND flash.
> 
> The following partitions are defined:
>   "boot":           CFE and nvram data
>   "rootfs":         Currently selected rootfs
>   "data":           Configuration data
>   "rootfs1_update": Container for the whole flash area used
>                     for the first rootfs to allow it to be
>                     updated
>   "rootfs2_update": Container for the whole flash area used
>                     for the second rootfs to allow it to be
>                     updated
>   "rootfs_other":   The other (not currently selected) rootfs
> 
> Example:
> [    2.157094] nand: device found, Manufacturer ID: 0xc2, Chip ID: 0xf1
> [    2.163796] nand: Macronix NAND 128MiB 3,3V 8-bit
> [    2.168648] nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
> [    2.176588] bcm6368_nand 10000200.nand: detected 128MiB total, 128KiB blocks, 2KiB pages, 16B OOB, 8-bit, Hamming ECC
> [    2.189782] Bad block table found at page 65472, version 0x01
> [    2.196910] Bad block table found at page 65408, version 0x01
> [    2.204003] nand_read_bbt: bad block at 0x000007480000
> [    2.225220] bcm63xxpart: rootfs1: CFE image tag found at 0x20000 with version 6, board type 963168VX
> [    2.236188] bcm63xxpart: rootfs2: CFE image tag found at 0x4000000 with version 6, board type 963168VX
> [    2.246165] bcm63xxpart: CFE bootline selected latest image rootfs1 (rootfs1_seq=2, rootfs2_seq=1)
> [    2.255800] 6 bcm63xxpart partitions found on MTD device brcmnand.0
> [    2.275360] Creating 6 MTD partitions on "brcmnand.0":
> [    2.280804] 0x000000000000-0x000000020000 : "boot"
> [    2.294609] 0x000000040000-0x000001120000 : "rootfs"
> [    2.310078] 0x000007b00000-0x000007f00000 : "data"
> [    2.324052] 0x000000020000-0x000003ac0000 : "rootfs1_update"
> [    2.339190] 0x000004000000-0x000007ac0000 : "rootfs2_update"
> [    2.354290] 0x000004020000-0x000005060000 : "rootfs_other"
> 
> The nvram contains the offset and size of the boot, rootfs1, rootfs2
> and data partitions. The presence of CFE and nvram is already verified
> by reading from the boot partition which is assumed to be at offset 0
> and the NAND process aborts if the nvram read indicates that this is
> not the case.
> 
> There is bcm_tag information at the start of each rootfs that is used
> to determine which rootfs is newer and what its real offset/size is.
> 
> The CFE bootline is used to select a rootfs.
> 
> Signed-off-by: Simon Arlott <simon@fire.lp0.eu>

Have we gotten any testing besides Simon? Or any Acked-by/Reviewed-by?

> ---
> v4: Reorganised functions based on earlier new patches in the series,
>     no real logic changes other than having to check for nvram->version
>     >= 6 within the nand function instead of the nvram read function.
> 
>     Renamed "curpart" to "i" because it allows the partition layout
>     lines to be under 80 characters.
> 
> v3: Use COMPILE_TEST.
> 
>     Ensure that strings read from flash are null terminated and validate
>     bcm_tag integer values (this also moves reporting of rootfs sequence
>     numbers to later on).
> 
> v2: Use external struct bcm963xx_nvram definition for bcm963268part.
> 
>     Removed support for the nand partition number field, it's not a
>     standard Broadcom field (was added by MitraStar Technology Corp.).
> 
>  drivers/mtd/bcm63xxpart.c | 196 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 190 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c
> index 26c38a1..4576b78 100644
> --- a/drivers/mtd/bcm63xxpart.c
> +++ b/drivers/mtd/bcm63xxpart.c
> @@ -16,10 +16,9 @@
>   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>   * GNU General Public License for more details.
>   *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> - *
> + * NAND flash layout derived from bcm963xx_4.12L.06B_consumer/bcmdrivers/opensource/char/board/bcm963xx/impl1/board.c,
> + *	bcm963xx_4.12L.06B_consumer/shared/opensource/include/bcm963xx/bcm_hwdefs.h:
> + * Copyright (c) 2002 Broadcom Corporation
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> @@ -124,6 +123,25 @@ static int bcm63xx_read_image_tag(struct mtd_info *master, const char *name,
>  	return 1;
>  }
>  
> +static bool bcm63xx_boot_latest(struct bcm963xx_nvram *nvram)
> +{
> +	char *p;
> +
> +	STR_NULL_TERMINATE(nvram->bootline);
> +
> +	/* Find previous image parameter "p" */
> +	if (!strncmp(nvram->bootline, "p=", 2))
> +		p = nvram->bootline;
> +	else
> +		p = strstr(nvram->bootline, " p=");
> +
> +	if (p == NULL)
> +		return true;
> +
> +	p += 2;
> +	return *p != '0';
> +}
> +
>  static int bcm63xx_parse_cfe_nor_partitions(struct mtd_info *master,
>  	const struct mtd_partition **pparts, struct bcm963xx_nvram *nvram)
>  {
> @@ -283,6 +301,171 @@ out:
>  	return nrparts;
>  }
>  
> +static bool bcm63xx_parse_nand_image_tag(struct mtd_info *master,
> +	const char *name, loff_t tag_offset, u64 *rootfs_offset,
> +	u64 *rootfs_size, unsigned int *rootfs_sequence)
> +{
> +	struct bcm_tag *buf;
> +	int ret;
> +	bool rootfs_ok = false;
> +
> +	*rootfs_offset = 0;
> +	*rootfs_size = 0;
> +	*rootfs_sequence = 0;
> +
> +	buf = vmalloc(sizeof(struct bcm_tag));
> +	if (!buf)
> +		goto out;
> +
> +	ret = bcm63xx_read_image_tag(master, name, tag_offset, buf);
> +	if (!ret) {
> +		/* Get rootfs offset and size from tag data */
> +		STR_NULL_TERMINATE(buf->flash_image_start);
> +		if (kstrtou64(buf->flash_image_start, 10, rootfs_offset) ||
> +				*rootfs_offset < BCM963XX_EXTENDED_SIZE) {
> +			pr_err("%s: invalid rootfs offset: %*ph\n", name,
> +				sizeof(buf->flash_image_start),
> +				buf->flash_image_start);
> +			goto out;
> +		}
> +
> +		STR_NULL_TERMINATE(buf->root_length);
> +		if (kstrtou64(buf->root_length, 10, rootfs_size) ||
> +				rootfs_size == 0) {

Sparse and Smatch have caught errors on this line:

  drivers/mtd/bcm63xxpart.c:334:48: warning: Using plain integer as NULL pointer [sparse]
  drivers/mtd/bcm63xxpart.c:334 bcm63xx_parse_nand_image_tag() warn: variable dereferenced before check 'rootfs_size' (see line 313) [smatch]


I think you mean '*rootfs_size == 0', not 'rootfs_size == 0'.

Brian

> +			pr_err("%s: invalid rootfs size: %*ph\n", name,
> +				sizeof(buf->root_length), buf->root_length);
> +			goto out;
> +		}
> +
> +		/* Adjust for flash offset */
> +		*rootfs_offset -= BCM963XX_EXTENDED_SIZE;
> +
> +		/* Remove bcm_tag data from length */
> +		*rootfs_size -= *rootfs_offset - tag_offset;
> +
> +		/* Get image sequence number to determine which one is newer */
> +		STR_NULL_TERMINATE(buf->image_sequence);
> +		if (kstrtouint(buf->image_sequence, 10, rootfs_sequence)) {
> +			pr_err("%s: invalid rootfs sequence: %*ph\n", name,
> +				sizeof(buf->image_sequence),
> +				buf->image_sequence);
> +			goto out;
> +		}
> +
> +		rootfs_ok = true;
> +	}
> +
> +out:
> +	vfree(buf);
> +	return rootfs_ok;
> +}
> +
> +static int bcm63xx_parse_cfe_nand_partitions(struct mtd_info *master,
> +	const struct mtd_partition **pparts,
> +	struct bcm963xx_nvram *nvram)
> +{
> +	int nrparts, i;
> +	struct mtd_partition *parts;
> +	u64 rootfs1_off, rootfs1_size;
> +	unsigned int rootfs1_seq;
> +	u64 rootfs2_off, rootfs2_size;
> +	unsigned int rootfs2_seq;
> +	bool rootfs1, rootfs2;
> +	bool use_first;
> +
> +	if (nvram->version < 6) {
> +		pr_warn("nvram version %u not supported\n", nvram->version);
> +		return -EINVAL;
> +	}
> +
> +	/* We've just read the nvram from offset 0,
> +	 * so it must be located there.
> +	 */
> +	if (BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, BOOT) != 0)
> +		return -EINVAL;
> +
> +	/* Get the rootfs partition locations */
> +	rootfs1 = bcm63xx_parse_nand_image_tag(master, "rootfs1",
> +		BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, ROOTFS_1),
> +		&rootfs1_off, &rootfs1_size, &rootfs1_seq);
> +	rootfs2 = bcm63xx_parse_nand_image_tag(master, "rootfs2",
> +		BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, ROOTFS_2),
> +		&rootfs2_off, &rootfs2_size, &rootfs2_seq);
> +
> +	/* Determine primary rootfs partition */
> +	if (rootfs1 && rootfs2) {
> +		bool use_latest = bcm63xx_boot_latest(nvram);
> +
> +		/* Compare sequence numbers */
> +		if (use_latest)
> +			use_first = rootfs1_seq > rootfs2_seq;
> +		else
> +			use_first = rootfs1_seq < rootfs2_seq;
> +
> +		pr_info("CFE bootline selected %s image rootfs%u (rootfs1_seq=%u, rootfs2_seq=%u)\n",
> +			use_latest ? "latest" : "previous",
> +			use_first ? 1 : 2,
> +			rootfs1_seq, rootfs2_seq);
> +	} else {
> +		use_first = rootfs1;
> +	}
> +
> +	/* Partitions:
> +	 * 1 boot
> +	 * 2 rootfs
> +	 * 3 data
> +	 * 4 rootfs1_update
> +	 * 5 rootfs2_update
> +	 * 6 rootfs_other
> +	 */
> +	nrparts = 6;
> +	i = 0;
> +
> +	parts = kcalloc(nrparts, sizeof(*parts), GFP_KERNEL);
> +	if (!parts)
> +		return -ENOMEM;
> +
> +	parts[i].name = "boot";
> +	parts[i].offset = BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, BOOT);
> +	parts[i].size = BCM963XX_NVRAM_NAND_PART_SIZE(nvram, BOOT);
> +	i++;
> +
> +	/* Primary rootfs if either is available */
> +	if (rootfs1 || rootfs2) {
> +		parts[i].name = "rootfs";
> +		parts[i].offset = use_first ? rootfs1_off : rootfs2_off;
> +		parts[i].size = use_first ? rootfs1_size : rootfs2_size;
> +		i++;
> +	}
> +
> +	parts[i].name = "data";
> +	parts[i].offset = BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, DATA);
> +	parts[i].size = BCM963XX_NVRAM_NAND_PART_SIZE(nvram, DATA);
> +	i++;
> +
> +	/* Full rootfs partitions for updates */
> +	parts[i].name = "rootfs1_update";
> +	parts[i].offset = BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, ROOTFS_1);
> +	parts[i].size = BCM963XX_NVRAM_NAND_PART_SIZE(nvram, ROOTFS_1);
> +	i++;
> +
> +	parts[i].name = "rootfs2_update";
> +	parts[i].offset = BCM963XX_NVRAM_NAND_PART_OFFSET(nvram, ROOTFS_2);
> +	parts[i].size = BCM963XX_NVRAM_NAND_PART_SIZE(nvram, ROOTFS_2);
> +	i++;
> +
> +	/* Other rootfs if both are available */
> +	if (rootfs1 && rootfs2) {
> +		parts[i].name = "rootfs_other";
> +		parts[i].offset = use_first ? rootfs2_off : rootfs1_off;
> +		parts[i].size = use_first ? rootfs2_size : rootfs1_size;
> +		i++;
> +	}
> +
> +	*pparts = parts;
> +	return nrparts;
> +}
> +
>  static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
>  					const struct mtd_partition **pparts,
>  					struct mtd_part_parser_data *data)
> @@ -304,7 +487,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master,
>  	if (!mtd_type_is_nand(master))
>  		ret = bcm63xx_parse_cfe_nor_partitions(master, pparts, nvram);
>  	else
> -		ret = -EINVAL;
> +		ret = bcm63xx_parse_cfe_nand_partitions(master, pparts, nvram);
>  
>  out:
>  	vfree(nvram);
> @@ -321,5 +504,6 @@ MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Daniel Dickinson <openwrt@cshore.neomailbox.net>");
>  MODULE_AUTHOR("Florian Fainelli <florian@openwrt.org>");
>  MODULE_AUTHOR("Mike Albon <malbon@openwrt.org>");
> -MODULE_AUTHOR("Jonas Gorski <jonas.gorski@gmail.com");
> +MODULE_AUTHOR("Jonas Gorski <jonas.gorski@gmail.com>");
> +MODULE_AUTHOR("Simon Arlott");
>  MODULE_DESCRIPTION("MTD partitioning for BCM63XX CFE bootloaders");
> -- 
> 2.1.4
> 
> -- 
> Simon Arlott