Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 14 Oct 2021 12:55:26 +0200
From:   Marek =?UTF-8?B?QmVow7pu?= <kabel@kernel.org>
To:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc:     robh+dt@kernel.org, devicetree@vger.kernel.org,
        U-Boot Mailing List <u-boot@lists.denx.de>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        Luka Kovacic <luka.kovacic@sartura.hr>
Subject: Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot
 environment NVMEM provider
Message-ID: <20211014125526.10d4861b@dellmb>
In-Reply-To: <857c27a6-5c4b-e0ed-a830-35762799613f@linaro.org>
References: <20211013232048.16559-1-kabel@kernel.org>
        <629c8ba1-c924-565f-0b3c-8b625f4e5fb0@linaro.org>
        <20211014120601.133e9a84@dellmb>
        <857c27a6-5c4b-e0ed-a830-35762799613f@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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

> On 14/10/2021 11:06, Marek Beh=C3=BAn wrote:
> > On Thu, 14 Oct 2021 09:26:27 +0100
> > Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> >  =20
> >> On 14/10/2021 00:20, Marek Beh=C3=BAn wrote: =20
> >>> 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.
> >>>     =20
> >>
> >> Have you looked at
> >> ./Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
> >> ? =20
> >=20
> > Hello srini,
> >=20
> > yes, I have. What about it? :)
> >=20
> > 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. =20
>=20
> 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=3Dvalue", separated by '\0's, for example:
> >    board=3Dturris_mox\0ethaddr=3D00:11:22:33:44:55\0bootcmd=3Drun
> > 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.=20
>=20
> 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,=20
> unless the provider driver is adding/updating dt entries dynamically=20
> 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