qcom: Add SMEM MTD parser
cover qcom: Add SMEM MTD parser
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
Signature · 1 line
2.1.4
PATCH 1/3 ARM: qcom: add SFPB nodes to IPQ806x dts
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.
--- arch/arm/boot/dts/qcom-ipq8064.dtsi | 12 ++++++++++++ 1 file changed, 12 insertions(+)
arch/arm/boot/dts/qcom-ipq8064.dtsi +12 -0
@@ -67,6 +67,18 @@ clock-frequency = <32768>; #clock-cells = <0>; }; + + sfpb_mutex_block: syscon at 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 {
Signature · 1 line
2.1.4
Re: [PATCH 1/3] ARM: qcom: add SFPB nodes to IPQ806x dts
Quoted 14 lines · Mathieu Olivari
Add one new node to the
ipq806x.dtsifile 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(+)arch/arm/boot/dts/qcom-ipq8064.dtsi +0
-0
arch/arm/boot/dts/qcom-ipq8064.dtsi line 71
Quoted 5 lines
arch/arm/boot/dts/qcom-ipq8064.dtsi +2
-0@@ -67,6 +67,18 @@ clock-frequency = <32768>; #clock-cells = <0>; }; + + sfpb_mutex_block: syscon at 01200600 {
I think you can drop the leading 0 here. Also this should be moved into the soc node.
arch/arm/boot/dts/qcom-ipq8064.dtsi line 81
Quoted 10 lines
arch/arm/boot/dts/qcom-ipq8064.dtsi +10
-0+ 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 /.
Quoted 3 lines
arch/arm/boot/dts/qcom-ipq8064.dtsi +0
-0}; soc: soc {
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 1/3] ARM: qcom: add SFPB nodes to IPQ806x dts
Quoted 24 lines · Stephen Boyd
Quoted 14 lines · Mathieu Olivari
Add one new node to the
ipq806x.dtsifile 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(+)arch/arm/boot/dts/qcom-ipq8064.dtsi +0
-0arch/arm/boot/dts/qcom-ipq8064.dtsi line 71
Quoted 5 lines
arch/arm/boot/dts/qcom-ipq8064.dtsi +2
-0@@ -67,6 +67,18 @@ clock-frequency = <32768>; #clock-cells = <0>; }; + + sfpb_mutex_block: syscon at 01200600 {I think you can drop the leading 0 here. Also this should be moved into the soc node.
Quoted 14 lines
arch/arm/boot/dts/qcom-ipq8064.dtsi line 81
Quoted 10 lines
arch/arm/boot/dts/qcom-ipq8064.dtsi +10
-0+ 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 /.
Quoted 2 lines
arch/arm/boot/dts/qcom-ipq8064.dtsi +0
-0}; soc: soc {
Right; I'll repost a patch to address both of these.
Quoted 6 lines
Signature · 3 lines
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 2/3 ARM: qcom: add SMEM device node to IPQ806x dts
In reply to Mathieu OlivariSMEM 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.
--- arch/arm/boot/dts/qcom-ipq8064.dtsi | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
arch/arm/boot/dts/qcom-ipq8064.dtsi +10 -1
@@ -55,7 +55,7 @@ no-map; }; - smem at 41000000 { + smem: smem at 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 {
Signature · 1 line
2.1.4
Re: [PATCH 2/3] ARM: qcom: add SMEM device node to IPQ806x dts
arch/arm/boot/dts/qcom-ipq8064.dtsi line 87
Quoted 10 lines · Mathieu Olivari
arch/arm/boot/dts/qcom-ipq8064.dtsi +6
-0@@ -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 /.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 2/3] ARM: qcom: add SMEM device node to IPQ806x dts
Quoted 14 lines · Stephen Boyd
arch/arm/boot/dts/qcom-ipq8064.dtsi line 86
Quoted 9 lines · Mathieu Olivari
arch/arm/boot/dts/qcom-ipq8064.dtsi +6
-0@@ -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.
Quoted 5 lines
Signature · 3 lines
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 3/3 mtd: add SMEM parser for QCOM platforms
In reply to Mathieu OlivariOn 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.
--- 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
drivers/mtd/Kconfig +7 -0
@@ -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" #
drivers/mtd/Makefile +1 -0
@@ -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
drivers/mtd/qcom_smem_part.c new file +224 -0
224 lines not on this page · raw message
Signature · 1 line
2.1.4
Re: [PATCH 3/3] mtd: add SMEM parser for QCOM platforms
drivers/mtd/qcom_smem_part.c line 48
Quoted 13 lines · Mathieu Olivari
drivers/mtd/qcom_smem_part.c new file +7
-0+#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.
drivers/mtd/qcom_smem_part.c line 54
Quoted 6 lines
drivers/mtd/qcom_smem_part.c new file +6
-0+}; + +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.
drivers/mtd/qcom_smem_part.c line 58
Quoted 4 lines
drivers/mtd/qcom_smem_part.c new file +4
-0+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.
drivers/mtd/qcom_smem_part.c line 64
Quoted 6 lines
drivers/mtd/qcom_smem_part.c new file +6
-0+{ +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?
Quoted 51 lines
drivers/mtd/qcom_smem_part.c new file +51
-0+ +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 ; +} +
drivers/mtd/qcom_smem_part.c line 125
Quoted 10 lines
drivers/mtd/qcom_smem_part.c new file +10
-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().
Quoted 41 lines
drivers/mtd/qcom_smem_part.c new file +41
-0+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 )
drivers/mtd/qcom_smem_part.c line 176
Quoted 10 lines
drivers/mtd/qcom_smem_part.c new file +10
-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)?
drivers/mtd/qcom_smem_part.c line 178
Quoted 2 lines
drivers/mtd/qcom_smem_part.c new file +2
-0+ GFP_KERNEL); +if (mtd_parts ==NULL )
if (!mtd_parts) is more kernel style.
Quoted 6 lines
drivers/mtd/qcom_smem_part.c new file +6
-0+return -ENOMEM ; + +for (i =0 ; i < smem_parts->len; i++) { +struct smem_partition *part = &smem_parts->parts[i]; + + mtd_parts[i].name = part->name;
drivers/mtd/qcom_smem_part.c line 194
Quoted 10 lines
drivers/mtd/qcom_smem_part.c new file +10
-0+ 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.
drivers/mtd/qcom_smem_part.c line 197
Quoted 3 lines
drivers/mtd/qcom_smem_part.c new file +3
-0+ } + + *pparts = mtd_parts;
Oh we already have the pointer, so just do this assignment before iterating.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 3/3] mtd: add SMEM parser for QCOM platforms
Quoted 7 lines · Stephen Boyd
Quoted 6 lines · Mathieu Olivari
drivers/mtd/qcom_smem_part.c new file +0
-0
Quoted 9 lines
drivers/mtd/qcom_smem_part.c line 48
Quoted 7 lines
drivers/mtd/qcom_smem_part.c new file +7
-0+#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.
Quoted 10 lines
drivers/mtd/qcom_smem_part.c line 54
Quoted 7 lines
drivers/mtd/qcom_smem_part.c new file +6
-0+}; + +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.
Quoted 8 lines
drivers/mtd/qcom_smem_part.c line 58
Quoted 5 lines
drivers/mtd/qcom_smem_part.c new file +4
-0+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.
Quoted 9 lines
drivers/mtd/qcom_smem_part.c line 64
Quoted 7 lines
drivers/mtd/qcom_smem_part.c new file +6
-0+{ +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.
Quoted 57 lines
Quoted 57 lines
drivers/mtd/qcom_smem_part.c new file +56
-0+ +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; +} +
Quoted 8 lines
drivers/mtd/qcom_smem_part.c line 125
Quoted 5 lines
drivers/mtd/qcom_smem_part.c new file +5
-0+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().
Quoted 46 lines
Quoted 46 lines
drivers/mtd/qcom_smem_part.c new file +45
-0+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 ) {
Quoted 8 lines
drivers/mtd/qcom_smem_part.c line 176
Quoted 6 lines
drivers/mtd/qcom_smem_part.c new file +6
-0+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.
Quoted 5 lines
drivers/mtd/qcom_smem_part.c line 178
Quoted 3 lines
drivers/mtd/qcom_smem_part.c new file +2
-0+ GFP_KERNEL); +if (mtd_parts ==NULL )if (!mtd_parts) is more kernel style.
OK.
Quoted 13 lines
Quoted 13 lines
drivers/mtd/qcom_smem_part.c new file +12
-0+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
Quoted 8 lines
drivers/mtd/qcom_smem_part.c line 194
Quoted 4 lines
drivers/mtd/qcom_smem_part.c new file +4
-0+* 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->partandmtd_partsand increment both in the loop to make tighter code.
OK; I'll simplify it by making a new pointer.
Quoted 6 lines
drivers/mtd/qcom_smem_part.c line 197
Quoted 4 lines
drivers/mtd/qcom_smem_part.c new file +3
-0+ } + + *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.
Quoted 5 lines
Signature · 3 lines
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project