2022-03-30 08:04:59

by Mikhail Zhilkin

[permalink] [raw]
Subject: [PATCH] mtd: parsers: add support for Sercomm partitions

This adds an MTD partition parser for the Sercomm partition table that
is used in some Beeline, Netgear and Sercomm routers.

The Sercomm partition map table contains real partition offsets, which
may differ from device to device depending on the number and location of
bad blocks on NAND.

Device tree example:
partitions {
compatible = "sercomm,sc-partitions", "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;

partition@0 {
label = "u-boot";
reg = <0x0 0x100000>;
scpart-id = <0>;
read-only;
};
};

This is essentially the same code as proposed by NOGUCHI Hiroshi
<[email protected]> here:
https://github.com/openwrt/openwrt/pull/1318#issuecomment-420607394

Signed-off-by: Mikhail Zhilkin <[email protected]>
---
drivers/mtd/parsers/Kconfig | 9 ++
drivers/mtd/parsers/Makefile | 1 +
drivers/mtd/parsers/scpart.c | 243 +++++++++++++++++++++++++++++++++++
3 files changed, 253 insertions(+)
create mode 100644 drivers/mtd/parsers/scpart.c

diff --git a/drivers/mtd/parsers/Kconfig b/drivers/mtd/parsers/Kconfig
index 23763d16e4f9..92a95297ca84 100644
--- a/drivers/mtd/parsers/Kconfig
+++ b/drivers/mtd/parsers/Kconfig
@@ -180,6 +180,15 @@ config MTD_REDBOOT_PARTS_READONLY

endif # MTD_REDBOOT_PARTS

+config MTD_SERCOMM_PARTS
+ tristate "Sercomm flash partition parser"
+ depends on MTD_OF_PARTS
+ help
+ This provides partitions table parser for devices with Sercomm
+ partition map. This partition table contains real partition
+ offsets, which may differ from device to device depending on the
+ number and location of bad blocks on NAND.
+
config MTD_QCOMSMEM_PARTS
tristate "Qualcomm SMEM flash partition parser"
depends on QCOM_SMEM
diff --git a/drivers/mtd/parsers/Makefile b/drivers/mtd/parsers/Makefile
index 2e98aa048278..2fcf0ab9e7da 100644
--- a/drivers/mtd/parsers/Makefile
+++ b/drivers/mtd/parsers/Makefile
@@ -10,6 +10,7 @@ ofpart-$(CONFIG_MTD_OF_PARTS_LINKSYS_NS)+= ofpart_linksys_ns.o
obj-$(CONFIG_MTD_PARSER_IMAGETAG) += parser_imagetag.o
obj-$(CONFIG_MTD_AFS_PARTS) += afs.o
obj-$(CONFIG_MTD_PARSER_TRX) += parser_trx.o
+obj-$(CONFIG_MTD_SERCOMM_PARTS) += scpart.o
obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpslpart.o
obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o
obj-$(CONFIG_MTD_QCOMSMEM_PARTS) += qcomsmempart.o
diff --git a/drivers/mtd/parsers/scpart.c b/drivers/mtd/parsers/scpart.c
new file mode 100644
index 000000000000..0d8b2bb3452e
--- /dev/null
+++ b/drivers/mtd/parsers/scpart.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * drivers/mtd/scpart.c: Sercomm Partition Parser
+ *
+ * Copyright (C) 2018 NOGUCHI Hiroshi
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/module.h>
+
+#define MOD_NAME "scpart"
+
+static const char sc_part_magic[] = {
+ 'S', 'C', 'F', 'L', 'M', 'A', 'P', 'O', 'K', '\0',
+};
+#define PART_MAGIC_LEN sizeof(sc_part_magic)
+
+/* assumes that all fields are set by CPU native endian */
+struct sc_part_desc {
+ uint32_t part_id;
+ uint32_t part_offs;
+ uint32_t part_bytes;
+};
+#define ID_ALREADY_FOUND (0xFFFFFFFFUL)
+
+#define MAP_OFFS_IN_BLK (0x800)
+
+#define MAP_MIRROR_NUM (2)
+
+static int scpart_desc_is_valid(struct sc_part_desc *pdesc)
+{
+ return !!((pdesc->part_id != 0xFFFFFFFFUL) &&
+ (pdesc->part_offs != 0xFFFFFFFFUL) &&
+ (pdesc->part_bytes != 0xFFFFFFFFUL));
+}
+
+static int scpart_scan_partmap(struct mtd_info *master, loff_t partmap_offs,
+ struct sc_part_desc **ppdesc)
+{
+ uint8_t *buf;
+ loff_t offs;
+ size_t retlen;
+ struct sc_part_desc *pdesc = NULL;
+ struct sc_part_desc *tmpdesc;
+ int cnt = 0;
+ int res2;
+ int res = 0;
+
+ buf = kzalloc(master->erasesize, GFP_KERNEL);
+ if (!buf) {
+ res = -ENOMEM;
+ goto out;
+ }
+
+ res2 = mtd_read(master, partmap_offs, master->erasesize, &retlen, buf);
+ if (res2 || (retlen != master->erasesize)) {
+ res = -EIO;
+ goto free;
+ }
+
+ offs = MAP_OFFS_IN_BLK;
+ while ((offs + sizeof(*tmpdesc)) < master->erasesize) {
+ tmpdesc = (struct sc_part_desc *)&(buf[offs]);
+ if (!scpart_desc_is_valid(tmpdesc))
+ break;
+ cnt++;
+ offs += sizeof(*tmpdesc);
+ }
+
+ if (cnt > 0) {
+ int bytes = cnt * sizeof(*pdesc);
+
+ pdesc = kzalloc(bytes, GFP_KERNEL);
+ if (!pdesc) {
+ res = -ENOMEM;
+ goto free;
+ }
+ memcpy(pdesc, &(buf[MAP_OFFS_IN_BLK]), bytes);
+
+ *ppdesc = pdesc;
+ res = cnt;
+ }
+
+free:
+ kfree(buf);
+
+out:
+ return res;
+}
+
+static int scpart_find_partmap(struct mtd_info *master,
+ struct sc_part_desc **ppdesc)
+{
+ loff_t offs;
+ uint8_t rdbuf[PART_MAGIC_LEN];
+ size_t retlen;
+ int magic_found = 0;
+ int res2;
+ int res = 0;
+
+ offs = 0;
+ while ((magic_found < MAP_MIRROR_NUM) &&
+ (offs < master->size) && !mtd_block_isbad(master, offs)) {
+ res2 = mtd_read(master, offs, PART_MAGIC_LEN, &retlen, rdbuf);
+ if (res2 || (retlen != PART_MAGIC_LEN)) {
+ res = -EIO;
+ goto out;
+ }
+ if (!memcmp(rdbuf, sc_part_magic, PART_MAGIC_LEN)) {
+ pr_debug("%s: signature found at 0x%llx\n", MOD_NAME, offs);
+ magic_found++;
+ res = scpart_scan_partmap(master, offs, ppdesc);
+ if (res > 0)
+ goto out;
+ }
+ offs += master->erasesize;
+ }
+
+out:
+ if (res > 0)
+ pr_info("%s: valid 'SC PART MAP' found (%d partitions)\n", MOD_NAME, res);
+ else
+ pr_info("%s: no valid 'SC PART MAP'\n", MOD_NAME);
+
+ return res;
+}
+
+static int scpart_parse(struct mtd_info *master,
+ const struct mtd_partition **pparts,
+ struct mtd_part_parser_data *data)
+{
+ struct sc_part_desc *scpart_map = NULL;
+ struct mtd_partition *parts = NULL;
+ struct device_node *mtd_node;
+ struct device_node *ofpart_node;
+ struct device_node *pp;
+ const char *partname;
+ int nr_scparts;
+ int nr_parts = 0;
+ int n;
+ int res = 0;
+
+ mtd_node = mtd_get_of_node(master);
+ if (!mtd_node)
+ goto out;
+
+ ofpart_node = of_get_child_by_name(mtd_node, "partitions");
+ if (!ofpart_node)
+ goto out;
+
+ nr_scparts = scpart_find_partmap(master, &scpart_map);
+ if (nr_scparts <= 0) {
+ res = nr_scparts;
+ goto free;
+ }
+
+ for_each_child_of_node(ofpart_node, pp) {
+ u32 scpart_id;
+ struct mtd_partition *parts_tmp;
+
+ if (of_property_read_u32(pp, "scpart-id", &scpart_id))
+ continue;
+
+ for (n = 0 ; n < nr_scparts ; n++)
+ if ((scpart_map[n].part_id != ID_ALREADY_FOUND) &&
+ (scpart_id == scpart_map[n].part_id))
+ break;
+ if (n >= nr_scparts)
+ /* not match */
+ continue;
+
+ /* add the partition found in OF into MTD partition array */
+
+ /* reallocate partition array */
+ parts_tmp = parts;
+ parts = kzalloc((nr_parts + 1) * sizeof(*parts), GFP_KERNEL);
+ if (!parts) {
+ kfree(parts_tmp);
+ res = -ENOMEM;
+ goto free;
+ }
+ if (parts_tmp) {
+ memcpy(parts, parts_tmp, nr_parts * sizeof(*parts));
+ kfree(parts_tmp);
+ }
+
+ parts[nr_parts].offset = scpart_map[n].part_offs;
+ parts[nr_parts].size = scpart_map[n].part_bytes;
+ parts[nr_parts].of_node = pp;
+
+ if (!of_property_read_string(pp, "label", &partname) ||
+ !of_property_read_string(pp, "name", &partname))
+ parts[nr_parts].name = partname;
+
+ if (of_property_read_bool(pp, "read-only"))
+ parts[nr_parts].mask_flags |= MTD_WRITEABLE;
+ if (of_property_read_bool(pp, "lock"))
+ parts[nr_parts].mask_flags |= MTD_POWERUP_LOCK;
+
+ /* mark as 'done' */
+ scpart_map[n].part_id = ID_ALREADY_FOUND;
+
+ nr_parts++;
+ }
+ if (nr_parts > 0) {
+ *pparts = parts;
+ res = nr_parts;
+ } else
+ pr_info("%s: No partition in OF matches partition ID with 'SC PART MAP'.\n",
+ MOD_NAME);
+
+ of_node_put(pp);
+
+free:
+ kfree(scpart_map);
+ if (res <= 0)
+ kfree(parts);
+
+out:
+ return res;
+}
+
+static const struct of_device_id scpart_parser_of_match_table[] = {
+ { .compatible = "sercomm,sc-partitions" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, scpart_parser_of_match_table);
+
+static struct mtd_part_parser scpart_parser = {
+ .parse_fn = scpart_parse,
+ .name = "scpart",
+ .of_match_table = scpart_parser_of_match_table,
+};
+module_mtd_part_parser(scpart_parser);
+
+/* mtd parsers will request the module by parser name */
+MODULE_ALIAS("scpart");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("NOGUCHI Hiroshi <[email protected]>");
+MODULE_DESCRIPTION("Sercomm partition parser");
--
2.25.1


2022-03-30 12:30:30

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: parsers: add support for Sercomm partitions

Hi Mikhail,

[email protected] wrote on Tue, 29 Mar 2022 12:20:16 +0000:

> This adds an MTD partition parser for the Sercomm partition table that
> is used in some Beeline, Netgear and Sercomm routers.
>
> The Sercomm partition map table contains real partition offsets, which
> may differ from device to device depending on the number and location of
> bad blocks on NAND.
>
> Device tree example:
> partitions {
> compatible = "sercomm,sc-partitions", "fixed-partitions";
> #address-cells = <1>;
> #size-cells = <1>;
>
> partition@0 {
> label = "u-boot";
> reg = <0x0 0x100000>;
> scpart-id = <0>;
> read-only;
> };
> };

You'll need a DT binding patch and Rob's ack!

>
> This is essentially the same code as proposed by NOGUCHI Hiroshi
> <[email protected]> here:

I would credit Hiroshi with a Suggested-by at least

> https://github.com/openwrt/openwrt/pull/1318#issuecomment-420607394

And use a Link: tag for this.

>
> Signed-off-by: Mikhail Zhilkin <[email protected]>
> ---


Thanks,
Miquèl

2022-05-11 00:44:45

by Mikhail Zhilkin

[permalink] [raw]
Subject: Re: [PATCH] mtd: parsers: add support for Sercomm partitions

Hi Miquel,

On 3/30/2022 11:09 AM, Miquel Raynal wrote:
> Hi Mikhail,
>
> [email protected] wrote on Tue, 29 Mar 2022 12:20:16 +0000:
>
>> This adds an MTD partition parser for the Sercomm partition table that
>> is used in some Beeline, Netgear and Sercomm routers.
>>
>> The Sercomm partition map table contains real partition offsets, which
>> may differ from device to device depending on the number and location of
>> bad blocks on NAND.
>>
>> Device tree example:
>> partitions {
>> compatible = "sercomm,sc-partitions", "fixed-partitions";
>> #address-cells = <1>;
>> #size-cells = <1>;
>>
>> partition@0 {
>> label = "u-boot";
>> reg = <0x0 0x100000>;
>> scpart-id = <0>;
>> read-only;
>> };
>> };
> You'll need a DT binding patch and Rob's ack!

I hope that I near to finish with DT binding...

Link:
https://lore.kernel.org/all/[email protected]/

Link:
https://lore.kernel.org/all/[email protected]/

>> This is essentially the same code as proposed by NOGUCHI Hiroshi
>> <[email protected]> here:
> I would credit Hiroshi with a Suggested-by at least

I read submitting patches rules again and thought that Signed-off-by is
suitable for this case. Is this ok?

Link:
https://lore.kernel.org/all/[email protected]/

>> https://github.com/openwrt/openwrt/pull/1318#issuecomment-420607394
> And use a Link: tag for this.
>
Fixed, thanks!

Link:
https://lore.kernel.org/all/[email protected]/

> Thanks,
> Miquèl
--
Best regards,
Mikhail


2022-05-11 09:39:01

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: parsers: add support for Sercomm partitions

Hi Mikhail,

[email protected] wrote on Tue, 10 May 2022 23:07:23 +0300:

> Hi Miquel,
>
> On 3/30/2022 11:09 AM, Miquel Raynal wrote:
> > Hi Mikhail,
> >
> > [email protected] wrote on Tue, 29 Mar 2022 12:20:16 +0000:
> >
> >> This adds an MTD partition parser for the Sercomm partition table that
> >> is used in some Beeline, Netgear and Sercomm routers.
> >>
> >> The Sercomm partition map table contains real partition offsets, which
> >> may differ from device to device depending on the number and location of
> >> bad blocks on NAND.
> >>
> >> Device tree example:
> >> partitions {
> >> compatible = "sercomm,sc-partitions", "fixed-partitions";
> >> #address-cells = <1>;
> >> #size-cells = <1>;
> >>
> >> partition@0 {
> >> label = "u-boot";
> >> reg = <0x0 0x100000>;
> >> scpart-id = <0>;
> >> read-only;
> >> };
> >> };
> > You'll need a DT binding patch and Rob's ack!
>
> I hope that I near to finish with DT binding...
>
> Link:
> https://lore.kernel.org/all/[email protected]/
>
> Link:
> https://lore.kernel.org/all/[email protected]/
>
> >> This is essentially the same code as proposed by NOGUCHI Hiroshi
> >> <[email protected]> here:
> > I would credit Hiroshi with a Suggested-by at least
>
> I read submitting patches rules again and thought that Signed-off-by is
> suitable for this case. Is this ok?

Either you take his work almost like it is and he must be the author
*and* the first signed-off-by line, or you take the authorship if you
think you did enough modifications to the code and in this case you can
either credit him with a suggested-by before your signed-off, or you
can credit him with a co-developed-by + his signed-off and then yours.

>
> Link:
> https://lore.kernel.org/all/[email protected]/
>
> >> https://github.com/openwrt/openwrt/pull/1318#issuecomment-420607394
> > And use a Link: tag for this.
> >
> Fixed, thanks!
>
> Link:
> https://lore.kernel.org/all/[email protected]/
>
> > Thanks,
> > Miquèl


Thanks,
Miquèl

2022-05-11 22:33:51

by Mikhail Zhilkin

[permalink] [raw]
Subject: Re: [PATCH] mtd: parsers: add support for Sercomm partitions

On 5/11/2022 10:16 AM, Miquel Raynal wrote:

>>>> This is essentially the same code as proposed by NOGUCHI Hiroshi
>>>> <[email protected]> here:
>>> I would credit Hiroshi with a Suggested-by at least
>> I read submitting patches rules again and thought that Signed-off-by is
>> suitable for this case. Is this ok?
> Either you take his work almost like it is and he must be the author
> *and* the first signed-off-by line, or you take the authorship if you
> think you did enough modifications to the code and in this case you can
> either credit him with a suggested-by before your signed-off, or you
> can credit him with a co-developed-by + his signed-off and then yours.

Thank you for clarification! My modifications were quite small:
- Remove redundant braces and logical NOT operator
- Define pr_fmt
- Replace kcalloc by kzalloc
- Use of_get_child_count() and alloc big enough array before the
for_each_child_of_node()

Therefore, I prefer to keep the authorship of the original author (NOGUCHI Hiroshi).

Patch is really good and was successfully tested on the hundreds of Sercomm devices (5.4, 5.10, 5.15 Kernel). My motivation for upstreaming is work under official support for these devices in OpenWrt.

What are the next steps from my side in this case? Should I wait for review of this patch here (Link: https://lore.kernel.org/linux-mtd/[email protected]/T/#u
) or do I need to do something additionally (for example, send updated set v5 with Acked-by from Krzysztof in the first patch)?

And sorry for the many questions... I haven't fully mastered the Linux change management process and best practices yet.

> Thanks,
> Miquèl

--
Best regards,
Mikhail


2022-05-14 02:08:56

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] mtd: parsers: add support for Sercomm partitions

Hi Mikhail,

[email protected] wrote on Wed, 11 May 2022 23:36:18 +0300:

> On 5/11/2022 10:16 AM, Miquel Raynal wrote:
>
> >>>> This is essentially the same code as proposed by NOGUCHI Hiroshi
> >>>> <[email protected]> here:
> >>> I would credit Hiroshi with a Suggested-by at least
> >> I read submitting patches rules again and thought that Signed-off-by is
> >> suitable for this case. Is this ok?
> > Either you take his work almost like it is and he must be the author
> > *and* the first signed-off-by line, or you take the authorship if you
> > think you did enough modifications to the code and in this case you can
> > either credit him with a suggested-by before your signed-off, or you
> > can credit him with a co-developed-by + his signed-off and then yours.
>
> Thank you for clarification! My modifications were quite small:
> - Remove redundant braces and logical NOT operator
> - Define pr_fmt
> - Replace kcalloc by kzalloc
> - Use of_get_child_count() and alloc big enough array before the
> for_each_child_of_node()
>
> Therefore, I prefer to keep the authorship of the original author (NOGUCHI Hiroshi).
>
> Patch is really good and was successfully tested on the hundreds of Sercomm devices (5.4, 5.10, 5.15 Kernel). My motivation for upstreaming is work under official support for these devices in OpenWrt.
>
> What are the next steps from my side in this case? Should I wait for review of this patch here (Link: https://lore.kernel.org/linux-mtd/[email protected]/T/#u
> ) or do I need to do something additionally (for example, send updated set v5 with Acked-by from Krzysztof in the first patch)?
>
> And sorry for the many questions... I haven't fully mastered the Linux change management process and best practices yet.

We are used to incremental changes, so please send another version with
the tags collected, the changes that I requested regarding the
authorship and we'll see what happens to the next version.

Thanks,
Miquèl