From: Vadym Kochan <vadym.kochan@plvision.eu>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Vadym Kochan <vadym.kochan@plvision.eu>,
John Thomson <john@johnthomson.fastmail.com.au>,
Rob Herring <robh+dt@kernel.org>,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Robert Marko <robert.marko@sartura.hr>
Subject: Re: [PATCH v2 1/3] nvmem: core: introduce cells parser
Date: Tue, 28 Sep 2021 17:11:23 +0300 [thread overview]
Message-ID: <vrcxh2sfxoaiyc.fsf@plvision.eu> (raw)
In-Reply-To: <cbff6b53-d751-0a44-c3de-d31c26ed3d5c@linaro.org>
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> 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@nxp.com/
>>>>> (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
>>
next prev parent reply other threads:[~2021-09-28 14:11 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-08 19:03 [PATCH v2 0/3] nvmem: add ONIE NVMEM cells parser Vadym Kochan
2021-06-08 19:03 ` [PATCH v2 1/3] nvmem: core: introduce " Vadym Kochan
2021-06-08 22:49 ` kernel test robot
2021-06-09 3:05 ` kernel test robot
2021-06-14 10:44 ` Srinivas Kandagatla
2021-06-16 12:33 ` Vadym Kochan
2021-06-21 11:00 ` Srinivas Kandagatla
2021-09-08 9:38 ` Vadym Kochan
2021-09-13 14:19 ` Srinivas Kandagatla
2021-09-20 10:24 ` Vadym Kochan
2021-09-20 10:36 ` Srinivas Kandagatla
2021-09-20 11:25 ` Vadym Kochan
2021-09-20 11:32 ` Srinivas Kandagatla
2021-09-20 12:29 ` Vadym Kochan
2021-09-20 12:34 ` Srinivas Kandagatla
2021-09-20 13:29 ` Vadym Kochan
2021-09-20 13:40 ` Srinivas Kandagatla
2021-09-21 5:50 ` John Thomson
2021-09-27 7:50 ` Vadym Kochan
2021-09-27 10:12 ` Srinivas Kandagatla
2021-09-28 13:31 ` Vadym Kochan
2021-09-28 13:51 ` Srinivas Kandagatla
2021-09-28 14:11 ` Vadym Kochan [this message]
2021-09-28 14:39 ` Srinivas Kandagatla
2021-09-27 10:12 ` Srinivas Kandagatla
2021-09-27 12:38 ` John Thomson
2021-09-08 9:44 ` Vadym Kochan
2021-06-08 19:03 ` [PATCH v2 2/3] dt-bindings: nvmem: document nvmem-cells-parser-name property Vadym Kochan
2021-06-18 20:59 ` Rob Herring
2021-06-08 19:03 ` [PATCH v2 3/3] nvmem: add ONIE nvmem cells parser Vadym Kochan
2021-08-06 15:39 ` Jan Lübbe
2021-09-08 9:56 ` Vadym Kochan
2021-09-12 21:06 ` John Thomson
2021-09-13 14:20 ` Srinivas Kandagatla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=vrcxh2sfxoaiyc.fsf@plvision.eu \
--to=vadym.kochan@plvision.eu \
--cc=devicetree@vger.kernel.org \
--cc=john@johnthomson.fastmail.com.au \
--cc=linux-kernel@vger.kernel.org \
--cc=robert.marko@sartura.hr \
--cc=robh+dt@kernel.org \
--cc=srinivas.kandagatla@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).