diff mbox series

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

Message ID 20221027225020.215149-1-marex@denx.de (mailing list archive)
State New, archived
Headers show
Series [1/3,RFC] dt-bindings: nvmem: syscon: Add syscon backed nvmem bindings | expand

Commit Message

Marek Vasut Oct. 27, 2022, 10:50 p.m. UTC
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

Comments

Rob Herring (Arm) Oct. 28, 2022, 12:20 p.m. UTC | #1
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.
Rob Herring (Arm) Oct. 28, 2022, 9:28 p.m. UTC | #2
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
> 
>
Marek Vasut Nov. 27, 2022, 10:05 p.m. UTC | #3
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 .
diff mbox series

Patch

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>;
+        };
+    };