[PATCH 0/3] mtd: cleanup plat-nand/ts72xx partitions interface

[PATCH 0/3] mtd: cleanup plat-nand/ts72xx partitions interface

[Dmitry Eremin-Solenikov]

The following changes since commit d15fed205975d5b552c3f7deee5dc795b002932a:

  mtd: samsung/onenand don't specify default parsing options (2011-06-06 15:06:01 +0300)

are available in the git repository at:
  git://git.infradead.org/users/dbaryshkov/mtd-cleanup.git mtd-retain

Dmitry Eremin-Solenikov (3):
      mtd: add a flags for partitions which should just leave smth. after them
      ts72xx: use MTDPART_OFS_RETAIN for mtd partitioning
      mtd: plat-nand: drop unused fields from platform_nand_data

 arch/arm/mach-ep93xx/ts72xx.c  |   23 +++++------------------
 drivers/mtd/mtdpart.c          |   13 +++++++++++++
 drivers/mtd/nand/plat_nand.c   |    2 --
 include/linux/mtd/nand.h       |    2 --
 include/linux/mtd/partitions.h |    5 ++++-
 5 files changed, 22 insertions(+), 23 deletions(-)

[PATCH 1/3] mtd: add a flags for partitions which should just leave smth. after them

[Dmitry Eremin-Solenikov]

Add support for MTDPART_OFS_RETAIN: such partitions start at the current
offset, take as much space as possible, but rain part->size bytes after
the end of the partitions for other parts. Primarily this is intended
for ts72xx arm platforms cleanup.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/mtd/mtdpart.c          |   13 +++++++++++++
 include/linux/mtd/partitions.h |    5 ++++-
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 3477e16..63c19f9 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -479,6 +479,19 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 			       (unsigned long long)cur_offset, (unsigned long long)slave->offset);
 		}
 	}
+	if (slave->offset == MTDPART_OFS_RETAIN) {
+		slave->offset = cur_offset;
+		if (master->size - slave->offset >= slave->mtd.size) {
+			slave->mtd.size =
+				master->size - slave->offset - slave->mtd.size;
+		} else {
+			printk(KERN_ERR "mtd partition \"%s\" doesn't have enough space: %#llx < %#llx, disabled\n",
+				part->name,
+				master->size - slave->offset, slave->mtd.size);
+			/* register to preserve ordering */
+			goto out_register;
+		}
+	}
 	if (slave->mtd.size == MTDPART_SIZ_FULL)
 		slave->mtd.size = master->size - slave->offset;
 
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 08c9c7b..1431cf2 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -24,7 +24,9 @@
  * 	will extend to the end of the master MTD device.
  * offset: absolute starting position within the master MTD device; if
  * 	defined as MTDPART_OFS_APPEND, the partition will start where the
- * 	previous one ended; if MTDPART_OFS_NXTBLK, at the next erase block.
+ *	previous one ended; if MTDPART_OFS_NXTBLK, at the next erase block;
+ *	if MTDPART_OFS_RETAIN, consume as much as possible, leaving size
+ *	after the end of partition.
  * mask_flags: contains flags that have to be masked (removed) from the
  * 	master MTD flag set for the corresponding MTD partition.
  * 	For example, to force a read-only partition, simply adding
@@ -42,6 +44,7 @@ struct mtd_partition {
 	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only) */
 };
 
+#define MTDPART_OFS_RETAIN	(-3)
 #define MTDPART_OFS_NXTBLK	(-2)
 #define MTDPART_OFS_APPEND	(-1)
 #define MTDPART_SIZ_FULL	(0)
-- 
1.7.4.4

Re: [PATCH 1/3] mtd: add a flags for partitions which should just leave smth. after them

[Artem Bityutskiy]

On Mon, 2011-06-06 at 18:04 +0400, Dmitry Eremin-Solenikov wrote:
> +			printk(KERN_ERR "mtd partition \"%s\" doesn't have enough space: %#llx < %#llx, disabled\n",
> +				part->name,
> +				master->size - slave->offset, slave->mtd.size);
> +			/* register to preserve ordering */

Tweaked this patch a tiny bit, and push all 3 to l2-mtd-2.6.git, thanks.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCH 2/3] ts72xx: use MTDPART_OFS_RETAIN for mtd partitioning

[Dmitry Eremin-Solenikov]

Instead of specifying a callback for dynamic partitioning, use
MTDPART_OFS_RETAIN for reserving a place near the end of flash for
RedBoot.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ryan Mallon <ryan@bluewatersys.com>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-ep93xx/ts72xx.c |   23 +++++------------------
 1 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-ep93xx/ts72xx.c b/arch/arm/mach-ep93xx/ts72xx.c
index c2d2cf4..dea42e2 100644
--- a/arch/arm/mach-ep93xx/ts72xx.c
+++ b/arch/arm/mach-ep93xx/ts72xx.c
@@ -116,8 +116,9 @@ static struct mtd_partition ts72xx_nand_parts[] = {
 		.mask_flags	= MTD_WRITEABLE,	/* force read-only */
 	}, {
 		.name		= "Linux",
-		.offset		= MTDPART_OFS_APPEND,
-		.size		= 0,			/* filled in later */
+		.offset		= MTDPART_OFS_RETAIN,
+		.size		= TS72XX_REDBOOT_PART_SIZE,
+				/* leave so much for last partition */
 	}, {
 		.name		= "RedBoot",
 		.offset		= MTDPART_OFS_APPEND,
@@ -126,28 +127,14 @@ static struct mtd_partition ts72xx_nand_parts[] = {
 	},
 };
 
-static void ts72xx_nand_set_parts(uint64_t size,
-				  struct platform_nand_chip *chip)
-{
-	/* Factory TS-72xx boards only come with 32MiB or 128MiB NAND options */
-	if (size == SZ_32M || size == SZ_128M) {
-		/* Set the "Linux" partition size */
-		ts72xx_nand_parts[1].size = size - TS72XX_REDBOOT_PART_SIZE;
-
-		chip->partitions = ts72xx_nand_parts;
-		chip->nr_partitions = ARRAY_SIZE(ts72xx_nand_parts);
-	} else {
-		pr_warning("Unknown nand disk size:%lluMiB\n", size >> 20);
-	}
-}
-
 static struct platform_nand_data ts72xx_nand_data = {
 	.chip = {
 		.nr_chips	= 1,
 		.chip_offset	= 0,
 		.chip_delay	= 15,
 		.part_probe_types = ts72xx_nand_part_probes,
-		.set_parts	= ts72xx_nand_set_parts,
+		.partitions	= ts72xx_nand_parts,
+		.nr_partitions	= ARRAY_SIZE(ts72xx_nand_parts),
 	},
 	.ctrl = {
 		.cmd_ctrl	= ts72xx_nand_hwcontrol,
-- 
1.7.4.4

Re: [PATCH 2/3] ts72xx: use MTDPART_OFS_RETAIN for mtd partitioning

[Petr Štetiar]

Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> [2011-06-06 18:04:15]:

Hi,

> Instead of specifying a callback for dynamic partitioning, use
> MTDPART_OFS_RETAIN for reserving a place near the end of flash for
> RedBoot.

[...]

> -		.offset		= MTDPART_OFS_APPEND,
> -		.size		= 0,			/* filled in later */
> +		.offset		= MTDPART_OFS_RETAIN,
> +		.size		= TS72XX_REDBOOT_PART_SIZE,
> +				/* leave so much for last partition */

thank you for the mtd cleanup work. While testing this patch, I've noticed,
that the TS72XX_REDBOOT_PART_SIZE is by 16K bigger, then it should be:

Redboot:

	RedBoot> fis list
	Name              FLASH addr  Mem addr    Length      Entry point
	(reserved)        0x60000000  0x60000000  0x01D04000  0x00000000
	RedBoot           0x61D04000  0x61D04000  0x00040000  0x00000000
	vmlinux           0x61D44000  0x00218000  0x000C0000  0x00218000
	FIS directory     0x61FFC000  0x61FFC000  0x00003000  0x00000000
	RedBoot config    0x61FFF000  0x61FFF000  0x00001000  0x00000000

Linux:

	0x000000000000-0x000000004000 : "TS-BOOTROM"
	0x000000004000-0x000001d00000 : "Linux"
	0x000001d00000-0x000002000000 : "RedBoot"

So if I simply read now first 4 bytes from /dev/mtdblock2, there's no 'CRUS'
or 'SURC' boot HeaderID/signature and it means, that there's no valid Redboot
for ep93xx. The correct value is:

	#define TS72XX_REDBOOT_PART_SIZE	(SZ_2M + SZ_1M - SZ_16K)

And in the current state one could easily overwrite fis/fconfig while
upgrading the Redboot from the Linux userspace and brick the board, because
there's fis/fconfig stuff included in that Redboot partition.

I think, that it would be better to either make that Redboot partition
read-only by default, make it smaller so it wouldn't contain fconfig/fis parts
in it or add fis/fconfig partitions also. I don't know what's preffered way of
handling this, but since we're touching that part of the code, we should fix
it right.

Adding Mika to the Cc loop, since he has ts-7260.

-- ynezz

Re: [PATCH 2/3] ts72xx: use MTDPART_OFS_RETAIN for mtd partitioning

[Petr Štetiar]

Petr Štetiar <ynezz@true.cz> [2011-06-11 11:22:24]:

> I think, that it would be better to either make that Redboot partition
> read-only by default

Sorry for the noise, it's read-only already, I've just missed that mask.

-- ynezz

Re: [PATCH 2/3] ts72xx: use MTDPART_OFS_RETAIN for mtd partitioning

[Dmitry Eremin-Solenikov]

[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]

Hello, colleagues,

On 6/11/11, Petr Štetiar <ynezz@true.cz> wrote:
> Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> [2011-06-06 18:04:15]:
>> Instead of specifying a callback for dynamic partitioning, use
>> MTDPART_OFS_RETAIN for reserving a place near the end of flash for
>> RedBoot.
>
> [...]
>
>> -		.offset		= MTDPART_OFS_APPEND,
>> -		.size		= 0,			/* filled in later */
>> +		.offset		= MTDPART_OFS_RETAIN,
>> +		.size		= TS72XX_REDBOOT_PART_SIZE,
>> +				/* leave so much for last partition */
>
> thank you for the mtd cleanup work. While testing this patch, I've noticed,
> that the TS72XX_REDBOOT_PART_SIZE is by 16K bigger, then it should be:
>
> Redboot:
>
> 	RedBoot> fis list
> 	Name              FLASH addr  Mem addr    Length      Entry point
> 	(reserved)        0x60000000  0x60000000  0x01D04000  0x00000000
> 	RedBoot           0x61D04000  0x61D04000  0x00040000  0x00000000
> 	vmlinux           0x61D44000  0x00218000  0x000C0000  0x00218000
> 	FIS directory     0x61FFC000  0x61FFC000  0x00003000  0x00000000
> 	RedBoot config    0x61FFF000  0x61FFF000  0x00001000  0x00000000
>
> Linux:
>
> 	0x000000000000-0x000000004000 : "TS-BOOTROM"
> 	0x000000004000-0x000001d00000 : "Linux"
> 	0x000001d00000-0x000002000000 : "RedBoot"
>
> So if I simply read now first 4 bytes from /dev/mtdblock2, there's no 'CRUS'
> or 'SURC' boot HeaderID/signature and it means, that there's no valid
> Redboot
> for ep93xx. The correct value is:
>
> 	#define TS72XX_REDBOOT_PART_SIZE	(SZ_2M + SZ_1M - SZ_16K)

Could you please verify/ack the attached patch?

-- 
With best wishes
Dmitry

[-- Attachment #2: 0001-ts72xx-correct-partition-sizes.patch --]
[-- Type: text/x-patch, Size: 1133 bytes --]

From 9a7a69043004d2f738e321c2443f81f345bf9b1e Mon Sep 17 00:00:00 2001
From: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Date: Sun, 12 Jun 2011 00:58:38 +0400
Subject: [PATCH] ts72xx: correct partition sizes

Since afe43223d (ts72xx: use MTDPART_OFS_RETAIN for mtd partitioning),
ts72xx uses incorrect size for Linux partition (difference is 16k),
correct that so that users have correct access to RedBoot partition.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-ep93xx/ts72xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-ep93xx/ts72xx.c b/arch/arm/mach-ep93xx/ts72xx.c
index dea42e2..9510ae0 100644
--- a/arch/arm/mach-ep93xx/ts72xx.c
+++ b/arch/arm/mach-ep93xx/ts72xx.c
@@ -106,7 +106,7 @@ static int ts72xx_nand_device_ready(struct mtd_info *mtd)
 static const char *ts72xx_nand_part_probes[] = { "cmdlinepart", NULL };
 
 #define TS72XX_BOOTROM_PART_SIZE	(SZ_16K)
-#define TS72XX_REDBOOT_PART_SIZE	(SZ_2M + SZ_1M)
+#define TS72XX_REDBOOT_PART_SIZE	(SZ_2M + SZ_1M - SZ_16K)
 
 static struct mtd_partition ts72xx_nand_parts[] = {
 	{
-- 
1.7.5.3

[PATCH 3/3] mtd: plat-nand: drop unused fields from platform_nand_data

[Dmitry Eremin-Solenikov]

Drop now unused set_parts from struct platform_nand_data. Also, while we are
at it, drop long unused priv field from platform_nand_data.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/mtd/nand/plat_nand.c |    2 --
 include/linux/mtd/nand.h     |    2 --
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/plat_nand.c b/drivers/mtd/nand/plat_nand.c
index 633c04b..1339fa8 100644
--- a/drivers/mtd/nand/plat_nand.c
+++ b/drivers/mtd/nand/plat_nand.c
@@ -108,8 +108,6 @@ static int __devinit plat_nand_probe(struct platform_device *pdev)
 			return 0;
 		}
 	}
-	if (pdata->chip.set_parts)
-		pdata->chip.set_parts(data->mtd.size, &pdata->chip);
 	if (pdata->chip.partitions) {
 		data->parts = pdata->chip.partitions;
 		err = mtd_device_register(&data->mtd, data->parts,
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 585d49b..da6e252 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -617,8 +617,6 @@ struct platform_nand_chip {
 	int chip_delay;
 	unsigned int options;
 	const char **part_probe_types;
-	void (*set_parts)(uint64_t size, struct platform_nand_chip *chip);
-	void *priv;
 };
 
 /* Keep gcc happy */
-- 
1.7.4.4