QCOM platforms such as IPQ806x are using SMEM to store their flash
layout. This patch set adds the DT nodes required to instanciate SMEM
on IPQ806x and add an MTD parser using it.
This change is based on the SMEM driver posted here:
*https://lkml.org/lkml/2015/7/27/1125
Mathieu Olivari (3):
ARM: qcom: add SFPB nodes to IPQ806x dts
ARM: qcom: add SMEM device node to IPQ806x dts
mtd: add SMEM parser for QCOM platforms
arch/arm/boot/dts/qcom-ipq8064.dtsi | 23 +++-
drivers/mtd/Kconfig | 7 ++
drivers/mtd/Makefile | 1 +
drivers/mtd/qcom_smem_part.c | 224 ++++++++++++++++++++++++++++++++++++
4 files changed, 254 insertions(+), 1 deletion(-)
create mode 100644 drivers/mtd/qcom_smem_part.c
--
2.1.4
Add one new node to the ipq806x.dtsi file to declare & register the
hardware spinlock devices. This mechanism is required to be used by
other drivers such as SMEM.
Signed-off-by: Mathieu Olivari <[email protected]>
---
arch/arm/boot/dts/qcom-ipq8064.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 9f727d8..ea7b959 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -67,6 +67,18 @@
clock-frequency = <32768>;
#clock-cells = <0>;
};
+
+ sfpb_mutex_block: syscon@01200600 {
+ compatible = "syscon";
+ reg = <0x01200600 0x100>;
+ };
+
+ sfpb_mutex: sfpb-mutex {
+ compatible = "qcom,sfpb-mutex";
+ syscon = <&sfpb_mutex_block 4 4>;
+
+ #hwlock-cells = <1>;
+ };
};
soc: soc {
--
2.1.4
SMEM is used on IPQ806x to store various board related information such
as boot device and flash partition layout. We'll declare it as a device
so we can make use of it thanks to the new SMEM soc driver.
Signed-off-by: Mathieu Olivari <[email protected]>
---
arch/arm/boot/dts/qcom-ipq8064.dtsi | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index ea7b959..9ee9780 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -55,7 +55,7 @@
no-map;
};
- smem@41000000 {
+ smem: smem@41000000 {
reg = <0x41000000 0x200000>;
no-map;
};
@@ -79,6 +79,15 @@
#hwlock-cells = <1>;
};
+
+ smem {
+ compatible = "qcom,smem";
+
+ memory-region = <&smem>;
+ reg = <0x41000000 0x4000>;
+
+ hwlocks = <&sfpb_mutex 3>;
+ };
};
soc: soc {
--
2.1.4
On QCOM platforms using MTD devices storage (such as IPQ806x), SMEM is
used to store partition layout. This new parser can now be used to read
SMEM and use it to register an MTD layout according to its content.
Signed-off-by: Mathieu Olivari <[email protected]>
---
drivers/mtd/Kconfig | 7 ++
drivers/mtd/Makefile | 1 +
drivers/mtd/qcom_smem_part.c | 224 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 232 insertions(+)
create mode 100644 drivers/mtd/qcom_smem_part.c
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index a03ad29..debc887 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -155,6 +155,13 @@ config MTD_BCM47XX_PARTS
This provides partitions parser for devices based on BCM47xx
boards.
+config MTD_QCOM_SMEM_PARTS
+ tristate "QCOM SMEM partitioning support"
+ depends on QCOM_SMEM
+ help
+ This provides partitions parser for QCOM devices using SMEM
+ such as IPQ806x.
+
comment "User Modules And Translation Layers"
#
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 99bb9a1..b3c7de4 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -13,6 +13,7 @@ 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_QCOM_SMEM_PARTS) += qcom_smem_part.o
# 'Users' - code which presents functionality to userspace.
obj-$(CONFIG_MTD_BLKDEVS) += mtd_blkdevs.o
diff --git a/drivers/mtd/qcom_smem_part.c b/drivers/mtd/qcom_smem_part.c
new file mode 100644
index 0000000..6cbf610
--- /dev/null
+++ b/drivers/mtd/qcom_smem_part.c
@@ -0,0 +1,224 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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/device.h>
+#include <linux/slab.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+
+#include <linux/soc/qcom/smem.h>
+
+/* Processor/host identifier for the application processor */
+#define SMEM_HOST_APPS 0
+
+/* SMEM items index */
+#define SMEM_AARM_PARTITION_TABLE 9
+#define SMEM_BOOT_FLASH_TYPE 421
+#define SMEM_BOOT_FLASH_BLOCK_SIZE 424
+
+/* SMEM Magic values in partition table */
+#define SMEM_PTABLE_MAGIC_1 0x55ee73aa
+#define SMEM_PTABLE_MAGIC_2 0xe35ebddb
+
+/* SMEM Flash types */
+#define SMEM_FLASH_NAND 2
+#define SMEM_FLASH_SPI 6
+
+#define SMEM_PART_NAME_SZ 16
+#define SMEM_PARTS_MAX 32
+
+struct smem_partition {
+ char name[SMEM_PART_NAME_SZ];
+ uint32_t start;
+ uint32_t size;
+ uint32_t attr;
+};
+
+struct smem_partition_table {
+ uint32_t magic[2];
+ uint32_t version;
+ uint32_t len;
+ struct smem_partition parts[SMEM_PARTS_MAX];
+};
+
+static int qcom_smem_get_flash_blksz(uint64_t **smem_blksz)
+{
+ int ret;
+ size_t size;
+
+ ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_BOOT_FLASH_BLOCK_SIZE,
+ (void **) smem_blksz, &size);
+
+ if (ret < 0) {
+ pr_err("Unable to read flash blksz from SMEM\n");
+ return -ENOENT;
+ }
+
+ if (size != sizeof(**smem_blksz)) {
+ pr_err("Invalid flash blksz size in SMEM\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int qcom_smem_get_flash_type(uint64_t **smem_flash_type)
+{
+ int ret;
+ size_t size;
+
+ ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_BOOT_FLASH_TYPE,
+ (void **) smem_flash_type, &size);
+
+ if (ret < 0) {
+ pr_err("Unable to read flash type from SMEM\n");
+ return -ENOENT;
+ }
+
+ if (size != sizeof(**smem_flash_type)) {
+ pr_err("Invalid flash type size in SMEM\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int qcom_smem_get_flash_partitions(struct smem_partition_table **pparts)
+{
+ int ret;
+ size_t size;
+
+ ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_AARM_PARTITION_TABLE,
+ (void **) pparts, &size);
+
+ if (ret < 0) {
+ pr_err("Unable to read partition table from SMEM\n");
+ return -ENOENT;
+ }
+
+ return 0;
+}
+
+static int of_dev_node_match(struct device *dev, void *data)
+{
+ return dev->of_node == data;
+}
+
+static bool is_spi_device(struct device_node *np)
+{
+ struct device *dev;
+
+ dev = bus_find_device(&spi_bus_type, NULL, np, of_dev_node_match);
+ return dev ? true : false;
+}
+
+static int parse_qcom_smem_partitions(struct mtd_info *master,
+ struct mtd_partition **pparts,
+ struct mtd_part_parser_data *data)
+{
+ struct smem_partition_table *smem_parts;
+ uint64_t *smem_flash_type, *smem_blksz;
+ struct mtd_partition *mtd_parts;
+ struct device_node *of_node = data->of_node;
+ int i, ret;
+
+ /*
+ * SMEM will only store the partition table of the boot device.
+ * If this is not the boot device, do not return any partition.
+ */
+ ret = qcom_smem_get_flash_type(&smem_flash_type);
+ if (ret < 0)
+ return ret;
+
+ if ((*smem_flash_type == SMEM_FLASH_NAND && !mtd_type_is_nand(master))
+ || (*smem_flash_type == SMEM_FLASH_SPI && !is_spi_device(of_node)))
+ return 0;
+
+ /*
+ * Just for sanity purpose, make sure the block size in SMEM matches the
+ * block size of the MTD device
+ */
+ ret = qcom_smem_get_flash_blksz(&smem_blksz);
+ if (ret < 0)
+ return ret;
+
+ if (*smem_blksz != master->erasesize) {
+ pr_err("SMEM block size differs from MTD block size\n");
+ return -EINVAL;
+ }
+
+ /* Get partition pointer from SMEM */
+ ret = qcom_smem_get_flash_partitions(&smem_parts);
+ if (ret < 0)
+ return ret;
+
+ if (smem_parts->magic[0] != SMEM_PTABLE_MAGIC_1 ||
+ smem_parts->magic[1] != SMEM_PTABLE_MAGIC_2) {
+ pr_err("SMEM partition magic invalid\n");
+ return -EINVAL;
+ }
+
+ /* Allocate and populate the mtd structures */
+ mtd_parts = kcalloc(smem_parts->len, sizeof(struct mtd_partition),
+ GFP_KERNEL);
+ if (mtd_parts == NULL)
+ return -ENOMEM;
+
+ for (i = 0; i < smem_parts->len; i++) {
+ struct smem_partition *part = &smem_parts->parts[i];
+
+ mtd_parts[i].name = part->name;
+ mtd_parts[i].size = part->size * (*smem_blksz);
+ mtd_parts[i].offset = part->start * (*smem_blksz);
+
+ /*
+ * The last SMEM partition may have its size marked as
+ * something like 0xffffffff, which means "until the end of the
+ * flash device". In this case, truncate it.
+ */
+ if (mtd_parts[i].offset + mtd_parts[i].size > master->size)
+ mtd_parts[i].size = master->size - mtd_parts[i].offset;
+ }
+
+ *pparts = mtd_parts;
+
+ return smem_parts->len;
+}
+
+static struct mtd_part_parser qcom_smem_parser = {
+ .owner = THIS_MODULE,
+ .parse_fn = parse_qcom_smem_partitions,
+ .name = "qcom-smem",
+};
+
+static int __init qcom_smem_parser_init(void)
+{
+ register_mtd_parser(&qcom_smem_parser);
+ return 0;
+}
+
+static void __exit qcom_smem_parser_exit(void)
+{
+ deregister_mtd_parser(&qcom_smem_parser);
+}
+
+module_init(qcom_smem_parser_init);
+module_exit(qcom_smem_parser_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Mathieu Olivari <[email protected]>");
+MODULE_DESCRIPTION("Parsing code for SMEM based partition tables");
--
2.1.4
On 08/13/2015 02:33 PM, Mathieu Olivari wrote:
> Add one new node to the ipq806x.dtsi file to declare & register the
> hardware spinlock devices. This mechanism is required to be used by
> other drivers such as SMEM.
>
> Signed-off-by: Mathieu Olivari <[email protected]>
> ---
> arch/arm/boot/dts/qcom-ipq8064.dtsi | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> index 9f727d8..ea7b959 100644
> --- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> @@ -67,6 +67,18 @@
> clock-frequency = <32768>;
> #clock-cells = <0>;
> };
> +
> + sfpb_mutex_block: syscon@01200600 {
I think you can drop the leading 0 here. Also this should be moved into
the soc node.
> + compatible = "syscon";
> + reg = <0x01200600 0x100>;
> + };
> +
> + sfpb_mutex: sfpb-mutex {
> + compatible = "qcom,sfpb-mutex";
> + syscon = <&sfpb_mutex_block 4 4>;
> +
> + #hwlock-cells = <1>;
> + };
And this node should be moved to the root /.
> };
>
> soc: soc {
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 08/13/2015 02:33 PM, Mathieu Olivari wrote:
> @@ -79,6 +79,15 @@
>
> #hwlock-cells = <1>;
> };
> +
> + smem {
> + compatible = "qcom,smem";
> +
> + memory-region = <&smem>;
> + reg = <0x41000000 0x4000>;
Does this reg property do something? I thought we were supposed to leave
this out when not using the rpm msg ram. Also, if there's no reg, it
should be at the root /.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 08/13/2015 02:33 PM, Mathieu Olivari wrote:
> diff --git a/drivers/mtd/qcom_smem_part.c b/drivers/mtd/qcom_smem_part.c
> new file mode 100644
> index 0000000..6cbf610
> --- /dev/null
> +++ b/drivers/mtd/qcom_smem_part.c
>
> +#define SMEM_PARTS_MAX 32
> +
> +struct smem_partition {
> + char name[SMEM_PART_NAME_SZ];
> + uint32_t start;
> + uint32_t size;
> + uint32_t attr;
Is this little endian? __le32? I think so.
> +};
> +
> +struct smem_partition_table {
> + uint32_t magic[2];
> + uint32_t version;
> + uint32_t len;
Same for these being little endian. And if so, please make magic into a
byte array of size 8 and then do a memcmp with another static byte array.
> + struct smem_partition parts[SMEM_PARTS_MAX];
> +};
> +
> +static int qcom_smem_get_flash_blksz(uint64_t **smem_blksz)
I don't know, but I prefer u64 because it's short and sweet. But I'm not
the maintainer here.
> +{
> + int ret;
> + size_t size;
> +
> + ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_BOOT_FLASH_BLOCK_SIZE,
> + (void **) smem_blksz, &size);
Is the cast necessary?
> +
> + if (ret < 0) {
> + pr_err("Unable to read flash blksz from SMEM\n");
> + return -ENOENT;
> + }
> +
> + if (size != sizeof(**smem_blksz)) {
> + pr_err("Invalid flash blksz size in SMEM\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_smem_get_flash_type(uint64_t **smem_flash_type)
> +{
> + int ret;
> + size_t size;
> +
> + ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_BOOT_FLASH_TYPE,
> + (void **) smem_flash_type, &size);
> +
> + if (ret < 0) {
> + pr_err("Unable to read flash type from SMEM\n");
> + return -ENOENT;
> + }
> +
> + if (size != sizeof(**smem_flash_type)) {
> + pr_err("Invalid flash type size in SMEM\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_smem_get_flash_partitions(struct smem_partition_table **pparts)
> +{
> + int ret;
> + size_t size;
> +
> + ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_AARM_PARTITION_TABLE,
> + (void **) pparts, &size);
> +
> + if (ret < 0) {
> + pr_err("Unable to read partition table from SMEM\n");
> + return -ENOENT;
> + }
> +
> + return 0;
> +}
> +
> +static int of_dev_node_match(struct device *dev, void *data)
> +{
> + return dev->of_node == data;
> +}
> +
> +static bool is_spi_device(struct device_node *np)
> +{
> + struct device *dev;
> +
> + dev = bus_find_device(&spi_bus_type, NULL, np, of_dev_node_match);
I'm pretty sure this holds a reference to the device that needs to be
released with put_device().
> + return dev ? true : false;
> +}
> +
> +static int parse_qcom_smem_partitions(struct mtd_info *master,
> + struct mtd_partition **pparts,
> + struct mtd_part_parser_data *data)
> +{
> + struct smem_partition_table *smem_parts;
> + uint64_t *smem_flash_type, *smem_blksz;
> + struct mtd_partition *mtd_parts;
> + struct device_node *of_node = data->of_node;
> + int i, ret;
> +
> + /*
> + * SMEM will only store the partition table of the boot device.
> + * If this is not the boot device, do not return any partition.
> + */
> + ret = qcom_smem_get_flash_type(&smem_flash_type);
> + if (ret < 0)
> + return ret;
> +
> + if ((*smem_flash_type == SMEM_FLASH_NAND && !mtd_type_is_nand(master))
> + || (*smem_flash_type == SMEM_FLASH_SPI && !is_spi_device(of_node)))
> + return 0;
> +
> + /*
> + * Just for sanity purpose, make sure the block size in SMEM matches the
> + * block size of the MTD device
> + */
> + ret = qcom_smem_get_flash_blksz(&smem_blksz);
> + if (ret < 0)
> + return ret;
> +
> + if (*smem_blksz != master->erasesize) {
> + pr_err("SMEM block size differs from MTD block size\n");
> + return -EINVAL;
> + }
> +
> + /* Get partition pointer from SMEM */
> + ret = qcom_smem_get_flash_partitions(&smem_parts);
> + if (ret < 0)
> + return ret;
> +
> + if (smem_parts->magic[0] != SMEM_PTABLE_MAGIC_1 ||
> + smem_parts->magic[1] != SMEM_PTABLE_MAGIC_2) {
> + pr_err("SMEM partition magic invalid\n");
> + return -EINVAL;
> + }
> +
> + /* Allocate and populate the mtd structures */
> + mtd_parts = kcalloc(smem_parts->len, sizeof(struct mtd_partition),
sizeof(*mtd_parts)?
> + GFP_KERNEL);
> + if (mtd_parts == NULL)
if (!mtd_parts) is more kernel style.
> + return -ENOMEM;
> +
> + for (i = 0; i < smem_parts->len; i++) {
> + struct smem_partition *part = &smem_parts->parts[i];
> +
> + mtd_parts[i].name = part->name;
> + mtd_parts[i].size = part->size * (*smem_blksz);
> + mtd_parts[i].offset = part->start * (*smem_blksz);
> +
> + /*
> + * The last SMEM partition may have its size marked as
> + * something like 0xffffffff, which means "until the end of the
> + * flash device". In this case, truncate it.
> + */
> + if (mtd_parts[i].offset + mtd_parts[i].size > master->size)
> + mtd_parts[i].size = master->size - mtd_parts[i].offset;
That is 7 mtd_parts[i]. instances. Consider making a local variable
pointer instead. Actually we could make two pointers for
smem_parts->part and mtd_parts and increment both in the loop to make
tighter code.
> + }
> +
> + *pparts = mtd_parts;
Oh we already have the pointer, so just do this assignment before iterating.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Thu, Aug 13, 2015 at 04:18:14PM -0700, Stephen Boyd wrote:
> On 08/13/2015 02:33 PM, Mathieu Olivari wrote:
> >Add one new node to the ipq806x.dtsi file to declare & register the
> >hardware spinlock devices. This mechanism is required to be used by
> >other drivers such as SMEM.
> >
> >Signed-off-by: Mathieu Olivari <[email protected]>
> >---
> > arch/arm/boot/dts/qcom-ipq8064.dtsi | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> >diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> >index 9f727d8..ea7b959 100644
> >--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
> >+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
> >@@ -67,6 +67,18 @@
> > clock-frequency = <32768>;
> > #clock-cells = <0>;
> > };
> >+
> >+ sfpb_mutex_block: syscon@01200600 {
>
> I think you can drop the leading 0 here. Also this should be moved into the
> soc node.
>
> >+ compatible = "syscon";
> >+ reg = <0x01200600 0x100>;
> >+ };
> >+
> >+ sfpb_mutex: sfpb-mutex {
> >+ compatible = "qcom,sfpb-mutex";
> >+ syscon = <&sfpb_mutex_block 4 4>;
> >+
> >+ #hwlock-cells = <1>;
> >+ };
>
> And this node should be moved to the root /.
> > };
> > soc: soc {
Right; I'll repost a patch to address both of these.
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
On Thu, Aug 13, 2015 at 04:57:50PM -0700, Stephen Boyd wrote:
> On 08/13/2015 02:33 PM, Mathieu Olivari wrote:
> >@@ -79,6 +79,15 @@
> > #hwlock-cells = <1>;
> > };
> >+
> >+ smem {
> >+ compatible = "qcom,smem";
> >+
> >+ memory-region = <&smem>;
> >+ reg = <0x41000000 0x4000>;
>
> Does this reg property do something? I thought we were supposed to leave
> this out when not using the rpm msg ram. Also, if there's no reg, it should
> be at the root /.
Actually; no it doesn't do much. I'll remove it.
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
On Thu, Aug 13, 2015 at 05:38:02PM -0700, Stephen Boyd wrote:
> On 08/13/2015 02:33 PM, Mathieu Olivari wrote:
> >diff --git a/drivers/mtd/qcom_smem_part.c b/drivers/mtd/qcom_smem_part.c
> >new file mode 100644
> >index 0000000..6cbf610
> >--- /dev/null
> >+++ b/drivers/mtd/qcom_smem_part.c
> >
> >+#define SMEM_PARTS_MAX 32
> >+
> >+struct smem_partition {
> >+ char name[SMEM_PART_NAME_SZ];
> >+ uint32_t start;
> >+ uint32_t size;
> >+ uint32_t attr;
>
> Is this little endian? __le32? I think so.
Yes. I'll make the change.
>
> >+};
> >+
> >+struct smem_partition_table {
> >+ uint32_t magic[2];
> >+ uint32_t version;
> >+ uint32_t len;
>
> Same for these being little endian. And if so, please make magic into a byte
> array of size 8 and then do a memcmp with another static byte array.
Done.
>
> >+ struct smem_partition parts[SMEM_PARTS_MAX];
> >+};
> >+
> >+static int qcom_smem_get_flash_blksz(uint64_t **smem_blksz)
>
> I don't know, but I prefer u64 because it's short and sweet. But I'm not the
> maintainer here.
Agreed.
>
> >+{
> >+ int ret;
> >+ size_t size;
> >+
> >+ ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_BOOT_FLASH_BLOCK_SIZE,
> >+ (void **) smem_blksz, &size);
>
> Is the cast necessary?
Compiler will issue a warning if it's not there.
>
> >+
> >+ if (ret < 0) {
> >+ pr_err("Unable to read flash blksz from SMEM\n");
> >+ return -ENOENT;
> >+ }
> >+
> >+ if (size != sizeof(**smem_blksz)) {
> >+ pr_err("Invalid flash blksz size in SMEM\n");
> >+ return -EINVAL;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static int qcom_smem_get_flash_type(uint64_t **smem_flash_type)
> >+{
> >+ int ret;
> >+ size_t size;
> >+
> >+ ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_BOOT_FLASH_TYPE,
> >+ (void **) smem_flash_type, &size);
> >+
> >+ if (ret < 0) {
> >+ pr_err("Unable to read flash type from SMEM\n");
> >+ return -ENOENT;
> >+ }
> >+
> >+ if (size != sizeof(**smem_flash_type)) {
> >+ pr_err("Invalid flash type size in SMEM\n");
> >+ return -EINVAL;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static int qcom_smem_get_flash_partitions(struct smem_partition_table **pparts)
> >+{
> >+ int ret;
> >+ size_t size;
> >+
> >+ ret = qcom_smem_get(SMEM_HOST_APPS, SMEM_AARM_PARTITION_TABLE,
> >+ (void **) pparts, &size);
> >+
> >+ if (ret < 0) {
> >+ pr_err("Unable to read partition table from SMEM\n");
> >+ return -ENOENT;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static int of_dev_node_match(struct device *dev, void *data)
> >+{
> >+ return dev->of_node == data;
> >+}
> >+
> >+static bool is_spi_device(struct device_node *np)
> >+{
> >+ struct device *dev;
> >+
> >+ dev = bus_find_device(&spi_bus_type, NULL, np, of_dev_node_match);
>
> I'm pretty sure this holds a reference to the device that needs to be
> released with put_device().
You're right. I'll add the corresponding put_device().
>
> >+ return dev ? true : false;
> >+}
> >+
> >+static int parse_qcom_smem_partitions(struct mtd_info *master,
> >+ struct mtd_partition **pparts,
> >+ struct mtd_part_parser_data *data)
> >+{
> >+ struct smem_partition_table *smem_parts;
> >+ uint64_t *smem_flash_type, *smem_blksz;
> >+ struct mtd_partition *mtd_parts;
> >+ struct device_node *of_node = data->of_node;
> >+ int i, ret;
> >+
> >+ /*
> >+ * SMEM will only store the partition table of the boot device.
> >+ * If this is not the boot device, do not return any partition.
> >+ */
> >+ ret = qcom_smem_get_flash_type(&smem_flash_type);
> >+ if (ret < 0)
> >+ return ret;
> >+
> >+ if ((*smem_flash_type == SMEM_FLASH_NAND && !mtd_type_is_nand(master))
> >+ || (*smem_flash_type == SMEM_FLASH_SPI && !is_spi_device(of_node)))
> >+ return 0;
> >+
> >+ /*
> >+ * Just for sanity purpose, make sure the block size in SMEM matches the
> >+ * block size of the MTD device
> >+ */
> >+ ret = qcom_smem_get_flash_blksz(&smem_blksz);
> >+ if (ret < 0)
> >+ return ret;
> >+
> >+ if (*smem_blksz != master->erasesize) {
> >+ pr_err("SMEM block size differs from MTD block size\n");
> >+ return -EINVAL;
> >+ }
> >+
> >+ /* Get partition pointer from SMEM */
> >+ ret = qcom_smem_get_flash_partitions(&smem_parts);
> >+ if (ret < 0)
> >+ return ret;
> >+
> >+ if (smem_parts->magic[0] != SMEM_PTABLE_MAGIC_1 ||
> >+ smem_parts->magic[1] != SMEM_PTABLE_MAGIC_2) {
> >+ pr_err("SMEM partition magic invalid\n");
> >+ return -EINVAL;
> >+ }
> >+
> >+ /* Allocate and populate the mtd structures */
> >+ mtd_parts = kcalloc(smem_parts->len, sizeof(struct mtd_partition),
>
> sizeof(*mtd_parts)?
OK.
>
> >+ GFP_KERNEL);
> >+ if (mtd_parts == NULL)
>
> if (!mtd_parts) is more kernel style.
OK.
>
> >+ return -ENOMEM;
> >+
> >+ for (i = 0; i < smem_parts->len; i++) {
> >+ struct smem_partition *part = &smem_parts->parts[i];
> >+
> >+ mtd_parts[i].name = part->name;
> >+ mtd_parts[i].size = part->size * (*smem_blksz);
> >+ mtd_parts[i].offset = part->start * (*smem_blksz);
> >+
> >+ /*
> >+ * The last SMEM partition may have its size marked as
> >+ * something like 0xffffffff, which means "until the end of the
> >+ * flash device". In this case, truncate it.
> >+ */
> >+ if (mtd_parts[i].offset + mtd_parts[i].size > master->size)
> >+ mtd_parts[i].size = master->size - mtd_parts[i].offset;
>
> That is 7 mtd_parts[i]. instances. Consider making a local variable pointer
> instead. Actually we could make two pointers for smem_parts->part and
> mtd_parts and increment both in the loop to make tighter code.
OK; I'll simplify it by making a new pointer.
>
> >+ }
> >+
> >+ *pparts = mtd_parts;
>
> Oh we already have the pointer, so just do this assignment before iterating.
pparts is an argument; I'd rather not make double use of it for clarity
purpose. I'll create a new one similar to the SMEM one instead.
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>