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

[Marek Vasut <marex@denx.de>]

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.

[...]