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

On 31/05/2023 22:44, Marek Vasut wrote:

On 5/31/23 21:37, Krzysztof Kozlowski wrote:

quoted

On 24/05/2023 05:30, Marek Vasut wrote:

quoted

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

quoted

On 17/05/2023 17:25, Marek Vasut wrote:

quoted

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 <redacted>
Cc: Marek Vasut <marex@denx.de>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Srinivas Kandagatla <redacted>
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.

quoted

+
+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.

quoted

quoted

quoted

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel