Hi,
This is a continuation of my old patchset from 2019. [1]
Back then, few drivers set driver_override wrong. I fixed Exynos
in a different way after discussions. QCOM NGD was not fixed
and a new user appeared - IMX SCU.
It seems "char *" in driver_override looks too consty, so we
tend to make a mistake of storing there string literals.
Changes since latest v3
=======================
1. Wrap comments, extend comment in driver_set_override() about newline.
2. Minor commit msg fixes.
3. Add tags.
Changes since latest v2
=======================
1. Make all driver_override fields as "const char *", just like SPI
and VDPA. (Mark)
2. Move "count" check to the new helper and add "count" argument. (Michael)
3. Fix typos in docs, patch subject. Extend doc. (Michael, Bjorn)
4. Compare pointers to reduce number of string readings in the helper.
5. Fix clk-imx return value.
Changes since latest v1 (not the old 2019 solution):
====================================================
https://lore.kernel.org/all/[email protected]/
1. Add helper for setting driver_override.
2. Use the helper.
Dependencies, merging and stable
================================
1. All patches, including last three fixes, depend on the first patch
introducing the helper.
2. Merging idea: everything via driver core (Greg KH?).
2. The last three commits - fixes - are probably not backportable
directly, because of this dependency. I don't know how to express
this dependency here, since stable-kernel-rules.rst mentions only commits as
possible dependencies.
[1] https://lore.kernel.org/all/[email protected]/
Best regards,
Krzysztof
Krzysztof Kozlowski (11):
driver: platform: Add helper for safer setting of driver_override
amba: Use driver_set_override() instead of open-coding
fsl-mc: Use driver_set_override() instead of open-coding
hv: Use driver_set_override() instead of open-coding
PCI: Use driver_set_override() instead of open-coding
s390/cio: Use driver_set_override() instead of open-coding
spi: Use helper for safer setting of driver_override
vdpa: Use helper for safer setting of driver_override
clk: imx: scu: Fix kfree() of static memory on setting driver_override
slimbus: qcom-ngd: Fix kfree() of static memory on setting
driver_override
rpmsg: Fix kfree() of static memory on setting driver_override
drivers/amba/bus.c | 28 +++--------------
drivers/base/driver.c | 56 +++++++++++++++++++++++++++++++++
drivers/base/platform.c | 28 +++--------------
drivers/bus/fsl-mc/fsl-mc-bus.c | 25 +++------------
drivers/clk/imx/clk-scu.c | 7 ++++-
drivers/hv/vmbus_drv.c | 28 +++--------------
drivers/pci/pci-sysfs.c | 28 +++--------------
drivers/rpmsg/rpmsg_core.c | 3 +-
drivers/rpmsg/rpmsg_internal.h | 13 ++++++--
drivers/rpmsg/rpmsg_ns.c | 14 +++++++--
drivers/s390/cio/cio.h | 6 +++-
drivers/s390/cio/css.c | 28 +++--------------
drivers/slimbus/qcom-ngd-ctrl.c | 13 +++++++-
drivers/spi/spi.c | 26 +++------------
drivers/vdpa/vdpa.c | 29 +++--------------
include/linux/amba/bus.h | 6 +++-
include/linux/device/driver.h | 2 ++
include/linux/fsl/mc.h | 6 ++--
include/linux/hyperv.h | 6 +++-
include/linux/pci.h | 6 +++-
include/linux/platform_device.h | 6 +++-
include/linux/rpmsg.h | 6 ++--
include/linux/spi/spi.h | 2 ++
include/linux/vdpa.h | 4 ++-
24 files changed, 171 insertions(+), 205 deletions(-)
--
2.32.0
Use a helper to set driver_override to reduce amount of duplicated code.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Mark Brown <[email protected]>
---
drivers/spi/spi.c | 26 ++++----------------------
include/linux/spi/spi.h | 2 ++
2 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 6937cf2d59e0..34f311224c47 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -71,29 +71,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct spi_device *spi = to_spi_device(dev);
- const char *end = memchr(buf, '\n', count);
- const size_t len = end ? end - buf : count;
- const char *driver_override, *old;
-
- /* We need to keep extra room for a newline when displaying value */
- if (len >= (PAGE_SIZE - 1))
- return -EINVAL;
-
- driver_override = kstrndup(buf, len, GFP_KERNEL);
- if (!driver_override)
- return -ENOMEM;
+ int ret;
- device_lock(dev);
- old = spi->driver_override;
- if (len) {
- spi->driver_override = driver_override;
- } else {
- /* Empty string, disable driver override */
- spi->driver_override = NULL;
- kfree(driver_override);
- }
- device_unlock(dev);
- kfree(old);
+ ret = driver_set_override(dev, &spi->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 5f8c063ddff4..f0177f9b6e13 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -138,6 +138,8 @@ extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
* for driver coldplugging, and in uevents used for hotplugging
* @driver_override: If the name of a driver is written to this attribute, then
* the device will bind to the named driver and only the named driver.
+ * Do not set directly, because core frees it; use driver_set_override() to
+ * set or clear it.
* @cs_gpiod: gpio descriptor of the chipselect line (optional, NULL when
* not using a GPIO line)
* @word_delay: delay to be inserted between consecutive
--
2.32.0
The driver_override field from platform driver should not be initialized
from static memory (string literal) because the core later kfree() it,
for example when driver_override is set via sysfs.
Use dedicated helper to set driver_override properly.
Fixes: 77d8f3068c63 ("clk: imx: scu: add two cells binding support")
Cc: <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/clk/imx/clk-scu.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index 083da31dc3ea..4b2268b7d0d0 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -683,7 +683,12 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
return ERR_PTR(ret);
}
- pdev->driver_override = "imx-scu-clk";
+ ret = driver_set_override(&pdev->dev, &pdev->driver_override,
+ "imx-scu-clk", strlen("imx-scu-clk"));
+ if (ret) {
+ platform_device_put(pdev);
+ return ERR_PTR(ret);
+ }
ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);
if (ret)
--
2.32.0
Several core drivers and buses expect that driver_override is a
dynamically allocated memory thus later they can kfree() it.
However such assumption is not documented, there were in the past and
there are already users setting it to a string literal. This leads to
kfree() of static memory during device release (e.g. in error paths or
during unbind):
kernel BUG at ../mm/slub.c:3960!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
...
(kfree) from [<c058da50>] (platform_device_release+0x88/0xb4)
(platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90)
(device_release) from [<c0a69050>] (kobject_put+0xec/0x20c)
(kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c)
(exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4)
(platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414)
(really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4)
(driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8)
(bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c)
(__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90)
(bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c)
(device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc)
(of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc)
(of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc)
(of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118)
(of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8)
(of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404)
(do_one_initcall) from [<c0f012c0>] (kernel_init_freeable+0x3d0/0x4d8)
(kernel_init_freeable) from [<c0a7def0>] (kernel_init+0x8/0x114)
(kernel_init) from [<c01010b4>] (ret_from_fork+0x14/0x20)
Provide a helper which clearly documents the usage of driver_override.
This will allow later to reuse the helper and reduce amount of
duplicated code.
Convert the platform driver to use new helper and make the
driver_override field const char (it is not modified by the core).
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/base/driver.c | 56 +++++++++++++++++++++++++++++++++
drivers/base/platform.c | 28 +++--------------
include/linux/device/driver.h | 2 ++
include/linux/platform_device.h | 6 +++-
4 files changed, 67 insertions(+), 25 deletions(-)
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 8c0d33e182fd..ba2510117484 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -30,6 +30,62 @@ static struct device *next_device(struct klist_iter *i)
return dev;
}
+/**
+ * driver_set_override() - Helper to set or clear driver override.
+ * @dev: Device to change
+ * @override: Address of string to change (e.g. &device->driver_override);
+ * The contents will be freed and hold newly allocated override.
+ * @s: NUL terminated string, new driver name to force a match, pass empty
+ * string to clear it
+ * @len: length of @s
+ *
+ * Helper to set or clear driver override in a device, intended for the cases
+ * when the driver_override field is allocated by driver/bus code.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int driver_set_override(struct device *dev, const char **override,
+ const char *s, size_t len)
+{
+ const char *new, *old;
+ char *cp;
+
+ if (!dev || !override || !s)
+ return -EINVAL;
+
+ /*
+ * The stored value will be used in sysfs show callback (sysfs_emit()),
+ * which has a length limit of PAGE_SIZE and adds a trailing newline.
+ * Thus we can store one character less to avoid truncation during sysfs
+ * show.
+ */
+ if (len >= (PAGE_SIZE - 1))
+ return -EINVAL;
+
+ new = kstrndup(s, len, GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ cp = strchr(new, '\n');
+ if (cp)
+ *cp = '\0';
+
+ device_lock(dev);
+ old = *override;
+ if (cp != new) {
+ *override = new;
+ } else {
+ kfree(new);
+ *override = NULL;
+ }
+ device_unlock(dev);
+
+ kfree(old);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(driver_set_override);
+
/**
* driver_for_each_device - Iterator for devices bound to a driver.
* @drv: Driver we're iterating.
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 8cc272fd5c99..b684157b7f2f 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1275,31 +1275,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct platform_device *pdev = to_platform_device(dev);
- char *driver_override, *old, *cp;
-
- /* We need to keep extra room for a newline */
- if (count >= (PAGE_SIZE - 1))
- return -EINVAL;
-
- driver_override = kstrndup(buf, count, GFP_KERNEL);
- if (!driver_override)
- return -ENOMEM;
-
- cp = strchr(driver_override, '\n');
- if (cp)
- *cp = '\0';
-
- device_lock(dev);
- old = pdev->driver_override;
- if (strlen(driver_override)) {
- pdev->driver_override = driver_override;
- } else {
- kfree(driver_override);
- pdev->driver_override = NULL;
- }
- device_unlock(dev);
+ int ret;
- kfree(old);
+ ret = driver_set_override(dev, &pdev->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index 15e7c5e15d62..700453017e1c 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -151,6 +151,8 @@ extern int __must_check driver_create_file(struct device_driver *driver,
extern void driver_remove_file(struct device_driver *driver,
const struct driver_attribute *attr);
+int driver_set_override(struct device *dev, const char **override,
+ const char *s, size_t len);
extern int __must_check driver_for_each_device(struct device_driver *drv,
struct device *start,
void *data,
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 7c96f169d274..582d83ed9a91 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -31,7 +31,11 @@ struct platform_device {
struct resource *resource;
const struct platform_device_id *id_entry;
- char *driver_override; /* Driver name to force a match */
+ /*
+ * Driver name to force a match. Do not set directly, because core
+ * frees it. Use driver_set_override() to set or clear it.
+ */
+ const char *driver_override;
/* MFD cell pointer */
struct mfd_cell *mfd_cell;
--
2.32.0
Use a helper to set driver_override to reduce amount of duplicated code.
Make the driver_override field const char, because it is not modified by
the core and it matches other subsystems.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Michael Kelley <[email protected]>
---
drivers/hv/vmbus_drv.c | 28 ++++------------------------
include/linux/hyperv.h | 6 +++++-
2 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 60ee8b329f9e..66213ce5579d 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -575,31 +575,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct hv_device *hv_dev = device_to_hv_device(dev);
- char *driver_override, *old, *cp;
-
- /* We need to keep extra room for a newline */
- if (count >= (PAGE_SIZE - 1))
- return -EINVAL;
-
- driver_override = kstrndup(buf, count, GFP_KERNEL);
- if (!driver_override)
- return -ENOMEM;
-
- cp = strchr(driver_override, '\n');
- if (cp)
- *cp = '\0';
-
- device_lock(dev);
- old = hv_dev->driver_override;
- if (strlen(driver_override)) {
- hv_dev->driver_override = driver_override;
- } else {
- kfree(driver_override);
- hv_dev->driver_override = NULL;
- }
- device_unlock(dev);
+ int ret;
- kfree(old);
+ ret = driver_set_override(dev, &hv_dev->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index fe2e0179ed51..12e2336b23b7 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1257,7 +1257,11 @@ struct hv_device {
u16 device_id;
struct device device;
- char *driver_override; /* Driver name to force a match */
+ /*
+ * Driver name to force a match. Do not set directly, because core
+ * frees it. Use driver_set_override() to set or clear it.
+ */
+ const char *driver_override;
struct vmbus_channel *channel;
struct kset *channels_kset;
--
2.32.0
Use a helper to set driver_override to reduce amount of duplicated code.
Make the driver_override field const char, because it is not modified by
the core and it matches other subsystems.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Vineeth Vijayan <[email protected]>
---
drivers/s390/cio/cio.h | 6 +++++-
drivers/s390/cio/css.c | 28 ++++------------------------
2 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h
index 1cb9daf9c645..fa8df50bb49e 100644
--- a/drivers/s390/cio/cio.h
+++ b/drivers/s390/cio/cio.h
@@ -103,7 +103,11 @@ struct subchannel {
struct work_struct todo_work;
struct schib_config config;
u64 dma_mask;
- char *driver_override; /* Driver name to force a match */
+ /*
+ * Driver name to force a match. Do not set directly, because core
+ * frees it. Use driver_set_override() to set or clear it.
+ */
+ const char *driver_override;
} __attribute__ ((aligned(8)));
DECLARE_PER_CPU_ALIGNED(struct irb, cio_irb);
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index fa8293335077..913b6ddd040b 100644
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -338,31 +338,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct subchannel *sch = to_subchannel(dev);
- char *driver_override, *old, *cp;
-
- /* We need to keep extra room for a newline */
- if (count >= (PAGE_SIZE - 1))
- return -EINVAL;
-
- driver_override = kstrndup(buf, count, GFP_KERNEL);
- if (!driver_override)
- return -ENOMEM;
-
- cp = strchr(driver_override, '\n');
- if (cp)
- *cp = '\0';
-
- device_lock(dev);
- old = sch->driver_override;
- if (strlen(driver_override)) {
- sch->driver_override = driver_override;
- } else {
- kfree(driver_override);
- sch->driver_override = NULL;
- }
- device_unlock(dev);
+ int ret;
- kfree(old);
+ ret = driver_set_override(dev, &sch->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
--
2.32.0
On 15/03/2022 18:59, Andy Shevchenko wrote:
> On Sat, Mar 12, 2022 at 5:16 PM Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> Several core drivers and buses expect that driver_override is a
>> dynamically allocated memory thus later they can kfree() it.
>>
>> However such assumption is not documented, there were in the past and
>> there are already users setting it to a string literal. This leads to
>> kfree() of static memory during device release (e.g. in error paths or
>> during unbind):
>>
>> kernel BUG at ../mm/slub.c:3960!
>> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
>> ...
>> (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4)
>> (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90)
>> (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c)
>> (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c)
>> (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4)
>> (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414)
>> (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4)
>> (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8)
>> (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c)
>> (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90)
>> (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c)
>> (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc)
>> (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc)
>> (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc)
>> (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118)
>> (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8)
>> (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404)
>
>> (do_one_initcall) from [<c0f012c0>] (kernel_init_freeable+0x3d0/0x4d8)
>> (kernel_init_freeable) from [<c0a7def0>] (kernel_init+0x8/0x114)
>> (kernel_init) from [<c01010b4>] (ret_from_fork+0x14/0x20)
>
> I believe you may remove these three.
Sure (for this and later comments).
>
>> Provide a helper which clearly documents the usage of driver_override.
>> This will allow later to reuse the helper and reduce amount of
>
> the amount
>
>> duplicated code.
>
>> Convert the platform driver to use new helper and make the
>
> a new
>
>> driver_override field const char (it is not modified by the core).
>
> ...
>
>> +/**
>> + * driver_set_override() - Helper to set or clear driver override.
>> + * @dev: Device to change
>> + * @override: Address of string to change (e.g. &device->driver_override);
>> + * The contents will be freed and hold newly allocated override.
>> + * @s: NUL terminated string, new driver name to force a match, pass empty
>
> NUL-terminated? (44 vs 115 occurrences)
>
>> + * string to clear it
>> + * @len: length of @s
>> + *
>> + * Helper to set or clear driver override in a device, intended for the cases
>> + * when the driver_override field is allocated by driver/bus code.
>> + *
>> + * Returns: 0 on success or a negative error code on failure.
>> + */
>> +int driver_set_override(struct device *dev, const char **override,
>> + const char *s, size_t len)
>> +{
>> + const char *new, *old;
>> + char *cp;
>> +
>> + if (!dev || !override || !s)
>> + return -EINVAL;
>> +
>> + /*
>> + * The stored value will be used in sysfs show callback (sysfs_emit()),
>> + * which has a length limit of PAGE_SIZE and adds a trailing newline.
>> + * Thus we can store one character less to avoid truncation during sysfs
>> + * show.
>> + */
>> + if (len >= (PAGE_SIZE - 1))
>> + return -EINVAL;
>> +
>> + new = kstrndup(s, len, GFP_KERNEL);
>> + if (!new)
>> + return -ENOMEM;
>> +
>> + cp = strchr(new, '\n');
>> + if (cp)
>> + *cp = '\0';
>
> AFAIU you may reduce memory footprint by
>
> cp = strnchr(new, len, '\n');
> if (cp)
> len = s - cp;
>
> new = kstrndup(...);
>
Indeed, thanks.
Best regards,
Krzysztof
On Sat, Mar 12, 2022 at 5:16 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> Several core drivers and buses expect that driver_override is a
> dynamically allocated memory thus later they can kfree() it.
>
> However such assumption is not documented, there were in the past and
> there are already users setting it to a string literal. This leads to
> kfree() of static memory during device release (e.g. in error paths or
> during unbind):
>
> kernel BUG at ../mm/slub.c:3960!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> ...
> (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4)
> (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90)
> (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c)
> (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c)
> (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4)
> (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414)
> (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4)
> (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8)
> (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c)
> (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90)
> (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c)
> (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc)
> (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc)
> (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc)
> (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118)
> (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8)
> (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404)
> (do_one_initcall) from [<c0f012c0>] (kernel_init_freeable+0x3d0/0x4d8)
> (kernel_init_freeable) from [<c0a7def0>] (kernel_init+0x8/0x114)
> (kernel_init) from [<c01010b4>] (ret_from_fork+0x14/0x20)
I believe you may remove these three.
> Provide a helper which clearly documents the usage of driver_override.
> This will allow later to reuse the helper and reduce amount of
the amount
> duplicated code.
> Convert the platform driver to use new helper and make the
a new
> driver_override field const char (it is not modified by the core).
...
> +/**
> + * driver_set_override() - Helper to set or clear driver override.
> + * @dev: Device to change
> + * @override: Address of string to change (e.g. &device->driver_override);
> + * The contents will be freed and hold newly allocated override.
> + * @s: NUL terminated string, new driver name to force a match, pass empty
NUL-terminated? (44 vs 115 occurrences)
> + * string to clear it
> + * @len: length of @s
> + *
> + * Helper to set or clear driver override in a device, intended for the cases
> + * when the driver_override field is allocated by driver/bus code.
> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +int driver_set_override(struct device *dev, const char **override,
> + const char *s, size_t len)
> +{
> + const char *new, *old;
> + char *cp;
> +
> + if (!dev || !override || !s)
> + return -EINVAL;
> +
> + /*
> + * The stored value will be used in sysfs show callback (sysfs_emit()),
> + * which has a length limit of PAGE_SIZE and adds a trailing newline.
> + * Thus we can store one character less to avoid truncation during sysfs
> + * show.
> + */
> + if (len >= (PAGE_SIZE - 1))
> + return -EINVAL;
> +
> + new = kstrndup(s, len, GFP_KERNEL);
> + if (!new)
> + return -ENOMEM;
> +
> + cp = strchr(new, '\n');
> + if (cp)
> + *cp = '\0';
AFAIU you may reduce memory footprint by
cp = strnchr(new, len, '\n');
if (cp)
len = s - cp;
new = kstrndup(...);
> + device_lock(dev);
> + old = *override;
> + if (cp != new) {
> + *override = new;
> + } else {
> + kfree(new);
> + *override = NULL;
> + }
> + device_unlock(dev);
> +
> + kfree(old);
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko