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;
References: <20210608190327.22071-1-vadym.kochan@plvision.eu>
 <20210608190327.22071-2-vadym.kochan@plvision.eu>
 <43023500-dd6a-5180-057e-cecc1f1b6500@linaro.org>
 <20210616123356.GA9951@plvision.eu>
 <fea907ed-06ce-5c82-667d-d11f3e902616@linaro.org>
 <vrcxh2zgsnl841.fsf@plvision.eu>
 <7e6d75ed-cebc-597f-7062-34261d184968@linaro.org>
 <vrcxh2pmt3bl4h.fsf@plvision.eu>
 <0e471789-fe29-b747-5153-75c9b4616c7f@linaro.org>
 <vrcxh2o88nbias.fsf@plvision.eu>
 <1da03714-8f23-1004-e89a-891e4599e04a@linaro.org>
 <vrcxh2mto7bfcb.fsf@plvision.eu>
 <1e146349-9fef-972b-9084-577f02d5168b@linaro.org>
 <vrcxh2lf3rbcjc.fsf@plvision.eu>
 <169d3f36-4297-32a3-3d23-824989625b26@linaro.org>
 <77b11bf7-3003-483f-b91e-bd93576eaae1@www.fastmail.com>
 <vrcxh2y27ia23n.fsf@plvision.eu>
 <56fb5c64-4142-03ef-2ea8-fc586fd239e1@linaro.org>
 <vrcxh2tui4akta.fsf@plvision.eu>
 <cbff6b53-d751-0a44-c3de-d31c26ed3d5c@linaro.org>
User-agent: mu4e 0.9.18; emacs 28.0.50
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
In-reply-to: <cbff6b53-d751-0a44-c3de-d31c26ed3d5c@linaro.org>
Date:   Tue, 28 Sep 2021 17:11:23 +0300
Message-ID: <vrcxh2sfxoaiyc.fsf@plvision.eu>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?lOJyEUB6vBjGBlaQFaSbdLrOOkcEr+9XpuwphqI0rKKFJkUi/NNl+KTwjWfs?=
 =?us-ascii?Q?5TDEtLEN0fk3HU75o8SkWXp2N4sOa/WHIfgG/HNvthA3IGEt2qoskTWFf5kI?=
 =?us-ascii?Q?mMJoow+94QBjc4zAnsCSt+QpxdS5o5OedkZoceT3fCRF0eLhigJ433OlNvov?=
 =?us-ascii?Q?aeXMoaCZUKW0xnMaB9IXWR2DgDD0If63S3cdmLRAQtnAb1a1ugnq/419nLYs?=
 =?us-ascii?Q?BvcYhN37Vj1dGngdrycLn3LBHhYV+k9K2UVsVAav+WAqYuIbdsn2MyWlrV3L?=
 =?us-ascii?Q?SnaRXB4OUpKcqG5/R3OCMh61gEwIMuBmJ5KUyhFf9K5fc9RQ16jYh+8OJrNe?=
 =?us-ascii?Q?gMx01lZdrtpz4LhY2tgKlIrWTgIXOzYA9lQ0DmDfKM3f6qxxq2bOLx9mOB6f?=
 =?us-ascii?Q?WoerfXKVyGpfn6Jsu3q+tPU5ZgXMIWEVoqZ2yjtbRYFWNqBanzz2JeRsWpuZ?=
 =?us-ascii?Q?kcZvXn25R/kWwKbTiI88XlWtH0q/Ph9OnEzloA+79i8wgsXrUT+DO5EYOAxG?=
 =?us-ascii?Q?E06foSMzNTYN0PmLe2I8J1LJosr595h7Td7m3kXywQf/s2nABFLCBBFPZe8h?=
 =?us-ascii?Q?e6QuyGydRD2Z2mCB+xWBTo6rbXCm8nHi7KGR3Y88uBAhhwHYGqfjr++q6mGO?=
 =?us-ascii?Q?DtMtA2yw7X/lnANQoST/sSFT7f7BECMkxc600oMFRCr8ZB3PAR8nc9DORhyG?=
 =?us-ascii?Q?2LffPo8QTS2JyOrLBjRLuQjfnHytYOYcCUcoJr2eCaUOZglER3cT1ilUY1xc?=
 =?us-ascii?Q?cnmrRjUHFCRUK5+cLT893lBOMHNbIyOYpjA79DyplBMsmGA0r4Hx5rjSlSM9?=
 =?us-ascii?Q?e7qnfc8cf8F+PiirGDPkOYyjDZiX/qe4YUdIkXBfnkB8k7EXILdkBu0aWwPL?=
 =?us-ascii?Q?E4SGtq7LQDQUotUNTpk6EI//4SbZjqahZotr+neMtRe37YVLeNOxYHY0NpHe?=
 =?us-ascii?Q?sR77jJr0EEcQZJ7/HyzlfDaJM61C9fNtkDNU/veiNtpB7pUri+5r64gtzb/M?=
 =?us-ascii?Q?v8lhy8V/1sd5GR7sBZQHZ02ZnN6ZXRyHOu6L+n52r5D9z+WWBxx5p7a2Vaxd?=
 =?us-ascii?Q?bu8M4THpnvFNahn/eUwIVcACsyCqIW0Z0k8vvjWpGxGRkgquW2PfH0N6062x?=
 =?us-ascii?Q?sQZDxzWgCahUSVKJosyrysNkzVZvU1bOhPr9vXUuOzhFUBcFb6WiMuenyeKu?=
 =?us-ascii?Q?2146SLPKx3BcN06Ejx15yl/NkjHxMazRMUrQIU+Gri1W3Hsk0A3gIwSxLPAW?=
 =?us-ascii?Q?URwFKVpgW7/If0leIenwpLbd4/emG+KelT95y0aQ9ilp5i4hpDrxUhtVd8/t?=
 =?us-ascii?Q?U8VPXXpah0NDWIIxRDdkcmgV?=
X-OriginatorOrg: plvision.eu
X-MS-Exchange-CrossTenant-Network-Message-Id: 61593e97-4533-41bc-3719-08d98289db47
X-MS-Exchange-CrossTenant-AuthSource: AS8P190MB1063.EURP190.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Sep 2021 14:11:25.2415
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 03707b74-30f3-46b6-a0e0-ff0a7438c9c4
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bKeYVq4fcGudCKQ6eSy9sIrHQ9+GueM5U7mB3fewAwy/bsmEURi+XHoAyEU/0e4lx2kUSQk4dnFCqJYG7Ziw+cz3kOMUEOq7YkMAUj3cmw4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8P190MB1079
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.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
>>