Re: NVMEM address DT post processing [Was: Re: [PATCH net 0/3] add property "nvmem_macaddr_swap" to swap macaddr bytes order]

[Rob Herring <robh+dt@xxxxxxxxxx>]

On Mon, May 13, 2019 at 6:16 AM Petr Štetiar <ynezz@xxxxxxx> wrote:
>
> Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> [2019-05-13 11:06:48]:
>
> > On 13/05/2019 10:07, Petr Štetiar wrote:
> > > Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> [2019-05-13 09:25:55]:
> > >
> > > > My initial idea was to add compatible strings to the cell so that most of
> > > > the encoding information can be derived from it. For example if the encoding
> > > > representing in your example is pretty standard or vendor specific we could
> > > > just do with a simple compatible like below:
> > >
> > > that vendor/compatible list would be quite long[1], there are hundreds of
> >
> > You are right just vendor list could be very long, but I was hoping that the
> > post-processing would fall in some categories which can be used in
> > compatible string.
> >
> > Irrespective of which we need to have some sort of compatible string to
> > enable nvmem core to know that there is some form of post processing to be
> > done on the cells!. Without which there is a danger of continuing to adding
> > new properties to the cell bindings which have no relation to each other.
>
> makes sense, so something like this would be acceptable?

No. Don't try to put a data processing language into DT.

>
>  eth1_addr: eth-mac-addr@18a {
>      /* or rather linux,nvmem-post-process ? */
>      compatible = "openwrt,nvmem-post-process";
>      reg = <0x189 0x11>;
>      indices = < 0 2
>                  3 2
>                  6 2
>                  9 2
>                 12 2
>                 15 2>;
>      transform = "ascii";
>      increment = <1>;
>      increment-at = <5>;
>      result-swap;
>  };
>
>  &eth1 {
>      nvmem-cells = <&eth1_addr>;
>      nvmem-cell-names = "mac-address";
>  };
>
> > > was very compeling as it would kill two birds with one stone (fix outstanding
> > > MTD/NVMEM OF clash as well[2]),
> >
> > Changes to nvmem dt bindings have been already rejected, for this more
> > discussion at: https://lore.kernel.org/patchwork/patch/936312/
>
> I know, I've re-read this thread several times, but it's still unclear to me,
> how this should be approached in order to be accepted by you and by DT
> maintainers as well.
>
> Anyway, to sum it up, from your point of view, following is fine?
>
>  flash@0 {
>     partitions {
>         art: partition@ff0000 {
>             nvmem_dev: nvmem-cells {
>                 compatible = "nvmem-cells";

My suggestion would be to add a specific compatible here and that can
provide a driver or hooks to read and transform the data.

Rob

>                 eth1_addr: eth-mac-addr@189 {
>                     compatible = "openwrt,nvmem-post-process";
>                     reg = <0x189 0x6>;
>                     increment = <1>;
>                     increment-at = <5>;
>                     result-swap;
>                 };
>             };
>         };
>     };
>  };
>
>  &eth1 {
>     nvmem = <&nvmem_dev>;
>     nvmem-cells = <&eth1_addr>;
>     nvmem-cell-names = "mac-address";
>  };
>
> -- ynezz