Messages in this thread | | | Subject | Re: [PATCH v2 1/3] nvmem: core: introduce cells parser | From | Srinivas Kandagatla <> | Date | Mon, 20 Sep 2021 14:40:49 +0100 |
| |
On 20/09/2021 14:29, Vadym Kochan wrote: > > Srinivas Kandagatla <srinivas.kandagatla@linaro.org> writes: > >> On 20/09/2021 13:29, Vadym Kochan wrote: >>> >>> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> 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 >> >> >>> >>>> >>>> >>>>> >>> >
| |