Re: [PATCH v2 1/4] dt-bindings: nvmem: Add bootcount-nvmem

Nandor Han
2021-04-30T08:21:04Z

Hi,

Thanks for your feedback.

<snip>

>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - linux,bootcount-nvmem
>
> What makes this Linux specific? IIRC, u-boot has boot counting function
> too.
>

U-Boot has indeed the counterpart functionality of bootcount feature,
however, in this particularly case is not called `u-boot,bootcount-nvmem`.
If you have any suggestions I'm happy to change it.
Should I remove the `linux` prefix?

<snip>

>> +
>> + linux,bootcount-magic:
>> + description: Override default mask value.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>
> I don't understand what this is. Is it magic or a mask?

It's the magic value. Seems to be a mistake in the description.
I'll correct this.

>
>> +
>> +dependencies:
>> + nvmem-cell-names: [ nvmem-cells ]
>
> Core schema takes care of this.
>

Ok. I will remove it in that case.

<snip>

>> +examples:
>> + # example with 16 bit nvram cell:
>> + - |
>> + bootcount {
>> + compatible = "linux,bootcount-nvmem";
>> + nvmem-cells = <&bootcount_regs>;
>> + nvmem-cell-names = "bootcount-regs";
>> + };
>> +
>> + rtc: rtc@68 {
>> + bootcount_regs: bootcount_nvmem_regs@e {
>> + reg = <0x0e 0x2>;
>
> It would be simpler to just add a compatible here and get rid of the
> 'bootcount' node here.
>

This is the configuration for NVMEM cell provider and is done according
to bindings/nvmem.yaml document.
Is here something that I'm missing?

At least this method of declaring NVMEM cells providers are decoupling
the code from hardware declaration resulting in a more robust code.

Regards,
Nandor