Hi Greg,
Here are some nvmem core fixes around nvmem provider device register
and error paths.
Most of these patches have been in next for 2-3 weeks.
Am really not sure if you are taking fixes late in this cycle.
In case you are not could you please apply them for 6.3
Thanks,
Srini
Jiasheng Jiang (1):
nvmem: brcm_nvram: Add check for kzalloc
Johan Hovold (1):
nvmem: qcom-spmi-sdam: fix module autoloading
Michael Walle (2):
nvmem: core: fix device node refcounting
nvmem: core: fix cell removal on error
Russell King (Oracle) (5):
nvmem: core: initialise nvmem->id early
nvmem: core: remove nvmem_config wp_gpio
nvmem: core: fix cleanup after dev_set_name()
nvmem: core: fix registration vs use race
nvmem: core: fix return value
Samuel Holland (1):
nvmem: sunxi_sid: Always use 32-bit MMIO reads
drivers/nvmem/brcm_nvram.c | 3 ++
drivers/nvmem/core.c | 60 +++++++++++++++++-----------------
drivers/nvmem/qcom-spmi-sdam.c | 1 +
drivers/nvmem/sunxi_sid.c | 15 ++++++++-
include/linux/nvmem-provider.h | 2 --
5 files changed, 48 insertions(+), 33 deletions(-)
--
2.25.1
From: Jiasheng Jiang <[email protected]>
Add the check for the return value of kzalloc in order to avoid
NULL pointer dereference.
Cc: [email protected]
Fixes: 6e977eaa8280 ("nvmem: brcm_nvram: parse NVRAM content into NVMEM cells")
Signed-off-by: Jiasheng Jiang <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/brcm_nvram.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/nvmem/brcm_nvram.c b/drivers/nvmem/brcm_nvram.c
index 34130449f2d2..39aa27942f28 100644
--- a/drivers/nvmem/brcm_nvram.c
+++ b/drivers/nvmem/brcm_nvram.c
@@ -98,6 +98,9 @@ static int brcm_nvram_parse(struct brcm_nvram *priv)
len = le32_to_cpu(header.len);
data = kzalloc(len, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
memcpy_fromio(data, priv->base, len);
data[len - 1] = '\0';
--
2.25.1
From: Samuel Holland <[email protected]>
The SID SRAM on at least some SoCs (A64 and D1) returns different values
when read with bus cycles narrower than 32 bits. This is not immediately
obvious, because memcpy_fromio() uses word-size accesses as long as
enough data is being copied.
The vendor driver always uses 32-bit MMIO reads, so do the same here.
This is faster than the register-based method, which is currently used
as a workaround on A64. And it fixes the values returned on D1, where
the SRAM method was being used.
The special case for the last word is needed to maintain .word_size == 1
for sysfs ABI compatibility, as noted previously in commit de2a3eaea552
("nvmem: sunxi_sid: Optimize register read-out method").
Cc: [email protected]
Fixes: 07ae4fde9efa ("nvmem: sunxi_sid: Add support for D1 variant")
Tested-by: Heiko Stuebner <[email protected]>
Signed-off-by: Samuel Holland <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/sunxi_sid.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
index 5750e1f4bcdb..92dfe4cb10e3 100644
--- a/drivers/nvmem/sunxi_sid.c
+++ b/drivers/nvmem/sunxi_sid.c
@@ -41,8 +41,21 @@ static int sunxi_sid_read(void *context, unsigned int offset,
void *val, size_t bytes)
{
struct sunxi_sid *sid = context;
+ u32 word;
+
+ /* .stride = 4 so offset is guaranteed to be aligned */
+ __ioread32_copy(val, sid->base + sid->value_offset + offset, bytes / 4);
- memcpy_fromio(val, sid->base + sid->value_offset + offset, bytes);
+ val += round_down(bytes, 4);
+ offset += round_down(bytes, 4);
+ bytes = bytes % 4;
+
+ if (!bytes)
+ return 0;
+
+ /* Handle any trailing bytes */
+ word = readl_relaxed(sid->base + sid->value_offset + offset);
+ memcpy(val, &word, bytes);
return 0;
}
--
2.25.1
From: "Russell King (Oracle)" <[email protected]>
The error path for wp_gpio attempts to free the IDA nvmem->id, but
this has yet to be assigned, so will always be zero - leaking the
ID allocated by ida_alloc(). Fix this by moving the initialisation
of nvmem->id earlier.
Cc: [email protected]
Fixes: f7d8d7dcd978 ("nvmem: fix memory leak in error path")
Signed-off-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 321d7d63e068..7394a7598efa 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -770,6 +770,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
return ERR_PTR(rval);
}
+ nvmem->id = rval;
+
if (config->wp_gpio)
nvmem->wp_gpio = config->wp_gpio;
else if (!config->ignore_wp)
@@ -785,7 +787,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
kref_init(&nvmem->refcnt);
INIT_LIST_HEAD(&nvmem->cells);
- nvmem->id = rval;
nvmem->owner = config->owner;
if (!nvmem->owner && config->dev->driver)
nvmem->owner = config->dev->driver->owner;
--
2.25.1
From: "Russell King (Oracle)" <[email protected]>
No one provides wp_gpio, so let's remove it to avoid issues with
the nvmem core putting this gpio.
Cc: [email protected]
Signed-off-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 4 +---
include/linux/nvmem-provider.h | 2 --
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 7394a7598efa..608f3ad2e2e4 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -772,9 +772,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
nvmem->id = rval;
- if (config->wp_gpio)
- nvmem->wp_gpio = config->wp_gpio;
- else if (!config->ignore_wp)
+ if (!config->ignore_wp)
nvmem->wp_gpio = gpiod_get_optional(config->dev, "wp",
GPIOD_OUT_HIGH);
if (IS_ERR(nvmem->wp_gpio)) {
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 50caa117cb62..bb15c9234e21 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -70,7 +70,6 @@ struct nvmem_keepout {
* @word_size: Minimum read/write access granularity.
* @stride: Minimum read/write access stride.
* @priv: User context passed to read/write callbacks.
- * @wp-gpio: Write protect pin
* @ignore_wp: Write Protect pin is managed by the provider.
*
* Note: A default "nvmem<id>" name will be assigned to the device if
@@ -85,7 +84,6 @@ struct nvmem_config {
const char *name;
int id;
struct module *owner;
- struct gpio_desc *wp_gpio;
const struct nvmem_cell_info *cells;
int ncells;
const struct nvmem_keepout *keepout;
--
2.25.1
From: "Russell King (Oracle)" <[email protected]>
If dev_set_name() fails, we leak nvmem->wp_gpio as the cleanup does not
put this. While a minimal fix for this would be to add the gpiod_put()
call, we can do better if we split device_register(), and use the
tested nvmem_release() cleanup code by initialising the device early,
and putting the device.
This results in a slightly larger fix, but results in clear code.
Note: this patch depends on "nvmem: core: initialise nvmem->id early"
and "nvmem: core: remove nvmem_config wp_gpio".
Cc: [email protected]
Fixes: 5544e90c8126 ("nvmem: core: add error handling for dev_set_name")
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
[Srini: Fixed subject line and error code handing with wp_gpio while applying.]
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 608f3ad2e2e4..ac77a019aed7 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -772,14 +772,18 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
nvmem->id = rval;
+ nvmem->dev.type = &nvmem_provider_type;
+ nvmem->dev.bus = &nvmem_bus_type;
+ nvmem->dev.parent = config->dev;
+
+ device_initialize(&nvmem->dev);
+
if (!config->ignore_wp)
nvmem->wp_gpio = gpiod_get_optional(config->dev, "wp",
GPIOD_OUT_HIGH);
if (IS_ERR(nvmem->wp_gpio)) {
- ida_free(&nvmem_ida, nvmem->id);
rval = PTR_ERR(nvmem->wp_gpio);
- kfree(nvmem);
- return ERR_PTR(rval);
+ goto err_put_device;
}
kref_init(&nvmem->refcnt);
@@ -791,9 +795,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
nvmem->stride = config->stride ?: 1;
nvmem->word_size = config->word_size ?: 1;
nvmem->size = config->size;
- nvmem->dev.type = &nvmem_provider_type;
- nvmem->dev.bus = &nvmem_bus_type;
- nvmem->dev.parent = config->dev;
nvmem->root_only = config->root_only;
nvmem->priv = config->priv;
nvmem->type = config->type;
@@ -821,11 +822,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
break;
}
- if (rval) {
- ida_free(&nvmem_ida, nvmem->id);
- kfree(nvmem);
- return ERR_PTR(rval);
- }
+ if (rval)
+ goto err_put_device;
nvmem->read_only = device_property_present(config->dev, "read-only") ||
config->read_only || !nvmem->reg_write;
@@ -836,7 +834,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
- rval = device_register(&nvmem->dev);
+ rval = device_add(&nvmem->dev);
if (rval)
goto err_put_device;
--
2.25.1
From: "Russell King (Oracle)" <[email protected]>
The i.MX6 CPU frequency driver sometimes fails to register at boot time
due to nvmem_cell_read_u32() sporadically returning -ENOENT.
This happens because there is a window where __nvmem_device_get() in
of_nvmem_cell_get() is able to return the nvmem device, but as cells
have been setup, nvmem_find_cell_entry_by_node() returns NULL.
The occurs because the nvmem core registration code violates one of the
fundamental principles of kernel programming: do not publish data
structures before their setup is complete.
Fix this by making nvmem core code conform with this principle.
Cc: [email protected]
Fixes: eace75cfdcf7 ("nvmem: Add a simple NVMEM framework for nvmem providers")
Signed-off-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ac77a019aed7..e92c6f1aadbb 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -832,22 +832,16 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
nvmem->dev.groups = nvmem_dev_groups;
#endif
- dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
-
- rval = device_add(&nvmem->dev);
- if (rval)
- goto err_put_device;
-
if (nvmem->nkeepout) {
rval = nvmem_validate_keepouts(nvmem);
if (rval)
- goto err_device_del;
+ goto err_put_device;
}
if (config->compat) {
rval = nvmem_sysfs_setup_compat(nvmem, config);
if (rval)
- goto err_device_del;
+ goto err_put_device;
}
if (config->cells) {
@@ -864,6 +858,12 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
if (rval)
goto err_remove_cells;
+ dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
+
+ rval = device_add(&nvmem->dev);
+ if (rval)
+ goto err_remove_cells;
+
blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem);
return nvmem;
@@ -873,8 +873,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
err_teardown_compat:
if (config->compat)
nvmem_sysfs_remove_compat(nvmem, config);
-err_device_del:
- device_del(&nvmem->dev);
err_put_device:
put_device(&nvmem->dev);
--
2.25.1
From: Michael Walle <[email protected]>
In of_nvmem_cell_get(), of_get_next_parent() is used on cell_np. This
will decrement the refcount on cell_np, but cell_np is still used later
in the code. Use of_get_parent() instead and of_node_put() in the
appropriate places.
Cc: [email protected]
Fixes: 69aba7948cbe("nvmem: Add a simple NVMEM framework for consumers")
Fixes: 7ae6478b304b ("nvmem: core: rework nvmem cell instance creation")
Signed-off-by: Michael Walle <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index e92c6f1aadbb..cbe5df99db82 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1237,16 +1237,21 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, const char *id)
if (!cell_np)
return ERR_PTR(-ENOENT);
- nvmem_np = of_get_next_parent(cell_np);
- if (!nvmem_np)
+ nvmem_np = of_get_parent(cell_np);
+ if (!nvmem_np) {
+ of_node_put(cell_np);
return ERR_PTR(-EINVAL);
+ }
nvmem = __nvmem_device_get(nvmem_np, device_match_of_node);
of_node_put(nvmem_np);
- if (IS_ERR(nvmem))
+ if (IS_ERR(nvmem)) {
+ of_node_put(cell_np);
return ERR_CAST(nvmem);
+ }
cell_entry = nvmem_find_cell_entry_by_node(nvmem, cell_np);
+ of_node_put(cell_np);
if (!cell_entry) {
__nvmem_device_put(nvmem);
return ERR_PTR(-ENOENT);
--
2.25.1
From: Michael Walle <[email protected]>
nvmem_add_cells() could return an error after some cells are already
added to the provider. In this case, the added cells are not removed.
Remove any registered cells if nvmem_add_cells() fails.
Cc: [email protected]
Fixes: fa72d847d68d7 ("nvmem: check the return value of nvmem_add_cells()")
Signed-off-by: Michael Walle <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index cbe5df99db82..563116405b3d 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -847,7 +847,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
if (config->cells) {
rval = nvmem_add_cells(nvmem, config->cells, config->ncells);
if (rval)
- goto err_teardown_compat;
+ goto err_remove_cells;
}
rval = nvmem_add_cells_from_table(nvmem);
@@ -870,7 +870,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
err_remove_cells:
nvmem_device_remove_all_cells(nvmem);
-err_teardown_compat:
if (config->compat)
nvmem_sysfs_remove_compat(nvmem, config);
err_put_device:
--
2.25.1
From: Johan Hovold <[email protected]>
Add the missing module device table so that the driver can be autoloaded
when built as a module.
Fixes: 40ce9798794f ("nvmem: add QTI SDAM driver")
Cc: [email protected] # 5.6
Signed-off-by: Johan Hovold <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/qcom-spmi-sdam.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
index 4fcb63507ecd..8499892044b7 100644
--- a/drivers/nvmem/qcom-spmi-sdam.c
+++ b/drivers/nvmem/qcom-spmi-sdam.c
@@ -166,6 +166,7 @@ static const struct of_device_id sdam_match_table[] = {
{ .compatible = "qcom,spmi-sdam" },
{},
};
+MODULE_DEVICE_TABLE(of, sdam_match_table);
static struct platform_driver sdam_driver = {
.driver = {
--
2.25.1
From: "Russell King (Oracle)" <[email protected]>
Dan Carpenter points out that the return code was not set in commit
60c8b4aebd8e ("nvmem: core: fix cleanup after dev_set_name()"), but
this is not the only issue - we also need to zero wp_gpio to prevent
gpiod_put() being called on an error value.
Cc: [email protected]
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Fixes: 60c8b4aebd8e ("nvmem: core: fix cleanup after dev_set_name()")
Signed-off-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 563116405b3d..34ee9d36ee7b 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -783,6 +783,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
GPIOD_OUT_HIGH);
if (IS_ERR(nvmem->wp_gpio)) {
rval = PTR_ERR(nvmem->wp_gpio);
+ nvmem->wp_gpio = NULL;
goto err_put_device;
}
--
2.25.1
On Fri, Jan 27, 2023 at 10:40:05AM +0000, Srinivas Kandagatla wrote:
> Hi Greg,
>
> Here are some nvmem core fixes around nvmem provider device register
> and error paths.
> Most of these patches have been in next for 2-3 weeks.
>
> Am really not sure if you are taking fixes late in this cycle.
> In case you are not could you please apply them for 6.3
When I apply them, I get the following errors from the scripts:
Commit: 36f5dbea16ad ("nvmem: core: fix return value")
Fixes tag: Fixes: 60c8b4aebd8e ("nvmem: core: fix cleanup after dev_set_name()")
Has these problem(s):
- Target SHA1 does not exist
Commit: 7de8892c0527 ("nvmem: core: fix device node refcounting")
Fixes tag: Fixes: 69aba7948cbe("nvmem: Add a simple NVMEM framework for consumers")
Has these problem(s):
- missing space between the SHA1 and the subject
The first one is because you have your own git tree, that's fine. But the
second one should have given you an error when it was in linux-next, what
happened?
Let me see if I can fix this up...
greg k-h
Thanks Greg,
On 28/01/2023 13:39, Greg KH wrote:
> On Fri, Jan 27, 2023 at 10:40:05AM +0000, Srinivas Kandagatla wrote:
>> Hi Greg,
>>
>> Here are some nvmem core fixes around nvmem provider device register
>> and error paths.
>> Most of these patches have been in next for 2-3 weeks.
>>
>> Am really not sure if you are taking fixes late in this cycle.
>> In case you are not could you please apply them for 6.3
>
> When I apply them, I get the following errors from the scripts:
>
> Commit: 36f5dbea16ad ("nvmem: core: fix return value")
> Fixes tag: Fixes: 60c8b4aebd8e ("nvmem: core: fix cleanup after dev_set_name()")
> Has these problem(s):
> - Target SHA1 does not exist
> Commit: 7de8892c0527 ("nvmem: core: fix device node refcounting")
> Fixes tag: Fixes: 69aba7948cbe("nvmem: Add a simple NVMEM framework for consumers")
> Has these problem(s):
> - missing space between the SHA1 and the subject
>
> The first one is because you have your own git tree, that's fine. But the
> second one should have given you an error when it was in linux-next, what
> happened?
>
> Let me see if I can fix this up...
Thankyou very much for fixing this up before applying, will take extra
care next time. :-)
thanks,
srini
>
> greg k-h