[V1 2/2] drivers soc: add support for ST stm32mp13xx family

Rodolfo Giometti posted 2 patches 7 months ago
[V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Rodolfo Giometti 7 months ago
This patch adds SoC support for the ST stm32mp13xx family. It also
adds the special attribute "secure" which returns the CPU's secure
mode status.

Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
---
 drivers/soc/st/Makefile        |   1 +
 drivers/soc/st/soc-stm32mp13.c | 253 +++++++++++++++++++++++++++++++++
 2 files changed, 254 insertions(+)
 create mode 100644 drivers/soc/st/soc-stm32mp13.c

diff --git a/drivers/soc/st/Makefile b/drivers/soc/st/Makefile
index 6c71607f6c89..c84bf510928d 100644
--- a/drivers/soc/st/Makefile
+++ b/drivers/soc/st/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_STM32_PM_DOMAINS) += stm32_pm_domain.o
 obj-$(CONFIG_STM32_RISAB) += stm32_risab.o
 obj-$(CONFIG_STM32_RISAF) += stm32_risaf.o
+obj-$(CONFIG_MACH_STM32MP13) += soc-stm32mp13.o
diff --git a/drivers/soc/st/soc-stm32mp13.c b/drivers/soc/st/soc-stm32mp13.c
new file mode 100644
index 000000000000..cf45dbeb926a
--- /dev/null
+++ b/drivers/soc/st/soc-stm32mp13.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025 Rodolfo Giometti <giometti@enneenne.com>
+ */
+
+#include <linux/cpu.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/sys_soc.h>
+
+#define STM32MP131A	0x6C9
+#define STM32MP131C	0x6C8
+#define STM32MP131D	0xEC9
+#define STM32MP131F	0xEC8
+#define STM32MP133A	0x0C1
+#define STM32MP133C	0x0C0
+#define STM32MP133D	0x8C1
+#define STM32MP133F	0x8C0
+#define STM32MP135A	0x001
+#define STM32MP135C	0x000
+#define STM32MP135D	0x801
+#define STM32MP135F	0x800
+
+#define BSEC_RPN	0x204
+#define BSEC_UID	0x234
+#define SYSCFG_IDC	0x380
+
+/*
+ * SoC attributes
+ */
+
+static ssize_t
+secure_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	u16 val;
+	char *str;
+	int ret;
+	struct device *cpu_dev;
+
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev) {
+		dev_err(dev, "failed to get cpu0 device\n");
+		return -ENODEV;
+	}
+	ret = nvmem_cell_read_u16(cpu_dev, "encoding_mode", &val);
+	if (ret)
+		return ret;
+
+	switch (val) {
+	case 0b0000010111:
+		str = "open";
+		break;
+	case 0b0000111111:
+		str = "closed";
+		break;
+	case 0b0101111111:
+		str = "closed boundary-scan-disabled]";
+		break;
+	case 0b1111111111:
+		str = "closed JTAG-disabled";
+		break;
+	default:
+		str = "unknown";
+	}
+
+	return sprintf(buf, "%s\n", str);
+}
+static DEVICE_ATTR_RO(secure);
+
+static struct attribute *stm32mp13_soc_attrs[] = {
+	&dev_attr_secure.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(stm32mp13_soc);
+
+/*
+ * Driver init functions
+ */
+
+static int __init stm32mp13_soc_get_rpn_uid(u32 *rpn, u32 uid[3])
+{
+	struct device_node *np;
+	void __iomem *regs;
+	static const struct of_device_id devids[] = {
+		{ .compatible = "st,stm32mp13-bsec" },
+		{ },
+	};
+
+	np = of_find_matching_node(NULL, devids);
+	if (!np)
+		return -ENODEV;
+
+	regs = of_iomap(np, 0);
+	of_node_put(np);
+
+	if (!regs) {
+		pr_warn("Could not map BSEC iomem range");
+		return -ENXIO;
+	}
+
+	*rpn = readl(regs + BSEC_RPN) & 0x0fff;
+	uid[0] = readl(regs + BSEC_UID + 0);
+	uid[1] = readl(regs + BSEC_UID + 4);
+	uid[2] = readl(regs + BSEC_UID + 8);
+
+	iounmap(regs);
+
+	return 0;
+}
+
+static int __init stm32mp13_soc_get_idc(u32 *idc)
+{
+	struct device_node *np;
+	void __iomem *regs;
+	static const struct of_device_id devids[] = {
+		{ .compatible = "st,stm32mp157-syscfg" },
+		{ },
+	};
+
+	np = of_find_matching_node(NULL, devids);
+	if (!np)
+		return -ENODEV;
+
+	regs = of_iomap(np, 0);
+	of_node_put(np);
+
+	if (!regs) {
+		pr_warn("Could not map BSEC iomem range");
+		return -ENXIO;
+	}
+
+	*idc = readl(regs + SYSCFG_IDC);
+
+	iounmap(regs);
+
+	return 0;
+}
+
+static int __init stm32mp13_soc_device_init(void)
+{
+	u32 part_number, rev, chipid[3];
+	struct soc_device_attribute *soc_dev_attr;
+	struct soc_device *soc_dev;
+	struct device_node *root;
+	const char *soc_id;
+	int ret;
+
+	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+	if (!soc_dev_attr)
+		return -ENOMEM;
+	soc_dev_attr->family = "STM STM32MP13xx";
+
+	root = of_find_node_by_path("/");
+	ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
+	if (ret)
+		of_property_read_string_index(root, "compatible", 0,
+						&soc_dev_attr->machine);
+	of_node_put(root);
+	if (ret)
+		goto free_soc;
+
+	/* Get chip info */
+	ret = stm32mp13_soc_get_rpn_uid(&part_number, chipid);
+	if (ret) {
+		pr_err("failed to get chip part number: %d\n", ret);
+		goto free_soc;
+	}
+	switch (part_number) {
+	case STM32MP131A:
+		soc_id = "131a";
+		break;
+	case STM32MP131C:
+		soc_id = "131c";
+		break;
+	case STM32MP131D:
+		soc_id = "131d";
+		break;
+	case STM32MP131F:
+		soc_id = "131f";
+		break;
+	case STM32MP133A:
+		soc_id = "133a";
+		break;
+	case STM32MP133C:
+		soc_id = "133c";
+		break;
+	case STM32MP133D:
+		soc_id = "133d";
+		break;
+	case STM32MP133F:
+		soc_id = "133f";
+		break;
+	case STM32MP135A:
+		soc_id = "135a";
+		break;
+	case STM32MP135C:
+		soc_id = "135c";
+		break;
+	case STM32MP135D:
+		soc_id = "135d";
+		break;
+	case STM32MP135F:
+		soc_id = "135f";
+		break;
+	default:
+		soc_id = "unknown";
+	}
+	soc_dev_attr->soc_id = soc_id;
+
+	ret = stm32mp13_soc_get_idc(&rev);
+	if (ret)
+		goto free_soc;
+	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%X", rev >> 16);
+	if (!soc_dev_attr->revision) {
+		ret = -ENOMEM;
+		goto free_soc;
+	}
+
+	soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%08X%08X%08X",
+					chipid[0], chipid[1], chipid[2]);
+	if (!soc_dev_attr->serial_number) {
+		ret = -ENOMEM;
+		goto free_rev;
+	}
+
+	/* Add custom attributes group */
+	soc_dev_attr->custom_attr_group = stm32mp13_soc_groups[0];
+
+	/* Register the SOC device */
+	soc_dev = soc_device_register(soc_dev_attr);
+	if (IS_ERR(soc_dev)) {
+		ret = PTR_ERR(soc_dev);
+		goto free_serial_number;
+	}
+
+	pr_info("SoC Machine: %s\n", soc_dev_attr->machine);
+	pr_info("SoC family: %s\n", soc_dev_attr->family);
+	pr_info("SoC ID: %s, Revision: %s\n",
+		soc_dev_attr->soc_id, soc_dev_attr->revision);
+
+	return 0;
+
+free_serial_number:
+	kfree(soc_dev_attr->serial_number);
+free_rev:
+	kfree(soc_dev_attr->revision);
+free_soc:
+	kfree(soc_dev_attr);
+	return ret;
+}
+device_initcall(stm32mp13_soc_device_init);
-- 
2.25.1
Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Yann Gautier 7 months ago
On 5/19/25 15:08, Rodolfo Giometti wrote:
> This patch adds SoC support for the ST stm32mp13xx family. It also
> adds the special attribute "secure" which returns the CPU's secure
> mode status.
> 
> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
> ---
>   drivers/soc/st/Makefile        |   1 +
>   drivers/soc/st/soc-stm32mp13.c | 253 +++++++++++++++++++++++++++++++++
>   2 files changed, 254 insertions(+)
>   create mode 100644 drivers/soc/st/soc-stm32mp13.c
> 
> diff --git a/drivers/soc/st/Makefile b/drivers/soc/st/Makefile
> index 6c71607f6c89..c84bf510928d 100644
> --- a/drivers/soc/st/Makefile
> +++ b/drivers/soc/st/Makefile
> @@ -1,3 +1,4 @@
>   obj-$(CONFIG_STM32_PM_DOMAINS) += stm32_pm_domain.o
>   obj-$(CONFIG_STM32_RISAB) += stm32_risab.o
>   obj-$(CONFIG_STM32_RISAF) += stm32_risaf.o
> +obj-$(CONFIG_MACH_STM32MP13) += soc-stm32mp13.o
> diff --git a/drivers/soc/st/soc-stm32mp13.c b/drivers/soc/st/soc-stm32mp13.c
> new file mode 100644
> index 000000000000..cf45dbeb926a
> --- /dev/null
> +++ b/drivers/soc/st/soc-stm32mp13.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025 Rodolfo Giometti <giometti@enneenne.com>
> + */
> +
> +#include <linux/cpu.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/sys_soc.h>
> +
> +#define STM32MP131A	0x6C9
> +#define STM32MP131C	0x6C8
> +#define STM32MP131D	0xEC9
> +#define STM32MP131F	0xEC8
> +#define STM32MP133A	0x0C1
> +#define STM32MP133C	0x0C0
> +#define STM32MP133D	0x8C1
> +#define STM32MP133F	0x8C0
> +#define STM32MP135A	0x001
> +#define STM32MP135C	0x000
> +#define STM32MP135D	0x801
> +#define STM32MP135F	0x800
> +
> +#define BSEC_RPN	0x204
> +#define BSEC_UID	0x234
> +#define SYSCFG_IDC	0x380
> +
> +/*
> + * SoC attributes
> + */
> +
> +static ssize_t
> +secure_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	u16 val;
> +	char *str;
> +	int ret;
> +	struct device *cpu_dev;
> +
> +	cpu_dev = get_cpu_device(0);
> +	if (!cpu_dev) {
> +		dev_err(dev, "failed to get cpu0 device\n");
> +		return -ENODEV;
> +	}
> +	ret = nvmem_cell_read_u16(cpu_dev, "encoding_mode", &val);
> +	if (ret)
> +		return ret;
> +
> +	switch (val) {
> +	case 0b0000010111:
> +		str = "open";
> +		break;
> +	case 0b0000111111:
> +		str = "closed";
> +		break;
> +	case 0b0101111111:
> +		str = "closed boundary-scan-disabled]";
> +		break;
> +	case 0b1111111111:
> +		str = "closed JTAG-disabled";
> +		break;
> +	default:
> +		str = "unknown";
> +	}
> +
> +	return sprintf(buf, "%s\n", str);
> +}
> +static DEVICE_ATTR_RO(secure);
> +
> +static struct attribute *stm32mp13_soc_attrs[] = {
> +	&dev_attr_secure.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(stm32mp13_soc);
> +
> +/*
> + * Driver init functions
> + */
> +
> +static int __init stm32mp13_soc_get_rpn_uid(u32 *rpn, u32 uid[3])
> +{
> +	struct device_node *np;
> +	void __iomem *regs;
> +	static const struct of_device_id devids[] = {
> +		{ .compatible = "st,stm32mp13-bsec" },
> +		{ },
> +	};

As said by Krzysztof, you cannot access the OTP fuses this way.
There is already a driver for that: drivers/nvmem/stm32-romem.c.
And the information there should be accessed through nvmem framework.

For the UID, you should add this in your first patch:
			uid_otp: uid-otp@34 {
				reg = <0x34 0xc>;
			};

And add &part_number_otp and &uid_otp to your driver nvmem-cells property:
nvmem-cells = <&cfg0_otp>, <&part_number_otp>, <&uid_otp>;

> +
> +	np = of_find_matching_node(NULL, devids);
> +	if (!np)
> +		return -ENODEV;
> +
> +	regs = of_iomap(np, 0);
> +	of_node_put(np);
> +
> +	if (!regs) {
> +		pr_warn("Could not map BSEC iomem range");
> +		return -ENXIO;
> +	}
> +
> +	*rpn = readl(regs + BSEC_RPN) & 0x0fff;
> +	uid[0] = readl(regs + BSEC_UID + 0);
> +	uid[1] = readl(regs + BSEC_UID + 4);
> +	uid[2] = readl(regs + BSEC_UID + 8);
> +
> +	iounmap(regs);
> +
> +	return 0;
> +}
> +
> +static int __init stm32mp13_soc_get_idc(u32 *idc)
> +{
> +	struct device_node *np;
> +	void __iomem *regs;
> +	static const struct of_device_id devids[] = {
> +		{ .compatible = "st,stm32mp157-syscfg" },
> +		{ },
> +	};

For this one you should use st,syscfg in DT, see other example in SoC DT
file, and access the info with syscon_regmap_lookup_by_phandle(), if it
is still the good API.
I'll let Krzysztof comment on that.

> +
> +	np = of_find_matching_node(NULL, devids);
> +	if (!np)
> +		return -ENODEV;
> +
> +	regs = of_iomap(np, 0);
> +	of_node_put(np);
> +
> +	if (!regs) {
> +		pr_warn("Could not map BSEC iomem range");
> +		return -ENXIO;
> +	}
> +
> +	*idc = readl(regs + SYSCFG_IDC);
> +
> +	iounmap(regs);
> +
> +	return 0;
> +}
> +
> +static int __init stm32mp13_soc_device_init(void)
> +{
> +	u32 part_number, rev, chipid[3];
> +	struct soc_device_attribute *soc_dev_attr;
> +	struct soc_device *soc_dev;
> +	struct device_node *root;
> +	const char *soc_id;
> +	int ret;
> +
> +	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> +	if (!soc_dev_attr)
> +		return -ENOMEM;
> +	soc_dev_attr->family = "STM STM32MP13xx";
> +
> +	root = of_find_node_by_path("/");
> +	ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
> +	if (ret)
> +		of_property_read_string_index(root, "compatible", 0,
> +						&soc_dev_attr->machine);
> +	of_node_put(root);
> +	if (ret)
> +		goto free_soc;
> +
> +	/* Get chip info */
> +	ret = stm32mp13_soc_get_rpn_uid(&part_number, chipid);
> +	if (ret) {
> +		pr_err("failed to get chip part number: %d\n", ret);
> +		goto free_soc;
> +	}
> +	switch (part_number) {
> +	case STM32MP131A:
> +		soc_id = "131a";

Prefer capital letters for SoC part number: 131A


Best regards,
Yann

> +		break;
> +	case STM32MP131C:
> +		soc_id = "131c";
> +		break;
> +	case STM32MP131D:
> +		soc_id = "131d";
> +		break;
> +	case STM32MP131F:
> +		soc_id = "131f";
> +		break;
> +	case STM32MP133A:
> +		soc_id = "133a";
> +		break;
> +	case STM32MP133C:
> +		soc_id = "133c";
> +		break;
> +	case STM32MP133D:
> +		soc_id = "133d";
> +		break;
> +	case STM32MP133F:
> +		soc_id = "133f";
> +		break;
> +	case STM32MP135A:
> +		soc_id = "135a";
> +		break;
> +	case STM32MP135C:
> +		soc_id = "135c";
> +		break;
> +	case STM32MP135D:
> +		soc_id = "135d";
> +		break;
> +	case STM32MP135F:
> +		soc_id = "135f";
> +		break;
> +	default:
> +		soc_id = "unknown";
> +	}
> +	soc_dev_attr->soc_id = soc_id;
> +
> +	ret = stm32mp13_soc_get_idc(&rev);
> +	if (ret)
> +		goto free_soc;
> +	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%X", rev >> 16);
> +	if (!soc_dev_attr->revision) {
> +		ret = -ENOMEM;
> +		goto free_soc;
> +	}
> +
> +	soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%08X%08X%08X",
> +					chipid[0], chipid[1], chipid[2]);
> +	if (!soc_dev_attr->serial_number) {
> +		ret = -ENOMEM;
> +		goto free_rev;
> +	}
> +
> +	/* Add custom attributes group */
> +	soc_dev_attr->custom_attr_group = stm32mp13_soc_groups[0];
> +
> +	/* Register the SOC device */
> +	soc_dev = soc_device_register(soc_dev_attr);
> +	if (IS_ERR(soc_dev)) {
> +		ret = PTR_ERR(soc_dev);
> +		goto free_serial_number;
> +	}
> +
> +	pr_info("SoC Machine: %s\n", soc_dev_attr->machine);
> +	pr_info("SoC family: %s\n", soc_dev_attr->family);
> +	pr_info("SoC ID: %s, Revision: %s\n",
> +		soc_dev_attr->soc_id, soc_dev_attr->revision);
> +
> +	return 0;
> +
> +free_serial_number:
> +	kfree(soc_dev_attr->serial_number);
> +free_rev:
> +	kfree(soc_dev_attr->revision);
> +free_soc:
> +	kfree(soc_dev_attr);
> +	return ret;
> +}
> +device_initcall(stm32mp13_soc_device_init);
Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Rodolfo Giometti 7 months ago
On 20/05/25 18:58, Yann Gautier wrote:
> On 5/19/25 15:08, Rodolfo Giometti wrote:

[snip]

>> +static int __init stm32mp13_soc_get_rpn_uid(u32 *rpn, u32 uid[3])
>> +{
>> +    struct device_node *np;
>> +    void __iomem *regs;
>> +    static const struct of_device_id devids[] = {
>> +        { .compatible = "st,stm32mp13-bsec" },
>> +        { },
>> +    };
> 
> As said by Krzysztof, you cannot access the OTP fuses this way.
> There is already a driver for that: drivers/nvmem/stm32-romem.c.
> And the information there should be accessed through nvmem framework.
> 
> For the UID, you should add this in your first patch:
>              uid_otp: uid-otp@34 {
>                  reg = <0x34 0xc>;
>              };
> 
> And add &part_number_otp and &uid_otp to your driver nvmem-cells property:
> nvmem-cells = <&cfg0_otp>, <&part_number_otp>, <&uid_otp>;

I see, but since the device is called by the device_initcall(), when I try to 
use the nvmem framework I always get the EPROBE_DEFER error. :(

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Krzysztof Kozlowski 7 months ago
On 21/05/2025 17:44, Rodolfo Giometti wrote:
> On 20/05/25 18:58, Yann Gautier wrote:
>> On 5/19/25 15:08, Rodolfo Giometti wrote:
> 
> [snip]
> 
>>> +static int __init stm32mp13_soc_get_rpn_uid(u32 *rpn, u32 uid[3])
>>> +{
>>> +    struct device_node *np;
>>> +    void __iomem *regs;
>>> +    static const struct of_device_id devids[] = {
>>> +        { .compatible = "st,stm32mp13-bsec" },
>>> +        { },
>>> +    };
>>
>> As said by Krzysztof, you cannot access the OTP fuses this way.
>> There is already a driver for that: drivers/nvmem/stm32-romem.c.
>> And the information there should be accessed through nvmem framework.
>>
>> For the UID, you should add this in your first patch:
>>              uid_otp: uid-otp@34 {
>>                  reg = <0x34 0xc>;
>>              };
>>
>> And add &part_number_otp and &uid_otp to your driver nvmem-cells property:
>> nvmem-cells = <&cfg0_otp>, <&part_number_otp>, <&uid_otp>;
> 
> I see, but since the device is called by the device_initcall(), when I try to 
> use the nvmem framework I always get the EPROBE_DEFER error. :(

.... for a reason.

I already said, this must not be device_initcall(), at least not without
proper arguments which were not given. IMHO, these should be also
documented, but we have poor history of doing that.

In any case manual probe ordering is fragile and should be avoided.

Best regards,
Krzysztof
Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Alexandre TORGUE 7 months ago
hi

On 5/19/25 15:08, Rodolfo Giometti wrote:
> This patch adds SoC support for the ST stm32mp13xx family. It also
> adds the special attribute "secure" which returns the CPU's secure
> mode status.
> 
> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
> ---
>   drivers/soc/st/Makefile        |   1 +
>   drivers/soc/st/soc-stm32mp13.c | 253 +++++++++++++++++++++++++++++++++
>   2 files changed, 254 insertions(+)
>   create mode 100644 drivers/soc/st/soc-stm32mp13.c
> 
> diff --git a/drivers/soc/st/Makefile b/drivers/soc/st/Makefile
> index 6c71607f6c89..c84bf510928d 100644
> --- a/drivers/soc/st/Makefile
> +++ b/drivers/soc/st/Makefile
> @@ -1,3 +1,4 @@
>   obj-$(CONFIG_STM32_PM_DOMAINS) += stm32_pm_domain.o
>   obj-$(CONFIG_STM32_RISAB) += stm32_risab.o
>   obj-$(CONFIG_STM32_RISAF) += stm32_risaf.o
> +obj-$(CONFIG_MACH_STM32MP13) += soc-stm32mp13.o

Your patch does not applied because the file does not exist. You can't 
take a patch on our github, push it as it is without rebase it on 
mainline kernel.

regards
alex

> diff --git a/drivers/soc/st/soc-stm32mp13.c b/drivers/soc/st/soc-stm32mp13.c
> new file mode 100644
> index 000000000000..cf45dbeb926a
> --- /dev/null
> +++ b/drivers/soc/st/soc-stm32mp13.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025 Rodolfo Giometti <giometti@enneenne.com>
> + */
> +
> +#include <linux/cpu.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/sys_soc.h>
> +
> +#define STM32MP131A	0x6C9
> +#define STM32MP131C	0x6C8
> +#define STM32MP131D	0xEC9
> +#define STM32MP131F	0xEC8
> +#define STM32MP133A	0x0C1
> +#define STM32MP133C	0x0C0
> +#define STM32MP133D	0x8C1
> +#define STM32MP133F	0x8C0
> +#define STM32MP135A	0x001
> +#define STM32MP135C	0x000
> +#define STM32MP135D	0x801
> +#define STM32MP135F	0x800
> +
> +#define BSEC_RPN	0x204
> +#define BSEC_UID	0x234
> +#define SYSCFG_IDC	0x380
> +
> +/*
> + * SoC attributes
> + */
> +
> +static ssize_t
> +secure_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	u16 val;
> +	char *str;
> +	int ret;
> +	struct device *cpu_dev;
> +
> +	cpu_dev = get_cpu_device(0);
> +	if (!cpu_dev) {
> +		dev_err(dev, "failed to get cpu0 device\n");
> +		return -ENODEV;
> +	}
> +	ret = nvmem_cell_read_u16(cpu_dev, "encoding_mode", &val);
> +	if (ret)
> +		return ret;
> +
> +	switch (val) {
> +	case 0b0000010111:
> +		str = "open";
> +		break;
> +	case 0b0000111111:
> +		str = "closed";
> +		break;
> +	case 0b0101111111:
> +		str = "closed boundary-scan-disabled]";
> +		break;
> +	case 0b1111111111:
> +		str = "closed JTAG-disabled";
> +		break;
> +	default:
> +		str = "unknown";
> +	}
> +
> +	return sprintf(buf, "%s\n", str);
> +}
> +static DEVICE_ATTR_RO(secure);
> +
> +static struct attribute *stm32mp13_soc_attrs[] = {
> +	&dev_attr_secure.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(stm32mp13_soc);
> +
> +/*
> + * Driver init functions
> + */
> +
> +static int __init stm32mp13_soc_get_rpn_uid(u32 *rpn, u32 uid[3])
> +{
> +	struct device_node *np;
> +	void __iomem *regs;
> +	static const struct of_device_id devids[] = {
> +		{ .compatible = "st,stm32mp13-bsec" },
> +		{ },
> +	};
> +
> +	np = of_find_matching_node(NULL, devids);
> +	if (!np)
> +		return -ENODEV;
> +
> +	regs = of_iomap(np, 0);
> +	of_node_put(np);
> +
> +	if (!regs) {
> +		pr_warn("Could not map BSEC iomem range");
> +		return -ENXIO;
> +	}
> +
> +	*rpn = readl(regs + BSEC_RPN) & 0x0fff;
> +	uid[0] = readl(regs + BSEC_UID + 0);
> +	uid[1] = readl(regs + BSEC_UID + 4);
> +	uid[2] = readl(regs + BSEC_UID + 8);
> +
> +	iounmap(regs);
> +
> +	return 0;
> +}
> +
> +static int __init stm32mp13_soc_get_idc(u32 *idc)
> +{
> +	struct device_node *np;
> +	void __iomem *regs;
> +	static const struct of_device_id devids[] = {
> +		{ .compatible = "st,stm32mp157-syscfg" },
> +		{ },
> +	};
> +
> +	np = of_find_matching_node(NULL, devids);
> +	if (!np)
> +		return -ENODEV;
> +
> +	regs = of_iomap(np, 0);
> +	of_node_put(np);
> +
> +	if (!regs) {
> +		pr_warn("Could not map BSEC iomem range");
> +		return -ENXIO;
> +	}
> +
> +	*idc = readl(regs + SYSCFG_IDC);
> +
> +	iounmap(regs);
> +
> +	return 0;
> +}
> +
> +static int __init stm32mp13_soc_device_init(void)
> +{
> +	u32 part_number, rev, chipid[3];
> +	struct soc_device_attribute *soc_dev_attr;
> +	struct soc_device *soc_dev;
> +	struct device_node *root;
> +	const char *soc_id;
> +	int ret;
> +
> +	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> +	if (!soc_dev_attr)
> +		return -ENOMEM;
> +	soc_dev_attr->family = "STM STM32MP13xx";
> +
> +	root = of_find_node_by_path("/");
> +	ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
> +	if (ret)
> +		of_property_read_string_index(root, "compatible", 0,
> +						&soc_dev_attr->machine);
> +	of_node_put(root);
> +	if (ret)
> +		goto free_soc;
> +
> +	/* Get chip info */
> +	ret = stm32mp13_soc_get_rpn_uid(&part_number, chipid);
> +	if (ret) {
> +		pr_err("failed to get chip part number: %d\n", ret);
> +		goto free_soc;
> +	}
> +	switch (part_number) {
> +	case STM32MP131A:
> +		soc_id = "131a";
> +		break;
> +	case STM32MP131C:
> +		soc_id = "131c";
> +		break;
> +	case STM32MP131D:
> +		soc_id = "131d";
> +		break;
> +	case STM32MP131F:
> +		soc_id = "131f";
> +		break;
> +	case STM32MP133A:
> +		soc_id = "133a";
> +		break;
> +	case STM32MP133C:
> +		soc_id = "133c";
> +		break;
> +	case STM32MP133D:
> +		soc_id = "133d";
> +		break;
> +	case STM32MP133F:
> +		soc_id = "133f";
> +		break;
> +	case STM32MP135A:
> +		soc_id = "135a";
> +		break;
> +	case STM32MP135C:
> +		soc_id = "135c";
> +		break;
> +	case STM32MP135D:
> +		soc_id = "135d";
> +		break;
> +	case STM32MP135F:
> +		soc_id = "135f";
> +		break;
> +	default:
> +		soc_id = "unknown";
> +	}
> +	soc_dev_attr->soc_id = soc_id;
> +
> +	ret = stm32mp13_soc_get_idc(&rev);
> +	if (ret)
> +		goto free_soc;
> +	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%X", rev >> 16);
> +	if (!soc_dev_attr->revision) {
> +		ret = -ENOMEM;
> +		goto free_soc;
> +	}
> +
> +	soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%08X%08X%08X",
> +					chipid[0], chipid[1], chipid[2]);
> +	if (!soc_dev_attr->serial_number) {
> +		ret = -ENOMEM;
> +		goto free_rev;
> +	}
> +
> +	/* Add custom attributes group */
> +	soc_dev_attr->custom_attr_group = stm32mp13_soc_groups[0];
> +
> +	/* Register the SOC device */
> +	soc_dev = soc_device_register(soc_dev_attr);
> +	if (IS_ERR(soc_dev)) {
> +		ret = PTR_ERR(soc_dev);
> +		goto free_serial_number;
> +	}
> +
> +	pr_info("SoC Machine: %s\n", soc_dev_attr->machine);
> +	pr_info("SoC family: %s\n", soc_dev_attr->family);
> +	pr_info("SoC ID: %s, Revision: %s\n",
> +		soc_dev_attr->soc_id, soc_dev_attr->revision);
> +
> +	return 0;
> +
> +free_serial_number:
> +	kfree(soc_dev_attr->serial_number);
> +free_rev:
> +	kfree(soc_dev_attr->revision);
> +free_soc:
> +	kfree(soc_dev_attr);
> +	return ret;
> +}
> +device_initcall(stm32mp13_soc_device_init);
Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Rodolfo Giometti 6 months, 3 weeks ago
On 20/05/25 10:55, Alexandre TORGUE wrote:
> hi
> 
> On 5/19/25 15:08, Rodolfo Giometti wrote:
>> This patch adds SoC support for the ST stm32mp13xx family. It also
>> adds the special attribute "secure" which returns the CPU's secure
>> mode status.
>>
>> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
>> ---
>>   drivers/soc/st/Makefile        |   1 +
>>   drivers/soc/st/soc-stm32mp13.c | 253 +++++++++++++++++++++++++++++++++
>>   2 files changed, 254 insertions(+)
>>   create mode 100644 drivers/soc/st/soc-stm32mp13.c
>>
>> diff --git a/drivers/soc/st/Makefile b/drivers/soc/st/Makefile
>> index 6c71607f6c89..c84bf510928d 100644
>> --- a/drivers/soc/st/Makefile
>> +++ b/drivers/soc/st/Makefile
>> @@ -1,3 +1,4 @@
>>   obj-$(CONFIG_STM32_PM_DOMAINS) += stm32_pm_domain.o
>>   obj-$(CONFIG_STM32_RISAB) += stm32_risab.o
>>   obj-$(CONFIG_STM32_RISAF) += stm32_risaf.o
>> +obj-$(CONFIG_MACH_STM32MP13) += soc-stm32mp13.o
> 
> Your patch does not applied because the file does not exist. You can't take a 
> patch on our github, push it as it is without rebase it on mainline kernel.
OK, my patch set doesn't apply to the vanilla kernel, and I'm going to remove 
linux-kernel@vger.kernel.org from the recipients list.

Stated this, where should I send my patches? Would the 
linux-stm32@st-md-mailman.stormreply.com list be a suitable candidate? :)

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming

Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Krzysztof Kozlowski 7 months ago
On 19/05/2025 15:08, Rodolfo Giometti wrote:
> This patch adds SoC support for the ST stm32mp13xx family. It also

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95


> adds the special attribute "secure" which returns the CPU's secure
> mode status.
> 
> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>


You Cc-ed linux-kernel, so I assume this is patch for mainline kernel.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument, so you will
not CC people just because they made one commit years ago). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

...

> +
> +	switch (val) {
> +	case 0b0000010111:
> +		str = "open";
> +		break;
> +	case 0b0000111111:
> +		str = "closed";
> +		break;
> +	case 0b0101111111:
> +		str = "closed boundary-scan-disabled]";
> +		break;
> +	case 0b1111111111:
> +		str = "closed JTAG-disabled";
> +		break;
> +	default:
> +		str = "unknown";
> +	}
> +
> +	return sprintf(buf, "%s\n", str);
> +}
> +static DEVICE_ATTR_RO(secure);

Where is ABI documented?

> +
> +static struct attribute *stm32mp13_soc_attrs[] = {
> +	&dev_attr_secure.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(stm32mp13_soc);


> +
> +static int __init stm32mp13_soc_get_idc(u32 *idc)
> +{
> +	struct device_node *np;
> +	void __iomem *regs;
> +	static const struct of_device_id devids[] = {
> +		{ .compatible = "st,stm32mp157-syscfg" },

No, don't add compatibles for other devices into the driver functions.
Use standard methods for binding, like every driver does.

> +		{ },
> +	};
> +
> +	np = of_find_matching_node(NULL, devids);
> +	if (!np)
> +		return -ENODEV;
> +
> +	regs = of_iomap(np, 0);
> +	of_node_put(np);
> +
> +	if (!regs) {
> +		pr_warn("Could not map BSEC iomem range");
> +		return -ENXIO;
> +	}
> +
> +	*idc = readl(regs + SYSCFG_IDC);
> +
> +	iounmap(regs);
> +
> +	return 0;
> +}
> +
> +static int __init stm32mp13_soc_device_init(void)
> +{
> +	u32 part_number, rev, chipid[3];
> +	struct soc_device_attribute *soc_dev_attr;
> +	struct soc_device *soc_dev;
> +	struct device_node *root;
> +	const char *soc_id;
> +	int ret;
> +
> +	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> +	if (!soc_dev_attr)
> +		return -ENOMEM;
> +	soc_dev_attr->family = "STM STM32MP13xx";
> +
> +	root = of_find_node_by_path("/");
> +	ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
> +	if (ret)
> +		of_property_read_string_index(root, "compatible", 0,
> +						&soc_dev_attr->machine);
> +	of_node_put(root);
> +	if (ret)
> +		goto free_soc;
> +
> +	/* Get chip info */
> +	ret = stm32mp13_soc_get_rpn_uid(&part_number, chipid);
> +	if (ret) {
> +		pr_err("failed to get chip part number: %d\n", ret);
> +		goto free_soc;
> +	}
> +	switch (part_number) {
> +	case STM32MP131A:
> +		soc_id = "131a";
> +		break;
> +	case STM32MP131C:
> +		soc_id = "131c";
> +		break;
> +	case STM32MP131D:
> +		soc_id = "131d";
> +		break;
> +	case STM32MP131F:
> +		soc_id = "131f";
> +		break;
> +	case STM32MP133A:
> +		soc_id = "133a";
> +		break;
> +	case STM32MP133C:
> +		soc_id = "133c";
> +		break;
> +	case STM32MP133D:
> +		soc_id = "133d";
> +		break;
> +	case STM32MP133F:
> +		soc_id = "133f";
> +		break;
> +	case STM32MP135A:
> +		soc_id = "135a";
> +		break;
> +	case STM32MP135C:
> +		soc_id = "135c";
> +		break;
> +	case STM32MP135D:
> +		soc_id = "135d";
> +		break;
> +	case STM32MP135F:
> +		soc_id = "135f";
> +		break;
> +	default:
> +		soc_id = "unknown";
> +	}
> +	soc_dev_attr->soc_id = soc_id;
> +
> +	ret = stm32mp13_soc_get_idc(&rev);
> +	if (ret)
> +		goto free_soc;
> +	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%X", rev >> 16);
> +	if (!soc_dev_attr->revision) {
> +		ret = -ENOMEM;
> +		goto free_soc;
> +	}
> +
> +	soc_dev_attr->serial_number = kasprintf(GFP_KERNEL, "%08X%08X%08X",
> +					chipid[0], chipid[1], chipid[2]);
> +	if (!soc_dev_attr->serial_number) {
> +		ret = -ENOMEM;
> +		goto free_rev;
> +	}
> +
> +	/* Add custom attributes group */
> +	soc_dev_attr->custom_attr_group = stm32mp13_soc_groups[0];
> +
> +	/* Register the SOC device */
> +	soc_dev = soc_device_register(soc_dev_attr);
> +	if (IS_ERR(soc_dev)) {
> +		ret = PTR_ERR(soc_dev);
> +		goto free_serial_number;
> +	}
> +
> +	pr_info("SoC Machine: %s\n", soc_dev_attr->machine);
> +	pr_info("SoC family: %s\n", soc_dev_attr->family);
> +	pr_info("SoC ID: %s, Revision: %s\n",
> +		soc_dev_attr->soc_id, soc_dev_attr->revision);

So this will print and spam every dmesg for every user of the kernel.
No, this has to be regular platform driver and only one print (see
kernel coding style about drivers being silent).


Best regards,
Krzysztof
Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Rodolfo Giometti 7 months ago
On 19/05/25 20:34, Krzysztof Kozlowski wrote:
> On 19/05/2025 15:08, Rodolfo Giometti wrote:

[snip]

>> +
>> +static int __init stm32mp13_soc_get_idc(u32 *idc)
>> +{
>> +	struct device_node *np;
>> +	void __iomem *regs;
>> +	static const struct of_device_id devids[] = {
>> +		{ .compatible = "st,stm32mp157-syscfg" },
> 
> No, don't add compatibles for other devices into the driver functions.
> Use standard methods for binding, like every driver does.

I need to access a region assigned to another driver very early on boot, and 
this is the only way I've found to solve the problem. Can you please give me an 
example of these standard binding methods?

Thanks in advance,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming
Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
Posted by Krzysztof Kozlowski 7 months ago
On 20/05/2025 11:01, Rodolfo Giometti wrote:
> On 19/05/25 20:34, Krzysztof Kozlowski wrote:
>> On 19/05/2025 15:08, Rodolfo Giometti wrote:
> 
> [snip]
> 
>>> +
>>> +static int __init stm32mp13_soc_get_idc(u32 *idc)
>>> +{
>>> +	struct device_node *np;
>>> +	void __iomem *regs;
>>> +	static const struct of_device_id devids[] = {
>>> +		{ .compatible = "st,stm32mp157-syscfg" },
>>
>> No, don't add compatibles for other devices into the driver functions.
>> Use standard methods for binding, like every driver does.
> 
> I need to access a region assigned to another driver very early on boot, and 
> this is the only way I've found to solve the problem. Can you please give me an 

Why do you need to access it very early? The code did not suggest that.

> example of these standard binding methods?

Driver should bind just like all regular drivers, so other socinfo
drivers as well. If you need other nodes, then use phandles for a syscon
- that's the standard method.

Best regards,
Krzysztof