Re: [PATCH] nvmem: core: add fixed-layout declaration

From: POPESCU Catalin
Date: Mon Jan 29 2024 - 10:30:04 EST


Hi Miquel,

Now, that "specific" layouts are considered like drivers and rely on
NVMEM_LAYOUTS to populate the nvmem cells (layouts.c), I guess it's not
possible anymore to consider "fixed-layout" as a normal layout that
should be treated the same way than any layout. Unless, we move
"fixed-layout" under drivers/nvmem/layouts. But, this also means that
"fixed-layout" won't be supported anymore out-of-the-box (by nvmem core)
and will require additional kernel configuration change. And I don't
know if this is actually acceptable ...

BR,
Catalin

On 11.01.24 16:41, Miquel Raynal wrote:
> [Some people who received this message don't often get email from miquel.raynal@xxxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>
>
> Hi Catalin,
>
> catalin.popescu@xxxxxxxxxxxxxxxxxxxx wrote on Thu, 11 Jan 2024 16:28:49
> +0100:
>
>> Declare fixed-layout as a supported layout and use add_cells
>> callback to parse the cells. This adds consistency over layouts
>> parsing as fixed-layout parsing is now handled in the same way
>> than others nvmem layouts.
> Thanks for submitting a new version, next time you'll need to increase the version number in the title (use git-send-email -v <version>) and also please append a changelog below the three dashes...
>
>> Signed-off-by: Catalin Popescu <catalin.popescu@xxxxxxxxxxxxxxxxxxxx>
>> ---
> ...here.
>
> Also, I think you should rebase on top of -rc1 when it will be out (in
> less than two weeks) because I believe the code snipped below will no
> longer apply...
>
>> drivers/nvmem/core.c | 24 +++++++++++++++++-------
>> 1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index 608b352a7d91..d7f34cbea34b 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -746,7 +746,9 @@ static int nvmem_add_cells_from_legacy_of(struct nvmem_device *nvmem)
>> return nvmem_add_cells_from_dt(nvmem, nvmem->dev.of_node);
>> }
>>
>> -static int nvmem_add_cells_from_fixed_layout(struct nvmem_device *nvmem)
>> +static int nvmem_add_cells_from_fixed_layout(struct device *dev,
>> + struct nvmem_device *nvmem,
>> + struct nvmem_layout *layout)
> ... this one.
>
> Otherwise LGTM.
>
> Thanks,
> Miquèl