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

[PATCH 1/3] [RFC] 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: Rafał Miłecki <rafal@milecki.pl>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
To: linux-arm-kernel@lists.infradead.org
---
 .../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..3035a0b2cd24a
--- /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:
+  - |
+    tamp@5c00a000 {
+        compatible = "st,stm32-tamp", "syscon", "simple-mfd";
+        reg = <0x5c00a000 0x400>;
+
+        nvmem-syscon {
+            compatible = "nvmem-syscon";
+            reg = <0x14c 0x4>;
+        };
+    };
-- 
2.35.1

[PATCH 2/3] [RFC] 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: Rafał Miłecki <rafal@milecki.pl>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
To: linux-arm-kernel@lists.infradead.org
---
 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 ec8a49c040031..cd9a7b00bc1ab 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -290,6 +290,16 @@ config NVMEM_SPRD_EFUSE
 	  This driver can also be built as a module. If so, the module
 	  will be called nvmem-sprd-efuse.
 
+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 fa80fe17e567e..6e170b30a1d1d 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -59,6 +59,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
 obj-$(CONFIG_NVMEM_SUNPLUS_OCOTP)	+= nvmem_sunplus_ocotp.o
diff --git a/drivers/nvmem/nvmem-syscon.c b/drivers/nvmem/nvmem-syscon.c
new file mode 100644
index 0000000000000..9a7f9a5e37609
--- /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.35.1

[PATCH 3/3] [RFC] 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: Rafał Miłecki <rafal@milecki.pl>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
To: linux-arm-kernel@lists.infradead.org
---
 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 155843e1402fb..8277837be8a51 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcom-som.dtsi
@@ -533,6 +533,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 134a798ad3f23..42a8d5cdd7024 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-som.dtsi
@@ -271,3 +271,14 @@ &rng1 {
 &rtc {
 	status = "okay";
 };
+
+&tamp {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	/* Boot counter */
+	nvmem {
+		compatible = "nvmem-syscon";
+		reg = <0x14c 0x4>;
+	};
+};
-- 
2.35.1

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

[Rob Herring]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3363 bytes --]

On Fri, 28 Oct 2022 00:50:18 +0200, 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: Rafał Miłecki <rafal@milecki.pl>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> To: linux-arm-kernel@lists.infradead.org
> ---
>  .../bindings/nvmem/nvmem-syscon.yaml          | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem-syscon.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dts:24.17-35: Warning (reg_format): /example-0/tamp@5c00a000/nvmem-syscon:reg: property has invalid length (8 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dts:22.26-25.15: Warning (unit_address_vs_reg): /example-0/tamp@5c00a000/nvmem-syscon: node has a reg or ranges property, but no unit name
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dts:22.26-25.15: Warning (avoid_default_addr_size): /example-0/tamp@5c00a000/nvmem-syscon: Relying on default #address-cells value
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dts:22.26-25.15: Warning (avoid_default_addr_size): /example-0/tamp@5c00a000/nvmem-syscon: Relying on default #size-cells value
Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/nvmem/nvmem-syscon.example.dtb: tamp@5c00a000: 'nvmem-syscon' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

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

[Rob Herring]

On Fri, Oct 28, 2022 at 12:50:18AM +0200, 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.

Generic bindings always start trivial until they get appended one 
property at a time...

What happens when you have more than 1 field and/or more than 1 
register?

> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> To: linux-arm-kernel@lists.infradead.org
> ---
>  .../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..3035a0b2cd24a
> --- /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:
> +  - |
> +    tamp@5c00a000 {
> +        compatible = "st,stm32-tamp", "syscon", "simple-mfd";

This is very common, but personally I think "syscon" and "simple-mfd" 
should be mutually exclusive. "simple-mfd" is saying the children have 
no dependency on the parent, yet the child nodes need a regmap from the 
parent. Sounds like a dependency.

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

How does one identify this is the bootloader's boot count? How does the 
bootloader know it can write to this?

> +        };
> +    };
> -- 
> 2.35.1
> 
> 

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

[Marek Vasut]

On 10/28/22 23:28, Rob Herring wrote:
> On Fri, Oct 28, 2022 at 12:50:18AM +0200, 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.
> 
> Generic bindings always start trivial until they get appended one
> property at a time...
> 
> What happens when you have more than 1 field and/or more than 1
> register?

If it is a continuous register array, the user can use the size field to 
describe such register array here.

If it is a sparse register array, multiple nvmem-syscon nodes would be 
needed. I haven't seen anything which would require one node for sparse 
register arrays, like boot counter distributed across multiple 
non-continuous registers or such.

>> +properties:
>> +  compatible:
>> +    enum:
>> +      - nvmem-syscon
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    tamp@5c00a000 {
>> +        compatible = "st,stm32-tamp", "syscon", "simple-mfd";
> 
> This is very common, but personally I think "syscon" and "simple-mfd"
> should be mutually exclusive. "simple-mfd" is saying the children have
> no dependency on the parent, yet the child nodes need a regmap from the
> parent. Sounds like a dependency.

So what exactly should be changed here?

>> +        reg = <0x5c00a000 0x400>;
>> +
>> +        nvmem-syscon {
>> +            compatible = "nvmem-syscon";
>> +            reg = <0x14c 0x4>;
> 
> How does one identify this is the bootloader's boot count?

The user has to know where the boot counter is stored (by hardware 
path). I wouldn't attempt to assign any complex logic here, since the 
boot counter could be implemented in various ways. Besides, this may not 
even be a boot counter, but some other variable exposed to user space. 
Maybe a unique node name can be used to discern the different 
nvmem-syscon nodes representing different variables if needed.

> How does the
> bootloader know it can write to this?

The bootloader implementer selected the bootcounter register based on 
hardware knowledge .