[PATCH v6.8 1/2] nvmem: layouts: refactor .add_cells() callback arguments

[PATCH v6.8 1/2] nvmem: layouts: refactor .add_cells() callback arguments

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Simply pass whole "struct nvmem_layout" instead of single variables.
There is nothing in "struct nvmem_layout" that we have to hide from
layout drivers. They also access it during .probe() and .remove().

Thanks to this change:

1. API gets more consistent
   All layouts drivers callbacks get the same argument

2. Layouts get correct device
   Before this change NVMEM core code was passing NVMEM device instead
   of layout device. That resulted in:
   * Confusing prints
   * Calling devm_*() helpers on wrong device
   * Helpers like of_device_get_match_data() dereferencing NULLs

3. It gets possible to get match data
   First of all nvmem_layout_get_match_data() requires passing "struct
   nvmem_layout" which .add_cells() callback didn't have before this. It
   doesn't matter much as it's rather useless now anyway (and will be
   dropped).
   What's more important however is that of_device_get_match_data() can
   be used now thanks to owning a proper device pointer.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
Those changes complete layouts refactoring process so I'd very much like
them to go with the rest of refactoring stuff to v6.8.

It's difficult to add Fixes tag due to the nature of refactoring.
Callback .add_cells() existed even before refactoring so I'm not sure
what commit would be fair to blame.

Srini, Greg: could you see if this could still make it into v6.8?

 drivers/nvmem/core.c             | 2 +-
 drivers/nvmem/layouts/onie-tlv.c | 4 +++-
 drivers/nvmem/layouts/sl28vpd.c  | 4 +++-
 include/linux/nvmem-provider.h   | 2 +-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ba559e81f77f..441d132ebb61 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -854,7 +854,7 @@ int nvmem_layout_register(struct nvmem_layout *layout)
 		return -EINVAL;
 
 	/* Populate the cells */
-	ret = layout->add_cells(&layout->nvmem->dev, layout->nvmem);
+	ret = layout->add_cells(layout);
 	if (ret)
 		return ret;
 
diff --git a/drivers/nvmem/layouts/onie-tlv.c b/drivers/nvmem/layouts/onie-tlv.c
index b24cc5dcc6ee..9d2ad5f2dc10 100644
--- a/drivers/nvmem/layouts/onie-tlv.c
+++ b/drivers/nvmem/layouts/onie-tlv.c
@@ -182,8 +182,10 @@ static bool onie_tlv_crc_is_valid(struct device *dev, size_t table_len, u8 *tabl
 	return true;
 }
 
-static int onie_tlv_parse_table(struct device *dev, struct nvmem_device *nvmem)
+static int onie_tlv_parse_table(struct nvmem_layout *layout)
 {
+	struct nvmem_device *nvmem = layout->nvmem;
+	struct device *dev = &layout->dev;
 	struct onie_tlv_hdr hdr;
 	size_t table_len, data_len, hdr_len;
 	u8 *table, *data;
diff --git a/drivers/nvmem/layouts/sl28vpd.c b/drivers/nvmem/layouts/sl28vpd.c
index b8ffae646cc2..53fa50f17dca 100644
--- a/drivers/nvmem/layouts/sl28vpd.c
+++ b/drivers/nvmem/layouts/sl28vpd.c
@@ -80,8 +80,10 @@ static int sl28vpd_v1_check_crc(struct device *dev, struct nvmem_device *nvmem)
 	return 0;
 }
 
-static int sl28vpd_add_cells(struct device *dev, struct nvmem_device *nvmem)
+static int sl28vpd_add_cells(struct nvmem_layout *layout)
 {
+	struct nvmem_device *nvmem = layout->nvmem;
+	struct device *dev = &layout->dev;
 	const struct nvmem_cell_info *pinfo;
 	struct nvmem_cell_info info = {0};
 	struct device_node *layout_np;
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 6fe65b35ea97..81a67642ac55 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -173,7 +173,7 @@ struct nvmem_cell_table {
 struct nvmem_layout {
 	struct device dev;
 	struct nvmem_device *nvmem;
-	int (*add_cells)(struct device *dev, struct nvmem_device *nvmem);
+	int (*add_cells)(struct nvmem_layout *layout);
 };
 
 struct nvmem_layout_driver {
-- 
2.35.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6.8 2/2] nvmem: drop nvmem_layout_get_match_data()

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Thanks for layouts refactoring we now have "struct device" associated
with layout. Also its OF pointer points directly to the "nvmem-layout"
DT node.

All it takes to get match data is a generic of_device_get_match_data().

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/nvmem/core.c           | 13 -------------
 include/linux/nvmem-provider.h | 10 ----------
 2 files changed, 23 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 441d132ebb61..4ed54076346d 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -876,19 +876,6 @@ void nvmem_layout_unregister(struct nvmem_layout *layout)
 }
 EXPORT_SYMBOL_GPL(nvmem_layout_unregister);
 
-const void *nvmem_layout_get_match_data(struct nvmem_device *nvmem,
-					struct nvmem_layout *layout)
-{
-	struct device_node __maybe_unused *layout_np;
-	const struct of_device_id *match;
-
-	layout_np = of_nvmem_layout_get_container(nvmem);
-	match = of_match_node(layout->dev.driver->of_match_table, layout_np);
-
-	return match ? match->data : NULL;
-}
-EXPORT_SYMBOL_GPL(nvmem_layout_get_match_data);
-
 /**
  * nvmem_register() - Register a nvmem device for given nvmem_config.
  * Also creates a binary entry in /sys/bus/nvmem/devices/dev-name/nvmem
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 81a67642ac55..f0ba0e03218f 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -205,9 +205,6 @@ void nvmem_layout_driver_unregister(struct nvmem_layout_driver *drv);
 	module_driver(__nvmem_layout_driver, nvmem_layout_driver_register, \
 		      nvmem_layout_driver_unregister)
 
-const void *nvmem_layout_get_match_data(struct nvmem_device *nvmem,
-					struct nvmem_layout *layout);
-
 #else
 
 static inline struct nvmem_device *nvmem_register(const struct nvmem_config *c)
@@ -238,13 +235,6 @@ static inline int nvmem_layout_register(struct nvmem_layout *layout)
 
 static inline void nvmem_layout_unregister(struct nvmem_layout *layout) {}
 
-static inline const void *
-nvmem_layout_get_match_data(struct nvmem_device *nvmem,
-			    struct nvmem_layout *layout)
-{
-	return NULL;
-}
-
 #endif /* CONFIG_NVMEM */
 
 #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
-- 
2.35.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6.8 2/2] nvmem: drop nvmem_layout_get_match_data()

[Michael Walle]

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Thanks for layouts refactoring we now have "struct device" associated
> with layout. Also its OF pointer points directly to the "nvmem-layout"
> DT node.
> 
> All it takes to get match data is a generic of_device_get_match_data().

Isn't device_get_match_data() preferred?

> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Michael Walle <michael@walle.cc>

-michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6.8 1/2] nvmem: layouts: refactor .add_cells() callback arguments

[Michael Walle]

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Simply pass whole "struct nvmem_layout" instead of single variables.
> There is nothing in "struct nvmem_layout" that we have to hide from
> layout drivers. They also access it during .probe() and .remove().
> 
> Thanks to this change:
> 
> 1. API gets more consistent
>    All layouts drivers callbacks get the same argument
> 
> 2. Layouts get correct device
>    Before this change NVMEM core code was passing NVMEM device instead
>    of layout device. That resulted in:
>    * Confusing prints
>    * Calling devm_*() helpers on wrong device
>    * Helpers like of_device_get_match_data() dereferencing NULLs
> 
> 3. It gets possible to get match data
>    First of all nvmem_layout_get_match_data() requires passing "struct
>    nvmem_layout" which .add_cells() callback didn't have before this. 
> It
>    doesn't matter much as it's rather useless now anyway (and will be
>    dropped).
>    What's more important however is that of_device_get_match_data() can
>    be used now thanks to owning a proper device pointer.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

LGTM,
Reviewed-by: Michael Walle <michael@walle.cc>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6.8 2/2] nvmem: drop nvmem_layout_get_match_data()

[Rafał Miłecki]

On 19.12.2023 13:21, Michael Walle wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Thanks for layouts refactoring we now have "struct device" associated
>> with layout. Also its OF pointer points directly to the "nvmem-layout"
>> DT node.
>>
>> All it takes to get match data is a generic of_device_get_match_data().
> 
> Isn't device_get_match_data() preferred?

You're right. I think a big crusade against of_device_get_match_data()
is still ahead of us but I'll make sure to use device_get_match_data()
in U-Boot env layout to shine as an example :)


>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> Reviewed-by: Michael Walle <michael@walle.cc>

Thank you for quick reviews!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6.8 1/2] nvmem: layouts: refactor .add_cells() callback arguments

[Miquel Raynal]

Hi Rafał,

zajec5@gmail.com wrote on Tue, 19 Dec 2023 13:01:03 +0100:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Simply pass whole "struct nvmem_layout" instead of single variables.
> There is nothing in "struct nvmem_layout" that we have to hide from
> layout drivers. They also access it during .probe() and .remove().
> 
> Thanks to this change:
> 
> 1. API gets more consistent
>    All layouts drivers callbacks get the same argument
> 
> 2. Layouts get correct device
>    Before this change NVMEM core code was passing NVMEM device instead
>    of layout device. That resulted in:
>    * Confusing prints
>    * Calling devm_*() helpers on wrong device
>    * Helpers like of_device_get_match_data() dereferencing NULLs
> 
> 3. It gets possible to get match data
>    First of all nvmem_layout_get_match_data() requires passing "struct
>    nvmem_layout" which .add_cells() callback didn't have before this. It
>    doesn't matter much as it's rather useless now anyway (and will be
>    dropped).
>    What's more important however is that of_device_get_match_data() can
>    be used now thanks to owning a proper device pointer.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6.8 2/2] nvmem: drop nvmem_layout_get_match_data()

[Miquel Raynal]

Hi Rafał,

zajec5@gmail.com wrote on Tue, 19 Dec 2023 13:01:04 +0100:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Thanks for layouts refactoring we now have "struct device" associated
> with layout. Also its OF pointer points directly to the "nvmem-layout"
> DT node.
> 
> All it takes to get match data is a generic of_device_get_match_data().
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel