[PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information

Andrei Stefanescu posted 12 patches 2 months, 4 weeks ago
[PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Andrei Stefanescu 2 months, 4 weeks ago
The SIUL2 hardware module has registers which expose information about
the given SoC (version, SRAM size, presence of some hw modules).

Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
 drivers/mfd/nxp-siul2.c           |   6 +-
 drivers/nvmem/Kconfig             |  10 ++
 drivers/nvmem/Makefile            |   2 +
 drivers/nvmem/s32g2_siul2_nvmem.c | 232 ++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+), 1 deletion(-)
 create mode 100644 drivers/nvmem/s32g2_siul2_nvmem.c

diff --git a/drivers/mfd/nxp-siul2.c b/drivers/mfd/nxp-siul2.c
index 904f41b3c61b..edf643e4bcba 100644
--- a/drivers/mfd/nxp-siul2.c
+++ b/drivers/mfd/nxp-siul2.c
@@ -31,7 +31,11 @@
 static const struct mfd_cell nxp_siul2_devs[] = {
 	{
 		.name = "s32g-siul2-pinctrl",
-	}
+	},
+	{
+
+		.name = "s32g2-siul2-nvmem",
+	},
 };
 
 /**
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index d370b2ad11e7..6efd23a2ee17 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -454,4 +454,14 @@ config NVMEM_QORIQ_EFUSE
 	  This driver can also be built as a module. If so, the module
 	  will be called nvmem_qoriq_efuse.
 
+config NVMEM_S32G2_SIUL2
+	tristate "S32G2 SIUL2 nvmem support: SoC revision"
+	depends on ARCH_S32 || COMPILE_TEST
+	default y
+	help
+	  This is a driver to access hardware related data like SoC revision
+	  for S32G2/S32G3 SoCs.
+
+	  The revision information is retrieved from the SIUL2 module.
+
 endif
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 2021d59688db..84fef48b7ff6 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -89,3 +89,5 @@ obj-$(CONFIG_NVMEM_ZYNQMP)		+= nvmem_zynqmp_nvmem.o
 nvmem_zynqmp_nvmem-y			:= zynqmp_nvmem.o
 obj-$(CONFIG_NVMEM_QORIQ_EFUSE)		+= nvmem-qoriq-efuse.o
 nvmem-qoriq-efuse-y			:= qoriq-efuse.o
+obj-$(CONFIG_NVMEM_S32G2_SIUL2) 	+= nvmem-s32g2-siul2.o
+nvmem-s32g2-siul2-y 			:= s32g2_siul2_nvmem.o
diff --git a/drivers/nvmem/s32g2_siul2_nvmem.c b/drivers/nvmem/s32g2_siul2_nvmem.c
new file mode 100644
index 000000000000..bf62c8885ff5
--- /dev/null
+++ b/drivers/nvmem/s32g2_siul2_nvmem.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021-2025 NXP
+ */
+
+#include <linux/mfd/nxp-siul2.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* SoC revision */
+#define NVRAM_CELL_SIZE			4
+#define SIUL2_MIDR1_OFF			(0x00000000)
+#define SIUL2_MIDR2_OFF			(0x00000004)
+
+#define SIUL20_CELL(c)			(c)
+#define SIUL21_CELL(c)			(100u + (c))
+#define SOC_MAJOR_CELL_OFFSET		SIUL20_CELL(0)
+#define SOC_MINOR_CELL_OFFSET		SIUL20_CELL(1)
+#define PCIE_DEV_ID_CELL_OFFSET		SIUL20_CELL(2)
+#define SERDES_PRESENCE_CELL_OFFSET	SIUL21_CELL(0)
+
+/* SIUL20_MIDR1 masks */
+#define SIUL20_MIDR1_MINOR_MASK		(0xF << 0)
+#define SIUL20_MIDR1_MAJOR_SHIFT	(4)
+#define SIUL20_MIDR1_MAJOR_MASK		(0xF << SIUL20_MIDR1_MAJOR_SHIFT)
+#define SIUL20_MIDR1_PART_NO_SHIFT	(16)
+#define SIUL20_MIDR1_PART_NO_MASK	GENMASK(25, 16)
+
+/* SIUL21_MIDR2 masks */
+#define SIUL21_MIDR2_SERDES_MASK	BIT(15)
+
+#define SIUL2_QUIRK_MIDR1_DECREMENT_VAL	BIT(1)
+
+struct s32g2_nvmem_drvdata {
+	u32 quirks;
+};
+
+struct s32g2_siul2_nvmem_data {
+	struct device *dev;
+	struct nvmem_device *nvmem;
+	struct regmap **regmaps;
+	struct s32g2_nvmem_drvdata drvdata;
+	u8 num_siul2;
+};
+
+static int needs_minor_decrement(const struct s32g2_nvmem_drvdata *data)
+{
+	return data->quirks & SIUL2_QUIRK_MIDR1_DECREMENT_VAL;
+}
+
+/* 3 digit part number */
+static int get_part_no(struct s32g2_siul2_nvmem_data *priv, u32 *part)
+{
+	int ret;
+
+	ret = regmap_read(priv->regmaps[0], SIUL2_MIDR1_OFF, part);
+	if (ret)
+		dev_err(priv->dev, "Failed to read SIUL2 PART_NO!\n");
+
+	*part &= SIUL20_MIDR1_PART_NO_MASK;
+	*part >>= SIUL20_MIDR1_PART_NO_SHIFT;
+
+	return ret;
+}
+
+static u32 get_variant_bits(u32 value)
+{
+	/*
+	 * Mapping between G3 variant ID and the PCIe Device ID,
+	 * as described in the "S32G3 Reference Manual",
+	 * chapter SerDes Subsystem, section "Device and revision IDs",
+	 * where: index = last 2 digits of the variant
+	 *        value = last hex digit of the PCIe Device ID"
+	 */
+	static const u32 s32g3_variants[] = {
+		[78] = 0x6,
+		[79] = 0x4,
+		[98] = 0x2,
+		[99] = 0x0,
+	};
+
+	/* PCIe variant bits with respect to PCIe Device ID update
+	 * applies only to S32G3 platforms.
+	 */
+	if (value / 100 != 3)
+		return 0;
+
+	value %= 100;
+
+	if (value < ARRAY_SIZE(s32g3_variants))
+		return s32g3_variants[value];
+
+	return 0;
+}
+
+static int s32g2_siul2_nvmem_read(void *context, unsigned int offset,
+				  void *val, size_t bytes)
+{
+	u32 major, minor, part_no, serdes, midr1, midr2;
+	struct s32g2_siul2_nvmem_data *priv = context;
+	int ret;
+
+	if (bytes != NVRAM_CELL_SIZE)
+		return -EOPNOTSUPP;
+
+	switch (offset) {
+	/* SIUL20 cells */
+	case SOC_MAJOR_CELL_OFFSET:
+		ret = regmap_read(priv->regmaps[0], SIUL2_MIDR1_OFF, &midr1);
+		if (ret)
+			return ret;
+		major = (midr1 & SIUL20_MIDR1_MAJOR_MASK) >> SIUL20_MIDR1_MAJOR_SHIFT;
+
+		/* Bytes format: 0.0.0.MAJOR */
+		*(u32 *)val = major + 1;
+
+		return 0;
+
+	case SOC_MINOR_CELL_OFFSET:
+		ret = regmap_read(priv->regmaps[0], SIUL2_MIDR1_OFF, &midr1);
+		if (ret)
+			return ret;
+
+		minor = midr1 & SIUL20_MIDR1_MINOR_MASK;
+
+		if (minor > 0 && needs_minor_decrement(&priv->drvdata))
+			minor--;
+
+		/* Bytes format: 0.0.0.MINOR */
+		*(u32 *)val = minor;
+
+		return 0;
+
+	case PCIE_DEV_ID_CELL_OFFSET:
+		ret = get_part_no(priv, &part_no);
+		if (ret)
+			return ret;
+
+		/* Bytes format: 0.0.0.PCIE_VARIANT */
+		*(u32 *)val = get_variant_bits(part_no);
+
+		return 0;
+
+	/* SIUL21 cells */
+	case SERDES_PRESENCE_CELL_OFFSET:
+		ret = regmap_read(priv->regmaps[1], SIUL2_MIDR2_OFF, &midr2);
+		if (ret)
+			return ret;
+
+		serdes = !!(midr2 & SIUL21_MIDR2_SERDES_MASK);
+		*(u32 *)val = serdes;
+		return 0;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int s32g2_siul2_nvmem_probe(struct platform_device *pdev)
+{
+	struct nxp_siul2_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
+	struct s32g2_siul2_nvmem_data *priv;
+	struct nvmem_config econfig = {
+		.name = "s32g2-siul2_nvmem",
+		.add_legacy_fixed_of_cells = false,
+		.owner = THIS_MODULE,
+		.word_size = 4,
+		.size = 4,
+		.read_only = true,
+	};
+	int i, ret;
+	u32 part;
+
+	if (!mfd)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "Invalid SIUL2 NVMEM parent!\n");
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct s32g2_siul2_nvmem_data),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->num_siul2 = mfd->num_siul2;
+	priv->regmaps = devm_kmalloc_array(&pdev->dev, priv->num_siul2,
+					   sizeof(*priv->regmaps), GFP_KERNEL);
+	if (!priv->regmaps)
+		return -ENOMEM;
+
+	for (i = 0; i < priv->num_siul2; i++)
+		priv->regmaps[i] = mfd->siul2[i].regmaps[SIUL2_MIDR];
+
+	priv->dev = &pdev->dev;
+	econfig.reg_read = s32g2_siul2_nvmem_read;
+	econfig.dev = pdev->dev.parent;
+	econfig.priv = priv;
+
+	ret = get_part_no(priv, &part);
+	if (ret)
+		return ret;
+
+	/* S32G2 SoCs have a special case. */
+	if (part / 100 == 2)
+		priv->drvdata.quirks |= SIUL2_QUIRK_MIDR1_DECREMENT_VAL;
+
+	priv->nvmem = devm_nvmem_register(pdev->dev.parent, &econfig);
+	if (IS_ERR(priv->nvmem))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->nvmem),
+				     "Failed to probe SIUL2 NVMEM!\n");
+
+	dev_info(&pdev->dev, "Initialized S32G%u SIUL2 nvmem driver\n",
+		 part / 100);
+
+	return 0;
+}
+
+static struct platform_driver s32g2_siul2_nvmem_driver = {
+	.probe = s32g2_siul2_nvmem_probe,
+	.driver = {
+		.name = "s32g2-siul2-nvmem",
+	},
+};
+
+module_platform_driver(s32g2_siul2_nvmem_driver);
+
+MODULE_AUTHOR("Catalin Udma <catalin-dan.udma@nxp.com>");
+MODULE_DESCRIPTION("S32G2 SIUL2 NVMEM driver");
+MODULE_LICENSE("GPL");
-- 
2.45.2
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Lee Jones 2 months, 2 weeks ago
On Thu, 10 Jul 2025, Andrei Stefanescu wrote:

> The SIUL2 hardware module has registers which expose information about
> the given SoC (version, SRAM size, presence of some hw modules).
> 
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> ---
>  drivers/mfd/nxp-siul2.c           |   6 +-

When you re-submit - this needs to be in its own commit.

>  drivers/nvmem/Kconfig             |  10 ++
>  drivers/nvmem/Makefile            |   2 +
>  drivers/nvmem/s32g2_siul2_nvmem.c | 232 ++++++++++++++++++++++++++++++
>  4 files changed, 249 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/nvmem/s32g2_siul2_nvmem.c

-- 
Lee Jones [李琼斯]
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Arnd Bergmann 2 months, 4 weeks ago
On Thu, Jul 10, 2025, at 16:20, Andrei Stefanescu wrote:
> The SIUL2 hardware module has registers which expose information about
> the given SoC (version, SRAM size, presence of some hw modules).
>
> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>

This does not look like an nvmem at all, it appears that you
are creating an alternative to the soc_device infrastructure
based on a binary interface tunneled through the nvmem subsystem.

Why not just make this a soc_device and have drivers use
soc_device_match() if they need to know what chip they are
running on?

    Arnd
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Andrei Stefanescu 2 months, 3 weeks ago
Hi Arnd,

>> The SIUL2 hardware module has registers which expose information about
>> the given SoC (version, SRAM size, presence of some hw modules).
>>
>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
> 
> This does not look like an nvmem at all, it appears that you
> are creating an alternative to the soc_device infrastructure
> based on a binary interface tunneled through the nvmem subsystem.
> 
> Why not just make this a soc_device and have drivers use
> soc_device_match() if they need to know what chip they are
> running on?

Thank you for the review! I've just taken a look over soc_device
and I agree, this driver should be a soc_device. I will convert
it in the next revision.

Best regards,
Andrei
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Andrei Stefanescu 2 months ago
> Thank you for the review! I've just taken a look over soc_device
> and I agree, this driver should be a soc_device. I will convert
> it in the next revision.
> 

Hi Arnd,

I took a more in-depth look over soc_device and how to apply it to SIUL2
and I have encountered an issue. Downstream [1], [2], [3] we use SIUL2
nvmem-cells to set the PCIe vendor id partially based on the part number
and also to ensure that the SerDes subsystem is present. I don't think we can
achieve this with a soc_device driver. I saw that we could export a custom
attribute but I don't think we can read it from the PCIe driver.

Apart from the proposed NVMEM driver, there is also an option of exporting
a syscon regmap for the registers which provide information about the SoC.

I have seen that typically NVMEM drivers export information read from fuses
but I think having a NVMEM driver is nicer way to access the information
instead of using a syscon regmap and manually extracting the needed bits. 

To provide a bit of context: the SIUL2 IP has two registers called
SIUL2 MCU ID Register (MIDR1/2) which export information such as:
the part number, major, minor, package, maximum frequency,
flash size, SRAM size, SerDes susbsytem presence and so on.

S32G2/3 SoCs have two SIUL2 blocks named SIUL2_0 and SIUL2_1. We need
to export the MIDR1/2 registers of both SIUL2 hardware blocks.

What do you think? Would it be ok to keep the existing NVMEM implementation?
Do you have any other suggestions?

Best regards,
Andrei

[1] - https://github.com/nxp-auto-linux/linux/blob/release/bsp44.0-6.6.85-rt/arch/arm64/boot/dts/freescale/s32cc.dtsi#L1036
[2] - https://github.com/nxp-auto-linux/linux/blob/release/bsp44.0-6.6.85-rt/drivers/pci/controller/dwc/pci-s32cc.c#L832
[3] - https://github.com/nxp-auto-linux/linux/blob/release/bsp44.0-6.6.85-rt/drivers/pci/controller/dwc/pci-s32cc.c#L163
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Krzysztof Kozlowski 2 months ago
On 01/08/2025 16:36, Andrei Stefanescu wrote:
> Apart from the proposed NVMEM driver, there is also an option of exporting
> a syscon regmap for the registers which provide information about the SoC.
> 
> I have seen that typically NVMEM drivers export information read from fuses
> but I think having a NVMEM driver is nicer way to access the information
> instead of using a syscon regmap and manually extracting the needed bits. 


nvmem is not a syscon. Mixing these two means device is something
completely else.

Best regards,
Krzysztof
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Krzysztof Kozlowski 2 months ago
On 02/08/2025 10:28, Krzysztof Kozlowski wrote:
> On 01/08/2025 16:36, Andrei Stefanescu wrote:
>> Apart from the proposed NVMEM driver, there is also an option of exporting
>> a syscon regmap for the registers which provide information about the SoC.
>>
>> I have seen that typically NVMEM drivers export information read from fuses
>> but I think having a NVMEM driver is nicer way to access the information
>> instead of using a syscon regmap and manually extracting the needed bits. 
> 
> 
> nvmem is not a syscon. Mixing these two means device is something
> completely else.


... and yes, I am aware that three FSL/NXP bindings use nvmem as syscon
already. People are mixing hardware description (nvmem) with some
purpose for drivers (syscon) forgetting that syscon are miscellaneous
SoC internal registers, not non-volatile memory.

Best regards,
Krzysztof
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Andrei Stefanescu 2 months ago
Hi Krzysztof,

Thank you for the quick response!
On 02/08/2025 11:32, Krzysztof Kozlowski wrote:
> On 02/08/2025 10:28, Krzysztof Kozlowski wrote:
>> On 01/08/2025 16:36, Andrei Stefanescu wrote:
>>> Apart from the proposed NVMEM driver, there is also an option of exporting
>>> a syscon regmap for the registers which provide information about the SoC.
>>>
>>> I have seen that typically NVMEM drivers export information read from fuses
>>> but I think having a NVMEM driver is nicer way to access the information
>>> instead of using a syscon regmap and manually extracting the needed bits. 
>>
>>
>> nvmem is not a syscon. Mixing these two means device is something
>> completely else.

Yes, I don't want to mix them. The driver will either be a NVMEM driver or
a syscon. These registers are read-only. I suggested NVMEM because it's a
an abstraction layer which makes it easier for drivers which want to use
that information without knowing where to actually read it i.e. reg address,
bit mask.

What do think? Which one would be better? Do you have other suggestions?

Best regards,
Andrei

> 
> 
> ... and yes, I am aware that three FSL/NXP bindings use nvmem as syscon
> already. People are mixing hardware description (nvmem) with some
> purpose for drivers (syscon) forgetting that syscon are miscellaneous
> SoC internal registers, not non-volatile memory.
> 
> Best regards,
> Krzysztof
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Krzysztof Kozlowski 2 months ago
On 04/08/2025 09:12, Andrei Stefanescu wrote:
> Hi Krzysztof,
> 
> Thank you for the quick response!
> On 02/08/2025 11:32, Krzysztof Kozlowski wrote:
>> On 02/08/2025 10:28, Krzysztof Kozlowski wrote:
>>> On 01/08/2025 16:36, Andrei Stefanescu wrote:
>>>> Apart from the proposed NVMEM driver, there is also an option of exporting
>>>> a syscon regmap for the registers which provide information about the SoC.
>>>>
>>>> I have seen that typically NVMEM drivers export information read from fuses
>>>> but I think having a NVMEM driver is nicer way to access the information
>>>> instead of using a syscon regmap and manually extracting the needed bits. 
>>>
>>>
>>> nvmem is not a syscon. Mixing these two means device is something
>>> completely else.
> 
> Yes, I don't want to mix them. The driver will either be a NVMEM driver or
> a syscon. These registers are read-only. I suggested NVMEM because it's a

We do not talk about drivers here, but hardware.

> an abstraction layer which makes it easier for drivers which want to use
> that information without knowing where to actually read it i.e. reg address,
> bit mask.

Sorry, but no. You design it for drivers, that's not the way. Describe
properly the hardware.


Best regards,
Krzysztof
Re: [PATCH v7 10/12] nvmem: s32g2_siul2: add NVMEM driver for SoC information
Posted by Andrei Stefanescu 2 months ago
Hi Krzysztof,

On 04/08/2025 10:26, Krzysztof Kozlowski wrote:
> On 04/08/2025 09:12, Andrei Stefanescu wrote:
>> Hi Krzysztof,
>>
>> Thank you for the quick response!
>> On 02/08/2025 11:32, Krzysztof Kozlowski wrote:
>>> On 02/08/2025 10:28, Krzysztof Kozlowski wrote:
>>>> On 01/08/2025 16:36, Andrei Stefanescu wrote:
>>>>> Apart from the proposed NVMEM driver, there is also an option of exporting
>>>>> a syscon regmap for the registers which provide information about the SoC.
>>>>>
>>>>> I have seen that typically NVMEM drivers export information read from fuses
>>>>> but I think having a NVMEM driver is nicer way to access the information
>>>>> instead of using a syscon regmap and manually extracting the needed bits. 
>>>>
>>>>
>>>> nvmem is not a syscon. Mixing these two means device is something
>>>> completely else.
>>
>> Yes, I don't want to mix them. The driver will either be a NVMEM driver or
>> a syscon. These registers are read-only. I suggested NVMEM because it's a
> 
> We do not talk about drivers here, but hardware.
> 
>> an abstraction layer which makes it easier for drivers which want to use
>> that information without knowing where to actually read it i.e. reg address,
>> bit mask.
> 
> Sorry, but no. You design it for drivers, that's not the way. Describe
> properly the hardware.

I don't think there's a clear way. These are read-only registers, I am not sure
if exporting them as a syscon regmap properly describes the hardware. Moreover,
I saw the following phrase in the NVMEM documentation [1]:

"NVMEM is the abbreviation for Non Volatile Memory layer. It is used to retrieve
configuration of SOC or Device specific data from non volatile memories like eeprom,
efuses and so on."

This suggests that NVMEM might be a fit. I incline more towards the NVVMEM driver
because it will also reduce code duplication (if multiple drivers need to read the
part number for example).

What do you think? Would you consider NVMEM the way to go in this case?

Best regards,
Andrei

[1] - https://www.kernel.org/doc/html/latest/driver-api/nvmem.html