Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> writes: > On 28/09/2021 14:31, Vadym Kochan wrote: >>>>> Can I note here that I would like to parse >>>>> TLV data from an SPI-NOR device to NVMEM cells. >>>>> The same general use case (getting mac-address from OEM data). >>>>> >>>>> Was planning to base my work on this series, as well as >>>>> https://lore.kernel.org/all/20210908100257.17833-1-qiangqing.zhang@xxxxxxx/ >>>>> (thanks for pointing that out Srinivas) >>>>> >>>>> Cheers, >>>> What about at least to have just one call in core.c to make it a bit >>>> de-coupled, like: >>> Why do you want to decouple this? the provider driver should be very >>> well aware of the format the data layout. >>> >> In my understanding nvmem device should not aware about the data layout >> (in case it does not rely on device's specific characteristics). Same >> cells layout (TLV, etc) might exist on other nvmem devices. >> > How would provider driver parse this without even knowing data layout? > > >>> Its fine to an extent to adding parse_cells() callback in nvmem_config. >>> >> OK, in that case it will require small change in the core. >> >>>> core.c >>>> >>>> struct nvmem_device *nvmem_register(const struct nvmem_config *config) >>>> { >>>> ... >>>> rval = nvmem_add_cells_from_table(nvmem); >>>> if (rval) >>>> goto err_remove_cells; >>>> >>>> + rval = nvmem_parse_cells(nvmem, of); >>>> + if (rval) { >>>> + /* err handling */ >>>> + } >>>> + >>>> rval = nvmem_add_cells_from_of(nvmem); >>>> if (rval) >>>> goto err_remove_cells; >>>> >>>> blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem); >>>> >>>> return nvmem; >>>> >>>> ... >>>> >>>> } >>>> >>>> somewhere in nvmem-parser.c: >>> However this is totally over kill. >>> >>>> /* retreive parser name from of_node and call appropriate function to parse >>>> non-fixed cells and update via of_update */ >>> This is completely provider drivers job, nothing nvmem core should worry >>> about. >>> >>> If you have concern of having code duplicated then we could make some of >>> the common functions as library functions, But it still is within the >>> scope of provider drivers. >>> >> Do I understand correctly that this parser function should be exported >> from at24.c (in case of ONIE) and not from a separate C module ? Or >> it just means that if there will be more users of this parsing function >> then it might be moved to separate C module ? > yes. > For now am not really sure how many users are for such parsing function. > >> >>> --srini >>> >> BTW, what if such change will be declined by particular nvmem driver >> maintainer ? > > You would need some changes to provider driver to be able to flag that > there is some kind of parsing required anyway. > It might be some new property in device-tree which can be used also for the other providers. > --srini >>
