* [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD partition parser
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-06-09 1:30 ` Brian Norris
2017-06-09 1:42 ` Brian Norris
2017-05-31 22:40 ` [PATCH v3 2/9] mtd: nand: sharpsl.h: support partition parser types Andrea Adami
` (8 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
The Sharp SL Series (Zaurus) PXA handhelds have 16/64/128M of NAND flash
and share the same layout of the first 7M partition, managed by Sharp FTL.
The purpose of this self-contained patch is to add a common parser and
remove the hardcoded sizes in the board files (these devices are not yet
converted to devicetree).
Users will have benefits because the mtdparts= tag will not be necessary
anymore and they will be free to repartition the little sized flash.
The obsolete bootloader can not pass the partitioning info to modern
kernels anymore so it has to be read from flash at known logical addresses.
(see http://www.h5.dion.ne.jp/~rimemoon/zaurus/memo_006.htm )
In kernel, under arch/arm/mach-pxa we have already 8 machines:
MACH_POODLE, MACH_CORGI, MACH_SHEPERD, MACH_HUSKY, MACH_AKITA, MACH_SPITZ,
MACH_BORZOI, MACH_TOSA.
Lost after the 2.4 vendor kernel are MACH_BOXER and MACH_TERRIER.
Almost every model has different factory partitioning: add to this the
units can be repartitioned by users with userspace tools (nandlogical)
and installers for popular (back then) linux distributions.
The Parameter Area in the first (boot) partition extends from 0x00040000 to
0x0007bfff (176k) and contains two copies of the partition table:
...
0x00060000: Partition Info1 16k
0x00064000: Partition Info2 16k
0x00668000: Model 16k
...
The first 7M partition is managed by the Sharp FTL reserving 5% + 1 blocks
for wear-leveling: some blocks are remapped and one layer of translation
(logical to physical) is necessary.
There isn't much documentation about this FTL in the 2.4 sources, just the
MTD methods for reading and writing using logical addresses and the block
management (wear-leveling, use counter).
For the purpose of the MTD parser only the read part of the code was taken.
The NAND drivers that can use this parser are sharpsl.c and tmio_nand.c.
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
drivers/mtd/Kconfig | 8 ++
drivers/mtd/Makefile | 2 +
drivers/mtd/sharpsl_ftl.c | 216 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/mtd/sharpsl_ftl.h | 34 ++++++++
drivers/mtd/sharpslpart.c | 132 ++++++++++++++++++++++++++++
5 files changed, 392 insertions(+)
create mode 100644 drivers/mtd/sharpsl_ftl.c
create mode 100644 drivers/mtd/sharpsl_ftl.h
create mode 100644 drivers/mtd/sharpslpart.c
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index e83a279..5c96127 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -155,6 +155,14 @@ config MTD_BCM47XX_PARTS
This provides partitions parser for devices based on BCM47xx
boards.
+config MTD_SHARPSL_PARTS
+ tristate "Sharp SL Series NAND flash partition parser"
+ depends on MTD_NAND_SHARPSL || MTD_NAND_TMIO
+ help
+ This provides the read-only FTL logic necessary to read the partition
+ table from the NAND flash of Sharp SL Series (Zaurus) and the MTD
+ partition parser using this code.
+
comment "User Modules And Translation Layers"
#
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 99bb9a1..89f707b 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -13,6 +13,8 @@ obj-$(CONFIG_MTD_AFS_PARTS) += afs.o
obj-$(CONFIG_MTD_AR7_PARTS) += ar7part.o
obj-$(CONFIG_MTD_BCM63XX_PARTS) += bcm63xxpart.o
obj-$(CONFIG_MTD_BCM47XX_PARTS) += bcm47xxpart.o
+obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpsl-part.o
+sharpsl-part-objs := sharpsl_ftl.o sharpslpart.o
# 'Users' - code which presents functionality to userspace.
obj-$(CONFIG_MTD_BLKDEVS) += mtd_blkdevs.o
diff --git a/drivers/mtd/sharpsl_ftl.c b/drivers/mtd/sharpsl_ftl.c
new file mode 100644
index 0000000..1796d03
--- /dev/null
+++ b/drivers/mtd/sharpsl_ftl.c
@@ -0,0 +1,216 @@
+/*
+ * MTD method for NAND accessing via logical address (SHARP FTL)
+ *
+ * Copyright (C) 2017 Andrea Adami <andrea.adami@gmail.com>
+ *
+ * Based on 2.4 sources: drivers/mtd/nand/sharp_sl_logical.c
+ * Copyright (C) 2002 SHARP
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include "sharpsl_ftl.h"
+
+/* oob structure */
+#define NAND_NOOB_LOGADDR_00 8
+#define NAND_NOOB_LOGADDR_01 9
+#define NAND_NOOB_LOGADDR_10 10
+#define NAND_NOOB_LOGADDR_11 11
+#define NAND_NOOB_LOGADDR_20 12
+#define NAND_NOOB_LOGADDR_21 13
+
+/* Logical Table */
+struct mtd_logical {
+ u32 size; /* size of the handled partition */
+ int index; /* mtd->index */
+ u_int phymax; /* physical blocks */
+ u_int logmax; /* logical blocks */
+ u_int *log2phy; /* the logical-to-physical table */
+};
+
+static struct mtd_logical *sharpsl_mtd_logical;
+
+static int sharpsl_nand_read_oob(struct mtd_info *mtd, loff_t offs, size_t len,
+ uint8_t *buf)
+{
+ loff_t mask = mtd->writesize - 1;
+ struct mtd_oob_ops ops;
+ int ret;
+
+ ops.mode = MTD_OPS_PLACE_OOB;
+ ops.ooboffs = offs & mask;
+ ops.ooblen = len;
+ ops.oobbuf = buf;
+ ops.datbuf = NULL;
+
+ ret = mtd_read_oob(mtd, offs & ~mask, &ops);
+ if (ret != 0 || len != ops.oobretlen)
+ return -1;
+
+ return 0;
+}
+
+static u_int sharpsl_nand_get_logical_num(u_char *oob)
+{
+ u16 us;
+ int good0, good1;
+
+ if (oob[NAND_NOOB_LOGADDR_00] == oob[NAND_NOOB_LOGADDR_10] &&
+ oob[NAND_NOOB_LOGADDR_01] == oob[NAND_NOOB_LOGADDR_11]) {
+ good0 = NAND_NOOB_LOGADDR_00;
+ good1 = NAND_NOOB_LOGADDR_01;
+ } else if (oob[NAND_NOOB_LOGADDR_10] == oob[NAND_NOOB_LOGADDR_20] &&
+ oob[NAND_NOOB_LOGADDR_11] == oob[NAND_NOOB_LOGADDR_21]) {
+ good0 = NAND_NOOB_LOGADDR_10;
+ good1 = NAND_NOOB_LOGADDR_11;
+ } else if (oob[NAND_NOOB_LOGADDR_20] == oob[NAND_NOOB_LOGADDR_00] &&
+ oob[NAND_NOOB_LOGADDR_21] == oob[NAND_NOOB_LOGADDR_01]) {
+ good0 = NAND_NOOB_LOGADDR_20;
+ good1 = NAND_NOOB_LOGADDR_21;
+ } else {
+ return UINT_MAX;
+ }
+
+ us = oob[good0] | oob[good1] << 8;
+
+ /* parity check */
+ if (hweight16(us) & 1)
+ return (UINT_MAX - 1);
+
+ /* reserved */
+ if (us == 0xffff)
+ return 0xffff;
+ else
+ return (us & 0x07fe) >> 1;
+}
+
+int sharpsl_nand_init_logical(struct mtd_info *mtd, u32 partition_size)
+{
+ struct mtd_logical *logical = NULL;
+ u_int block_num, log_num;
+ loff_t block_adr;
+ u_char *oob = NULL;
+ int i, readretry;
+
+ logical = kzalloc(sizeof(*logical), GFP_KERNEL);
+ if (!logical)
+ return -ENOMEM;
+
+ oob = kzalloc(mtd->oobsize, GFP_KERNEL);
+ if (!oob) {
+ kfree(logical);
+ return -ENOMEM;
+ }
+
+ /* initialize management structure */
+ logical->size = partition_size;
+ logical->index = mtd->index;
+ logical->phymax = (partition_size / mtd->erasesize);
+
+ /* FTL reserves 5% of the blocks + 1 spare */
+ logical->logmax = ((logical->phymax * 95) / 100) - 1;
+
+ logical->log2phy = NULL;
+ logical->log2phy = kcalloc(logical->logmax, sizeof(u_int), GFP_KERNEL);
+ if (!logical->log2phy) {
+ kfree(logical);
+ kfree(oob);
+ return -ENOMEM;
+ }
+
+ /* initialize logical->log2phy */
+ for (i = 0; i < logical->logmax; i++)
+ logical->log2phy[i] = UINT_MAX;
+
+ /* create physical-logical table */
+ for (block_num = 0; block_num < logical->phymax; block_num++) {
+ block_adr = block_num * mtd->erasesize;
+
+ if (mtd_block_isbad(mtd, block_adr))
+ continue;
+
+ readretry = 3;
+read_retry:
+ if (sharpsl_nand_read_oob(mtd, block_adr, mtd->oobsize, oob))
+ continue;
+
+ /* get logical block */
+ log_num = sharpsl_nand_get_logical_num(oob);
+
+ /* skip out of range and not unique values */
+ if ((int)log_num >= 0 && (log_num < logical->logmax)) {
+ if (logical->log2phy[log_num] == UINT_MAX)
+ logical->log2phy[log_num] = block_num;
+ } else {
+ readretry--;
+ if (readretry)
+ goto read_retry;
+ }
+ }
+ kfree(oob);
+ sharpsl_mtd_logical = logical;
+
+ pr_info("Sharp SL FTL: %d blocks used (%d logical, %d reserved)\n",
+ logical->phymax, logical->logmax,
+ logical->phymax - logical->logmax);
+
+ return 0;
+}
+
+void sharpsl_nand_cleanup_logical(void)
+{
+ struct mtd_logical *logical = sharpsl_mtd_logical;
+
+ sharpsl_mtd_logical = NULL;
+
+ kfree(logical->log2phy);
+ logical->log2phy = NULL;
+ kfree(logical);
+ logical = NULL;
+}
+
+int sharpsl_nand_read_laddr(struct mtd_info *mtd,
+ loff_t from,
+ size_t len,
+ u_char *buf)
+{
+ struct mtd_logical *logical;
+ u_int log_num, log_new;
+ u_int block_num;
+ loff_t block_adr;
+ loff_t block_ofs;
+ size_t retlen;
+ int ret;
+
+ logical = sharpsl_mtd_logical;
+ log_num = (u32)from / mtd->erasesize;
+ log_new = ((u32)from + len - 1) / mtd->erasesize;
+
+ if (len <= 0 || log_num >= logical->logmax || log_new > log_num)
+ return -EINVAL;
+
+ block_num = logical->log2phy[log_num];
+ block_adr = block_num * mtd->erasesize;
+ block_ofs = (u32)from % mtd->erasesize;
+
+ ret = mtd_read(mtd, block_adr + block_ofs, len, &retlen, buf);
+ if (ret != 0 || len != retlen)
+ return -EINVAL;
+
+ return 0;
+}
diff --git a/drivers/mtd/sharpsl_ftl.h b/drivers/mtd/sharpsl_ftl.h
new file mode 100644
index 0000000..2880cbe
--- /dev/null
+++ b/drivers/mtd/sharpsl_ftl.h
@@ -0,0 +1,34 @@
+/*
+ * Header file for NAND accessing via logical address (SHARP FTL)
+ *
+ * Copyright (C) 2017 Andrea Adami <andrea.adami@gmail.com>
+ *
+ * Based on 2.4 sources: linux/include/asm-arm/sharp_nand_logical.h
+ * Copyright (C) 2002 SHARP
+ *
+ * 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
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __SHARPSL_NAND_LOGICAL_H__
+#define __SHARPSL_NAND_LOGICAL_H__
+
+#include <linux/types.h>
+#include <linux/mtd/mtd.h>
+
+int sharpsl_nand_init_logical(struct mtd_info *mtd, u32 partition_size);
+
+void sharpsl_nand_cleanup_logical(void);
+
+int sharpsl_nand_read_laddr(struct mtd_info *mtd, loff_t from, size_t len,
+ u_char *buf);
+
+#endif
diff --git a/drivers/mtd/sharpslpart.c b/drivers/mtd/sharpslpart.c
new file mode 100644
index 0000000..40aebe9
--- /dev/null
+++ b/drivers/mtd/sharpslpart.c
@@ -0,0 +1,132 @@
+/*
+ * MTD partition parser for NAND flash on Sharp SL Series
+ *
+ * Copyright (C) 2017 Andrea Adami <andrea.adami@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
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include "sharpsl_ftl.h"
+
+/* factory defaults */
+#define SHARPSL_NAND_PARTS 3
+#define SHARPSL_FTL_PARTITION_SIZE (7 * 1024 * 1024)
+#define PARAM_BLOCK_PARTITIONINFO1 0x00060000
+#define PARAM_BLOCK_PARTITIONINFO2 0x00064000
+
+#define BOOT_MAGIC be32_to_cpu(0x424f4f54) /* BOOT */
+#define FSRO_MAGIC be32_to_cpu(0x4653524f) /* FSRO */
+#define FSRW_MAGIC be32_to_cpu(0x46535257) /* FSRW */
+
+/*
+ * Sample values read from SL-C860
+ *
+ * # cat /proc/mtd
+ * dev: size erasesize name
+ * mtd0: 006d0000 00020000 "Filesystem"
+ * mtd1: 00700000 00004000 "smf"
+ * mtd2: 03500000 00004000 "root"
+ * mtd3: 04400000 00004000 "home"
+ *
+ * PARTITIONINFO1
+ * 0x00060000: 00 00 00 00 00 00 70 00 42 4f 4f 54 00 00 00 00 ......p.BOOT....
+ * 0x00060010: 00 00 70 00 00 00 c0 03 46 53 52 4f 00 00 00 00 ..p.....FSRO....
+ * 0x00060020: 00 00 c0 03 00 00 00 04 46 53 52 57 00 00 00 00 ........FSRW....
+ * 0x00060030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
+ *
+ */
+
+struct sharpsl_nand_partitioninfo {
+ u32 start;
+ u32 end;
+ u32 magic;
+ u32 reserved;
+};
+
+static int sharpsl_parse_mtd_partitions(struct mtd_info *master,
+ const struct mtd_partition **pparts,
+ struct mtd_part_parser_data *data)
+{
+ struct sharpsl_nand_partitioninfo buf1[SHARPSL_NAND_PARTS];
+ struct sharpsl_nand_partitioninfo buf2[SHARPSL_NAND_PARTS];
+ struct mtd_partition *sharpsl_nand_parts;
+
+ /* init logical mgmt (FTL) */
+ if (sharpsl_nand_init_logical(master, SHARPSL_FTL_PARTITION_SIZE))
+ return -EINVAL;
+
+ /* read the two partition tables */
+ if (sharpsl_nand_read_laddr(master,
+ PARAM_BLOCK_PARTITIONINFO1,
+ sizeof(buf1), (u_char *)&buf1) ||
+ sharpsl_nand_read_laddr(master,
+ PARAM_BLOCK_PARTITIONINFO2,
+ sizeof(buf2), (u_char *)&buf2))
+ return -EINVAL;
+
+ /* cleanup logical mgmt (FTL) */
+ sharpsl_nand_cleanup_logical();
+
+ /* compare the two buffers */
+ if (memcmp(&buf1, &buf2, sizeof(buf1))) {
+ pr_err("sharpslpart: PARTITIONINFO 1,2 differ. Quit parser.\n");
+ return -EINVAL;
+ }
+
+ /* check for magics (just in the first) */
+ if (buf1[0].magic != BOOT_MAGIC ||
+ buf1[1].magic != FSRO_MAGIC ||
+ buf1[2].magic != FSRW_MAGIC) {
+ pr_err("sharpslpart: magic values mismatch. Quit parser.\n");
+ return -EINVAL;
+ }
+
+ sharpsl_nand_parts = kzalloc(sizeof(*sharpsl_nand_parts) *
+ SHARPSL_NAND_PARTS, GFP_KERNEL);
+ if (!sharpsl_nand_parts)
+ return -ENOMEM;
+
+ /* original names */
+ sharpsl_nand_parts[0].name = "smf";
+ sharpsl_nand_parts[0].offset = buf1[0].start;
+ sharpsl_nand_parts[0].size = buf1[0].end - buf1[0].start;
+ sharpsl_nand_parts[0].mask_flags = 0;
+
+ sharpsl_nand_parts[1].name = "root";
+ sharpsl_nand_parts[1].offset = buf1[1].start;
+ sharpsl_nand_parts[1].size = buf1[1].end - buf1[1].start;
+ sharpsl_nand_parts[1].mask_flags = 0;
+
+ sharpsl_nand_parts[2].name = "home";
+ sharpsl_nand_parts[2].offset = buf1[2].start;
+ /* discard buf1[2].end, was for older models with 64M flash */
+ sharpsl_nand_parts[2].size = master->size - buf1[2].start;
+ sharpsl_nand_parts[2].mask_flags = 0;
+
+ *pparts = sharpsl_nand_parts;
+ return SHARPSL_NAND_PARTS;
+}
+
+static struct mtd_part_parser sharpsl_mtd_parser = {
+ .parse_fn = sharpsl_parse_mtd_partitions,
+ .name = "sharpslpart",
+};
+module_mtd_part_parser(sharpsl_mtd_parser);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Andrea Adami <andrea.adami@gmail.com>");
+MODULE_DESCRIPTION("MTD partitioning for NAND flash on Sharp SL Series");
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD partition parser
2017-05-31 22:40 ` [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD " Andrea Adami
@ 2017-06-09 1:30 ` Brian Norris
2017-06-20 8:52 ` Andrea Adami
2017-06-09 1:42 ` Brian Norris
1 sibling, 1 reply; 22+ messages in thread
From: Brian Norris @ 2017-06-09 1:30 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Hi,
On Thu, Jun 01, 2017 at 12:40:50AM +0200, Andrea Adami wrote:
> The Sharp SL Series (Zaurus) PXA handhelds have 16/64/128M of NAND flash
> and share the same layout of the first 7M partition, managed by Sharp FTL.
>
> The purpose of this self-contained patch is to add a common parser and
> remove the hardcoded sizes in the board files (these devices are not yet
> converted to devicetree).
> Users will have benefits because the mtdparts= tag will not be necessary
> anymore and they will be free to repartition the little sized flash.
>
> The obsolete bootloader can not pass the partitioning info to modern
> kernels anymore so it has to be read from flash at known logical addresses.
> (see http://www.h5.dion.ne.jp/~rimemoon/zaurus/memo_006.htm )
>
> In kernel, under arch/arm/mach-pxa we have already 8 machines:
> MACH_POODLE, MACH_CORGI, MACH_SHEPERD, MACH_HUSKY, MACH_AKITA, MACH_SPITZ,
> MACH_BORZOI, MACH_TOSA.
> Lost after the 2.4 vendor kernel are MACH_BOXER and MACH_TERRIER.
>
> Almost every model has different factory partitioning: add to this the
> units can be repartitioned by users with userspace tools (nandlogical)
> and installers for popular (back then) linux distributions.
>
> The Parameter Area in the first (boot) partition extends from 0x00040000 to
> 0x0007bfff (176k) and contains two copies of the partition table:
> ...
> 0x00060000: Partition Info1 16k
> 0x00064000: Partition Info2 16k
> 0x00668000: Model 16k
> ...
>
> The first 7M partition is managed by the Sharp FTL reserving 5% + 1 blocks
> for wear-leveling: some blocks are remapped and one layer of translation
> (logical to physical) is necessary.
>
> There isn't much documentation about this FTL in the 2.4 sources, just the
> MTD methods for reading and writing using logical addresses and the block
> management (wear-leveling, use counter).
> For the purpose of the MTD parser only the read part of the code was taken.
>
> The NAND drivers that can use this parser are sharpsl.c and tmio_nand.c.
>
> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> ---
> drivers/mtd/Kconfig | 8 ++
> drivers/mtd/Makefile | 2 +
> drivers/mtd/sharpsl_ftl.c | 216 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/mtd/sharpsl_ftl.h | 34 ++++++++
> drivers/mtd/sharpslpart.c | 132 ++++++++++++++++++++++++++++
> 5 files changed, 392 insertions(+)
> create mode 100644 drivers/mtd/sharpsl_ftl.c
> create mode 100644 drivers/mtd/sharpsl_ftl.h
> create mode 100644 drivers/mtd/sharpslpart.c
>
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index e83a279..5c96127 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -155,6 +155,14 @@ config MTD_BCM47XX_PARTS
> This provides partitions parser for devices based on BCM47xx
> boards.
>
> +config MTD_SHARPSL_PARTS
> + tristate "Sharp SL Series NAND flash partition parser"
> + depends on MTD_NAND_SHARPSL || MTD_NAND_TMIO
Please include '|| COMPILE_TEST'.
> + help
> + This provides the read-only FTL logic necessary to read the partition
> + table from the NAND flash of Sharp SL Series (Zaurus) and the MTD
> + partition parser using this code.
> +
> comment "User Modules And Translation Layers"
>
> #
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index 99bb9a1..89f707b 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -13,6 +13,8 @@ obj-$(CONFIG_MTD_AFS_PARTS) += afs.o
> obj-$(CONFIG_MTD_AR7_PARTS) += ar7part.o
> obj-$(CONFIG_MTD_BCM63XX_PARTS) += bcm63xxpart.o
> obj-$(CONFIG_MTD_BCM47XX_PARTS) += bcm47xxpart.o
> +obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpsl-part.o
> +sharpsl-part-objs := sharpsl_ftl.o sharpslpart.o
>
> # 'Users' - code which presents functionality to userspace.
> obj-$(CONFIG_MTD_BLKDEVS) += mtd_blkdevs.o
> diff --git a/drivers/mtd/sharpsl_ftl.c b/drivers/mtd/sharpsl_ftl.c
> new file mode 100644
> index 0000000..1796d03
> --- /dev/null
> +++ b/drivers/mtd/sharpsl_ftl.c
> @@ -0,0 +1,216 @@
> +/*
> + * MTD method for NAND accessing via logical address (SHARP FTL)
> + *
> + * Copyright (C) 2017 Andrea Adami <andrea.adami@gmail.com>
> + *
> + * Based on 2.4 sources: drivers/mtd/nand/sharp_sl_logical.c
> + * Copyright (C) 2002 SHARP
> + *
> + * 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
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include "sharpsl_ftl.h"
> +
> +/* oob structure */
> +#define NAND_NOOB_LOGADDR_00 8
> +#define NAND_NOOB_LOGADDR_01 9
> +#define NAND_NOOB_LOGADDR_10 10
> +#define NAND_NOOB_LOGADDR_11 11
> +#define NAND_NOOB_LOGADDR_20 12
> +#define NAND_NOOB_LOGADDR_21 13
> +
> +/* Logical Table */
> +struct mtd_logical {
> + u32 size; /* size of the handled partition */
> + int index; /* mtd->index */
> + u_int phymax; /* physical blocks */
> + u_int logmax; /* logical blocks */
> + u_int *log2phy; /* the logical-to-physical table */
> +};
> +
> +static struct mtd_logical *sharpsl_mtd_logical;
Please do not use a static variable like this. References should be
dynamic.
Can the pointer just be passed back to the caller, and passed back to
the cleanup function when finished?
Related: do you foresee this parsing code being useful for anything
besides partitions? If not, then it seems like we should just merge the
"ftl" and "part" files. Not a requirement, but it might be cleaner.
> +
> +static int sharpsl_nand_read_oob(struct mtd_info *mtd, loff_t offs, size_t len,
> + uint8_t *buf)
> +{
> + loff_t mask = mtd->writesize - 1;
> + struct mtd_oob_ops ops;
> + int ret;
> +
> + ops.mode = MTD_OPS_PLACE_OOB;
> + ops.ooboffs = offs & mask;
> + ops.ooblen = len;
> + ops.oobbuf = buf;
> + ops.datbuf = NULL;
> +
> + ret = mtd_read_oob(mtd, offs & ~mask, &ops);
> + if (ret != 0 || len != ops.oobretlen)
> + return -1;
> +
> + return 0;
> +}
> +
> +static u_int sharpsl_nand_get_logical_num(u_char *oob)
> +{
> + u16 us;
> + int good0, good1;
> +
> + if (oob[NAND_NOOB_LOGADDR_00] == oob[NAND_NOOB_LOGADDR_10] &&
> + oob[NAND_NOOB_LOGADDR_01] == oob[NAND_NOOB_LOGADDR_11]) {
> + good0 = NAND_NOOB_LOGADDR_00;
> + good1 = NAND_NOOB_LOGADDR_01;
> + } else if (oob[NAND_NOOB_LOGADDR_10] == oob[NAND_NOOB_LOGADDR_20] &&
> + oob[NAND_NOOB_LOGADDR_11] == oob[NAND_NOOB_LOGADDR_21]) {
> + good0 = NAND_NOOB_LOGADDR_10;
> + good1 = NAND_NOOB_LOGADDR_11;
> + } else if (oob[NAND_NOOB_LOGADDR_20] == oob[NAND_NOOB_LOGADDR_00] &&
> + oob[NAND_NOOB_LOGADDR_21] == oob[NAND_NOOB_LOGADDR_01]) {
> + good0 = NAND_NOOB_LOGADDR_20;
> + good1 = NAND_NOOB_LOGADDR_21;
> + } else {
> + return UINT_MAX;
> + }
These seems like a pretty weird form of error protection. IIUC, there
are just 3 copies of the 2-byte sequence number, and we take the first
pair that matches at least one of the others? Could use a comment above
the if/else block, to explain what the logic is. Or perhaps even some
comments at the top of the file, to roughly describe this FTL.
> +
> + us = oob[good0] | oob[good1] << 8;
> +
> + /* parity check */
> + if (hweight16(us) & 1)
> + return (UINT_MAX - 1);
> +
> + /* reserved */
> + if (us == 0xffff)
Magic number should get a descriptive macro?
> + return 0xffff;
What does this mean? IIUC, this is essentially going to be an
out-of-bounds value that gets ignored, right? Should this just be
'return UINT_MAX'?
> + else
> + return (us & 0x07fe) >> 1;
0x07fe could use a macro definition. (And since it seems complementary,
I guess the 1 in 'hweight16(us) & 1' should also be a macro.)
> +}
> +
> +int sharpsl_nand_init_logical(struct mtd_info *mtd, u32 partition_size)
> +{
> + struct mtd_logical *logical = NULL;
You don't need the NULL initialization.
> + u_int block_num, log_num;
> + loff_t block_adr;
> + u_char *oob = NULL;
Same here.
> + int i, readretry;
> +
> + logical = kzalloc(sizeof(*logical), GFP_KERNEL);
> + if (!logical)
> + return -ENOMEM;
> +
> + oob = kzalloc(mtd->oobsize, GFP_KERNEL);
> + if (!oob) {
> + kfree(logical);
> + return -ENOMEM;
> + }
> +
> + /* initialize management structure */
> + logical->size = partition_size;
> + logical->index = mtd->index;
> + logical->phymax = (partition_size / mtd->erasesize);
> +
> + /* FTL reserves 5% of the blocks + 1 spare */
> + logical->logmax = ((logical->phymax * 95) / 100) - 1;
> +
> + logical->log2phy = NULL;
> + logical->log2phy = kcalloc(logical->logmax, sizeof(u_int), GFP_KERNEL);
> + if (!logical->log2phy) {
> + kfree(logical);
> + kfree(oob);
> + return -ENOMEM;
> + }
> +
> + /* initialize logical->log2phy */
> + for (i = 0; i < logical->logmax; i++)
> + logical->log2phy[i] = UINT_MAX;
The 0-initialized kcalloc() is a little superfluous, since you reinit
here. I guess it's a matter of taste.
> +
> + /* create physical-logical table */
> + for (block_num = 0; block_num < logical->phymax; block_num++) {
> + block_adr = block_num * mtd->erasesize;
> +
> + if (mtd_block_isbad(mtd, block_adr))
> + continue;
> +
> + readretry = 3;
> +read_retry:
> + if (sharpsl_nand_read_oob(mtd, block_adr, mtd->oobsize, oob))
> + continue;
> +
> + /* get logical block */
> + log_num = sharpsl_nand_get_logical_num(oob);
> +
> + /* skip out of range and not unique values */
> + if ((int)log_num >= 0 && (log_num < logical->logmax)) {
Why the (int) cast? You should already be eliminating anything larger
than INT_MAX, because they'll be larger than logmax. You should only
need the second comparison.
> + if (logical->log2phy[log_num] == UINT_MAX)
> + logical->log2phy[log_num] = block_num;
> + } else {
> + readretry--;
> + if (readretry)
> + goto read_retry;
What's the idea on the retries? Is this somehow supposed to make NAND
more reliable? That seems unlikely to work... Although notably, OOB is
often not covered by ECC, so maybe this is a really poor attempt at
making up for that?
> + }
> + }
> + kfree(oob);
> + sharpsl_mtd_logical = logical;
> +
> + pr_info("Sharp SL FTL: %d blocks used (%d logical, %d reserved)\n",
> + logical->phymax, logical->logmax,
> + logical->phymax - logical->logmax);
> +
> + return 0;
> +}
> +
> +void sharpsl_nand_cleanup_logical(void)
> +{
> + struct mtd_logical *logical = sharpsl_mtd_logical;
Please make this take a pointer arg, instead of relying on a static var.
> +
> + sharpsl_mtd_logical = NULL;
> +
> + kfree(logical->log2phy);
> + logical->log2phy = NULL;
> + kfree(logical);
> + logical = NULL;
> +}
> +
> +int sharpsl_nand_read_laddr(struct mtd_info *mtd,
> + loff_t from,
> + size_t len,
> + u_char *buf)
> +{
> + struct mtd_logical *logical;
> + u_int log_num, log_new;
> + u_int block_num;
> + loff_t block_adr;
> + loff_t block_ofs;
> + size_t retlen;
> + int ret;
> +
> + logical = sharpsl_mtd_logical;
> + log_num = (u32)from / mtd->erasesize;
> + log_new = ((u32)from + len - 1) / mtd->erasesize;
> +
> + if (len <= 0 || log_num >= logical->logmax || log_new > log_num)
> + return -EINVAL;
> +
> + block_num = logical->log2phy[log_num];
> + block_adr = block_num * mtd->erasesize;
> + block_ofs = (u32)from % mtd->erasesize;
> +
> + ret = mtd_read(mtd, block_adr + block_ofs, len, &retlen, buf);
> + if (ret != 0 || len != retlen)
What about ret == -EUCLEAN? Do you want to handle corrected bitflips?
> + return -EINVAL;
> +
> + return 0;
> +}
> diff --git a/drivers/mtd/sharpsl_ftl.h b/drivers/mtd/sharpsl_ftl.h
> new file mode 100644
> index 0000000..2880cbe
> --- /dev/null
> +++ b/drivers/mtd/sharpsl_ftl.h
> @@ -0,0 +1,34 @@
> +/*
> + * Header file for NAND accessing via logical address (SHARP FTL)
> + *
> + * Copyright (C) 2017 Andrea Adami <andrea.adami@gmail.com>
> + *
> + * Based on 2.4 sources: linux/include/asm-arm/sharp_nand_logical.h
> + * Copyright (C) 2002 SHARP
> + *
> + * 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
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifndef __SHARPSL_NAND_LOGICAL_H__
> +#define __SHARPSL_NAND_LOGICAL_H__
> +
> +#include <linux/types.h>
> +#include <linux/mtd/mtd.h>
> +
> +int sharpsl_nand_init_logical(struct mtd_info *mtd, u32 partition_size);
> +
> +void sharpsl_nand_cleanup_logical(void);
> +
> +int sharpsl_nand_read_laddr(struct mtd_info *mtd, loff_t from, size_t len,
> + u_char *buf);
> +
> +#endif
> diff --git a/drivers/mtd/sharpslpart.c b/drivers/mtd/sharpslpart.c
> new file mode 100644
> index 0000000..40aebe9
> --- /dev/null
> +++ b/drivers/mtd/sharpslpart.c
> @@ -0,0 +1,132 @@
> +/*
> + * MTD partition parser for NAND flash on Sharp SL Series
> + *
> + * Copyright (C) 2017 Andrea Adami <andrea.adami@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
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include "sharpsl_ftl.h"
> +
> +/* factory defaults */
> +#define SHARPSL_NAND_PARTS 3
> +#define SHARPSL_FTL_PARTITION_SIZE (7 * 1024 * 1024)
> +#define PARAM_BLOCK_PARTITIONINFO1 0x00060000
> +#define PARAM_BLOCK_PARTITIONINFO2 0x00064000
> +
> +#define BOOT_MAGIC be32_to_cpu(0x424f4f54) /* BOOT */
> +#define FSRO_MAGIC be32_to_cpu(0x4653524f) /* FSRO */
> +#define FSRW_MAGIC be32_to_cpu(0x46535257) /* FSRW */
> +
> +/*
> + * Sample values read from SL-C860
> + *
> + * # cat /proc/mtd
> + * dev: size erasesize name
> + * mtd0: 006d0000 00020000 "Filesystem"
> + * mtd1: 00700000 00004000 "smf"
> + * mtd2: 03500000 00004000 "root"
> + * mtd3: 04400000 00004000 "home"
> + *
> + * PARTITIONINFO1
> + * 0x00060000: 00 00 00 00 00 00 70 00 42 4f 4f 54 00 00 00 00 ......p.BOOT....
> + * 0x00060010: 00 00 70 00 00 00 c0 03 46 53 52 4f 00 00 00 00 ..p.....FSRO....
> + * 0x00060020: 00 00 c0 03 00 00 00 04 46 53 52 57 00 00 00 00 ........FSRW....
> + * 0x00060030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
> + *
> + */
> +
> +struct sharpsl_nand_partitioninfo {
> + u32 start;
> + u32 end;
> + u32 magic;
> + u32 reserved;
> +};
> +
> +static int sharpsl_parse_mtd_partitions(struct mtd_info *master,
> + const struct mtd_partition **pparts,
> + struct mtd_part_parser_data *data)
> +{
> + struct sharpsl_nand_partitioninfo buf1[SHARPSL_NAND_PARTS];
> + struct sharpsl_nand_partitioninfo buf2[SHARPSL_NAND_PARTS];
> + struct mtd_partition *sharpsl_nand_parts;
> +
> + /* init logical mgmt (FTL) */
Do you have any kind of validation logic, to ensure that this parser
actually matches? What if the flash was erased? Or what if somebody
managed to reformat to some other flash layout?
I suppose for many cases like that, the logical mapping will turn up
with a lot of UINT_MAX entries, and then sharpsl_nand_read_laddr() will
return -EINVAL? It'd be nice if there were some more clear sanity
checks though, if possible. Does this FTL have a header we should be
looking for?
> + if (sharpsl_nand_init_logical(master, SHARPSL_FTL_PARTITION_SIZE))
> + return -EINVAL;
> +
> + /* read the two partition tables */
> + if (sharpsl_nand_read_laddr(master,
> + PARAM_BLOCK_PARTITIONINFO1,
> + sizeof(buf1), (u_char *)&buf1) ||
> + sharpsl_nand_read_laddr(master,
> + PARAM_BLOCK_PARTITIONINFO2,
> + sizeof(buf2), (u_char *)&buf2))
> + return -EINVAL;
You've failed to cleanup in the error case here:
sharpsl_nand_cleanup_logical()
Also, consider passing through the actual error code?
> +
> + /* cleanup logical mgmt (FTL) */
> + sharpsl_nand_cleanup_logical();
> +
> + /* compare the two buffers */
> + if (memcmp(&buf1, &buf2, sizeof(buf1))) {
> + pr_err("sharpslpart: PARTITIONINFO 1,2 differ. Quit parser.\n");
> + return -EINVAL;
> + }
> +
> + /* check for magics (just in the first) */
> + if (buf1[0].magic != BOOT_MAGIC ||
> + buf1[1].magic != FSRO_MAGIC ||
> + buf1[2].magic != FSRW_MAGIC) {
> + pr_err("sharpslpart: magic values mismatch. Quit parser.\n");
> + return -EINVAL;
> + }
> +
> + sharpsl_nand_parts = kzalloc(sizeof(*sharpsl_nand_parts) *
> + SHARPSL_NAND_PARTS, GFP_KERNEL);
> + if (!sharpsl_nand_parts)
> + return -ENOMEM;
> +
> + /* original names */
> + sharpsl_nand_parts[0].name = "smf";
> + sharpsl_nand_parts[0].offset = buf1[0].start;
> + sharpsl_nand_parts[0].size = buf1[0].end - buf1[0].start;
Want to sanity check that 'end > start'?
> + sharpsl_nand_parts[0].mask_flags = 0;
Don't need to zero this out, when you've used kzalloc(). Same comments
apply below.
> +
> + sharpsl_nand_parts[1].name = "root";
> + sharpsl_nand_parts[1].offset = buf1[1].start;
> + sharpsl_nand_parts[1].size = buf1[1].end - buf1[1].start;
> + sharpsl_nand_parts[1].mask_flags = 0;
> +
> + sharpsl_nand_parts[2].name = "home";
> + sharpsl_nand_parts[2].offset = buf1[2].start;
> + /* discard buf1[2].end, was for older models with 64M flash */
> + sharpsl_nand_parts[2].size = master->size - buf1[2].start;
> + sharpsl_nand_parts[2].mask_flags = 0;
> +
> + *pparts = sharpsl_nand_parts;
> + return SHARPSL_NAND_PARTS;
> +}
> +
> +static struct mtd_part_parser sharpsl_mtd_parser = {
> + .parse_fn = sharpsl_parse_mtd_partitions,
> + .name = "sharpslpart",
> +};
> +module_mtd_part_parser(sharpsl_mtd_parser);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Andrea Adami <andrea.adami@gmail.com>");
> +MODULE_DESCRIPTION("MTD partitioning for NAND flash on Sharp SL Series");
Brian
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD partition parser
2017-06-09 1:30 ` Brian Norris
@ 2017-06-20 8:52 ` Andrea Adami
2017-06-20 22:05 ` Brian Norris
0 siblings, 1 reply; 22+ messages in thread
From: Andrea Adami @ 2017-06-20 8:52 UTC (permalink / raw)
To: Brian Norris
Cc: linux-mtd, David Woodhouse, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Brian,
thanks for the review and for the unvaluable advices.
I have almost fixed the problems you spotted and I am almost ready to
send the new v4 of the patchset.
Some doubts are still present: I'll comment under your remarks:
On Fri, Jun 9, 2017 at 3:30 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> Hi,
>
> On Thu, Jun 01, 2017 at 12:40:50AM +0200, Andrea Adami wrote:
>> The Sharp SL Series (Zaurus) PXA handhelds have 16/64/128M of NAND flash
>> and share the same layout of the first 7M partition, managed by Sharp FTL.
>>
>> The purpose of this self-contained patch is to add a common parser and
>> remove the hardcoded sizes in the board files (these devices are not yet
>> converted to devicetree).
>> Users will have benefits because the mtdparts= tag will not be necessary
>> anymore and they will be free to repartition the little sized flash.
>>
>> The obsolete bootloader can not pass the partitioning info to modern
>> kernels anymore so it has to be read from flash at known logical addresses.
>> (see http://www.h5.dion.ne.jp/~rimemoon/zaurus/memo_006.htm )
>>
>> In kernel, under arch/arm/mach-pxa we have already 8 machines:
>> MACH_POODLE, MACH_CORGI, MACH_SHEPERD, MACH_HUSKY, MACH_AKITA, MACH_SPITZ,
>> MACH_BORZOI, MACH_TOSA.
>> Lost after the 2.4 vendor kernel are MACH_BOXER and MACH_TERRIER.
>>
>> Almost every model has different factory partitioning: add to this the
>> units can be repartitioned by users with userspace tools (nandlogical)
>> and installers for popular (back then) linux distributions.
>>
>> The Parameter Area in the first (boot) partition extends from 0x00040000 to
>> 0x0007bfff (176k) and contains two copies of the partition table:
>> ...
>> 0x00060000: Partition Info1 16k
>> 0x00064000: Partition Info2 16k
>> 0x00668000: Model 16k
>> ...
>>
>> The first 7M partition is managed by the Sharp FTL reserving 5% + 1 blocks
>> for wear-leveling: some blocks are remapped and one layer of translation
>> (logical to physical) is necessary.
>>
>> There isn't much documentation about this FTL in the 2.4 sources, just the
>> MTD methods for reading and writing using logical addresses and the block
>> management (wear-leveling, use counter).
>> For the purpose of the MTD parser only the read part of the code was taken.
>>
>> The NAND drivers that can use this parser are sharpsl.c and tmio_nand.c.
>>
>> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
>> ---
>> drivers/mtd/Kconfig | 8 ++
>> drivers/mtd/Makefile | 2 +
>> drivers/mtd/sharpsl_ftl.c | 216 ++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/mtd/sharpsl_ftl.h | 34 ++++++++
>> drivers/mtd/sharpslpart.c | 132 ++++++++++++++++++++++++++++
>> 5 files changed, 392 insertions(+)
>> create mode 100644 drivers/mtd/sharpsl_ftl.c
>> create mode 100644 drivers/mtd/sharpsl_ftl.h
>> create mode 100644 drivers/mtd/sharpslpart.c
>>
>> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
>> index e83a279..5c96127 100644
>> --- a/drivers/mtd/Kconfig
>> +++ b/drivers/mtd/Kconfig
>> @@ -155,6 +155,14 @@ config MTD_BCM47XX_PARTS
>> This provides partitions parser for devices based on BCM47xx
>> boards.
>>
>> +config MTD_SHARPSL_PARTS
>> + tristate "Sharp SL Series NAND flash partition parser"
>> + depends on MTD_NAND_SHARPSL || MTD_NAND_TMIO
>
> Please include '|| COMPILE_TEST'.
>
>> + help
>> + This provides the read-only FTL logic necessary to read the partition
>> + table from the NAND flash of Sharp SL Series (Zaurus) and the MTD
>> + partition parser using this code.
>> +
>> comment "User Modules And Translation Layers"
>>
>> #
>> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
>> index 99bb9a1..89f707b 100644
>> --- a/drivers/mtd/Makefile
>> +++ b/drivers/mtd/Makefile
>> @@ -13,6 +13,8 @@ obj-$(CONFIG_MTD_AFS_PARTS) += afs.o
>> obj-$(CONFIG_MTD_AR7_PARTS) += ar7part.o
>> obj-$(CONFIG_MTD_BCM63XX_PARTS) += bcm63xxpart.o
>> obj-$(CONFIG_MTD_BCM47XX_PARTS) += bcm47xxpart.o
>> +obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpsl-part.o
>> +sharpsl-part-objs := sharpsl_ftl.o sharpslpart.o
>>
>> # 'Users' - code which presents functionality to userspace.
>> obj-$(CONFIG_MTD_BLKDEVS) += mtd_blkdevs.o
>> diff --git a/drivers/mtd/sharpsl_ftl.c b/drivers/mtd/sharpsl_ftl.c
>> new file mode 100644
>> index 0000000..1796d03
>> --- /dev/null
>> +++ b/drivers/mtd/sharpsl_ftl.c
>> @@ -0,0 +1,216 @@
>> +/*
>> + * MTD method for NAND accessing via logical address (SHARP FTL)
>> + *
>> + * Copyright (C) 2017 Andrea Adami <andrea.adami@gmail.com>
>> + *
>> + * Based on 2.4 sources: drivers/mtd/nand/sharp_sl_logical.c
>> + * Copyright (C) 2002 SHARP
>> + *
>> + * 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
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/partitions.h>
>> +#include "sharpsl_ftl.h"
>> +
>> +/* oob structure */
>> +#define NAND_NOOB_LOGADDR_00 8
>> +#define NAND_NOOB_LOGADDR_01 9
>> +#define NAND_NOOB_LOGADDR_10 10
>> +#define NAND_NOOB_LOGADDR_11 11
>> +#define NAND_NOOB_LOGADDR_20 12
>> +#define NAND_NOOB_LOGADDR_21 13
>> +
>> +/* Logical Table */
>> +struct mtd_logical {
>> + u32 size; /* size of the handled partition */
>> + int index; /* mtd->index */
>> + u_int phymax; /* physical blocks */
>> + u_int logmax; /* logical blocks */
>> + u_int *log2phy; /* the logical-to-physical table */
>> +};
>> +
>> +static struct mtd_logical *sharpsl_mtd_logical;
>
> Please do not use a static variable like this. References should be
> dynamic.
>
> Can the pointer just be passed back to the caller, and passed back to
> the cleanup function when finished?
>
I'll fix this.
> Related: do you foresee this parsing code being useful for anything
> besides partitions? If not, then it seems like we should just merge the
> "ftl" and "part" files. Not a requirement, but it might be cleaner.
>
I think it makes sense to merge back together.
It was earlier split to better expose the read-only FTL.
>> +
>> +static int sharpsl_nand_read_oob(struct mtd_info *mtd, loff_t offs, size_t len,
>> + uint8_t *buf)
>> +{
>> + loff_t mask = mtd->writesize - 1;
>> + struct mtd_oob_ops ops;
>> + int ret;
>> +
>> + ops.mode = MTD_OPS_PLACE_OOB;
>> + ops.ooboffs = offs & mask;
>> + ops.ooblen = len;
>> + ops.oobbuf = buf;
>> + ops.datbuf = NULL;
>> +
>> + ret = mtd_read_oob(mtd, offs & ~mask, &ops);
>> + if (ret != 0 || len != ops.oobretlen)
>> + return -1;
>> +
>> + return 0;
>> +}
>> +
>> +static u_int sharpsl_nand_get_logical_num(u_char *oob)
>> +{
>> + u16 us;
>> + int good0, good1;
>> +
>> + if (oob[NAND_NOOB_LOGADDR_00] == oob[NAND_NOOB_LOGADDR_10] &&
>> + oob[NAND_NOOB_LOGADDR_01] == oob[NAND_NOOB_LOGADDR_11]) {
>> + good0 = NAND_NOOB_LOGADDR_00;
>> + good1 = NAND_NOOB_LOGADDR_01;
>> + } else if (oob[NAND_NOOB_LOGADDR_10] == oob[NAND_NOOB_LOGADDR_20] &&
>> + oob[NAND_NOOB_LOGADDR_11] == oob[NAND_NOOB_LOGADDR_21]) {
>> + good0 = NAND_NOOB_LOGADDR_10;
>> + good1 = NAND_NOOB_LOGADDR_11;
>> + } else if (oob[NAND_NOOB_LOGADDR_20] == oob[NAND_NOOB_LOGADDR_00] &&
>> + oob[NAND_NOOB_LOGADDR_21] == oob[NAND_NOOB_LOGADDR_01]) {
>> + good0 = NAND_NOOB_LOGADDR_20;
>> + good1 = NAND_NOOB_LOGADDR_21;
>> + } else {
>> + return UINT_MAX;
>> + }
>
> These seems like a pretty weird form of error protection. IIUC, there
> are just 3 copies of the 2-byte sequence number, and we take the first
> pair that matches at least one of the others? Could use a comment above
> the if/else block, to explain what the logic is. Or perhaps even some
> comments at the top of the file, to roughly describe this FTL.
>
I am sorry I don't know much about error correction.
No info, no comments in the 2.4 sources.
My tests did indeed show that only the first condition is matched
(good0=8 good1=9) so I'll retry and then I'll simplify the code..
>> +
>> + us = oob[good0] | oob[good1] << 8;
>> +
>> + /* parity check */
>> + if (hweight16(us) & 1)
>> + return (UINT_MAX - 1);
>> +
>> + /* reserved */
>> + if (us == 0xffff)
>
> Magic number should get a descriptive macro?
>
done (BLOCK_IS_RESERVED)
>> + return 0xffff;
>
> What does this mean? IIUC, this is essentially going to be an
> out-of-bounds value that gets ignored, right? Should this just be
> 'return UINT_MAX'?
>
Done.
it is the same with the simplified code we are using.
If you look in the 2.4 sources there was a further check to verify the usage [1]
[ 1] https://github.com/LinuxPDA/Sharp_FTL_2.4.20/blob/master/linux/drivers/mtd/nand/sharp_sl_logical.c#L453
>> + else
>> + return (us & 0x07fe) >> 1;
>
> 0x07fe could use a macro definition. (And since it seems complementary,
> I guess the 1 in 'hweight16(us) & 1' should also be a macro.)
>
Done
BLOCK_UNMASK 0x07fe
BLOCK_UNMASK_COMPLEMENT 1
>> +}
>> +
>> +int sharpsl_nand_init_logical(struct mtd_info *mtd, u32 partition_size)
>> +{
>> + struct mtd_logical *logical = NULL;
>
> You don't need the NULL initialization.
Fixed (all these are remnants of the 2.4 conversion...)
>
>> + u_int block_num, log_num;
>> + loff_t block_adr;
>> + u_char *oob = NULL;
>
> Same here.
>
Done
>> + int i, readretry;
>> +
>> + logical = kzalloc(sizeof(*logical), GFP_KERNEL);
>> + if (!logical)
>> + return -ENOMEM;
>> +
>> + oob = kzalloc(mtd->oobsize, GFP_KERNEL);
>> + if (!oob) {
>> + kfree(logical);
>> + return -ENOMEM;
>> + }
>> +
>> + /* initialize management structure */
>> + logical->size = partition_size;
>> + logical->index = mtd->index;
>> + logical->phymax = (partition_size / mtd->erasesize);
>> +
>> + /* FTL reserves 5% of the blocks + 1 spare */
>> + logical->logmax = ((logical->phymax * 95) / 100) - 1;
>> +
>> + logical->log2phy = NULL;
>> + logical->log2phy = kcalloc(logical->logmax, sizeof(u_int), GFP_KERNEL);
>> + if (!logical->log2phy) {
>> + kfree(logical);
>> + kfree(oob);
>> + return -ENOMEM;
>> + }
>> +
>> + /* initialize logical->log2phy */
>> + for (i = 0; i < logical->logmax; i++)
>> + logical->log2phy[i] = UINT_MAX;
>
> The 0-initialized kcalloc() is a little superfluous, since you reinit
> here. I guess it's a matter of taste.
>
Well, right, my bad.
The original code [2] did kmalloc() + memset so checkpatch suggested
to use kcalloc().
But the simplified code we are using does not need it (we ignore 'usage').
I'll change back to kmalloc() and initialize to UINT_MAX.
[2] https://github.com/LinuxPDA/Sharp_FTL_2.4.20/blob/master/linux/drivers/mtd/nand/sharp_sl_logical.c#L395
>> +
>> + /* create physical-logical table */
>> + for (block_num = 0; block_num < logical->phymax; block_num++) {
>> + block_adr = block_num * mtd->erasesize;
>> +
>> + if (mtd_block_isbad(mtd, block_adr))
>> + continue;
>> +
>> + readretry = 3;
>> +read_retry:
>> + if (sharpsl_nand_read_oob(mtd, block_adr, mtd->oobsize, oob))
>> + continue;
>> +
>> + /* get logical block */
>> + log_num = sharpsl_nand_get_logical_num(oob);
>> +
>> + /* skip out of range and not unique values */
>> + if ((int)log_num >= 0 && (log_num < logical->logmax)) {
>
> Why the (int) cast? You should already be eliminating anything larger
> than INT_MAX, because they'll be larger than logmax. You should only
> need the second comparison.
>
Fixed both.
The cast is another remnant of the (many) checks done in the old sources.
My bad I have let some superfluous code during conversion....
>> + if (logical->log2phy[log_num] == UINT_MAX)
>> + logical->log2phy[log_num] = block_num;
>> + } else {
>> + readretry--;
>> + if (readretry)
>> + goto read_retry;
>
> What's the idea on the retries? Is this somehow supposed to make NAND
> more reliable? That seems unlikely to work... Although notably, OOB is
> often not covered by ECC, so maybe this is a really poor attempt at
> making up for that?
>
:)
Yes, it seems poor-man's solution but is indeed in the 2.4 sources.
Not needed (tested). Removed.
>> + }
>> + }
>> + kfree(oob);
>> + sharpsl_mtd_logical = logical;
>> +
>> + pr_info("Sharp SL FTL: %d blocks used (%d logical, %d reserved)\n",
>> + logical->phymax, logical->logmax,
>> + logical->phymax - logical->logmax);
>> +
>> + return 0;
>> +}
>> +
>> +void sharpsl_nand_cleanup_logical(void)
>> +{
>> + struct mtd_logical *logical = sharpsl_mtd_logical;
>
> Please make this take a pointer arg, instead of relying on a static var.
>
Sure, as above I'll avoid the static var.
>> +
>> + sharpsl_mtd_logical = NULL;
>> +
>> + kfree(logical->log2phy);
>> + logical->log2phy = NULL;
>> + kfree(logical);
>> + logical = NULL;
>> +}
>> +
>> +int sharpsl_nand_read_laddr(struct mtd_info *mtd,
>> + loff_t from,
>> + size_t len,
>> + u_char *buf)
>> +{
>> + struct mtd_logical *logical;
>> + u_int log_num, log_new;
>> + u_int block_num;
>> + loff_t block_adr;
>> + loff_t block_ofs;
>> + size_t retlen;
>> + int ret;
>> +
>> + logical = sharpsl_mtd_logical;
>> + log_num = (u32)from / mtd->erasesize;
>> + log_new = ((u32)from + len - 1) / mtd->erasesize;
>> +
>> + if (len <= 0 || log_num >= logical->logmax || log_new > log_num)
>> + return -EINVAL;
>> +
>> + block_num = logical->log2phy[log_num];
>> + block_adr = block_num * mtd->erasesize;
>> + block_ofs = (u32)from % mtd->erasesize;
>> +
>> + ret = mtd_read(mtd, block_adr + block_ofs, len, &retlen, buf);
>> + if (ret != 0 || len != retlen)
>
> What about ret == -EUCLEAN? Do you want to handle corrected bitflips?
>
I have changed the checks here and have taken the code of mtdtest_read()
so to ignorer bitflips and pass the error through.
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> diff --git a/drivers/mtd/sharpsl_ftl.h b/drivers/mtd/sharpsl_ftl.h
>> new file mode 100644
>> index 0000000..2880cbe
>> --- /dev/null
>> +++ b/drivers/mtd/sharpsl_ftl.h
>> @@ -0,0 +1,34 @@
>> +/*
>> + * Header file for NAND accessing via logical address (SHARP FTL)
>> + *
>> + * Copyright (C) 2017 Andrea Adami <andrea.adami@gmail.com>
>> + *
>> + * Based on 2.4 sources: linux/include/asm-arm/sharp_nand_logical.h
>> + * Copyright (C) 2002 SHARP
>> + *
>> + * 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
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#ifndef __SHARPSL_NAND_LOGICAL_H__
>> +#define __SHARPSL_NAND_LOGICAL_H__
>> +
>> +#include <linux/types.h>
>> +#include <linux/mtd/mtd.h>
>> +
>> +int sharpsl_nand_init_logical(struct mtd_info *mtd, u32 partition_size);
>> +
>> +void sharpsl_nand_cleanup_logical(void);
>> +
>> +int sharpsl_nand_read_laddr(struct mtd_info *mtd, loff_t from, size_t len,
>> + u_char *buf);
>> +
>> +#endif
>> diff --git a/drivers/mtd/sharpslpart.c b/drivers/mtd/sharpslpart.c
>> new file mode 100644
>> index 0000000..40aebe9
>> --- /dev/null
>> +++ b/drivers/mtd/sharpslpart.c
>> @@ -0,0 +1,132 @@
>> +/*
>> + * MTD partition parser for NAND flash on Sharp SL Series
>> + *
>> + * Copyright (C) 2017 Andrea Adami <andrea.adami@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
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/module.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/partitions.h>
>> +#include "sharpsl_ftl.h"
>> +
>> +/* factory defaults */
>> +#define SHARPSL_NAND_PARTS 3
>> +#define SHARPSL_FTL_PARTITION_SIZE (7 * 1024 * 1024)
>> +#define PARAM_BLOCK_PARTITIONINFO1 0x00060000
>> +#define PARAM_BLOCK_PARTITIONINFO2 0x00064000
>> +
>> +#define BOOT_MAGIC be32_to_cpu(0x424f4f54) /* BOOT */
>> +#define FSRO_MAGIC be32_to_cpu(0x4653524f) /* FSRO */
>> +#define FSRW_MAGIC be32_to_cpu(0x46535257) /* FSRW */
>> +
>> +/*
>> + * Sample values read from SL-C860
>> + *
>> + * # cat /proc/mtd
>> + * dev: size erasesize name
>> + * mtd0: 006d0000 00020000 "Filesystem"
>> + * mtd1: 00700000 00004000 "smf"
>> + * mtd2: 03500000 00004000 "root"
>> + * mtd3: 04400000 00004000 "home"
>> + *
>> + * PARTITIONINFO1
>> + * 0x00060000: 00 00 00 00 00 00 70 00 42 4f 4f 54 00 00 00 00 ......p.BOOT....
>> + * 0x00060010: 00 00 70 00 00 00 c0 03 46 53 52 4f 00 00 00 00 ..p.....FSRO....
>> + * 0x00060020: 00 00 c0 03 00 00 00 04 46 53 52 57 00 00 00 00 ........FSRW....
>> + * 0x00060030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
>> + *
>> + */
>> +
>> +struct sharpsl_nand_partitioninfo {
>> + u32 start;
>> + u32 end;
>> + u32 magic;
>> + u32 reserved;
>> +};
>> +
>> +static int sharpsl_parse_mtd_partitions(struct mtd_info *master,
>> + const struct mtd_partition **pparts,
>> + struct mtd_part_parser_data *data)
>> +{
>> + struct sharpsl_nand_partitioninfo buf1[SHARPSL_NAND_PARTS];
>> + struct sharpsl_nand_partitioninfo buf2[SHARPSL_NAND_PARTS];
>> + struct mtd_partition *sharpsl_nand_parts;
>> +
>> + /* init logical mgmt (FTL) */
>
> Do you have any kind of validation logic, to ensure that this parser
> actually matches? What if the flash was erased? Or what if somebody
> managed to reformat to some other flash layout?
>
The bootloader loads and launches the zImage1 found at a specific
logical address.
If mtd1 were invalid there would be no boot.
See, 10yrs ago there was an unofficial u-boot port for experienced users.
That was requiring to full-erase the nand, so removing the maintenance
and diag code
This would be the only case of modified partition layout I can think of.
> I suppose for many cases like that, the logical mapping will turn up
> with a lot of UINT_MAX entries, and then sharpsl_nand_read_laddr() will
> return -EINVAL? It'd be nice if there were some more clear sanity
> checks though, if possible. Does this FTL have a header we should be
> looking for?
I expect -EINVAL in that case.
BUT if one would be usingh u-boot then he'd have partitions in
u-boot.env or in cmdline.
The (infamous) header is copied here:
https://github.com/LinuxPDA/Sharp_FTL_2.4.20/blob/master/linux/drivers/mtd/nand/sharp_sl_logical.c
Full sources available at
http://support.ezaurus.com/developer/source/source_dl.asp
>
>> + if (sharpsl_nand_init_logical(master, SHARPSL_FTL_PARTITION_SIZE))
>> + return -EINVAL;
>> +
>> + /* read the two partition tables */
>> + if (sharpsl_nand_read_laddr(master,
>> + PARAM_BLOCK_PARTITIONINFO1,
>> + sizeof(buf1), (u_char *)&buf1) ||
>> + sharpsl_nand_read_laddr(master,
>> + PARAM_BLOCK_PARTITIONINFO2,
>> + sizeof(buf2), (u_char *)&buf2))
>> + return -EINVAL;
>
> You've failed to cleanup in the error case here:
> sharpsl_nand_cleanup_logical()
>
Argh, fixed.
> Also, consider passing through the actual error code?
>
Yes, done,
>> +
>> + /* cleanup logical mgmt (FTL) */
>> + sharpsl_nand_cleanup_logical();
>> +
>> + /* compare the two buffers */
>> + if (memcmp(&buf1, &buf2, sizeof(buf1))) {
>> + pr_err("sharpslpart: PARTITIONINFO 1,2 differ. Quit parser.\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* check for magics (just in the first) */
>> + if (buf1[0].magic != BOOT_MAGIC ||
>> + buf1[1].magic != FSRO_MAGIC ||
>> + buf1[2].magic != FSRW_MAGIC) {
>> + pr_err("sharpslpart: magic values mismatch. Quit parser.\n");
>> + return -EINVAL;
>> + }
>> +
>> + sharpsl_nand_parts = kzalloc(sizeof(*sharpsl_nand_parts) *
>> + SHARPSL_NAND_PARTS, GFP_KERNEL);
>> + if (!sharpsl_nand_parts)
>> + return -ENOMEM;
>> +
>> + /* original names */
>> + sharpsl_nand_parts[0].name = "smf";
>> + sharpsl_nand_parts[0].offset = buf1[0].start;
>> + sharpsl_nand_parts[0].size = buf1[0].end - buf1[0].start;
>
> Want to sanity check that 'end > start'?
>
Done, I have added a sanity check.
>> + sharpsl_nand_parts[0].mask_flags = 0;
>
> Don't need to zero this out, when you've used kzalloc(). Same comments
> apply below.
>
Yes, fixed.
(I have copied without much thinking from ar7part.c)
>> +
>> + sharpsl_nand_parts[1].name = "root";
>> + sharpsl_nand_parts[1].offset = buf1[1].start;
>> + sharpsl_nand_parts[1].size = buf1[1].end - buf1[1].start;
>> + sharpsl_nand_parts[1].mask_flags = 0;
>> +
>> + sharpsl_nand_parts[2].name = "home";
>> + sharpsl_nand_parts[2].offset = buf1[2].start;
>> + /* discard buf1[2].end, was for older models with 64M flash */
>> + sharpsl_nand_parts[2].size = master->size - buf1[2].start;
>> + sharpsl_nand_parts[2].mask_flags = 0;
>> +
>> + *pparts = sharpsl_nand_parts;
>> + return SHARPSL_NAND_PARTS;
>> +}
>> +
>> +static struct mtd_part_parser sharpsl_mtd_parser = {
>> + .parse_fn = sharpsl_parse_mtd_partitions,
>> + .name = "sharpslpart",
>> +};
>> +module_mtd_part_parser(sharpsl_mtd_parser);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Andrea Adami <andrea.adami@gmail.com>");
>> +MODULE_DESCRIPTION("MTD partitioning for NAND flash on Sharp SL Series");
>
> Brian
Thanks for your time.
Regards
Andrea
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD partition parser
2017-06-20 8:52 ` Andrea Adami
@ 2017-06-20 22:05 ` Brian Norris
0 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-06-20 22:05 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Hi,
On Tue, Jun 20, 2017 at 10:52:44AM +0200, Andrea Adami wrote:
> Brian,
> thanks for the review and for the unvaluable advices.
>
> I have almost fixed the problems you spotted and I am almost ready to
> send the new v4 of the patchset.
Great!
> Some doubts are still present: I'll comment under your remarks:
Perfect. I've trimmed, and responded to a few things. Let me know if you
hvae more questions.
> On Fri, Jun 9, 2017 at 3:30 AM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > Hi,
> >
> > On Thu, Jun 01, 2017 at 12:40:50AM +0200, Andrea Adami wrote:
> >> The Sharp SL Series (Zaurus) PXA handhelds have 16/64/128M of NAND flash
> >> and share the same layout of the first 7M partition, managed by Sharp FTL.
> >>
> >> The purpose of this self-contained patch is to add a common parser and
> >> remove the hardcoded sizes in the board files (these devices are not yet
> >> converted to devicetree).
> >> Users will have benefits because the mtdparts= tag will not be necessary
> >> anymore and they will be free to repartition the little sized flash.
> >>
> >> The obsolete bootloader can not pass the partitioning info to modern
> >> kernels anymore so it has to be read from flash at known logical addresses.
> >> (see http://www.h5.dion.ne.jp/~rimemoon/zaurus/memo_006.htm )
> >>
> >> In kernel, under arch/arm/mach-pxa we have already 8 machines:
> >> MACH_POODLE, MACH_CORGI, MACH_SHEPERD, MACH_HUSKY, MACH_AKITA, MACH_SPITZ,
> >> MACH_BORZOI, MACH_TOSA.
> >> Lost after the 2.4 vendor kernel are MACH_BOXER and MACH_TERRIER.
> >>
> >> Almost every model has different factory partitioning: add to this the
> >> units can be repartitioned by users with userspace tools (nandlogical)
> >> and installers for popular (back then) linux distributions.
> >>
> >> The Parameter Area in the first (boot) partition extends from 0x00040000 to
> >> 0x0007bfff (176k) and contains two copies of the partition table:
> >> ...
> >> 0x00060000: Partition Info1 16k
> >> 0x00064000: Partition Info2 16k
> >> 0x00668000: Model 16k
> >> ...
> >>
> >> The first 7M partition is managed by the Sharp FTL reserving 5% + 1 blocks
> >> for wear-leveling: some blocks are remapped and one layer of translation
> >> (logical to physical) is necessary.
> >>
> >> There isn't much documentation about this FTL in the 2.4 sources, just the
> >> MTD methods for reading and writing using logical addresses and the block
> >> management (wear-leveling, use counter).
> >> For the purpose of the MTD parser only the read part of the code was taken.
> >>
> >> The NAND drivers that can use this parser are sharpsl.c and tmio_nand.c.
> >>
> >> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> >> ---
> >> drivers/mtd/Kconfig | 8 ++
> >> drivers/mtd/Makefile | 2 +
> >> drivers/mtd/sharpsl_ftl.c | 216 ++++++++++++++++++++++++++++++++++++++++++++++
> >> drivers/mtd/sharpsl_ftl.h | 34 ++++++++
> >> drivers/mtd/sharpslpart.c | 132 ++++++++++++++++++++++++++++
> >> 5 files changed, 392 insertions(+)
> >> create mode 100644 drivers/mtd/sharpsl_ftl.c
> >> create mode 100644 drivers/mtd/sharpsl_ftl.h
> >> create mode 100644 drivers/mtd/sharpslpart.c
> >>
...
> >> diff --git a/drivers/mtd/sharpsl_ftl.c b/drivers/mtd/sharpsl_ftl.c
> >> new file mode 100644
> >> index 0000000..1796d03
> >> --- /dev/null
> >> +++ b/drivers/mtd/sharpsl_ftl.c
> >> @@ -0,0 +1,216 @@
...
> >> +static u_int sharpsl_nand_get_logical_num(u_char *oob)
> >> +{
> >> + u16 us;
> >> + int good0, good1;
> >> +
> >> + if (oob[NAND_NOOB_LOGADDR_00] == oob[NAND_NOOB_LOGADDR_10] &&
> >> + oob[NAND_NOOB_LOGADDR_01] == oob[NAND_NOOB_LOGADDR_11]) {
> >> + good0 = NAND_NOOB_LOGADDR_00;
> >> + good1 = NAND_NOOB_LOGADDR_01;
> >> + } else if (oob[NAND_NOOB_LOGADDR_10] == oob[NAND_NOOB_LOGADDR_20] &&
> >> + oob[NAND_NOOB_LOGADDR_11] == oob[NAND_NOOB_LOGADDR_21]) {
> >> + good0 = NAND_NOOB_LOGADDR_10;
> >> + good1 = NAND_NOOB_LOGADDR_11;
> >> + } else if (oob[NAND_NOOB_LOGADDR_20] == oob[NAND_NOOB_LOGADDR_00] &&
> >> + oob[NAND_NOOB_LOGADDR_21] == oob[NAND_NOOB_LOGADDR_01]) {
> >> + good0 = NAND_NOOB_LOGADDR_20;
> >> + good1 = NAND_NOOB_LOGADDR_21;
> >> + } else {
> >> + return UINT_MAX;
> >> + }
> >
> > These seems like a pretty weird form of error protection. IIUC, there
> > are just 3 copies of the 2-byte sequence number, and we take the first
> > pair that matches at least one of the others? Could use a comment above
> > the if/else block, to explain what the logic is. Or perhaps even some
> > comments at the top of the file, to roughly describe this FTL.
> >
>
> I am sorry I don't know much about error correction.
> No info, no comments in the 2.4 sources.
>
> My tests did indeed show that only the first condition is matched
> (good0=8 good1=9) so I'll retry and then I'll simplify the code..
I don't think removing the secondary conditions above is a good idea; it
seems to provide some value as a backup, in case of (for example)
bitflips in the OOB. I was just commenting that it was a little strange,
and that it might be good to write up a comment based on our
understanding of this OOB header format. Either text, or some small
ASCII art. (Along the lines of "logical block number assigned to a
physical block is stored in OOB of the first page, in 3 16-bit copies
layed out as <foo>; in case of errors, we check that a 2 or more of
these copies agree. Reserved values <bar> mean <blah>.")
IOW, your job isn't just to prune down the vendor's code, but to make it
cleaner and clearer to the reader/reviewer.
...
> >> +
> >> + /* create physical-logical table */
> >> + for (block_num = 0; block_num < logical->phymax; block_num++) {
> >> + block_adr = block_num * mtd->erasesize;
> >> +
> >> + if (mtd_block_isbad(mtd, block_adr))
> >> + continue;
> >> +
> >> + readretry = 3;
> >> +read_retry:
> >> + if (sharpsl_nand_read_oob(mtd, block_adr, mtd->oobsize, oob))
> >> + continue;
> >> +
> >> + /* get logical block */
> >> + log_num = sharpsl_nand_get_logical_num(oob);
> >> +
> >> + /* skip out of range and not unique values */
> >> + if ((int)log_num >= 0 && (log_num < logical->logmax)) {
...
> >> + if (logical->log2phy[log_num] == UINT_MAX)
> >> + logical->log2phy[log_num] = block_num;
> >> + } else {
> >> + readretry--;
> >> + if (readretry)
> >> + goto read_retry;
> >
> > What's the idea on the retries? Is this somehow supposed to make NAND
> > more reliable? That seems unlikely to work... Although notably, OOB is
> > often not covered by ECC, so maybe this is a really poor attempt at
> > making up for that?
> >
>
> :)
> Yes, it seems poor-man's solution but is indeed in the 2.4 sources.
> Not needed (tested). Removed.
Caveat: a simple test of one or a few device(s) is not exactly
verification that this wasn't useful at all, but given we can't figure
out a proper reason for it, removing it seems prudent.
> >> + }
> >> + }
> >> + kfree(oob);
> >> + sharpsl_mtd_logical = logical;
> >> +
> >> + pr_info("Sharp SL FTL: %d blocks used (%d logical, %d reserved)\n",
> >> + logical->phymax, logical->logmax,
> >> + logical->phymax - logical->logmax);
> >> +
> >> + return 0;
> >> +}
> >> +
...
> >> +int sharpsl_nand_read_laddr(struct mtd_info *mtd,
> >> + loff_t from,
> >> + size_t len,
> >> + u_char *buf)
> >> +{
> >> + struct mtd_logical *logical;
> >> + u_int log_num, log_new;
> >> + u_int block_num;
> >> + loff_t block_adr;
> >> + loff_t block_ofs;
> >> + size_t retlen;
> >> + int ret;
> >> +
> >> + logical = sharpsl_mtd_logical;
> >> + log_num = (u32)from / mtd->erasesize;
> >> + log_new = ((u32)from + len - 1) / mtd->erasesize;
> >> +
> >> + if (len <= 0 || log_num >= logical->logmax || log_new > log_num)
> >> + return -EINVAL;
> >> +
> >> + block_num = logical->log2phy[log_num];
> >> + block_adr = block_num * mtd->erasesize;
> >> + block_ofs = (u32)from % mtd->erasesize;
> >> +
> >> + ret = mtd_read(mtd, block_adr + block_ofs, len, &retlen, buf);
> >> + if (ret != 0 || len != retlen)
> >
> > What about ret == -EUCLEAN? Do you want to handle corrected bitflips?
> >
>
> I have changed the checks here and have taken the code of mtdtest_read()
> so to ignorer bitflips and pass the error through.
Yes, I suppose that is a fine pattern. You can't really do anything
about high numbers of bitflilps, unless you really want to add in write
support (in which case you could probably try to copy/migrate the table
to an available block).
> >> + return -EINVAL;
> >> +
> >> + return 0;
> >> +}
...
> >> diff --git a/drivers/mtd/sharpslpart.c b/drivers/mtd/sharpslpart.c
> >> new file mode 100644
> >> index 0000000..40aebe9
> >> --- /dev/null
> >> +++ b/drivers/mtd/sharpslpart.c
> >> @@ -0,0 +1,132 @@
> >> +/*
> >> + * MTD partition parser for NAND flash on Sharp SL Series
> >> + *
> >> + * Copyright (C) 2017 Andrea Adami <andrea.adami@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
> >> + * the Free Software Foundation; either version 2 of the License, or
> >> + * (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + */
> >> +
> >> +#include <linux/kernel.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/module.h>
> >> +#include <linux/mtd/mtd.h>
> >> +#include <linux/mtd/partitions.h>
> >> +#include "sharpsl_ftl.h"
> >> +
> >> +/* factory defaults */
> >> +#define SHARPSL_NAND_PARTS 3
> >> +#define SHARPSL_FTL_PARTITION_SIZE (7 * 1024 * 1024)
> >> +#define PARAM_BLOCK_PARTITIONINFO1 0x00060000
> >> +#define PARAM_BLOCK_PARTITIONINFO2 0x00064000
> >> +
> >> +#define BOOT_MAGIC be32_to_cpu(0x424f4f54) /* BOOT */
> >> +#define FSRO_MAGIC be32_to_cpu(0x4653524f) /* FSRO */
> >> +#define FSRW_MAGIC be32_to_cpu(0x46535257) /* FSRW */
> >> +
> >> +/*
> >> + * Sample values read from SL-C860
> >> + *
> >> + * # cat /proc/mtd
> >> + * dev: size erasesize name
> >> + * mtd0: 006d0000 00020000 "Filesystem"
> >> + * mtd1: 00700000 00004000 "smf"
> >> + * mtd2: 03500000 00004000 "root"
> >> + * mtd3: 04400000 00004000 "home"
> >> + *
> >> + * PARTITIONINFO1
> >> + * 0x00060000: 00 00 00 00 00 00 70 00 42 4f 4f 54 00 00 00 00 ......p.BOOT....
> >> + * 0x00060010: 00 00 70 00 00 00 c0 03 46 53 52 4f 00 00 00 00 ..p.....FSRO....
> >> + * 0x00060020: 00 00 c0 03 00 00 00 04 46 53 52 57 00 00 00 00 ........FSRW....
> >> + * 0x00060030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
> >> + *
> >> + */
> >> +
> >> +struct sharpsl_nand_partitioninfo {
> >> + u32 start;
> >> + u32 end;
> >> + u32 magic;
> >> + u32 reserved;
> >> +};
> >> +
> >> +static int sharpsl_parse_mtd_partitions(struct mtd_info *master,
> >> + const struct mtd_partition **pparts,
> >> + struct mtd_part_parser_data *data)
> >> +{
> >> + struct sharpsl_nand_partitioninfo buf1[SHARPSL_NAND_PARTS];
> >> + struct sharpsl_nand_partitioninfo buf2[SHARPSL_NAND_PARTS];
> >> + struct mtd_partition *sharpsl_nand_parts;
> >> +
> >> + /* init logical mgmt (FTL) */
> >
> > Do you have any kind of validation logic, to ensure that this parser
> > actually matches? What if the flash was erased? Or what if somebody
> > managed to reformat to some other flash layout?
> >
>
> The bootloader loads and launches the zImage1 found at a specific
> logical address.
> If mtd1 were invalid there would be no boot.
>
> See, 10yrs ago there was an unofficial u-boot port for experienced users.
> That was requiring to full-erase the nand, so removing the maintenance
> and diag code
> This would be the only case of modified partition layout I can think of.
My point wasn't really just for well-handled devices, or even Sharp
devices at all. What if this partition parser ends up in the parser list
for some other random device? e.g., when we add better device tree
support for other parsers, it'll start becoming easier for arbitrary
platforms to utilize arbitrary parsers; I wouldn't expect people to want
to use this parser, but if we have a quick way to say "this doesn't
match, skip me", then that would be helpful.
Or maybe another case: what if someone utilizes some completely
different partition layout (and gets their bootloader to handle it) but
is still otherwise using the same kernel/platform support? We don't want
to waste too much time scanning for this partition table if possible.
> > I suppose for many cases like that, the logical mapping will turn up
> > with a lot of UINT_MAX entries, and then sharpsl_nand_read_laddr() will
> > return -EINVAL? It'd be nice if there were some more clear sanity
> > checks though, if possible. Does this FTL have a header we should be
> > looking for?
>
> I expect -EINVAL in that case.
> BUT if one would be usingh u-boot then he'd have partitions in
> u-boot.env or in cmdline.
>
> The (infamous) header is copied here:
>
> https://github.com/LinuxPDA/Sharp_FTL_2.4.20/blob/master/linux/drivers/mtd/nand/sharp_sl_logical.c
That doesn't seem to have much more that can help us. Maybe your current
version is the best we can do.
> Full sources available at
> http://support.ezaurus.com/developer/source/source_dl.asp
>
> >
> >> + if (sharpsl_nand_init_logical(master, SHARPSL_FTL_PARTITION_SIZE))
> >> + return -EINVAL;
> >> +
> >> + /* read the two partition tables */
> >> + if (sharpsl_nand_read_laddr(master,
> >> + PARAM_BLOCK_PARTITIONINFO1,
> >> + sizeof(buf1), (u_char *)&buf1) ||
> >> + sharpsl_nand_read_laddr(master,
> >> + PARAM_BLOCK_PARTITIONINFO2,
> >> + sizeof(buf2), (u_char *)&buf2))
> >> + return -EINVAL;
...
Brian
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD partition parser
2017-05-31 22:40 ` [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD " Andrea Adami
2017-06-09 1:30 ` Brian Norris
@ 2017-06-09 1:42 ` Brian Norris
2017-06-20 8:56 ` Andrea Adami
1 sibling, 1 reply; 22+ messages in thread
From: Brian Norris @ 2017-06-09 1:42 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Oh, one more thing:
On Thu, Jun 01, 2017 at 12:40:50AM +0200, Andrea Adami wrote:
> diff --git a/drivers/mtd/sharpslpart.c b/drivers/mtd/sharpslpart.c
> new file mode 100644
> index 0000000..40aebe9
> --- /dev/null
> +++ b/drivers/mtd/sharpslpart.c
> @@ -0,0 +1,132 @@
> +/*
> + * MTD partition parser for NAND flash on Sharp SL Series
> + *
> + * Copyright (C) 2017 Andrea Adami <andrea.adami@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
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include "sharpsl_ftl.h"
> +
> +/* factory defaults */
> +#define SHARPSL_NAND_PARTS 3
> +#define SHARPSL_FTL_PARTITION_SIZE (7 * 1024 * 1024)
> +#define PARAM_BLOCK_PARTITIONINFO1 0x00060000
> +#define PARAM_BLOCK_PARTITIONINFO2 0x00064000
> +
> +#define BOOT_MAGIC be32_to_cpu(0x424f4f54) /* BOOT */
> +#define FSRO_MAGIC be32_to_cpu(0x4653524f) /* FSRO */
> +#define FSRW_MAGIC be32_to_cpu(0x46535257) /* FSRW */
[...]
> +struct sharpsl_nand_partitioninfo {
> + u32 start;
> + u32 end;
I was going to ignore this earlier, but I just noticed that you handle
endianness for the magic values above, so...
shouldn't you handle endianness for the start and end fields too?
> + u32 magic;
> + u32 reserved;
> +};
[...]
Brian
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD partition parser
2017-06-09 1:42 ` Brian Norris
@ 2017-06-20 8:56 ` Andrea Adami
0 siblings, 0 replies; 22+ messages in thread
From: Andrea Adami @ 2017-06-20 8:56 UTC (permalink / raw)
To: Brian Norris
Cc: linux-mtd, David Woodhouse, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
On Fri, Jun 9, 2017 at 3:42 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> Oh, one more thing:
>
> On Thu, Jun 01, 2017 at 12:40:50AM +0200, Andrea Adami wrote:
>> diff --git a/drivers/mtd/sharpslpart.c b/drivers/mtd/sharpslpart.c
>> new file mode 100644
>> index 0000000..40aebe9
>> --- /dev/null
>> +++ b/drivers/mtd/sharpslpart.c
>> @@ -0,0 +1,132 @@
>> +/*
>> + * MTD partition parser for NAND flash on Sharp SL Series
>> + *
>> + * Copyright (C) 2017 Andrea Adami <andrea.adami@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
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/module.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/partitions.h>
>> +#include "sharpsl_ftl.h"
>> +
>> +/* factory defaults */
>> +#define SHARPSL_NAND_PARTS 3
>> +#define SHARPSL_FTL_PARTITION_SIZE (7 * 1024 * 1024)
>> +#define PARAM_BLOCK_PARTITIONINFO1 0x00060000
>> +#define PARAM_BLOCK_PARTITIONINFO2 0x00064000
>> +
>> +#define BOOT_MAGIC be32_to_cpu(0x424f4f54) /* BOOT */
>> +#define FSRO_MAGIC be32_to_cpu(0x4653524f) /* FSRO */
>> +#define FSRW_MAGIC be32_to_cpu(0x46535257) /* FSRW */
>
> [...]
>
>> +struct sharpsl_nand_partitioninfo {
>> + u32 start;
>> + u32 end;
>
> I was going to ignore this earlier, but I just noticed that you handle
> endianness for the magic values above, so...
>
> shouldn't you handle endianness for the start and end fields too?
>
>> + u32 magic;
>> + u32 reserved;
>> +};
> [...]
>
> Brian
Brian,
I have added a macro so to avoid to repeat le32_to_cpu() for each value.
This seems even to work as intended...
Note that the magics are 'texts' so they appear as BE.
Some more tests and I'll send v4 of the patch to your attention.
Thanks in advance.
Andrea
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 2/9] mtd: nand: sharpsl.h: support partition parser types
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
2017-05-31 22:40 ` [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD " Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-05-31 22:40 ` [PATCH v3 3/9] mfd: tmio.h: " Andrea Adami
` (7 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
include/linux/mtd/sharpsl.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/mtd/sharpsl.h b/include/linux/mtd/sharpsl.h
index 65e91d0..c0e0be2 100644
--- a/include/linux/mtd/sharpsl.h
+++ b/include/linux/mtd/sharpsl.h
@@ -17,4 +17,5 @@ struct sharpsl_nand_platform_data {
const struct mtd_ooblayout_ops *ecc_layout;
struct mtd_partition *partitions;
unsigned int nr_partitions;
+ const char *const *types; /* names of parsers to use if any */
};
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 3/9] mfd: tmio.h: support partition parser types
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
2017-05-31 22:40 ` [PATCH v3 1/9] mtd: sharpslpart: add sharpslpart MTD " Andrea Adami
2017-05-31 22:40 ` [PATCH v3 2/9] mtd: nand: sharpsl.h: support partition parser types Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-06-01 8:40 ` Wolfram Sang
2017-06-01 14:35 ` Lee Jones
2017-05-31 22:40 ` [PATCH v3 4/9] mtd: nand: sharpsl.c: take in account " Andrea Adami
` (6 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
include/linux/mfd/tmio.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index a1520d8..23bb069 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -139,6 +139,7 @@ struct tmio_nand_data {
struct nand_bbt_descr *badblock_pattern;
struct mtd_partition *partition;
unsigned int num_partitions;
+ const char *const *types; /* names of parsers to use if any */
};
#define FBIO_TMIO_ACC_WRITE 0x7C639300
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 3/9] mfd: tmio.h: support partition parser types
2017-05-31 22:40 ` [PATCH v3 3/9] mfd: tmio.h: " Andrea Adami
@ 2017-06-01 8:40 ` Wolfram Sang
2017-06-01 14:35 ` Lee Jones
1 sibling, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2017-06-01 8:40 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Brian Norris, Boris Brezillon,
Marek Vasut, Richard Weinberger, Cyrille Pitchen, Wolfram Sang,
Lee Jones, Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 177 bytes --]
On Thu, Jun 01, 2017 at 12:40:52AM +0200, Andrea Adami wrote:
> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 3/9] mfd: tmio.h: support partition parser types
2017-05-31 22:40 ` [PATCH v3 3/9] mfd: tmio.h: " Andrea Adami
2017-06-01 8:40 ` Wolfram Sang
@ 2017-06-01 14:35 ` Lee Jones
2017-06-03 21:40 ` Andrea Adami
1 sibling, 1 reply; 22+ messages in thread
From: Lee Jones @ 2017-06-01 14:35 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Brian Norris, Boris Brezillon,
Marek Vasut, Richard Weinberger, Cyrille Pitchen, Wolfram Sang,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
On Thu, 01 Jun 2017, Andrea Adami wrote:
> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> ---
> include/linux/mfd/tmio.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index a1520d8..23bb069 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -139,6 +139,7 @@ struct tmio_nand_data {
> struct nand_bbt_descr *badblock_pattern;
> struct mtd_partition *partition;
> unsigned int num_partitions;
> + const char *const *types; /* names of parsers to use if any */
Please use kerneldoc instead.
> };
>
> #define FBIO_TMIO_ACC_WRITE 0x7C639300
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 3/9] mfd: tmio.h: support partition parser types
2017-06-01 14:35 ` Lee Jones
@ 2017-06-03 21:40 ` Andrea Adami
2017-06-05 7:59 ` Lee Jones
0 siblings, 1 reply; 22+ messages in thread
From: Andrea Adami @ 2017-06-03 21:40 UTC (permalink / raw)
To: Lee Jones
Cc: linux-mtd, David Woodhouse, Brian Norris, Boris Brezillon,
Marek Vasut, Richard Weinberger, Cyrille Pitchen, Wolfram Sang,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
On Thu, Jun 1, 2017 at 4:35 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 01 Jun 2017, Andrea Adami wrote:
>
>> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
>> ---
>> include/linux/mfd/tmio.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
>> index a1520d8..23bb069 100644
>> --- a/include/linux/mfd/tmio.h
>> +++ b/include/linux/mfd/tmio.h
>> @@ -139,6 +139,7 @@ struct tmio_nand_data {
>> struct nand_bbt_descr *badblock_pattern;
>> struct mtd_partition *partition;
>> unsigned int num_partitions;
>> + const char *const *types; /* names of parsers to use if any */
>
> Please use kerneldoc instead.
>
Thanks for the review.
tmio.h is not consistent and uses inline comments so I did not change the style.
Another round of patches will be necessary to sanitize the comments,
out of the scope of this little patchset.
Cheers
Andrea
>> };
>>
>> #define FBIO_TMIO_ACC_WRITE 0x7C639300
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 3/9] mfd: tmio.h: support partition parser types
2017-06-03 21:40 ` Andrea Adami
@ 2017-06-05 7:59 ` Lee Jones
0 siblings, 0 replies; 22+ messages in thread
From: Lee Jones @ 2017-06-05 7:59 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Brian Norris, Boris Brezillon,
Marek Vasut, Richard Weinberger, Cyrille Pitchen, Wolfram Sang,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
On Sat, 03 Jun 2017, Andrea Adami wrote:
> On Thu, Jun 1, 2017 at 4:35 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Thu, 01 Jun 2017, Andrea Adami wrote:
> >
> >> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> >> ---
> >> include/linux/mfd/tmio.h | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> >> index a1520d8..23bb069 100644
> >> --- a/include/linux/mfd/tmio.h
> >> +++ b/include/linux/mfd/tmio.h
> >> @@ -139,6 +139,7 @@ struct tmio_nand_data {
> >> struct nand_bbt_descr *badblock_pattern;
> >> struct mtd_partition *partition;
> >> unsigned int num_partitions;
> >> + const char *const *types; /* names of parsers to use if any */
> >
> > Please use kerneldoc instead.
> >
> Thanks for the review.
>
> tmio.h is not consistent and uses inline comments so I did not change the style.
> Another round of patches will be necessary to sanitize the comments,
> out of the scope of this little patchset.
Will you do the work?
> >> };
> >>
> >> #define FBIO_TMIO_ACC_WRITE 0x7C639300
> >
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 4/9] mtd: nand: sharpsl.c: take in account partition parser types
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
` (2 preceding siblings ...)
2017-05-31 22:40 ` [PATCH v3 3/9] mfd: tmio.h: " Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-06-01 8:35 ` Wolfram Sang
2017-05-31 22:40 ` [PATCH v3 5/9] mtd: nand: tmio_nand.c: " Andrea Adami
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
drivers/mtd/nand/sharpsl.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/sharpsl.c b/drivers/mtd/nand/sharpsl.c
index 064ca17..1a76779 100644
--- a/drivers/mtd/nand/sharpsl.c
+++ b/drivers/mtd/nand/sharpsl.c
@@ -183,8 +183,13 @@ static int sharpsl_nand_probe(struct platform_device *pdev)
/* Register the partitions */
mtd->name = "sharpsl-nand";
- err = mtd_device_parse_register(mtd, NULL, NULL,
- data->partitions, data->nr_partitions);
+ err = mtd_device_parse_register(&sharpsl->mtd,
+ data->types ? data->types : NULL,
+ NULL,
+ data->partitions ?
+ data->partitions : NULL,
+ data->nr_partitions ?
+ data->nr_partitions : 0);
if (err)
goto err_add;
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 4/9] mtd: nand: sharpsl.c: take in account partition parser types
2017-05-31 22:40 ` [PATCH v3 4/9] mtd: nand: sharpsl.c: take in account " Andrea Adami
@ 2017-06-01 8:35 ` Wolfram Sang
0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2017-06-01 8:35 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Brian Norris, Boris Brezillon,
Marek Vasut, Richard Weinberger, Cyrille Pitchen, Wolfram Sang,
Lee Jones, Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 264 bytes --]
> + data->types ? data->types : NULL,
This is equal to just 'data_types'.
> + NULL,
> + data->partitions ?
> + data->partitions : NULL,
> + data->nr_partitions ?
> + data->nr_partitions : 0);
All the ternary operators are superfluous.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 5/9] mtd: nand: tmio_nand.c: take in account partition parser types
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
` (3 preceding siblings ...)
2017-05-31 22:40 ` [PATCH v3 4/9] mtd: nand: sharpsl.c: take in account " Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-06-01 8:37 ` Wolfram Sang
2017-05-31 22:40 ` [PATCH v3 6/9] arm: pxa: corgi.c: remove hardcoded partitioning, use sharpslpart parser Andrea Adami
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
drivers/mtd/nand/tmio_nand.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/tmio_nand.c b/drivers/mtd/nand/tmio_nand.c
index fc5e773..bea05bb 100644
--- a/drivers/mtd/nand/tmio_nand.c
+++ b/drivers/mtd/nand/tmio_nand.c
@@ -440,7 +440,9 @@ static int tmio_probe(struct platform_device *dev)
goto err_irq;
/* Register the partitions */
- retval = mtd_device_parse_register(mtd, NULL, NULL,
+ retval = mtd_device_parse_register(mtd,
+ data->types ? data->types : NULL,
+ NULL,
data ? data->partition : NULL,
data ? data->num_partitions : 0);
if (!retval)
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 5/9] mtd: nand: tmio_nand.c: take in account partition parser types
2017-05-31 22:40 ` [PATCH v3 5/9] mtd: nand: tmio_nand.c: " Andrea Adami
@ 2017-06-01 8:37 ` Wolfram Sang
0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2017-06-01 8:37 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Brian Norris, Boris Brezillon,
Marek Vasut, Richard Weinberger, Cyrille Pitchen, Wolfram Sang,
Lee Jones, Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 387 bytes --]
> - retval = mtd_device_parse_register(mtd, NULL, NULL,
> + retval = mtd_device_parse_register(mtd,
> + data->types ? data->types : NULL,
This is superfluous. You want to check against 'data' instead as it is
done below. Otherwis you might get a NULL pointer dereference.
> + NULL,
> data ? data->partition : NULL,
> data ? data->num_partitions : 0);
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 6/9] arm: pxa: corgi.c: remove hardcoded partitioning, use sharpslpart parser
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
` (4 preceding siblings ...)
2017-05-31 22:40 ` [PATCH v3 5/9] mtd: nand: tmio_nand.c: " Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-05-31 22:40 ` [PATCH v3 7/9] arm: pxa: tosa.c: " Andrea Adami
` (3 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
arch/arm/mach-pxa/corgi.c | 31 ++++++++-----------------------
1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/arch/arm/mach-pxa/corgi.c b/arch/arm/mach-pxa/corgi.c
index 7270f0d..2412add 100644
--- a/arch/arm/mach-pxa/corgi.c
+++ b/arch/arm/mach-pxa/corgi.c
@@ -606,24 +606,6 @@ static void __init corgi_init_spi(void)
static inline void corgi_init_spi(void) {}
#endif
-static struct mtd_partition sharpsl_nand_partitions[] = {
- {
- .name = "System Area",
- .offset = 0,
- .size = 7 * 1024 * 1024,
- },
- {
- .name = "Root Filesystem",
- .offset = 7 * 1024 * 1024,
- .size = 25 * 1024 * 1024,
- },
- {
- .name = "Home Filesystem",
- .offset = MTDPART_OFS_APPEND,
- .size = MTDPART_SIZ_FULL,
- },
-};
-
static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
static struct nand_bbt_descr sharpsl_bbt = {
@@ -633,10 +615,16 @@ static struct nand_bbt_descr sharpsl_bbt = {
.pattern = scan_ff_pattern
};
+static const char * const probes[] = {
+ "cmdlinepart",
+ "ofpart",
+ "sharpslpart",
+ NULL,
+};
+
static struct sharpsl_nand_platform_data sharpsl_nand_platform_data = {
.badblock_pattern = &sharpsl_bbt,
- .partitions = sharpsl_nand_partitions,
- .nr_partitions = ARRAY_SIZE(sharpsl_nand_partitions),
+ .types = probes,
};
static struct resource sharpsl_nand_resources[] = {
@@ -750,9 +738,6 @@ static void __init corgi_init(void)
platform_scoop_config = &corgi_pcmcia_config;
- if (machine_is_husky())
- sharpsl_nand_partitions[1].size = 53 * 1024 * 1024;
-
platform_add_devices(devices, ARRAY_SIZE(devices));
regulator_has_full_constraints();
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 7/9] arm: pxa: tosa.c: remove hardcoded partitioning, use sharpslpart parser
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
` (5 preceding siblings ...)
2017-05-31 22:40 ` [PATCH v3 6/9] arm: pxa: corgi.c: remove hardcoded partitioning, use sharpslpart parser Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-05-31 22:40 ` [PATCH v3 8/9] arm: pxa: spitz.c: " Andrea Adami
` (2 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
arch/arm/mach-pxa/tosa.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/arch/arm/mach-pxa/tosa.c b/arch/arm/mach-pxa/tosa.c
index 13de660..3074aae 100644
--- a/arch/arm/mach-pxa/tosa.c
+++ b/arch/arm/mach-pxa/tosa.c
@@ -673,24 +673,6 @@ static int tosa_tc6393xb_suspend(struct platform_device *dev)
return 0;
}
-static struct mtd_partition tosa_nand_partition[] = {
- {
- .name = "smf",
- .offset = 0,
- .size = 7 * 1024 * 1024,
- },
- {
- .name = "root",
- .offset = MTDPART_OFS_APPEND,
- .size = 28 * 1024 * 1024,
- },
- {
- .name = "home",
- .offset = MTDPART_OFS_APPEND,
- .size = MTDPART_SIZ_FULL,
- },
-};
-
static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
static struct nand_bbt_descr tosa_tc6393xb_nand_bbt = {
@@ -700,10 +682,16 @@ static struct nand_bbt_descr tosa_tc6393xb_nand_bbt = {
.pattern = scan_ff_pattern
};
+static const char * const probes[] = {
+ "cmdlinepart",
+ "ofpart",
+ "sharpslpart",
+ NULL,
+};
+
static struct tmio_nand_data tosa_tc6393xb_nand_config = {
- .num_partitions = ARRAY_SIZE(tosa_nand_partition),
- .partition = tosa_nand_partition,
.badblock_pattern = &tosa_tc6393xb_nand_bbt,
+ .types = probes,
};
static int tosa_tc6393xb_setup(struct platform_device *dev)
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 8/9] arm: pxa: spitz.c: remove hardcoded partitioning, use sharpslpart parser
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
` (6 preceding siblings ...)
2017-05-31 22:40 ` [PATCH v3 7/9] arm: pxa: tosa.c: " Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-05-31 22:40 ` [PATCH v3 9/9] arm: pxa: poodle.c: " Andrea Adami
2017-06-09 1:35 ` [PATCH v3 0/9] mtd: sharpslpart partition parser Brian Norris
9 siblings, 0 replies; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
arch/arm/mach-pxa/spitz.c | 34 +++++++++-------------------------
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 67d66c7..21a2e42 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -739,21 +739,6 @@ static inline void spitz_lcd_init(void) {}
* NAND Flash
******************************************************************************/
#if defined(CONFIG_MTD_NAND_SHARPSL) || defined(CONFIG_MTD_NAND_SHARPSL_MODULE)
-static struct mtd_partition spitz_nand_partitions[] = {
- {
- .name = "System Area",
- .offset = 0,
- .size = 7 * 1024 * 1024,
- }, {
- .name = "Root Filesystem",
- .offset = 7 * 1024 * 1024,
- }, {
- .name = "Home Filesystem",
- .offset = MTDPART_OFS_APPEND,
- .size = MTDPART_SIZ_FULL,
- },
-};
-
static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
static struct nand_bbt_descr spitz_nand_bbt = {
@@ -808,10 +793,16 @@ static const struct mtd_ooblayout_ops akita_ooblayout_ops = {
.free = akita_ooblayout_free,
};
+static const char * const probes[] = {
+ "cmdlinepart",
+ "ofpart",
+ "sharpslpart",
+ NULL,
+};
+
static struct sharpsl_nand_platform_data spitz_nand_pdata = {
.badblock_pattern = &spitz_nand_bbt,
- .partitions = spitz_nand_partitions,
- .nr_partitions = ARRAY_SIZE(spitz_nand_partitions),
+ .types = probes,
};
static struct resource spitz_nand_resources[] = {
@@ -834,14 +825,7 @@ static struct platform_device spitz_nand_device = {
static void __init spitz_nand_init(void)
{
- if (machine_is_spitz()) {
- spitz_nand_partitions[1].size = 5 * 1024 * 1024;
- } else if (machine_is_akita()) {
- spitz_nand_partitions[1].size = 58 * 1024 * 1024;
- spitz_nand_bbt.len = 1;
- spitz_nand_pdata.ecc_layout = &akita_ooblayout_ops;
- } else if (machine_is_borzoi()) {
- spitz_nand_partitions[1].size = 32 * 1024 * 1024;
+ if (machine_is_akita() || machine_is_borzoi()) {
spitz_nand_bbt.len = 1;
spitz_nand_pdata.ecc_layout = &akita_ooblayout_ops;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 9/9] arm: pxa: poodle.c: remove hardcoded partitioning, use sharpslpart parser
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
` (7 preceding siblings ...)
2017-05-31 22:40 ` [PATCH v3 8/9] arm: pxa: spitz.c: " Andrea Adami
@ 2017-05-31 22:40 ` Andrea Adami
2017-06-09 1:35 ` [PATCH v3 0/9] mtd: sharpslpart partition parser Brian Norris
9 siblings, 0 replies; 22+ messages in thread
From: Andrea Adami @ 2017-05-31 22:40 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
arch/arm/mach-pxa/poodle.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/arch/arm/mach-pxa/poodle.c b/arch/arm/mach-pxa/poodle.c
index 62a1191..4881a43 100644
--- a/arch/arm/mach-pxa/poodle.c
+++ b/arch/arm/mach-pxa/poodle.c
@@ -333,24 +333,6 @@ static struct pxafb_mach_info poodle_fb_info = {
.lcd_conn = LCD_COLOR_TFT_16BPP,
};
-static struct mtd_partition sharpsl_nand_partitions[] = {
- {
- .name = "System Area",
- .offset = 0,
- .size = 7 * 1024 * 1024,
- },
- {
- .name = "Root Filesystem",
- .offset = 7 * 1024 * 1024,
- .size = 22 * 1024 * 1024,
- },
- {
- .name = "Home Filesystem",
- .offset = MTDPART_OFS_APPEND,
- .size = MTDPART_SIZ_FULL,
- },
-};
-
static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
static struct nand_bbt_descr sharpsl_bbt = {
@@ -360,10 +342,16 @@ static struct nand_bbt_descr sharpsl_bbt = {
.pattern = scan_ff_pattern
};
+static const char * const probes[] = {
+ "cmdlinepart",
+ "ofpart",
+ "sharpslpart",
+ NULL,
+};
+
static struct sharpsl_nand_platform_data sharpsl_nand_platform_data = {
.badblock_pattern = &sharpsl_bbt,
- .partitions = sharpsl_nand_partitions,
- .nr_partitions = ARRAY_SIZE(sharpsl_nand_partitions),
+ .types = probes,
};
static struct resource sharpsl_nand_resources[] = {
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 0/9] mtd: sharpslpart partition parser
2017-05-31 22:40 [PATCH v3 0/9] mtd: sharpslpart partition parser Andrea Adami
` (8 preceding siblings ...)
2017-05-31 22:40 ` [PATCH v3 9/9] arm: pxa: poodle.c: " Andrea Adami
@ 2017-06-09 1:35 ` Brian Norris
9 siblings, 0 replies; 22+ messages in thread
From: Brian Norris @ 2017-06-09 1:35 UTC (permalink / raw)
To: Andrea Adami
Cc: linux-mtd, David Woodhouse, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen, Wolfram Sang, Lee Jones,
Daniel Mack, Haojian Zhuang, Dmitry Eremin-Solenikov,
Robert Jarzmik, Linus Walleij, Russell King, linux-mmc,
linux-kernel, linux-arm-kernel
On Thu, Jun 01, 2017 at 12:40:49AM +0200, Andrea Adami wrote:
> This patchset introduces a simple partition parser for the Sharp SL
> Series PXA handhelds. More details in the commit text.
>
> I have set in cc the ARM PXA maintainers because this is the MTD part of
> a planned wider patchset cleaning the Zaurus board files. The MFD maintainers
> are also in cc (tmio.h change).
>
> Changelog:
> v1 initial import of 2.4 sources [1]
> v2 refactor applying many suggested fixes [2]
> v3 put the partition parser types in the platform data
>
> [1] https://github.com/LinuxPDA/Sharp_FTL_2.4.20
> [2] https://github.com/LinuxPDA/linux/commits/sharpslpart_v2
>
> Andrea Adami (9):
> mtd: sharpslpart: add sharpslpart MTD partition parser
> mtd: nand: sharpsl.h: support partition parser types
> mfd: nand: tmio.h: support partition parser types
> mtd: nand: sharpsl.c: take in account partition parser types
> mtd: nand: tmio_nand.c: take in account partition parser types
> arm: pxa: corgi.c: remove hardcoded partitioning, use sharpslpart
> parser
> arm: pxa: tosa.c: remove hardcoded partitioning, use sharpslpart
> parser
> arm: pxa: spitz.c: remove hardcoded partitioning, use sharpslpart
> parser
> arm: pxa: poodle.c: remove hardcoded partitioning, use sharpslpart
> parser
Barring my comments on patch 1 and Wolfram's comments on a few of the
others, for the whole series:
Reviewed-by: Brian Norris <computersforpeace@gmail.com>
Once things are fixed up, this should probably go through MTD.
Brian
^ permalink raw reply [flat|nested] 22+ messages in thread