[PATCH 0/5] nvmem: core: one fix and several cleanups for sysfs code

[PATCH 0/5] nvmem: core: one fix and several cleanups for sysfs code

[Thomas Weißschuh]

Patch 1 is a bugfix.
All other patches are small cleanups.

Hint about another nvmem bugfix at [0].

[0] https://lore.kernel.org/lkml/20240619-nvmem-cell-sysfs-perm-v1-1-e5b7882fdfa8@weissschuh.net/

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (5):
      nvmem: core: only change name to fram for current attribute
      nvmem: core: mark bin_attr_nvmem_eeprom_compat as const
      nvmem: core: add single sysfs group
      nvmem: core: remove global nvmem_cells_group
      nvmem: core: drop unnecessary range checks in sysfs callbacks

 drivers/nvmem/core.c | 52 +++++++++++++---------------------------------------
 1 file changed, 13 insertions(+), 39 deletions(-)
---
base-commit: e5b3efbe1ab1793bb49ae07d56d0973267e65112
change-id: 20240620-nvmem-compat-name-79474e8a0569

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>

[PATCH 1/5] nvmem: core: only change name to fram for current attribute

[Thomas Weißschuh]

bin_attr_nvmem_eeprom_compat is the template from which all future
compat attributes are created.
Changing it means to change all subsquent compat attributes, too.

Instead only use the "fram" name for the currently registered attribute.

Fixes: fd307a4ad332 ("nvmem: prepare basics for FRAM support")
Cc: stable@vger.kernel.org
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/nvmem/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index e1ec3b7200d7..1285300ed239 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -396,10 +396,9 @@ static int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
 	if (!config->base_dev)
 		return -EINVAL;
 
-	if (config->type == NVMEM_TYPE_FRAM)
-		bin_attr_nvmem_eeprom_compat.attr.name = "fram";
-
 	nvmem->eeprom = bin_attr_nvmem_eeprom_compat;
+	if (config->type == NVMEM_TYPE_FRAM)
+		nvmem->eeprom.attr.name = "fram";
 	nvmem->eeprom.attr.mode = nvmem_bin_attr_get_umode(nvmem);
 	nvmem->eeprom.size = nvmem->size;
 #ifdef CONFIG_DEBUG_LOCK_ALLOC

-- 
2.45.2

[PATCH 2/5] nvmem: core: mark bin_attr_nvmem_eeprom_compat as const

[Thomas Weißschuh]

The attribute is only a template from which copies are created.
It should not be changed, so mark it const.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/nvmem/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 1285300ed239..d74297d10631 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -372,7 +372,7 @@ static const struct attribute_group *nvmem_cells_groups[] = {
 	NULL,
 };
 
-static struct bin_attribute bin_attr_nvmem_eeprom_compat = {
+static const struct bin_attribute bin_attr_nvmem_eeprom_compat = {
 	.attr	= {
 		.name	= "eeprom",
 	},

-- 
2.45.2

[PATCH 3/5] nvmem: core: add single sysfs group

[Thomas Weißschuh]

The sysfs core provides a function to easily register a single group.
Use it and remove the now unnecessary nvmem_cells_groups array.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/nvmem/core.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index d74297d10631..4c250f0e55ef 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -367,11 +367,6 @@ static const struct attribute_group *nvmem_dev_groups[] = {
 	NULL,
 };
 
-static const struct attribute_group *nvmem_cells_groups[] = {
-	&nvmem_cells_group,
-	NULL,
-};
-
 static const struct bin_attribute bin_attr_nvmem_eeprom_compat = {
 	.attr	= {
 		.name	= "eeprom",
@@ -477,7 +472,7 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
 
 	nvmem_cells_group.bin_attrs = cells_attrs;
 
-	ret = device_add_groups(&nvmem->dev, nvmem_cells_groups);
+	ret = device_add_group(&nvmem->dev, &nvmem_cells_group);
 	if (ret)
 		goto unlock_mutex;
 

-- 
2.45.2

[PATCH 4/5] nvmem: core: remove global nvmem_cells_group

[Thomas Weißschuh]

nvmem_cells_groups is a global variable that is also mutated.
This is complicated and error-prone.

Instead use a normal stack variable.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/nvmem/core.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 4c250f0e55ef..ed077d7e3998 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -357,11 +357,6 @@ static const struct attribute_group nvmem_bin_group = {
 	.is_bin_visible = nvmem_bin_attr_is_visible,
 };
 
-/* Cell attributes will be dynamically allocated */
-static struct attribute_group nvmem_cells_group = {
-	.name		= "cells",
-};
-
 static const struct attribute_group *nvmem_dev_groups[] = {
 	&nvmem_bin_group,
 	NULL,
@@ -423,23 +418,24 @@ static void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
 
 static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
 {
-	struct bin_attribute **cells_attrs, *attrs;
+	struct attribute_group group = {
+		.name	= "cells",
+	};
 	struct nvmem_cell_entry *entry;
+	struct bin_attribute *attrs;
 	unsigned int ncells = 0, i = 0;
 	int ret = 0;
 
 	mutex_lock(&nvmem_mutex);
 
-	if (list_empty(&nvmem->cells) || nvmem->sysfs_cells_populated) {
-		nvmem_cells_group.bin_attrs = NULL;
+	if (list_empty(&nvmem->cells) || nvmem->sysfs_cells_populated)
 		goto unlock_mutex;
-	}
 
 	/* Allocate an array of attributes with a sentinel */
 	ncells = list_count_nodes(&nvmem->cells);
-	cells_attrs = devm_kcalloc(&nvmem->dev, ncells + 1,
-				   sizeof(struct bin_attribute *), GFP_KERNEL);
-	if (!cells_attrs) {
+	group.bin_attrs = devm_kcalloc(&nvmem->dev, ncells + 1,
+				       sizeof(struct bin_attribute *), GFP_KERNEL);
+	if (!group.bin_attrs) {
 		ret = -ENOMEM;
 		goto unlock_mutex;
 	}
@@ -466,13 +462,11 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
 			goto unlock_mutex;
 		}
 
-		cells_attrs[i] = &attrs[i];
+		group.bin_attrs[i] = &attrs[i];
 		i++;
 	}
 
-	nvmem_cells_group.bin_attrs = cells_attrs;
-
-	ret = device_add_group(&nvmem->dev, &nvmem_cells_group);
+	ret = device_add_group(&nvmem->dev, &group);
 	if (ret)
 		goto unlock_mutex;
 

-- 
2.45.2

[PATCH 5/5] nvmem: core: drop unnecessary range checks in sysfs callbacks

[Thomas Weißschuh]

The same checks have already been done in sysfs_kf_bin_write() and
sysfs_kf_bin_read() just before the callbacks are invoked.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/nvmem/core.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ed077d7e3998..5c1ee7f0da0a 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -203,19 +203,12 @@ static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
 		dev = kobj_to_dev(kobj);
 	nvmem = to_nvmem_device(dev);
 
-	/* Stop the user from reading */
-	if (pos >= nvmem->size)
-		return 0;
-
 	if (!IS_ALIGNED(pos, nvmem->stride))
 		return -EINVAL;
 
 	if (count < nvmem->word_size)
 		return -EINVAL;
 
-	if (pos + count > nvmem->size)
-		count = nvmem->size - pos;
-
 	count = round_down(count, nvmem->word_size);
 
 	if (!nvmem->reg_read)
@@ -243,19 +236,12 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
 		dev = kobj_to_dev(kobj);
 	nvmem = to_nvmem_device(dev);
 
-	/* Stop the user from writing */
-	if (pos >= nvmem->size)
-		return -EFBIG;
-
 	if (!IS_ALIGNED(pos, nvmem->stride))
 		return -EINVAL;
 
 	if (count < nvmem->word_size)
 		return -EINVAL;
 
-	if (pos + count > nvmem->size)
-		count = nvmem->size - pos;
-
 	count = round_down(count, nvmem->word_size);
 
 	if (!nvmem->reg_write)

-- 
2.45.2

Re: (subset) [PATCH 0/5] nvmem: core: one fix and several cleanups for sysfs code

[Srinivas Kandagatla]

On Thu, 20 Jun 2024 18:00:32 +0200, Thomas Weißschuh wrote:
> Patch 1 is a bugfix.
> All other patches are small cleanups.
> 
> Hint about another nvmem bugfix at [0].
> 
> [0] https://lore.kernel.org/lkml/20240619-nvmem-cell-sysfs-perm-v1-1-e5b7882fdfa8@weissschuh.net/
> 
> [...]

Applied, thanks!

[1/5] nvmem: core: only change name to fram for current attribute
      commit: 92e57866c8eeefd00ee0c05232b5134e11a66298

Best regards,
-- 
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Re: (subset) [PATCH 0/5] nvmem: core: one fix and several cleanups for sysfs code

[Srinivas Kandagatla]

On Thu, 20 Jun 2024 18:00:32 +0200, Thomas Weißschuh wrote:
> Patch 1 is a bugfix.
> All other patches are small cleanups.
> 
> Hint about another nvmem bugfix at [0].
> 
> [0] https://lore.kernel.org/lkml/20240619-nvmem-cell-sysfs-perm-v1-1-e5b7882fdfa8@weissschuh.net/
> 
> [...]

Applied, thanks!

[2/5] nvmem: core: mark bin_attr_nvmem_eeprom_compat as const
      commit: 178a9aea2c5db8328757fdea66922bda0236e95c
[3/5] nvmem: core: add single sysfs group
      commit: 80026ea9fdc22bbc8bfa9b41f54baba314bacc55
[4/5] nvmem: core: remove global nvmem_cells_group
      commit: e76590d9faf8c058df9faf0b6513f055beb84b57
[5/5] nvmem: core: drop unnecessary range checks in sysfs callbacks
      commit: 050e51c214c5bbe5ffd9e7f5927ccdcd2da18fe3

Best regards,
-- 
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Re: (subset) [PATCH 0/5] nvmem: core: one fix and several cleanups for sysfs code

[Thomas Weißschuh]

On 2024-06-22 11:49:25+0000, Srinivas Kandagatla wrote:
> On Thu, 20 Jun 2024 18:00:32 +0200, Thomas Weißschuh wrote:
> > Patch 1 is a bugfix.
> > All other patches are small cleanups.
> > 
> > Hint about another nvmem bugfix at [0].
> > 
> > [0] https://lore.kernel.org/lkml/20240619-nvmem-cell-sysfs-perm-v1-1-e5b7882fdfa8@weissschuh.net/
> > 
> > [...]
> 
> Applied, thanks!

Thanks!

> [2/5] nvmem: core: mark bin_attr_nvmem_eeprom_compat as const
>       commit: 178a9aea2c5db8328757fdea66922bda0236e95c

Please note that patch 2 has a dependency on patch 1.
In the current state this will probably lead to build errors in
linux-next, as nvmem-fixes is not part of linux-next.

I should have mentioned that.

In theory patch 2 could even be squashed into patch 1, as it really is
mostly an extension of it.

> [3/5] nvmem: core: add single sysfs group
>       commit: 80026ea9fdc22bbc8bfa9b41f54baba314bacc55
> [4/5] nvmem: core: remove global nvmem_cells_group
>       commit: e76590d9faf8c058df9faf0b6513f055beb84b57
> [5/5] nvmem: core: drop unnecessary range checks in sysfs callbacks
>       commit: 050e51c214c5bbe5ffd9e7f5927ccdcd2da18fe3