Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider

[Marek Behún]

On Thu, 14 Oct 2021 11:30:13 +0100
Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> wrote:

> On 14/10/2021 11:06, Marek Behún wrote:
> > On Thu, 14 Oct 2021 09:26:27 +0100
> > Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> wrote:
> >   
> >> On 14/10/2021 00:20, Marek Behún wrote:  
> >>> Add device tree bindings for U-Boot environment NVMEM provider.
> >>>
> >>> U-Boot environment can be stored at a specific offset of a MTD
> >>> device, EEPROM, MMC, NAND or SATA device, on an UBI volume, or in
> >>> a file on a filesystem.
> >>>
> >>> The environment can contain information such as device's MAC
> >>> address, which should be used by the ethernet controller node.
> >>>      
> >>
> >> Have you looked at
> >> ./Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
> >> ?  
> > 
> > Hello srini,
> > 
> > yes, I have. What about it? :)
> > 
> > That binding won't work for u-boot-env, because the data are stored
> > in a different way. A cell does not have a constant predetermined
> > offset on the MTD.  
> 
> Can't you dynamically update the nodes before nvmem-provider is
> registered?

Are you talking about dynamically updating nvmem-cell OF nodes, adding
reg properties with actual offsets and lengths found after parsing?

> > The variables are stored as a sequence of values of format
> > "name=value", separated by '\0's, for example:
> >    board=turris_mox\0ethaddr=00:11:22:33:44:55\0bootcmd=run
> > distro_bootcmd\0.... Chaning lengths of values of variables, or
> > deleting variables, moves the data around. Integers and MAC
> > addresses are stored as strings, and so on. 
> 
> Do you already have a provider driver for handing this.

Not yet, I will send the proposal together with a driver in next
round.

> How is pre parsing cell info and post processing data planned to be
> handled?

My plan was to read the variables from the u-boot-env partition, create
a nvmem-cell for each variable, and then pair the ones mentioned in
device tree with their DT nodes, and post-process according to type
(post-processing would be done only for those mentioned in device tree,
others would be left as strings).

> Currently in nvmem core we do check for "reg" property for each cell, 
> unless the provider driver is adding/updating dt entries dynamically 
> before registering nvmem provider

I don't think updaring DT entries dynamically is a correct solution at
all. Is this done in Linux? Updating device properties is something
different, but changing DT properties seems wrong to me.

> It will not work as it is. Alteast this is what I suggested in similar
> case where cell information is in tlv format.

Hmm. OK, I shall try to implement a driver for this and then will
return to you.

> Secondly mac-address seems to be delimited, we recently introduced
> post processing callback for provider driver [1], which should help
> in this case.

Cool, I shall use that.

> If the nvmem-cell names are standard like "mac-address" then you do
> not need to add a new "type" binding to cell too, you can do
> post-processing based on name.

I plan to add functions
  of_nvmem_get_mac_address()
  nvmem_get_mac_address()
which would look at (in this order):
  mac-address, address, mac-address-backup
We have to keep the name "address" for backwards compatibility with one
driver that uses this (drivers/net/ethernet/ni/nixge.c)

Thanks.

Marek