Re: [RFC PATCH] dt-bindings: nvmem: add transformation support

[Michael Walle]

Hi Srinivas,

Am 2021-11-29 13:54, schrieb Srinivas Kandagatla:
> On 23/11/2021 13:44, Michael Walle wrote:
> > This is my second attempt to solve the use case where there is only 
> > the
> > base MAC address stored in an EEPROM or similar storage provider. This
> > is the case for the Kontron sl28 board and multiple openwrt supported
> > boards.
> >
> > The first proposal [1] didn't find much appreciation and there wasn't
> > any reply to my question or new proposal [2]. So here we are with my 
> > new
> > proposal, that is more flexible and doesn't fix the ethernet mac only.
> > This is just an RFC for the device tree representation for now to see 
> > if
> > this is the correct way to tackle this.
> >
> > I'm also aware of the latest post process hook support [3]. This 
> > doesn't
> > fix the base mac address issue, but I think it also doesn't solve the
> > case with swapped ethernet addresses in the general case. That hook 
> > will
> > involve the driver to do the swapping, but how would the driver know
> > if that swapping is actually required. Usually the interpretation of 
> > the
> > content is opaque to the driver, after all it is the user/board
>
> But this is the path for any post processing hook, which ever
> direction we endup with(using core helpers or provider specific
> post-processing).

Mh? I don't understand. My point was that the driver is unlikely
to know what it should process. Take the mtd (or the mtd otp)
nvmem provider for example. If I understand it correctly, it just
gets the nvmem name, for example, "mac-address". How should
the post process hook know, what it should do? IMHO that just
works for very specific drivers, which tied to the content
they provide.

> > manufacturer who does program the storage device. We might be lucky in
> > the imx-ocotp case because the IMX reference manual actually states
> > where and in which format the mac address is programmed.
> >
> > Introduce a transformation property. This is intended to be just an
> > enumeration of operations. If there will be a new operation, support 
> > for
> > it has to be added to the nvmem core.
> >
> > A transformation might have multiple output values, like in the base 
> > mac
> > address case. It reads the mac address from the nvmem storage and
> > generates multiple individual addresses, i.e. on our board we reserve 
> > 8
> > consecutive addresses. These addresses then can be assigned to 
> > different
> > network interfaces. To make it possible to reference different values 
> > we
> > need to introduce an argument to the phandle. This additional argument
> > is then an index into a list of values.
> >
> > Example:
> >    mac_addresses: base-mac-address@10 {
> >      #nvmem-cell-cells = <1>;
> >      reg = <10 6>;
> >      transformation = <NVMEM_T_ETH_OFFSET 0 1 7>;
>
> IMO, this is totally redundant. we could probably encode this info
> directly in the cell specifiers, something like:
>
> >    }
> >
> >    &eth0 {
> >      nvmem-cells = <&mac_addresses 0>;
>
> nvmem-cells = <&base_mac_addresses NVMEM_T_ETH_OFFSET 0>;

I had he same idea, but keep in mind, that there could be more
than just one nvmem cells:

nvmem-cells = <&phandle1 arg1 &pandle2 arg2 arg3>;
nvmem-cell-names = "name1", "name2";

So you'd need the #nvmem-cell-cells either way.

> And value of #nvmem-cell-cells is dependent on the first cell 
> specifier.

What do you mean with first cell specifier? the phandle 
(base_mac_address
in the example) or the NVMEM_T_ETH_OFFSET? I guess the latter, because 
the
arguments depend on the transformation. But this is not how the
of_parse_phandle_with_args() works, it will look the '#nvmem-cell-cells'
property up, to see how many arguments it should expect, which is a
property to the referenced node. Thus I've come up with the additional
indirection. The number of arguments for the reference cell is either
0 or 1 and the transformation is part of the nvmem cell.

> I understand that these 3 bits of info is required for this type of
> post processing and this can only come from DT and its not possible to
> determine this at runtime.

ok :)

> Would this address other usecases?

I think so, but see above for why it can't work. Or I am missing
something.

> Are you in a position to test few of them?

Sure (at least after my vacation). And TBH I think the imxotp mac
swap should use the same or it will be likely that there are future
SoCs which will always swap the ethnet

> Lets wait for Rob's opinion on adding #nvmem-cell-cells property with
> cell specifiers describing the encoding information?

+1

Thanks for looking into this,
-michael