[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 at denx.de>
>>>>> ---
>>>>> Cc: Alexandre Torgue <alexandre.torgue at foss.st.com>
>>>>> Cc: Conor Dooley <conor+dt at kernel.org>
>>>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt at linaro.org>
>>>>> Cc: Marek Vasut <marex at denx.de>
>>>>> Cc: Maxime Coquelin <mcoquelin.stm32 at gmail.com>
>>>>> Cc: Rob Herring <robh+dt at kernel.org>
>>>>> Cc: Srinivas Kandagatla <srinivas.kandagatla at linaro.org>
>>>>> Cc: devicetree at vger.kernel.org
>>>>> Cc: kernel at dh-electronics.com
>>>>> Cc: linux-arm-kernel at lists.infradead.org
>>>>> Cc: linux-stm32 at 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 at 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