Date:   Mon, 20 May 2019 09:28:16 -0500
From:   Rob Herring <robh+dt@...nel.org>
To:     Petr Štetiar <ynezz@...e.cz>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        Andy Duan <fugang.duan@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "john@...ozen.org" <john@...ozen.org>,
        "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Frank Rowand <frowand.list@...il.com>,
        Mark Rutland <mark.rutland@....com>,
        Alban Bedel <albeu@...e.fr>, devicetree@...r.kernel.org
Subject: Re: NVMEM address DT post processing [Was: Re: [PATCH net 0/3] add
 property "nvmem_macaddr_swap" to swap macaddr bytes order]

On Mon, May 13, 2019 at 6:16 AM Petr Štetiar <ynezz@...e.cz> wrote:
>
> Srinivas Kandagatla <srinivas.kandagatla@...aro.org> [2019-05-13 11:06:48]:
>
> > On 13/05/2019 10:07, Petr Štetiar wrote:
> > > Srinivas Kandagatla <srinivas.kandagatla@...aro.org> [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@...000 {
>             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

Powered by blists - more mailing lists