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

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

[Catalin Popescu]

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.

Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
---
 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)
 {
 	struct device_node *layout_np;
 	int err = 0;
@@ -755,8 +757,7 @@ static int nvmem_add_cells_from_fixed_layout(struct nvmem_device *nvmem)
 	if (!layout_np)
 		return 0;
 
-	if (of_device_is_compatible(layout_np, "fixed-layout"))
-		err = nvmem_add_cells_from_dt(nvmem, layout_np);
+	err = nvmem_add_cells_from_dt(nvmem, layout_np);
 
 	of_node_put(layout_np);
 
@@ -1009,10 +1010,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 			goto err_remove_cells;
 	}
 
-	rval = nvmem_add_cells_from_fixed_layout(nvmem);
-	if (rval)
-		goto err_remove_cells;
-
 	rval = nvmem_add_cells_from_layout(nvmem);
 	if (rval)
 		goto err_remove_cells;
@@ -2132,6 +2129,19 @@ const char *nvmem_dev_name(struct nvmem_device *nvmem)
 }
 EXPORT_SYMBOL_GPL(nvmem_dev_name);
 
+static const struct of_device_id fixed_layout_of_match_table[] = {
+	{ .compatible = "fixed-layout", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, fixed_layout_of_match_table);
+
+static struct nvmem_layout fixed_layout = {
+	.name = "NVMEM fixed layout",
+	.of_match_table = fixed_layout_of_match_table,
+	.add_cells = nvmem_add_cells_from_fixed_layout,
+};
+module_nvmem_layout_driver(fixed_layout);
+
 static int __init nvmem_init(void)
 {
 	return bus_register(&nvmem_bus_type);
-- 
2.34.1

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

[Miquel Raynal]

Hi Catalin,

catalin.popescu@leica-geosystems.com 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@leica-geosystems.com>
> ---

...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

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

[POPESCU Catalin]

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@bootlin.com. 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@leica-geosystems.com 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@leica-geosystems.com>
>> ---
> ...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

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

[Miquel Raynal]

Hi Catalin,

catalin.popescu@leica-geosystems.com wrote on Mon, 29 Jan 2024 15:29:31
+0000:

> 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.

That would be the relevant approach, yes.

> But, this also means that 
> "fixed-layout" won't be supported anymore out-of-the-box (by nvmem core) 

That would not be acceptable indeed.

> and will require additional kernel configuration change.

This would presumably be manageable however.

No pressure if you don't feel like you could carry that task, it's not
longer trivial.

Thanks,
Miquèl