* Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
2025-05-19 13:08 ` [V1 2/2] drivers soc: add support for ST stm32mp13xx family Rodolfo Giometti
@ 2025-05-19 18:34 ` Krzysztof Kozlowski
2025-05-20 9:01 ` Rodolfo Giometti
2025-05-20 8:55 ` Alexandre TORGUE
2025-05-20 16:58 ` Yann Gautier
2 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-19 18:34 UTC (permalink / raw)
To: Rodolfo Giometti, linux-kernel
Cc: Maxime Coquelin, Alexandre Torgue, Eric Fourmont, Yann GAUTIER
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
2025-05-19 18:34 ` Krzysztof Kozlowski
@ 2025-05-20 9:01 ` Rodolfo Giometti
2025-05-20 9:15 ` Krzysztof Kozlowski
0 siblings, 1 reply; 15+ messages in thread
From: Rodolfo Giometti @ 2025-05-20 9:01 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Maxime Coquelin, Alexandre Torgue, Eric Fourmont, Yann GAUTIER,
linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
2025-05-20 9:01 ` Rodolfo Giometti
@ 2025-05-20 9:15 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-20 9:15 UTC (permalink / raw)
To: Rodolfo Giometti
Cc: Maxime Coquelin, Alexandre Torgue, Eric Fourmont, Yann GAUTIER,
linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
2025-05-19 13:08 ` [V1 2/2] drivers soc: add support for ST stm32mp13xx family Rodolfo Giometti
2025-05-19 18:34 ` Krzysztof Kozlowski
@ 2025-05-20 8:55 ` Alexandre TORGUE
2025-05-26 10:56 ` Rodolfo Giometti
2025-05-20 16:58 ` Yann Gautier
2 siblings, 1 reply; 15+ messages in thread
From: Alexandre TORGUE @ 2025-05-20 8:55 UTC (permalink / raw)
To: Rodolfo Giometti, linux-kernel
Cc: Maxime Coquelin, Eric Fourmont, Yann GAUTIER
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);
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
2025-05-20 8:55 ` Alexandre TORGUE
@ 2025-05-26 10:56 ` Rodolfo Giometti
0 siblings, 0 replies; 15+ messages in thread
From: Rodolfo Giometti @ 2025-05-26 10:56 UTC (permalink / raw)
To: Alexandre TORGUE
Cc: Maxime Coquelin, Eric Fourmont, Yann GAUTIER, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
2025-05-19 13:08 ` [V1 2/2] drivers soc: add support for ST stm32mp13xx family Rodolfo Giometti
2025-05-19 18:34 ` Krzysztof Kozlowski
2025-05-20 8:55 ` Alexandre TORGUE
@ 2025-05-20 16:58 ` Yann Gautier
2025-05-21 15:44 ` Rodolfo Giometti
2 siblings, 1 reply; 15+ messages in thread
From: Yann Gautier @ 2025-05-20 16:58 UTC (permalink / raw)
To: Rodolfo Giometti, linux-kernel, Krzysztof Kozlowski
Cc: Maxime Coquelin, Alexandre Torgue, Eric Fourmont
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);
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
2025-05-20 16:58 ` Yann Gautier
@ 2025-05-21 15:44 ` Rodolfo Giometti
2025-05-21 16:37 ` Krzysztof Kozlowski
0 siblings, 1 reply; 15+ messages in thread
From: Rodolfo Giometti @ 2025-05-21 15:44 UTC (permalink / raw)
To: Yann Gautier, Krzysztof Kozlowski
Cc: Maxime Coquelin, Alexandre Torgue, Eric Fourmont, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [V1 2/2] drivers soc: add support for ST stm32mp13xx family
2025-05-21 15:44 ` Rodolfo Giometti
@ 2025-05-21 16:37 ` Krzysztof Kozlowski
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-21 16:37 UTC (permalink / raw)
To: Rodolfo Giometti, Yann Gautier, Krzysztof Kozlowski
Cc: Maxime Coquelin, Alexandre Torgue, Eric Fourmont, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread