Re: [PATCH v2 1/3] nvmem: core: introduce cells parser

From: Srinivas Kandagatla
Date: Mon Sep 20 2021 - 09:41:14 EST




On 20/09/2021 14:29, Vadym Kochan wrote:

Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> writes:

On 20/09/2021 13:29, Vadym Kochan wrote:

Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> writes:

On 20/09/2021 12:25, Vadym Kochan wrote:
Or, treat cells with length "0" in a special way and allow to update
cell info later.you can update irrespective of the length, as long as this is done
before register.


};

some_dev_node {
compatible = "xxx";
nvmem-cells = <&mac_address>;
nvmem-cell-names = "mac-address";
};

== CODE ==
ret = of_get_mac_address(dev->of_node, base_mac);
==========


If you notice the mac_address node reg is 0.
This node "reg" property should be updated ( using of_update_property())
by nvmem-provider driver while tlv parsing and by matching the node name
with tlv name.

I assume parser driver can just invoke add_cell_table (with may be
by adding 'bool update' field to the cell_info struct) and core.c will just
update existing cells parsed from OF.

Lets keep the core code clean for now, I would expect the provider
driver along with parser function to do update the nodes.

--srini

core.c sequence:

1) after cells parsed from OF:

2) lookup the parser

3) parser->cells_parse(ctx, table)

3.a) update existing cells matched by name from table

4) parser->cells_clean(ctx, table)
/* to free cell's_info allocated by the parser driver */

as alternative way, I think it would be more generic to
provide nvmem-provider.h API to update the existing cell info,
however it makes sense only when cells were parsed
by the cell parser, in the other situations the
cell should be well defined.

with this approach the parser driver will be just called
via parser->cells_parse(ctx) and will call nvmem_cell_info_update()
for each parsed cells.

TBH, This is an over kill.

In nvmem provider driver probe you can parse the tlv data and update the
dt nodes before nvmem register.

rest of the code should just work as it is.

--srini

You mean to parse TLV in the particular nvmem provider driver (for
example in at24 driver) ? If so, then it will not allow to parse
this TLV format from the other kinds of nvmem.

Why not?


Well, I think that nvmem driver and TLV parsing should somehow be
de-coupled from each other like block devices and FS does. BUT,
in case this TLV data will be used only on at24 devices then
may be it is OK.


It has to be decoupled yes, which could be at any level with simple library function to a infrastructure level..

In this case with few or single users, doing with an additional infrastructure is a bit of over kill IMO.


--srini
Can you also tell us which other nvmem providers are you going to test
this on?


Currently I can test only on at24 devices. From the:

https://opencomputeproject.github.io/onie/design-spec/hw_requirements.html

"
Each ONIE system must include non-volatile storage which contains vital
product data assigned by the manufacturer. The non-volatile storage
could take the form of an EEPROM, a NOR-flash sector, a NAND-flash
sector or any other non-volatile storage medium.
"

I am not aware about other nvmem devices which are used for existing
ONIE supported boards.

As long as you represent this parsing function as some library function,
I do not see any issue.
If any exported symbol is available for this then any nvmem provider
could use it.

--srini