2023-11-16 14:06:07

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH v2] mfd: rk8xx: fixup devices registration with PLATFORM_DEVID_AUTO

Since commit 210f418f8ace ("mfd: rk8xx: Add rk806 support"), devices are
registered with "0" as id, causing devices to not have an automatic device id
and prevents having multiple RK8xx PMICs on the same system.

Properly pass PLATFORM_DEVID_AUTO to devm_mfd_add_devices() and since
it will ignore the cells .id with this special value, also cleanup
by removing all now ignored cells .id values.

Now we have the same behaviour as before rk806 introduction and rk806
retains the intented behavior.

This fixes a regression while booting the Odroid Go Ultra on v6.6.1:
sysfs: cannot create duplicate filename '/bus/platform/devices/rk808-clkout'
CPU: 3 PID: 97 Comm: kworker/u12:2 Not tainted 6.6.1 #1
Hardware name: Hardkernel ODROID-GO-Ultra (DT)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace+0x9c/0x11c
show_stack+0x18/0x24
dump_stack_lvl+0x78/0xc4
dump_stack+0x18/0x24
sysfs_warn_dup+0x64/0x80
sysfs_do_create_link_sd+0xf0/0xf8
sysfs_create_link+0x20/0x40
bus_add_device+0x114/0x160
device_add+0x3f0/0x7cc
platform_device_add+0x180/0x270
mfd_add_device+0x390/0x4a8
devm_mfd_add_devices+0xb0/0x150
rk8xx_probe+0x26c/0x410
rk8xx_i2c_probe+0x64/0x98
i2c_device_probe+0x104/0x2e8
really_probe+0x184/0x3c8
__driver_probe_device+0x7c/0x16c
driver_probe_device+0x3c/0x10c
__device_attach_driver+0xbc/0x158
bus_for_each_drv+0x80/0xdc
__device_attach+0x9c/0x1ac
device_initial_probe+0x14/0x20
bus_probe_device+0xac/0xb0
deferred_probe_work_func+0xa0/0xf4
process_one_work+0x1bc/0x378
worker_thread+0x1dc/0x3d4
kthread+0x104/0x118
ret_from_fork+0x10/0x20
rk8xx-i2c 0-001c: error -EEXIST: failed to add MFD devices
rk8xx-i2c: probe of 0-001c failed with error -17

Fixes: 210f418f8ace ("mfd: rk8xx: Add rk806 support")
Reported-by: Adam Green <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
Finally a full cleanup is required to fix the regression because rk806 had
a special treatment still allowing DEVID_AUTO while all the other cells
regressed to DEVID_NONE.

[1] https://lore.kernel.org/all/[email protected]/
---
Changes in v2:
- Do a full cleanup instead of simply changing the id value passed to register
- Link to v1: https://lore.kernel.org/r/20231116-topic-amlogic-upstream-fix-rk8xx-devid-auto-v1-1-75fa43575ab7@linaro.org
---
drivers/mfd/rk8xx-core.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/mfd/rk8xx-core.c b/drivers/mfd/rk8xx-core.c
index c47164a3ec1d..b1ffc3b9e2be 100644
--- a/drivers/mfd/rk8xx-core.c
+++ b/drivers/mfd/rk8xx-core.c
@@ -53,76 +53,68 @@ static const struct resource rk817_charger_resources[] = {
};

static const struct mfd_cell rk805s[] = {
- { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, },
- { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, },
- { .name = "rk805-pinctrl", .id = PLATFORM_DEVID_NONE, },
+ { .name = "rk808-clkout", },
+ { .name = "rk808-regulator", },
+ { .name = "rk805-pinctrl", },
{
.name = "rk808-rtc",
.num_resources = ARRAY_SIZE(rtc_resources),
.resources = &rtc_resources[0],
- .id = PLATFORM_DEVID_NONE,
},
{ .name = "rk805-pwrkey",
.num_resources = ARRAY_SIZE(rk805_key_resources),
.resources = &rk805_key_resources[0],
- .id = PLATFORM_DEVID_NONE,
},
};

static const struct mfd_cell rk806s[] = {
- { .name = "rk805-pinctrl", .id = PLATFORM_DEVID_AUTO, },
- { .name = "rk808-regulator", .id = PLATFORM_DEVID_AUTO, },
+ { .name = "rk805-pinctrl", },
+ { .name = "rk808-regulator", },
{
.name = "rk805-pwrkey",
.resources = rk806_pwrkey_resources,
.num_resources = ARRAY_SIZE(rk806_pwrkey_resources),
- .id = PLATFORM_DEVID_AUTO,
},
};

static const struct mfd_cell rk808s[] = {
- { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, },
- { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, },
+ { .name = "rk808-clkout", },
+ { .name = "rk808-regulator", },
{
.name = "rk808-rtc",
.num_resources = ARRAY_SIZE(rtc_resources),
.resources = rtc_resources,
- .id = PLATFORM_DEVID_NONE,
},
};

static const struct mfd_cell rk817s[] = {
- { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, },
- { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, },
+ { .name = "rk808-clkout", },
+ { .name = "rk808-regulator", },
{
.name = "rk805-pwrkey",
.num_resources = ARRAY_SIZE(rk817_pwrkey_resources),
.resources = &rk817_pwrkey_resources[0],
- .id = PLATFORM_DEVID_NONE,
},
{
.name = "rk808-rtc",
.num_resources = ARRAY_SIZE(rk817_rtc_resources),
.resources = &rk817_rtc_resources[0],
- .id = PLATFORM_DEVID_NONE,
},
- { .name = "rk817-codec", .id = PLATFORM_DEVID_NONE, },
+ { .name = "rk817-codec", },
{
.name = "rk817-charger",
.num_resources = ARRAY_SIZE(rk817_charger_resources),
.resources = &rk817_charger_resources[0],
- .id = PLATFORM_DEVID_NONE,
},
};

static const struct mfd_cell rk818s[] = {
- { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, },
- { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, },
+ { .name = "rk808-clkout", },
+ { .name = "rk808-regulator", },
{
.name = "rk808-rtc",
.num_resources = ARRAY_SIZE(rtc_resources),
.resources = rtc_resources,
- .id = PLATFORM_DEVID_NONE,
},
};

@@ -684,7 +676,7 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap
pre_init_reg[i].addr);
}

- ret = devm_mfd_add_devices(dev, 0, cells, nr_cells, NULL, 0,
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cells, nr_cells, NULL, 0,
regmap_irq_get_domain(rk808->irq_data));
if (ret)
return dev_err_probe(dev, ret, "failed to add MFD devices\n");

---
base-commit: f31817cbcf48d191faee7cebfb59197d2048cd64
change-id: 20231116-topic-amlogic-upstream-fix-rk8xx-devid-auto-59ce0d1b738a

Best regards,
--
Neil Armstrong <[email protected]>


2023-11-16 16:26:02

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v2] mfd: rk8xx: fixup devices registration with PLATFORM_DEVID_AUTO

Hi,

On Thu, Nov 16, 2023 at 03:05:13PM +0100, Neil Armstrong wrote:
> Since commit 210f418f8ace ("mfd: rk8xx: Add rk806 support"), devices are
> registered with "0" as id, causing devices to not have an automatic device id
> and prevents having multiple RK8xx PMICs on the same system.
>
> Properly pass PLATFORM_DEVID_AUTO to devm_mfd_add_devices() and since
> it will ignore the cells .id with this special value, also cleanup
> by removing all now ignored cells .id values.
>
> Now we have the same behaviour as before rk806 introduction and rk806
> retains the intented behavior.
>
> This fixes a regression while booting the Odroid Go Ultra on v6.6.1:
> sysfs: cannot create duplicate filename '/bus/platform/devices/rk808-clkout'
> CPU: 3 PID: 97 Comm: kworker/u12:2 Not tainted 6.6.1 #1
> Hardware name: Hardkernel ODROID-GO-Ultra (DT)
> Workqueue: events_unbound deferred_probe_work_func
> Call trace:
> dump_backtrace+0x9c/0x11c
> show_stack+0x18/0x24
> dump_stack_lvl+0x78/0xc4
> dump_stack+0x18/0x24
> sysfs_warn_dup+0x64/0x80
> sysfs_do_create_link_sd+0xf0/0xf8
> sysfs_create_link+0x20/0x40
> bus_add_device+0x114/0x160
> device_add+0x3f0/0x7cc
> platform_device_add+0x180/0x270
> mfd_add_device+0x390/0x4a8
> devm_mfd_add_devices+0xb0/0x150
> rk8xx_probe+0x26c/0x410
> rk8xx_i2c_probe+0x64/0x98
> i2c_device_probe+0x104/0x2e8
> really_probe+0x184/0x3c8
> __driver_probe_device+0x7c/0x16c
> driver_probe_device+0x3c/0x10c
> __device_attach_driver+0xbc/0x158
> bus_for_each_drv+0x80/0xdc
> __device_attach+0x9c/0x1ac
> device_initial_probe+0x14/0x20
> bus_probe_device+0xac/0xb0
> deferred_probe_work_func+0xa0/0xf4
> process_one_work+0x1bc/0x378
> worker_thread+0x1dc/0x3d4
> kthread+0x104/0x118
> ret_from_fork+0x10/0x20
> rk8xx-i2c 0-001c: error -EEXIST: failed to add MFD devices
> rk8xx-i2c: probe of 0-001c failed with error -17
>
> Fixes: 210f418f8ace ("mfd: rk8xx: Add rk806 support")
> Reported-by: Adam Green <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> Finally a full cleanup is required to fix the regression because rk806 had
> a special treatment still allowing DEVID_AUTO while all the other cells
> regressed to DEVID_NONE.
>
> [1] https://lore.kernel.org/all/[email protected]/
> ---

LGTM

Reviewed-by: Sebastian Reichel <[email protected]>

-- Sebastian

> Changes in v2:
> - Do a full cleanup instead of simply changing the id value passed to register
> - Link to v1: https://lore.kernel.org/r/20231116-topic-amlogic-upstream-fix-rk8xx-devid-auto-v1-1-75fa43575ab7@linaro.org
> ---
> drivers/mfd/rk8xx-core.c | 34 +++++++++++++---------------------
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mfd/rk8xx-core.c b/drivers/mfd/rk8xx-core.c
> index c47164a3ec1d..b1ffc3b9e2be 100644
> --- a/drivers/mfd/rk8xx-core.c
> +++ b/drivers/mfd/rk8xx-core.c
> @@ -53,76 +53,68 @@ static const struct resource rk817_charger_resources[] = {
> };
>
> static const struct mfd_cell rk805s[] = {
> - { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, },
> - { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, },
> - { .name = "rk805-pinctrl", .id = PLATFORM_DEVID_NONE, },
> + { .name = "rk808-clkout", },
> + { .name = "rk808-regulator", },
> + { .name = "rk805-pinctrl", },
> {
> .name = "rk808-rtc",
> .num_resources = ARRAY_SIZE(rtc_resources),
> .resources = &rtc_resources[0],
> - .id = PLATFORM_DEVID_NONE,
> },
> { .name = "rk805-pwrkey",
> .num_resources = ARRAY_SIZE(rk805_key_resources),
> .resources = &rk805_key_resources[0],
> - .id = PLATFORM_DEVID_NONE,
> },
> };
>
> static const struct mfd_cell rk806s[] = {
> - { .name = "rk805-pinctrl", .id = PLATFORM_DEVID_AUTO, },
> - { .name = "rk808-regulator", .id = PLATFORM_DEVID_AUTO, },
> + { .name = "rk805-pinctrl", },
> + { .name = "rk808-regulator", },
> {
> .name = "rk805-pwrkey",
> .resources = rk806_pwrkey_resources,
> .num_resources = ARRAY_SIZE(rk806_pwrkey_resources),
> - .id = PLATFORM_DEVID_AUTO,
> },
> };
>
> static const struct mfd_cell rk808s[] = {
> - { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, },
> - { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, },
> + { .name = "rk808-clkout", },
> + { .name = "rk808-regulator", },
> {
> .name = "rk808-rtc",
> .num_resources = ARRAY_SIZE(rtc_resources),
> .resources = rtc_resources,
> - .id = PLATFORM_DEVID_NONE,
> },
> };
>
> static const struct mfd_cell rk817s[] = {
> - { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, },
> - { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, },
> + { .name = "rk808-clkout", },
> + { .name = "rk808-regulator", },
> {
> .name = "rk805-pwrkey",
> .num_resources = ARRAY_SIZE(rk817_pwrkey_resources),
> .resources = &rk817_pwrkey_resources[0],
> - .id = PLATFORM_DEVID_NONE,
> },
> {
> .name = "rk808-rtc",
> .num_resources = ARRAY_SIZE(rk817_rtc_resources),
> .resources = &rk817_rtc_resources[0],
> - .id = PLATFORM_DEVID_NONE,
> },
> - { .name = "rk817-codec", .id = PLATFORM_DEVID_NONE, },
> + { .name = "rk817-codec", },
> {
> .name = "rk817-charger",
> .num_resources = ARRAY_SIZE(rk817_charger_resources),
> .resources = &rk817_charger_resources[0],
> - .id = PLATFORM_DEVID_NONE,
> },
> };
>
> static const struct mfd_cell rk818s[] = {
> - { .name = "rk808-clkout", .id = PLATFORM_DEVID_NONE, },
> - { .name = "rk808-regulator", .id = PLATFORM_DEVID_NONE, },
> + { .name = "rk808-clkout", },
> + { .name = "rk808-regulator", },
> {
> .name = "rk808-rtc",
> .num_resources = ARRAY_SIZE(rtc_resources),
> .resources = rtc_resources,
> - .id = PLATFORM_DEVID_NONE,
> },
> };
>
> @@ -684,7 +676,7 @@ int rk8xx_probe(struct device *dev, int variant, unsigned int irq, struct regmap
> pre_init_reg[i].addr);
> }
>
> - ret = devm_mfd_add_devices(dev, 0, cells, nr_cells, NULL, 0,
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cells, nr_cells, NULL, 0,
> regmap_irq_get_domain(rk808->irq_data));
> if (ret)
> return dev_err_probe(dev, ret, "failed to add MFD devices\n");
>
> ---
> base-commit: f31817cbcf48d191faee7cebfb59197d2048cd64
> change-id: 20231116-topic-amlogic-upstream-fix-rk8xx-devid-auto-59ce0d1b738a
>
> Best regards,
> --
> Neil Armstrong <[email protected]>
>


Attachments:
(No filename) (6.44 kB)
signature.asc (849.00 B)
Download all attachments

2023-11-23 15:09:28

by Lee Jones

[permalink] [raw]
Subject: Re: (subset) [PATCH v2] mfd: rk8xx: fixup devices registration with PLATFORM_DEVID_AUTO

On Thu, 16 Nov 2023 15:05:13 +0100, Neil Armstrong wrote:
> Since commit 210f418f8ace ("mfd: rk8xx: Add rk806 support"), devices are
> registered with "0" as id, causing devices to not have an automatic device id
> and prevents having multiple RK8xx PMICs on the same system.
>
> Properly pass PLATFORM_DEVID_AUTO to devm_mfd_add_devices() and since
> it will ignore the cells .id with this special value, also cleanup
> by removing all now ignored cells .id values.
>
> [...]

Applied, thanks!

[1/1] mfd: rk8xx: fixup devices registration with PLATFORM_DEVID_AUTO
commit: a6f86de216cd36e0f57f5797df7361c8900f93e6

--
Lee Jones [李琼斯]