[PATCH v2 1/3] dt-bindings: nvmem: syscon: Add syscon backed nvmem bindings

[PATCH v2 1/3] dt-bindings: nvmem: syscon: Add syscon backed nvmem bindings

[Marek Vasut]

Add trivial bindings for driver which permits exposing syscon backed
register to userspace. This is useful e.g. to expose U-Boot boot
counter on various platforms where the boot counter is stored in
random volatile register, like STM32MP15xx TAMP_BKPxR register.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Marek Vasut <marex@denx.de>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: kernel@dh-electronics.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
V2: Use generic syscon supernode
---
 .../bindings/nvmem/nvmem-syscon.yaml          | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml b/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
new file mode 100644
index 0000000000000..7c1173a1a6218
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/nvmem-syscon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic syscon backed nvmem
+
+maintainers:
+  - Marek Vasut <marex@denx.de>
+
+allOf:
+  - $ref: "nvmem.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - nvmem-syscon
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    syscon {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        syscon@14c {
+            compatible = "nvmem-syscon";
+            reg = <0x14c 0x4>;
+        };
+    };
-- 
2.39.2

[PATCH v2 2/3] nvmem: syscon: Add syscon backed nvmem driver

[Marek Vasut]

Add trivial driver which permits exposing syscon backed register
to userspace. This is useful e.g. to expose U-Boot boot counter
on various platforms where the boot counter is stored in random
volatile register, like STM32MP15xx TAMP_BKPxR register.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Marek Vasut <marex@denx.de>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: kernel@dh-electronics.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
V2: No change
---
 drivers/nvmem/Kconfig        |  10 ++++
 drivers/nvmem/Makefile       |   2 +
 drivers/nvmem/nvmem-syscon.c | 105 +++++++++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+)
 create mode 100644 drivers/nvmem/nvmem-syscon.c

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index b291b27048c76..4e4aecdd9c293 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -303,6 +303,16 @@ config NVMEM_STM32_BSEC_OPTEE_TA
 	  This library is a used by stm32-romem driver or included in the module
 	  called nvmem-stm32-romem.
 
+config NVMEM_SYSCON
+	tristate "Generic syscon backed nvmem"
+	help
+	  This is a driver for generic syscon backed nvmem. This can be
+	  used to expose arbitrary syscon backed register to user space
+	  via nvmem, like the U-Boot boot counter.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called nvmem-syscon.
+
 config NVMEM_STM32_ROMEM
 	tristate "STMicroelectronics STM32 factory-programmed memory support"
 	depends on ARCH_STM32 || COMPILE_TEST
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index f82431ec8aef0..d10e478e6a6c9 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -60,6 +60,8 @@ obj-$(CONFIG_NVMEM_SPMI_SDAM)		+= nvmem_qcom-spmi-sdam.o
 nvmem_qcom-spmi-sdam-y			+= qcom-spmi-sdam.o
 obj-$(CONFIG_NVMEM_SPRD_EFUSE)		+= nvmem_sprd_efuse.o
 nvmem_sprd_efuse-y			:= sprd-efuse.o
+obj-$(CONFIG_NVMEM_SYSCON)		+= nvmem_syscon.o
+nvmem_syscon-y				:= nvmem-syscon.o
 obj-$(CONFIG_NVMEM_STM32_ROMEM)		+= nvmem_stm32_romem.o
 nvmem_stm32_romem-y 			:= stm32-romem.o
 nvmem_stm32_romem-$(CONFIG_NVMEM_STM32_BSEC_OPTEE_TA) += stm32-bsec-optee-ta.o
diff --git a/drivers/nvmem/nvmem-syscon.c b/drivers/nvmem/nvmem-syscon.c
new file mode 100644
index 0000000000000..e0aa5af0300d3
--- /dev/null
+++ b/drivers/nvmem/nvmem-syscon.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Marek Vasut <marex@denx.de>
+ *
+ * Based on snvs_lpgpr.c .
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+struct nvmem_syscon_priv {
+	struct device_d		*dev;
+	struct regmap		*regmap;
+	struct nvmem_config	cfg;
+	unsigned int		off;
+};
+
+static int nvmem_syscon_write(void *context, unsigned int offset, void *val,
+			      size_t bytes)
+{
+	struct nvmem_syscon_priv *priv = context;
+
+	return regmap_bulk_write(priv->regmap, priv->off + offset,
+				 val, bytes / 4);
+}
+
+static int nvmem_syscon_read(void *context, unsigned int offset, void *val,
+			     size_t bytes)
+{
+	struct nvmem_syscon_priv *priv = context;
+
+	return regmap_bulk_read(priv->regmap, priv->off + offset,
+				val, bytes / 4);
+}
+
+static int nvmem_syscon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *syscon_node;
+	struct nvmem_syscon_priv *priv;
+	struct nvmem_device *nvmem;
+	struct nvmem_config *cfg;
+	int ret;
+
+	if (!node)
+		return -ENOENT;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_index(node, "reg", 0, &priv->off);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32_index(node, "reg", 1, &priv->cfg.size);
+	if (ret)
+		return ret;
+
+	syscon_node = of_get_parent(node);
+	if (!syscon_node)
+		return -ENODEV;
+
+	priv->regmap = syscon_node_to_regmap(syscon_node);
+	of_node_put(syscon_node);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	cfg = &priv->cfg;
+	cfg->priv = priv;
+	cfg->name = dev_name(dev);
+	cfg->dev = dev;
+	cfg->stride = 4;
+	cfg->word_size = 4;
+	cfg->owner = THIS_MODULE;
+	cfg->reg_read  = nvmem_syscon_read;
+	cfg->reg_write = nvmem_syscon_write;
+
+	nvmem = devm_nvmem_register(dev, cfg);
+
+	return PTR_ERR_OR_ZERO(nvmem);
+}
+
+static const struct of_device_id nvmem_syscon_dt_ids[] = {
+	{ .compatible = "nvmem-syscon" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, nvmem_syscon_dt_ids);
+
+static struct platform_driver nvmem_syscon_driver = {
+	.probe	= nvmem_syscon_probe,
+	.driver = {
+		.name	= "nvmem-syscon",
+		.of_match_table = nvmem_syscon_dt_ids,
+	},
+};
+module_platform_driver(nvmem_syscon_driver);
+
+MODULE_AUTHOR("Marek Vasut <marex@denx.de>");
+MODULE_DESCRIPTION("Generic syscon nvmem driver");
+MODULE_LICENSE("GPL");
-- 
2.39.2

[PATCH v2 3/3] ARM: dts: stm32: Add nvmem-syscon node to TAMP to expose boot count on DHSOM

[Marek Vasut]

Add nvmem-syscon subnode to expose TAMP_BKPxR register 19 to user space.
This register contains U-Boot boot counter, by exposing it to user space
the user space can reset the boot counter.

Read access example:
"
$ hexdump -vC /sys/bus/nvmem/devices/5c00a000.tamp\:nvmem0/nvmem
00000000  0c 00 c4 b0
"

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Marek Vasut <marex@denx.de>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: kernel@dh-electronics.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
V2: No change
---
 arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi | 11 +++++++++++
 arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
index 74735552f4803..b2557bb718f52 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
@@ -537,6 +537,17 @@ &sdmmc3 {
 	status = "okay";
 };
 
+&tamp {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	/* Boot counter */
+	nvmem {
+		compatible = "nvmem-syscon";
+		reg = <0x14c 0x4>;
+	};
+};
+
 &uart4 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart4_pins_a>;
diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
index bba19f21e5277..864960387e634 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
@@ -269,3 +269,14 @@ &rng1 {
 &rtc {
 	status = "okay";
 };
+
+&tamp {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	/* Boot counter */
+	nvmem {
+		compatible = "nvmem-syscon";
+		reg = <0x14c 0x4>;
+	};
+};
-- 
2.39.2

Re: [PATCH v2 2/3] nvmem: syscon: Add syscon backed nvmem driver

[Krzysztof Kozlowski]

On 17/05/2023 17:25, Marek Vasut wrote:
> Add trivial driver which permits exposing syscon backed register
> to userspace. This is useful e.g. to expose U-Boot boot counter
> on various platforms where the boot counter is stored in random
> volatile register, like STM32MP15xx TAMP_BKPxR register.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: kernel@dh-electronics.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> ---
> V2: No change
> ---
>  drivers/nvmem/Kconfig        |  10 ++++
>  drivers/nvmem/Makefile       |   2 +
>  drivers/nvmem/nvmem-syscon.c | 105 +++++++++++++++++++++++++++++++++++
>  3 files changed, 117 insertions(+)
>  create mode 100644 drivers/nvmem/nvmem-syscon.c
> 
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index b291b27048c76..4e4aecdd9c293 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -303,6 +303,16 @@ config NVMEM_STM32_BSEC_OPTEE_TA
>  	  This library is a used by stm32-romem driver or included in the module
>  	  called nvmem-stm32-romem.
>  
> +config NVMEM_SYSCON
> +	tristate "Generic syscon backed nvmem"
> +	help
> +	  This is a driver for generic syscon backed nvmem. This can be
> +	  used to expose arbitrary syscon backed register to user space
> +	  via nvmem, like the U-Boot boot counter.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called nvmem-syscon.
> +
>  config NVMEM_STM32_ROMEM
>  	tristate "STMicroelectronics STM32 factory-programmed memory support"
>  	depends on ARCH_STM32 || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index f82431ec8aef0..d10e478e6a6c9 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -60,6 +60,8 @@ obj-$(CONFIG_NVMEM_SPMI_SDAM)		+= nvmem_qcom-spmi-sdam.o
>  nvmem_qcom-spmi-sdam-y			+= qcom-spmi-sdam.o
>  obj-$(CONFIG_NVMEM_SPRD_EFUSE)		+= nvmem_sprd_efuse.o
>  nvmem_sprd_efuse-y			:= sprd-efuse.o
> +obj-$(CONFIG_NVMEM_SYSCON)		+= nvmem_syscon.o
> +nvmem_syscon-y				:= nvmem-syscon.o
>  obj-$(CONFIG_NVMEM_STM32_ROMEM)		+= nvmem_stm32_romem.o
>  nvmem_stm32_romem-y 			:= stm32-romem.o
>  nvmem_stm32_romem-$(CONFIG_NVMEM_STM32_BSEC_OPTEE_TA) += stm32-bsec-optee-ta.o
> diff --git a/drivers/nvmem/nvmem-syscon.c b/drivers/nvmem/nvmem-syscon.c
> new file mode 100644
> index 0000000000000..e0aa5af0300d3
> --- /dev/null
> +++ b/drivers/nvmem/nvmem-syscon.c
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Marek Vasut <marex@denx.de>
> + *
> + * Based on snvs_lpgpr.c .
> + */
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +
> +struct nvmem_syscon_priv {
> +	struct device_d		*dev;
> +	struct regmap		*regmap;
> +	struct nvmem_config	cfg;
> +	unsigned int		off;
> +};
> +
> +static int nvmem_syscon_write(void *context, unsigned int offset, void *val,
> +			      size_t bytes)
> +{
> +	struct nvmem_syscon_priv *priv = context;
> +
> +	return regmap_bulk_write(priv->regmap, priv->off + offset,
> +				 val, bytes / 4);
> +}
> +
> +static int nvmem_syscon_read(void *context, unsigned int offset, void *val,
> +			     size_t bytes)
> +{
> +	struct nvmem_syscon_priv *priv = context;
> +
> +	return regmap_bulk_read(priv->regmap, priv->off + offset,
> +				val, bytes / 4);
> +}
> +
> +static int nvmem_syscon_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct device_node *syscon_node;
> +	struct nvmem_syscon_priv *priv;
> +	struct nvmem_device *nvmem;
> +	struct nvmem_config *cfg;
> +	int ret;
> +
> +	if (!node)
> +		return -ENOENT;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_index(node, "reg", 0, &priv->off);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32_index(node, "reg", 1, &priv->cfg.size);
> +	if (ret)
> +		return ret;
> +
> +	syscon_node = of_get_parent(node);

This does not look correct. You hard-code dependency that it must be a
child of syscon node. This is weird requirement and not explained in the
bindings.

Why this cannot be then generic MMIO node? Why it has to be a child of
syscon?


> +	if (!syscon_node)
> +		return -ENODEV;
> +
> +	priv->regmap = syscon_node_to_regmap(syscon_node);
> +	of_node_put(syscon_node);
> +	if (IS_ERR(priv->regmap))
> +		return PTR_ERR(priv->regmap);
Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: nvmem: syscon: Add syscon backed nvmem bindings

[Krzysztof Kozlowski]

On 17/05/2023 17:25, Marek Vasut wrote:
> Add trivial bindings for driver which permits exposing syscon backed
> register to userspace. This is useful e.g. to expose U-Boot boot
> counter on various platforms where the boot counter is stored in
> random volatile register, like STM32MP15xx TAMP_BKPxR register.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: kernel@dh-electronics.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> ---
> V2: Use generic syscon supernode
> ---
>  .../bindings/nvmem/nvmem-syscon.yaml          | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml b/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
> new file mode 100644
> index 0000000000000..7c1173a1a6218
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/nvmem-syscon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic syscon backed nvmem
> +
> +maintainers:
> +  - Marek Vasut <marex@denx.de>
> +
> +allOf:
> +  - $ref: "nvmem.yaml#"

Usual comment: drop quotes. We removed them everywhere, so you based
your work on some old tree.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - nvmem-syscon
> +
> +  reg:
> +    maxItems: 1

Rob's questions are not solved. The nvmem.yaml schema expects here to
allow children. This should not be created per-register, but per entire
block of registers.

OTOH, using nvmem for syscon (which are MMIO and writeable registers
usually) seems odd.

> +

missing nvmem cells

> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false

unevaluatedProps: false

> +
> +examples:
> +  - |
> +    syscon {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        syscon@14c {

It's not really a syscon, but efuse, otp or nvmem.

> +            compatible = "nvmem-syscon";
> +            reg = <0x14c 0x4>;

Missing nvmem cells

> +        };
> +    };

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: nvmem: syscon: Add syscon backed nvmem bindings

[Krzysztof Kozlowski]

On 17/05/2023 17:25, Marek Vasut wrote:
> Add trivial bindings for driver which permits exposing syscon backed
> register to userspace. This is useful e.g. to expose U-Boot boot
> counter on various platforms where the boot counter is stored in
> random volatile register, like STM32MP15xx TAMP_BKPxR register.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---

Let me also leave a note here - cross linking for all parties:

Please propose a solution solving also mediatek,boottrap, probably
extendable to range of registers. Other solution is what Rob mentioned -
this might not be suitable for generic binding and device.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: nvmem: syscon: Add syscon backed nvmem bindings

[Marek Vasut]

On 5/18/23 16:30, Krzysztof Kozlowski wrote:
> On 17/05/2023 17:25, Marek Vasut wrote:
>> Add trivial bindings for driver which permits exposing syscon backed
>> register to userspace. This is useful e.g. to expose U-Boot boot
>> counter on various platforms where the boot counter is stored in
>> random volatile register, like STM32MP15xx TAMP_BKPxR register.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
> 
> Let me also leave a note here - cross linking for all parties:
> 
> Please propose a solution solving also mediatek,boottrap, probably
> extendable to range of registers. Other solution is what Rob mentioned -
> this might not be suitable for generic binding and device.

 From what I can tell, shouldn't the mediatek 1g MAC driver have a 
nvmem-cells phandle to this OTP/fuses/whatever area and shouldn't the 
driver read out and decode its settings within the kernel ?

That doesn't seem really related to this particular issue I'm trying to 
solve I'm afraid.

Re: [PATCH v2 1/3] dt-bindings: nvmem: syscon: Add syscon backed nvmem bindings

[Marek Vasut]

On 5/18/23 16:26, Krzysztof Kozlowski wrote:
> On 17/05/2023 17:25, Marek Vasut wrote:
>> Add trivial bindings for driver which permits exposing syscon backed
>> register to userspace. This is useful e.g. to expose U-Boot boot
>> counter on various platforms where the boot counter is stored in
>> random volatile register, like STM32MP15xx TAMP_BKPxR register.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> Cc: devicetree@vger.kernel.org
>> Cc: kernel@dh-electronics.com
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-stm32@st-md-mailman.stormreply.com
>> ---
>> V2: Use generic syscon supernode
>> ---
>>   .../bindings/nvmem/nvmem-syscon.yaml          | 39 +++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml b/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
>> new file mode 100644
>> index 0000000000000..7c1173a1a6218
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
>> @@ -0,0 +1,39 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/nvmem/nvmem-syscon.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Generic syscon backed nvmem
>> +
>> +maintainers:
>> +  - Marek Vasut <marex@denx.de>
>> +
>> +allOf:
>> +  - $ref: "nvmem.yaml#"
> 
> Usual comment: drop quotes. We removed them everywhere, so you based
> your work on some old tree.
> 
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - nvmem-syscon
>> +
>> +  reg:
>> +    maxItems: 1
> 
> Rob's questions are not solved.

Can you reiterate this one ? I likely missed it.

> The nvmem.yaml schema expects here to
> allow children. This should not be created per-register, but per entire
> block of registers.

This thing works the other way around, I have a syscon register block 
already, and I want to expose subset of it to userspace as read/write 
accessible file to expose bootcounter available in that register (so I 
can read it and reset it from user application).

> OTOH, using nvmem for syscon (which are MMIO and writeable registers
> usually) seems odd.
> 
>> +
> 
> missing nvmem cells

See above, I don't think nvmem-cells applies here.

[...]

Re: [PATCH v2 2/3] nvmem: syscon: Add syscon backed nvmem driver

[Marek Vasut]

On 5/18/23 16:22, Krzysztof Kozlowski wrote:

[...]

>> +++ b/drivers/nvmem/nvmem-syscon.c
>> @@ -0,0 +1,105 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2022 Marek Vasut <marex@denx.de>
>> + *
>> + * Based on snvs_lpgpr.c .
>> + */
>> +
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/nvmem-provider.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regmap.h>
>> +
>> +struct nvmem_syscon_priv {
>> +	struct device_d		*dev;
>> +	struct regmap		*regmap;
>> +	struct nvmem_config	cfg;
>> +	unsigned int		off;
>> +};
>> +
>> +static int nvmem_syscon_write(void *context, unsigned int offset, void *val,
>> +			      size_t bytes)
>> +{
>> +	struct nvmem_syscon_priv *priv = context;
>> +
>> +	return regmap_bulk_write(priv->regmap, priv->off + offset,
>> +				 val, bytes / 4);
>> +}
>> +
>> +static int nvmem_syscon_read(void *context, unsigned int offset, void *val,
>> +			     size_t bytes)
>> +{
>> +	struct nvmem_syscon_priv *priv = context;
>> +
>> +	return regmap_bulk_read(priv->regmap, priv->off + offset,
>> +				val, bytes / 4);
>> +}
>> +
>> +static int nvmem_syscon_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *node = dev->of_node;
>> +	struct device_node *syscon_node;
>> +	struct nvmem_syscon_priv *priv;
>> +	struct nvmem_device *nvmem;
>> +	struct nvmem_config *cfg;
>> +	int ret;
>> +
>> +	if (!node)
>> +		return -ENOENT;
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	ret = of_property_read_u32_index(node, "reg", 0, &priv->off);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = of_property_read_u32_index(node, "reg", 1, &priv->cfg.size);
>> +	if (ret)
>> +		return ret;
>> +
>> +	syscon_node = of_get_parent(node);
> 
> This does not look correct. You hard-code dependency that it must be a
> child of syscon node. This is weird requirement and not explained in the
> bindings.
> 
> Why this cannot be then generic MMIO node? Why it has to be a child of
> syscon?

Because I already have a syscon node and I want to expose only a subset 
of it to userspace (bootcounter in my case) . See 1/3, I replied there, 
let's continue the discussion there.

I fixed the rest in prep for V3, sorry for the horrid delays in replies.

Re: [PATCH v2 3/3] ARM: dts: stm32: Add nvmem-syscon node to TAMP to expose boot count on DHSOM

[Alexandre TORGUE]

On 5/17/23 17:25, Marek Vasut wrote:
> Add nvmem-syscon subnode to expose TAMP_BKPxR register 19 to user space.
> This register contains U-Boot boot counter, by exposing it to user space
> the user space can reset the boot counter.
> 
> Read access example:
> "
> $ hexdump -vC /sys/bus/nvmem/devices/5c00a000.tamp\:nvmem0/nvmem
> 00000000  0c 00 c4 b0
> "
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: kernel@dh-electronics.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> ---
> V2: No change
> ---
>   arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi | 11 +++++++++++
>   arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 11 +++++++++++
>   2 files changed, 22 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> index 74735552f4803..b2557bb718f52 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> @@ -537,6 +537,17 @@ &sdmmc3 {
>   	status = "okay";
>   };
>   
> +&tamp {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	/* Boot counter */
> +	nvmem {

nvmem@14c ?

> +		compatible = "nvmem-syscon";
> +		reg = <0x14c 0x4>;
> +	};
> +};
> +
>   &uart4 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&uart4_pins_a>;
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> index bba19f21e5277..864960387e634 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> @@ -269,3 +269,14 @@ &rng1 {
>   &rtc {
>   	status = "okay";
>   };
> +
> +&tamp {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	/* Boot counter */
> +	nvmem {

nvmem@14c ?

> +		compatible = "nvmem-syscon";
> +		reg = <0x14c 0x4>;
> +	};
> +};

RE: [PATCH v2 3/3] ARM: dts: stm32: Add nvmem-syscon node to TAMP to expose boot count on DHSOM

[patrick.delaunay]

Hi Marek,

> From: Marek Vasut <marex@denx.de>
> Sent: Wednesday, May 17, 2023 5:25 PM
> Subject: [PATCH v2 3/3] ARM: dts: stm32: Add nvmem-syscon node to TAMP to
> expose boot count on DHSOM
> 
> Add nvmem-syscon subnode to expose TAMP_BKPxR register 19 to user space.
> This register contains U-Boot boot counter, by exposing it to user space the user
> space can reset the boot counter.
> 
> Read access example:
> "
> $ hexdump -vC /sys/bus/nvmem/devices/5c00a000.tamp\:nvmem0/nvmem
> 00000000  0c 00 c4 b0
> "
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: kernel@dh-electronics.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> ---
> V2: No change
> ---
>  arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi | 11 +++++++++++
> arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 11 +++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> index 74735552f4803..b2557bb718f52 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
> @@ -537,6 +537,17 @@ &sdmmc3 {
>  	status = "okay";
>  };
> 
> +&tamp {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	/* Boot counter */
> +	nvmem {
> +		compatible = "nvmem-syscon";
> +		reg = <0x14c 0x4>;
> +	};
> +};
> +
>  &uart4 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart4_pins_a>;
> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> index bba19f21e5277..864960387e634 100644
> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
> @@ -269,3 +269,14 @@ &rng1 {
>  &rtc {
>  	status = "okay";
>  };
> +
> +&tamp {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	/* Boot counter */
> +	nvmem {

According binding you need to add "@<reg>" => nvmem@14c

And you export only TAMP_BKP19R directly in a nvmem region ?

> +		compatible = "nvmem-syscon";
> +		reg = <0x14c 0x4>;
> +	};
> +};


the boot counter could be a nvem cell so you could expose  other backup registers 

For example :

&tamp {
	#address-cells = <1>;
	#size-cells = <1>;

	nvmem@14c  {
		compatible = "nvmem-syscon";
		reg = <0x14c 0x4>;

		/* Data cells */
		boot_counter: boot-counter@14c {
			reg = <0x14c 0x4>;
		};
	};
};

Even if you export more backup register the cell will be correctly described in DT
and could be accessible directly  with sysfs without managed offset in userland

with https://lore.kernel.org/lkml/202305240724.z3McDuYM-lkp@intel.com/T/
Or previous serie https://lore.kernel.org/lkml/20211220064730.28806-1-zajec5@gmail.com/


for example to export all the free register:

Reference: https://wiki.st.com/stm32mpu/wiki/STM32MP15_backup_registers

the cell " boot-counter" will be always available for users.

&tamp {
	#address-cells = <1>;
	#size-cells = <1>;

	/* backup register: 10 to 21 */
	nvmem@0x128  {
		compatible = "nvmem-syscon";
		reg = <0x128 0x44>;

		/* Data cells */
		boot_counter: boot-counter@14c {
			reg = <0x14c 0x4>;
		};
		boot_mode: boot-mode@150 {
			reg = <0x150 0x4>;
		};
....
	};
};


Patrick

> --
> 2.39.2

ST Restricted

Re: [PATCH v2 3/3] ARM: dts: stm32: Add nvmem-syscon node to TAMP to expose boot count on DHSOM

[Marek Vasut]

On 5/26/23 17:28, patrick.delaunay@foss.st.com wrote:
> Hi Marek,

Hi,

>> From: Marek Vasut <marex@denx.de>
>> Sent: Wednesday, May 17, 2023 5:25 PM
>> Subject: [PATCH v2 3/3] ARM: dts: stm32: Add nvmem-syscon node to TAMP to
>> expose boot count on DHSOM
>>
>> Add nvmem-syscon subnode to expose TAMP_BKPxR register 19 to user space.
>> This register contains U-Boot boot counter, by exposing it to user space the user
>> space can reset the boot counter.
>>
>> Read access example:
>> "
>> $ hexdump -vC /sys/bus/nvmem/devices/5c00a000.tamp\:nvmem0/nvmem
>> 00000000  0c 00 c4 b0
>> "
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> Cc: devicetree@vger.kernel.org
>> Cc: kernel@dh-electronics.com
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-stm32@st-md-mailman.stormreply.com
>> ---
>> V2: No change
>> ---
>>   arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi | 11 +++++++++++
>> arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 11 +++++++++++
>>   2 files changed, 22 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
>> b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
>> index 74735552f4803..b2557bb718f52 100644
>> --- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
>> @@ -537,6 +537,17 @@ &sdmmc3 {
>>   	status = "okay";
>>   };
>>
>> +&tamp {
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +
>> +	/* Boot counter */
>> +	nvmem {
>> +		compatible = "nvmem-syscon";
>> +		reg = <0x14c 0x4>;
>> +	};
>> +};
>> +
>>   &uart4 {
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&uart4_pins_a>;
>> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>> b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>> index bba19f21e5277..864960387e634 100644
>> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>> @@ -269,3 +269,14 @@ &rng1 {
>>   &rtc {
>>   	status = "okay";
>>   };
>> +
>> +&tamp {
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +
>> +	/* Boot counter */
>> +	nvmem {
> 
> According binding you need to add "@<reg>" => nvmem@14c
> 
> And you export only TAMP_BKP19R directly in a nvmem region ?

4 bytes is more than plenty for boot counter , yes.

>> +		compatible = "nvmem-syscon";
>> +		reg = <0x14c 0x4>;
>> +	};
>> +};
> 
> 
> the boot counter could be a nvem cell so you could expose  other backup registers
> 
> For example :
> 
> &tamp {
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 
> 	nvmem@14c  {
> 		compatible = "nvmem-syscon";
> 		reg = <0x14c 0x4>;
> 
> 		/* Data cells */
> 		boot_counter: boot-counter@14c {
> 			reg = <0x14c 0x4>;
> 		};
> 	};
> };
> 
> Even if you export more backup register the cell will be correctly described in DT
> and could be accessible directly  with sysfs without managed offset in userland
> 
> with https://lore.kernel.org/lkml/202305240724.z3McDuYM-lkp@intel.com/T/
> Or previous serie https://lore.kernel.org/lkml/20211220064730.28806-1-zajec5@gmail.com/
> 
> 
> for example to export all the free register:
> 
> Reference: https://wiki.st.com/stm32mpu/wiki/STM32MP15_backup_registers
> 
> the cell " boot-counter" will be always available for users.
> 
> &tamp {
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 
> 	/* backup register: 10 to 21 */
> 	nvmem@0x128  {
> 		compatible = "nvmem-syscon";
> 		reg = <0x128 0x44>;
> 
> 		/* Data cells */
> 		boot_counter: boot-counter@14c {
> 			reg = <0x14c 0x4>;
> 		};
> 		boot_mode: boot-mode@150 {
> 			reg = <0x150 0x4>;
> 		};
> ....
> 	};
> };

Sure, thanks. I'm not sure I understood the message above.

Re: [PATCH v2 1/3] dt-bindings: nvmem: syscon: Add syscon backed nvmem bindings

[Krzysztof Kozlowski]

On 31/05/2023 22:44, Marek Vasut wrote:
> On 5/31/23 21:37, Krzysztof Kozlowski wrote:
>> On 24/05/2023 05:30, Marek Vasut wrote:
>>> On 5/18/23 16:26, Krzysztof Kozlowski wrote:
>>>> On 17/05/2023 17:25, Marek Vasut wrote:
>>>>> Add trivial bindings for driver which permits exposing syscon backed
>>>>> register to userspace. This is useful e.g. to expose U-Boot boot
>>>>> counter on various platforms where the boot counter is stored in
>>>>> random volatile register, like STM32MP15xx TAMP_BKPxR register.
>>>>>
>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>> ---
>>>>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>>>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>>> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>>> Cc: devicetree@vger.kernel.org
>>>>> Cc: kernel@dh-electronics.com
>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>> Cc: linux-stm32@st-md-mailman.stormreply.com
>>>>> ---
>>>>> V2: Use generic syscon supernode
>>>>> ---
>>>>>    .../bindings/nvmem/nvmem-syscon.yaml          | 39 +++++++++++++++++++
>>>>>    1 file changed, 39 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml b/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
>>>>> new file mode 100644
>>>>> index 0000000000000..7c1173a1a6218
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
>>>>> @@ -0,0 +1,39 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/nvmem/nvmem-syscon.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Generic syscon backed nvmem
>>>>> +
>>>>> +maintainers:
>>>>> +  - Marek Vasut <marex@denx.de>
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: "nvmem.yaml#"
>>>>
>>>> Usual comment: drop quotes. We removed them everywhere, so you based
>>>> your work on some old tree.
>>>>
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - nvmem-syscon
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>
>>>> Rob's questions are not solved.
>>>
>>> Can you reiterate this one ? I likely missed it.
>>
>> You did not solve the case of more than one register. This isn't an odd
>> case.
> 
> So why not just extend the bindings to support
> 
> reg = <0x14c 0x4>, <0x180 0x10>, ... ;
> 
> this kind of stuff ?

Does not look right. Look at nvmem bindings and don't do it differently.

> 
>>>> The nvmem.yaml schema expects here to
>>>> allow children. This should not be created per-register, but per entire
>>>> block of registers.
>>>
>>> This thing works the other way around, I have a syscon register block
>>> already, and I want to expose subset of it to userspace as read/write
>>> accessible file to expose bootcounter available in that register (so I
>>> can read it and reset it from user application).
>>
>> And this makes it too limited. I would expect one device exposing
>> multiple blocks or registers, just like all nvmem providers are doing.
> 
> What would be the real-world use case of that ?

The same as the real world use cases for nvmem. One syscon block has
multiple registers so I can easily imagine wanting boottrap, bootstat,
boot-whatever you want.

Best regards,
Krzysztof

Re: [PATCH v2 3/3] ARM: dts: stm32: Add nvmem-syscon node to TAMP to expose boot count on DHSOM

[Patrick DELAUNAY]

Hi,

On 6/1/23 01:09, Marek Vasut wrote:
> On 5/26/23 17:28, patrick.delaunay@foss.st.com wrote:
>> Hi Marek,
>
> Hi,
>
>>> From: Marek Vasut <marex@denx.de>
>>> Sent: Wednesday, May 17, 2023 5:25 PM
>>> Subject: [PATCH v2 3/3] ARM: dts: stm32: Add nvmem-syscon node to 
>>> TAMP to
>>> expose boot count on DHSOM
>>>
>>> Add nvmem-syscon subnode to expose TAMP_BKPxR register 19 to user 
>>> space.
>>> This register contains U-Boot boot counter, by exposing it to user 
>>> space the user
>>> space can reset the boot counter.
>>>
>>> Read access example:
>>> "
>>> $ hexdump -vC /sys/bus/nvmem/devices/5c00a000.tamp\:nvmem0/nvmem
>>> 00000000  0c 00 c4 b0
>>> "
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> ---
>>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: kernel@dh-electronics.com
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-stm32@st-md-mailman.stormreply.com
>>> ---
>>> V2: No change
>>> ---
>>>   arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi | 11 +++++++++++
>>> arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi | 11 +++++++++++
>>>   2 files changed, 22 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
>>> b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
>>> index 74735552f4803..b2557bb718f52 100644
>>> --- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
>>> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
>>> @@ -537,6 +537,17 @@ &sdmmc3 {
>>>       status = "okay";
>>>   };
>>>
>>> +&tamp {
>>> +    #address-cells = <1>;
>>> +    #size-cells = <1>;
>>> +
>>> +    /* Boot counter */
>>> +    nvmem {
>>> +        compatible = "nvmem-syscon";
>>> +        reg = <0x14c 0x4>;
>>> +    };
>>> +};
>>> +
>>>   &uart4 {
>>>       pinctrl-names = "default";
>>>       pinctrl-0 = <&uart4_pins_a>;
>>> diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>>> b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>>> index bba19f21e5277..864960387e634 100644
>>> --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>>> +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
>>> @@ -269,3 +269,14 @@ &rng1 {
>>>   &rtc {
>>>       status = "okay";
>>>   };
>>> +
>>> +&tamp {
>>> +    #address-cells = <1>;
>>> +    #size-cells = <1>;
>>> +
>>> +    /* Boot counter */
>>> +    nvmem {
>>
>> According binding you need to add "@<reg>" => nvmem@14c
>>
>> And you export only TAMP_BKP19R directly in a nvmem region ?
>
> 4 bytes is more than plenty for boot counter , yes.
>
>>> +        compatible = "nvmem-syscon";
>>> +        reg = <0x14c 0x4>;
>>> +    };
>>> +};
>>
>>
>> the boot counter could be a nvem cell so you could expose  other 
>> backup registers
>>
>> For example :
>>
>> &tamp {
>>     #address-cells = <1>;
>>     #size-cells = <1>;
>>
>>     nvmem@14c  {
>>         compatible = "nvmem-syscon";
>>         reg = <0x14c 0x4>;
>>
>>         /* Data cells */
>>         boot_counter: boot-counter@14c {
>>             reg = <0x14c 0x4>;
>>         };
>>     };
>> };
>>
>> Even if you export more backup register the cell will be correctly 
>> described in DT
>> and could be accessible directly  with sysfs without managed offset 
>> in userland
>>
>> with https://lore.kernel.org/lkml/202305240724.z3McDuYM-lkp@intel.com/T/
>> Or previous serie 
>> https://lore.kernel.org/lkml/20211220064730.28806-1-zajec5@gmail.com/
>>
>>
>> for example to export all the free register:
>>
>> Reference: https://wiki.st.com/stm32mpu/wiki/STM32MP15_backup_registers
>>
>> the cell " boot-counter" will be always available for users.
>>
>> &tamp {
>>     #address-cells = <1>;
>>     #size-cells = <1>;
>>
>>     /* backup register: 10 to 21 */
>>     nvmem@0x128  {
>>         compatible = "nvmem-syscon";
>>         reg = <0x128 0x44>;
>>
>>         /* Data cells */
>>         boot_counter: boot-counter@14c {
>>             reg = <0x14c 0x4>;
>>         };
>>         boot_mode: boot-mode@150 {
>>             reg = <0x150 0x4>;
>>         };
>> ....
>>     };
>> };
>
> Sure, thanks. I'm not sure I understood the message above.


sorry if it wasn't clear


TAMP register a nvmem driver = NVRAM provider

=> it should export ALL the free backup registers

       as they can used by application / kernel for many purpose....

       and not only for boot counterfor you use-case


So limit the exported backup register to this 4 bytes is strange for me.


and COUNTER is a nvem cell =  a part of backup register = TAMP_BKP19R

=> I agree 4 byte for this count is fine for this cell


NB: today we have no means to read only one nvmem cell with sysfs API

        but I see this feature is proposed to have something as

/sys/bus/nvmem/devices/nvmem@0x128/ => all the backup registers

/sys/bus/nvmem/devices/nvmem@0x128/cells/boot-mode => only the nvmem 
cell used as counter I think it is more safe for long term support to 
manage your counter as a nvmem cell. regards Patrick