[V3] dt-bindings: nvmem: convert base example to use "nvmem-layout" node

  From: Rafał Miłecki <rafal@milecki.pl>

With support for "fixed-layout" binding we can use now "nvmem-layout"
even for fixed NVMEM cells. Use that in the base example as it should be
preferred over placing cells directly in the device node.

New and other bindings should follow as old binding will get deprecated
at some point.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
 1 file changed, 24 insertions(+), 18 deletions(-)
  

Hi Rafał,

zajec5@gmail.com wrote on Fri, 10 Mar 2023 08:51:45 +0100:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> With support for "fixed-layout" binding we can use now "nvmem-layout"
> even for fixed NVMEM cells. Use that in the base example as it should be
> preferred over placing cells directly in the device node.
> 
> New and other bindings should follow as old binding will get deprecated
> at some point.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
>  1 file changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> index 732162e9d13e..c77be1c20e47 100644
> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> @@ -67,24 +67,30 @@ examples:
>  
>            /* ... */
>  
> -          /* Data cells */
> -          tsens_calibration: calib@404 {
> -              reg = <0x404 0x10>;
> -          };
> -
> -          tsens_calibration_bckp: calib_bckp@504 {
> -              reg = <0x504 0x11>;
> -              bits = <6 128>;
> -          };
> -
> -          pvs_version: pvs-version@6 {
> -              reg = <0x6 0x2>;
> -              bits = <7 2>;
> -          };
> -
> -          speed_bin: speed-bin@c{
> -              reg = <0xc 0x1>;
> -              bits = <2 3>;
> +          nvmem-layout {

I believe we should not introduce "intermediate state" bindings when
this is not strictly required, in order to avoid confusion with what is
backward and what is transitory. So I would expect this to only apply
after the switch to:

	nvmem-layout@xxx {
		reg = <xxx>;

I don't care who will take care of it, but I think it would be better
to have everything in one series.

Other than the "order" problematic which I think is important here, I
agree with this series.

> +              compatible = "fixed-layout";
> +              #address-cells = <1>;
> +              #size-cells = <1>;
> +
> +              /* Data cells */
> +              tsens_calibration: calib@404 {
> +                  reg = <0x404 0x10>;
> +              };
> +
> +              tsens_calibration_bckp: calib_bckp@504 {
> +                  reg = <0x504 0x11>;
> +                  bits = <6 128>;
> +              };
> +
> +              pvs_version: pvs-version@6 {
> +                  reg = <0x6 0x2>;
> +                  bits = <7 2>;
> +              };
> +
> +              speed_bin: speed-bin@c{
> +                  reg = <0xc 0x1>;
> +                  bits = <2 3>;
> +              };
>            };
>        };
>  


Thanks,
Miquèl
  

On 10.03.2023 09:59, Miquel Raynal wrote:
> Hi Rafał,
> 
> zajec5@gmail.com wrote on Fri, 10 Mar 2023 08:51:45 +0100:
> 
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> With support for "fixed-layout" binding we can use now "nvmem-layout"
>> even for fixed NVMEM cells. Use that in the base example as it should be
>> preferred over placing cells directly in the device node.
>>
>> New and other bindings should follow as old binding will get deprecated
>> at some point.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
>>   1 file changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> index 732162e9d13e..c77be1c20e47 100644
>> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> @@ -67,24 +67,30 @@ examples:
>>   
>>             /* ... */
>>   
>> -          /* Data cells */
>> -          tsens_calibration: calib@404 {
>> -              reg = <0x404 0x10>;
>> -          };
>> -
>> -          tsens_calibration_bckp: calib_bckp@504 {
>> -              reg = <0x504 0x11>;
>> -              bits = <6 128>;
>> -          };
>> -
>> -          pvs_version: pvs-version@6 {
>> -              reg = <0x6 0x2>;
>> -              bits = <7 2>;
>> -          };
>> -
>> -          speed_bin: speed-bin@c{
>> -              reg = <0xc 0x1>;
>> -              bits = <2 3>;
>> +          nvmem-layout {
> 
> I believe we should not introduce "intermediate state" bindings when
> this is not strictly required, in order to avoid confusion with what is
> backward and what is transitory. So I would expect this to only apply
> after the switch to:
> 
> 	nvmem-layout@xxx {
> 		reg = <xxx>;
> 
> I don't care who will take care of it, but I think it would be better
> to have everything in one series.
> 
> Other than the "order" problematic which I think is important here, I
> agree with this series.

I fail to see how / why:
1. Adding new NVMEM layout
2. Supporting mutliple NVMEM layouts
would depend on each other.

We already have 2 NVMEM layouts bindings. I'm just adding a new (third)
one.

If having NVMEM layouts bindings puts us in any kind of intermediate
state then we're already there. I don't think my new NVMEM layout
changes this situation.


>> +              compatible = "fixed-layout";
>> +              #address-cells = <1>;
>> +              #size-cells = <1>;
>> +
>> +              /* Data cells */
>> +              tsens_calibration: calib@404 {
>> +                  reg = <0x404 0x10>;
>> +              };
>> +
>> +              tsens_calibration_bckp: calib_bckp@504 {
>> +                  reg = <0x504 0x11>;
>> +                  bits = <6 128>;
>> +              };
>> +
>> +              pvs_version: pvs-version@6 {
>> +                  reg = <0x6 0x2>;
>> +                  bits = <7 2>;
>> +              };
>> +
>> +              speed_bin: speed-bin@c{
>> +                  reg = <0xc 0x1>;
>> +                  bits = <2 3>;
>> +              };
>>             };
>>         };
  

On 10/03/2023 07:51, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> With support for "fixed-layout" binding we can use now "nvmem-layout"
> even for fixed NVMEM cells. Use that in the base example as it should be
> preferred over placing cells directly in the device node.
> 
Fixed layouts are the core part of nvmem, am not sure why you want to 
deprecate this. Either we derive the cell information dt or via layouts 
or some post processing they should still endup as fixed layouts.
this way the core part is always same irrespective of where the cell 
info comes from.


--srini

> New and other bindings should follow as old binding will get deprecated
> at some point.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>   .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
>   1 file changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> index 732162e9d13e..c77be1c20e47 100644
> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> @@ -67,24 +67,30 @@ examples:
>   
>             /* ... */
>   
> -          /* Data cells */
> -          tsens_calibration: calib@404 {
> -              reg = <0x404 0x10>;
> -          };
> -
> -          tsens_calibration_bckp: calib_bckp@504 {
> -              reg = <0x504 0x11>;
> -              bits = <6 128>;
> -          };
> -
> -          pvs_version: pvs-version@6 {
> -              reg = <0x6 0x2>;
> -              bits = <7 2>;
> -          };
> -
> -          speed_bin: speed-bin@c{
> -              reg = <0xc 0x1>;
> -              bits = <2 3>;
> +          nvmem-layout {
> +              compatible = "fixed-layout";
> +              #address-cells = <1>;
> +              #size-cells = <1>;
> +
> +              /* Data cells */
> +              tsens_calibration: calib@404 {
> +                  reg = <0x404 0x10>;
> +              };
> +
> +              tsens_calibration_bckp: calib_bckp@504 {
> +                  reg = <0x504 0x11>;
> +                  bits = <6 128>;
> +              };
> +
> +              pvs_version: pvs-version@6 {
> +                  reg = <0x6 0x2>;
> +                  bits = <7 2>;
> +              };
> +
> +              speed_bin: speed-bin@c{
> +                  reg = <0xc 0x1>;
> +                  bits = <2 3>;
> +              };
>             };
>         };
>
  

On 10.03.2023 10:29, Srinivas Kandagatla wrote:
> 
> 
> On 10/03/2023 07:51, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> With support for "fixed-layout" binding we can use now "nvmem-layout"
>> even for fixed NVMEM cells. Use that in the base example as it should be
>> preferred over placing cells directly in the device node.
>>
> Fixed layouts are the core part of nvmem, am not sure why you want to deprecate this. Either we derive the cell information dt or via layouts or some post processing they should still endup as fixed layouts.
> this way the core part is always same irrespective of where the cell info comes from.

I really don't understand why my changes get misunderstood. It's just
happened another time. Yesterday Michael wrote: "your motivation to drop
handling the fixed cells".

Again:
I DON'T deprecate or drop support for fixed layouts (fixed NVMEM cells)
I DON'T deprecate or drop support for fixed layouts (fixed NVMEM cells)

I just want NVMEM bindings to move location of DT fixed NVMEM cells from
*device* node to *nvmem-layout* node.

Also:
I WON'T drop support for old binding. We stay backward compatible.
I WON'T drop support for old binding. We stay backward compatible.


>> New and other bindings should follow as old binding will get deprecated
>> at some point.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
>>   1 file changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> index 732162e9d13e..c77be1c20e47 100644
>> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> @@ -67,24 +67,30 @@ examples:
>>             /* ... */
>> -          /* Data cells */
>> -          tsens_calibration: calib@404 {
>> -              reg = <0x404 0x10>;
>> -          };
>> -
>> -          tsens_calibration_bckp: calib_bckp@504 {
>> -              reg = <0x504 0x11>;
>> -              bits = <6 128>;
>> -          };
>> -
>> -          pvs_version: pvs-version@6 {
>> -              reg = <0x6 0x2>;
>> -              bits = <7 2>;
>> -          };
>> -
>> -          speed_bin: speed-bin@c{
>> -              reg = <0xc 0x1>;
>> -              bits = <2 3>;
>> +          nvmem-layout {
>> +              compatible = "fixed-layout";
>> +              #address-cells = <1>;
>> +              #size-cells = <1>;
>> +
>> +              /* Data cells */
>> +              tsens_calibration: calib@404 {
>> +                  reg = <0x404 0x10>;
>> +              };
>> +
>> +              tsens_calibration_bckp: calib_bckp@504 {
>> +                  reg = <0x504 0x11>;
>> +                  bits = <6 128>;
>> +              };
>> +
>> +              pvs_version: pvs-version@6 {
>> +                  reg = <0x6 0x2>;
>> +                  bits = <7 2>;
>> +              };
>> +
>> +              speed_bin: speed-bin@c{
>> +                  reg = <0xc 0x1>;
>> +                  bits = <2 3>;
>> +              };
>>             };
>>         };
  

Hi Srinivas,

srinivas.kandagatla@linaro.org wrote on Fri, 10 Mar 2023 09:29:18 +0000:

> On 10/03/2023 07:51, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> > 
> > With support for "fixed-layout" binding we can use now "nvmem-layout"
> > even for fixed NVMEM cells. Use that in the base example as it should be
> > preferred over placing cells directly in the device node.
> >   
> Fixed layouts are the core part of nvmem, am not sure why you want to deprecate this. Either we derive the cell information dt or via layouts or some post processing they should still endup as fixed layouts.
> this way the core part is always same irrespective of where the cell info comes from.

Actually I was suspicious as first but we had a very similar case in
mtd:
- People need partitioning so we add partitions in the mtd node
- People need more advanced partitioning and partitioning becomes a
  mess so we containerize everything in a "partitions" subnode.
  It definitely improves the readability and makes the code more
  resilient: outside of the container, it's not a partition, period.

I believe that's what Rafał is trying to anticipate. Just moving the
fixed cells declaration into a container (and we have one already:
nvmem-layout).

Thanks,
Miquèl