Skip to contentSkip to conversation

mfd: Fix platform device ids to avoid probe failure

PATCH Superseded Bartlomiej Zolnierkiewicz 4 participants 40 messages Mar 20, 2015 – May 19, 2015

Subsystems (5)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover mfd: Fix platform device ids to avoid probe failure

Hi,

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

Relevant error messages (in this case for MAX8997 PMIC driver):

[    0.911674] ------------[ cut here ]------------
[    0.911706] WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x54/0x70()
[    0.911718] sysfs: cannot create duplicate filename '/devices/13860000.i2c/i2c-0/0-0066/max8997-led'
...
[    0.912382] ------------[ cut here ]------------
[    0.912402] WARNING: CPU: 0 PID: 1 at lib/kobject.c:240 kobject_add_internal+0x238/0x2c0()
[    0.912411] kobject_add_internal failed for max8997-led with -EEXIST, don't try to register things with the same name in th
...
[    0.920721] max8997 0-0066: failed to add MFD devices -17
[    0.921553] max8997: probe of 0-0066 failed with error -17

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

MAX8997 PMIC fix was tested on Exynos4210 Origen board, the rest of patches is compile tested only.

Changes since v1:
- corrected 'stable' mailing list address, no other changes

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Bartlomiej Zolnierkiewicz (5):
mfd: max8997: Fix platform device ids to avoid probe failure
mfd: da9055: Fix platform device ids to avoid probe failure
mfd: lp8788: Fix platform device ids to avoid probe failure
mfd: wm831x: Fix platform device ids to avoid probe failure
mfd: da9052: Fix platform device names

 drivers/mfd/da9052-core.c |  21 ++++--
 drivers/mfd/da9055-core.c |  21 ++++--
 drivers/mfd/lp8788.c      |  10 ++-
 drivers/mfd/max8997.c     |  16 ++--
 drivers/mfd/wm831x-core.c | 184 +++++++++++++++++++++++++++-------------------
 5 files changed, 150 insertions(+), 102 deletions(-)
Signature · 1 line
1.8.2.3
#1 lore raw

PATCH v2 1/5 mfd: max8997: Fix platform device ids to avoid probe failure

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

Relevant error messages in case of MAX8997 PMIC driver:

[    0.911674] ------------[ cut here ]------------
[    0.911706] WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x54/0x70()
[    0.911718] sysfs: cannot create duplicate filename '/devices/13860000.i2c/i2c-0/0-0066/max8997-led'
[    0.911725] Modules linked in:
[    0.911742] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.18.0-rc1-00027-g6e3f62f #1557
[    0.911782] [<c0013cb4>] (unwind_backtrace) from [<c0010d10>] (show_stack+0x10/0x14)
[    0.911809] [<c0010d10>] (show_stack) from [<c04619c8>] (dump_stack+0x68/0xb8)
[    0.911837] [<c04619c8>] (dump_stack) from [<c0020c80>] (warn_slowpath_common+0x64/0x84)
[    0.911854] [<c0020c80>] (warn_slowpath_common) from [<c0020d34>] (warn_slowpath_fmt+0x30/0x40)
[    0.911871] [<c0020d34>] (warn_slowpath_fmt) from [<c011ec50>] (sysfs_warn_dup+0x54/0x70)
[    0.911887] [<c011ec50>] (sysfs_warn_dup) from [<c011ecf4>] (sysfs_create_dir_ns+0x88/0x90)
[    0.911910] [<c011ecf4>] (sysfs_create_dir_ns) from [<c01d5524>] (kobject_add_internal+0x9c/0x2c0)
[    0.911926] [<c01d5524>] (kobject_add_internal) from [<c01d58d0>] (kobject_add+0x4c/0x94)
[    0.911950] [<c01d58d0>] (kobject_add) from [<c026f65c>] (device_add+0xb8/0x4d8)
[    0.911969] [<c026f65c>] (device_add) from [<c027330c>] (platform_device_add+0x114/0x214)
[    0.911996] [<c027330c>] (platform_device_add) from [<c028d0f0>] (mfd_add_device+0x240/0x328)
[    0.912013] [<c028d0f0>] (mfd_add_device) from [<c028d3d0>] (mfd_add_devices+0xa4/0xe8)
[    0.912030] [<c028d3d0>] (mfd_add_devices) from [<c028e3b0>] (max8997_i2c_probe+0x150/0x230)
[    0.912056] [<c028e3b0>] (max8997_i2c_probe) from [<c0329114>] (i2c_device_probe+0xe8/0x120)
[    0.912076] [<c0329114>] (i2c_device_probe) from [<c0271cf0>] (driver_probe_device+0x114/0x234)
[    0.912094] [<c0271cf0>] (driver_probe_device) from [<c0270630>] (bus_for_each_drv+0x5c/0x88)
[    0.912109] [<c0270630>] (bus_for_each_drv) from [<c0271ba8>] (device_attach+0x70/0x88)
[    0.912124] [<c0271ba8>] (device_attach) from [<c0271304>] (bus_probe_device+0x84/0xa8)
[    0.912138] [<c0271304>] (bus_probe_device) from [<c026f94c>] (device_add+0x3a8/0x4d8)
[    0.912153] [<c026f94c>] (device_add) from [<c03298c0>] (i2c_new_device+0xfc/0x16c)
[    0.912167] [<c03298c0>] (i2c_new_device) from [<c0329dd0>] (i2c_register_adapter+0x27c/0x490)
[    0.912186] [<c0329dd0>] (i2c_register_adapter) from [<c032f184>] (s3c24xx_i2c_probe+0x200/0x4dc)
[    0.912205] [<c032f184>] (s3c24xx_i2c_probe) from [<c027310c>] (platform_drv_probe+0x48/0x98)
[    0.912219] [<c027310c>] (platform_drv_probe) from [<c0271cf0>] (driver_probe_device+0x114/0x234)
[    0.912232] [<c0271cf0>] (driver_probe_device) from [<c0271e9c>] (__driver_attach+0x8c/0x90)
[    0.912246] [<c0271e9c>] (__driver_attach) from [<c02706b0>] (bus_for_each_dev+0x54/0x88)
[    0.912262] [<c02706b0>] (bus_for_each_dev) from [<c02714f4>] (bus_add_driver+0xd8/0x1cc)
[    0.912275] [<c02714f4>] (bus_add_driver) from [<c02724bc>] (driver_register+0x78/0xf4)
[    0.912291] [<c02724bc>] (driver_register) from [<c0008924>] (do_one_initcall+0x80/0x1c4)
[    0.912312] [<c0008924>] (do_one_initcall) from [<c062cd20>] (kernel_init_freeable+0x100/0x1cc)
[    0.912330] [<c062cd20>] (kernel_init_freeable) from [<c045da50>] (kernel_init+0x8/0xe4)
[    0.912348] [<c045da50>] (kernel_init) from [<c000e5b8>] (ret_from_fork+0x14/0x3c)
[    0.912370] ---[ end trace ca08768d4d1efd62 ]---
[    0.912382] ------------[ cut here ]------------
[    0.912402] WARNING: CPU: 0 PID: 1 at lib/kobject.c:240 kobject_add_internal+0x238/0x2c0()
[    0.912411] kobject_add_internal failed for max8997-led with -EEXIST, don't try to register things with the same name in th
e same directory.
[    0.912418] Modules linked in:
[    0.912433] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G        W      3.18.0-rc1-00027-g6e3f62f #1557
[    0.912458] [<c0013cb4>] (unwind_backtrace) from [<c0010d10>] (show_stack+0x10/0x14)
[    0.912479] [<c0010d10>] (show_stack) from [<c04619c8>] (dump_stack+0x68/0xb8)
[    0.912502] [<c04619c8>] (dump_stack) from [<c0020c80>] (warn_slowpath_common+0x64/0x84)
[    0.912519] [<c0020c80>] (warn_slowpath_common) from [<c0020d34>] (warn_slowpath_fmt+0x30/0x40)
[    0.912536] [<c0020d34>] (warn_slowpath_fmt) from [<c01d56c0>] (kobject_add_internal+0x238/0x2c0)
[    0.912553] [<c01d56c0>] (kobject_add_internal) from [<c01d58d0>] (kobject_add+0x4c/0x94)
[    0.912572] [<c01d58d0>] (kobject_add) from [<c026f65c>] (device_add+0xb8/0x4d8)
[    0.912588] [<c026f65c>] (device_add) from [<c027330c>] (platform_device_add+0x114/0x214)
[    0.912611] [<c027330c>] (platform_device_add) from [<c028d0f0>] (mfd_add_device+0x240/0x328)
[    0.912628] [<c028d0f0>] (mfd_add_device) from [<c028d3d0>] (mfd_add_devices+0xa4/0xe8)
[    0.912643] [<c028d3d0>] (mfd_add_devices) from [<c028e3b0>] (max8997_i2c_probe+0x150/0x230)
[    0.912665] [<c028e3b0>] (max8997_i2c_probe) from [<c0329114>] (i2c_device_probe+0xe8/0x120)
[    0.912682] [<c0329114>] (i2c_device_probe) from [<c0271cf0>] (driver_probe_device+0x114/0x234)
[    0.912700] [<c0271cf0>] (driver_probe_device) from [<c0270630>] (bus_for_each_drv+0x5c/0x88)
[    0.912714] [<c0270630>] (bus_for_each_drv) from [<c0271ba8>] (device_attach+0x70/0x88)
[    0.912729] [<c0271ba8>] (device_attach) from [<c0271304>] (bus_probe_device+0x84/0xa8)
[    0.912743] [<c0271304>] (bus_probe_device) from [<c026f94c>] (device_add+0x3a8/0x4d8)
[    0.912758] [<c026f94c>] (device_add) from [<c03298c0>] (i2c_new_device+0xfc/0x16c)
[    0.912771] [<c03298c0>] (i2c_new_device) from [<c0329dd0>] (i2c_register_adapter+0x27c/0x490)
[    0.912789] [<c0329dd0>] (i2c_register_adapter) from [<c032f184>] (s3c24xx_i2c_probe+0x200/0x4dc)
[    0.912805] [<c032f184>] (s3c24xx_i2c_probe) from [<c027310c>] (platform_drv_probe+0x48/0x98)
[    0.912820] [<c027310c>] (platform_drv_probe) from [<c0271cf0>] (driver_probe_device+0x114/0x234)
[    0.912833] [<c0271cf0>] (driver_probe_device) from [<c0271e9c>] (__driver_attach+0x8c/0x90)
[    0.912847] [<c0271e9c>] (__driver_attach) from [<c02706b0>] (bus_for_each_dev+0x54/0x88)
[    0.912862] [<c02706b0>] (bus_for_each_dev) from [<c02714f4>] (bus_add_driver+0xd8/0x1cc)
[    0.912875] [<c02714f4>] (bus_add_driver) from [<c02724bc>] (driver_register+0x78/0xf4)
[    0.912889] [<c02724bc>] (driver_register) from [<c0008924>] (do_one_initcall+0x80/0x1c4)
[    0.912908] [<c0008924>] (do_one_initcall) from [<c062cd20>] (kernel_init_freeable+0x100/0x1cc)
[    0.912925] [<c062cd20>] (kernel_init_freeable) from [<c045da50>] (kernel_init+0x8/0xe4)
[    0.912942] [<c045da50>] (kernel_init) from [<c000e5b8>] (ret_from_fork+0x14/0x3c)
[    0.912952] ---[ end trace ca08768d4d1efd63 ]---
[    0.920721] max8997 0-0066: failed to add MFD devices -17
[    0.921553] max8997: probe of 0-0066 failed with error -17

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

This fix was tested on Exynos4210 Origen board which is using MAX8997 PMIC (it also fixes Exynos cpufreq support which was broken because vdd_arm regulator was not available due to MAX8997 PMIC probe failure).

Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")
Cc Johan Hovold <[email protected]>

---
stable v3.19+

 drivers/mfd/max8997.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/mfd/max8997.c +8 -8
index 595364e..c93dabc 100644
@@ -41,13 +41,13 @@
 #define I2C_ADDR_HAPTIC	(0x90 >> 1)
 
 static const struct mfd_cell max8997_devs[] = {
-	{ .name = "max8997-pmic", },
-	{ .name = "max8997-rtc", },
-	{ .name = "max8997-battery", },
-	{ .name = "max8997-haptic", },
-	{ .name = "max8997-muic", },
-	{ .name = "max8997-led", .id = 1 },
-	{ .name = "max8997-led", .id = 2 },
+	{ .name = "max8997-pmic",	.id = -1 },
+	{ .name = "max8997-rtc",	.id = -1 },
+	{ .name = "max8997-battery",	.id = -1 },
+	{ .name = "max8997-haptic",	.id = -1 },
+	{ .name = "max8997-muic",	.id = -1 },
+	{ .name = "max8997-led",	.id = 0 },
+	{ .name = "max8997-led",	.id = 1 },
 };
 
 #ifdef CONFIG_OF
@@ -234,7 +234,7 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
 
 	max8997_irq_init(max8997);
 
-	ret = mfd_add_devices(max8997->dev, -1, max8997_devs,
+	ret = mfd_add_devices(max8997->dev, 0, max8997_devs,
 			ARRAY_SIZE(max8997_devs),
 			NULL, 0, NULL);
 	if (ret < 0) {
Signature · 1 line
1.8.2.3
#2 lore raw

PATCH v2 2/5 mfd: da9055: Fix platform device ids to avoid probe failure

In reply to Bartlomiej Zolnierkiewicz

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")
Cc Johan Hovold <[email protected]>
Cc Support Opensource <[email protected]>

---
stable v3.19+

 drivers/mfd/da9055-core.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/mfd/da9055-core.c +13 -8
index b4d920c..4dbe752 100644
@@ -298,6 +298,12 @@ static const struct mfd_cell da9055_devs[] = {
 	{
 		.of_compatible = "dlg,da9055-gpio",
 		.name = "da9055-gpio",
+		.id = -1,
+	},
+	{
+		.of_compatible = "dlg,da9055-regulator",
+		.name = "da9055-regulator",
+		.id = 0,
 	},
 	{
 		.of_compatible = "dlg,da9055-regulator",
@@ -327,43 +333,42 @@ static const struct mfd_cell da9055_devs[] = {
 	{
 		.of_compatible = "dlg,da9055-regulator",
 		.name = "da9055-regulator",
-		.id = 6,
-	},
-	{
-		.of_compatible = "dlg,da9055-regulator",
-		.name = "da9055-regulator",
-		.id = 7,
 		.resources = &da9055_ld05_6_resource,
 		.num_resources = 1,
+		.id = 6,
 	},
 	{
 		.of_compatible = "dlg,da9055-regulator",
 		.name = "da9055-regulator",
 		.resources = &da9055_ld05_6_resource,
 		.num_resources = 1,
-		.id = 8,
+		.id = 7,
 	},
 	{
 		.of_compatible = "dlg,da9055-onkey",
 		.name = "da9055-onkey",
 		.resources = &da9055_onkey_resource,
 		.num_resources = 1,
+		.id = -1,
 	},
 	{
 		.of_compatible = "dlg,da9055-rtc",
 		.name = "da9055-rtc",
 		.resources = da9055_rtc_resource,
 		.num_resources = ARRAY_SIZE(da9055_rtc_resource),
+		.id = -1,
 	},
 	{
 		.of_compatible = "dlg,da9055-hwmon",
 		.name = "da9055-hwmon",
 		.resources = &da9055_hwmon_resource,
 		.num_resources = 1,
+		.id = -1,
 	},
 	{
 		.of_compatible = "dlg,da9055-watchdog",
 		.name = "da9055-watchdog",
+		.id = -1,
 	},
 };
 
@@ -404,7 +409,7 @@ int da9055_device_init(struct da9055 *da9055)
 
 	da9055->irq_base = regmap_irq_chip_get_base(da9055->irq_data);
 
-	ret = mfd_add_devices(da9055->dev, -1,
+	ret = mfd_add_devices(da9055->dev, 0,
 			      da9055_devs, ARRAY_SIZE(da9055_devs),
 			      NULL, da9055->irq_base, NULL);
 	if (ret)
Signature · 1 line
1.8.2.3
#3 lore raw

PATCH v2 3/5 mfd: lp8788: Fix platform device ids to avoid probe failure

In reply to Bartlomiej Zolnierkiewicz

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")
Cc Johan Hovold <[email protected]>
Cc Milo Kim <[email protected]>

---
stable v3.19+

 drivers/mfd/lp8788.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/mfd/lp8788.c +6 -4
index a30bc15..2883b97 100644
@@ -23,6 +23,7 @@
 #define MFD_DEV_SIMPLE(_name)					\
 {								\
 	.name = LP8788_DEV_##_name,				\
+	.id = -1,						\
 }
 
 #define MFD_DEV_WITH_ID(_name, _id)				\
@@ -36,6 +37,7 @@
 	.name = LP8788_DEV_##_name,				\
 	.resources = _resource,					\
 	.num_resources = num_resource,				\
+	.id = -1,						\
 }
 
 static struct resource chg_irqs[] = {
@@ -73,12 +75,13 @@ static struct resource rtc_irqs[] = {
 
 static const struct mfd_cell lp8788_devs[] = {
 	/* 4 bucks */
+	MFD_DEV_WITH_ID(BUCK, 0),
 	MFD_DEV_WITH_ID(BUCK, 1),
 	MFD_DEV_WITH_ID(BUCK, 2),
 	MFD_DEV_WITH_ID(BUCK, 3),
-	MFD_DEV_WITH_ID(BUCK, 4),
 
 	/* 12 digital ldos */
+	MFD_DEV_WITH_ID(DLDO, 0),
 	MFD_DEV_WITH_ID(DLDO, 1),
 	MFD_DEV_WITH_ID(DLDO, 2),
 	MFD_DEV_WITH_ID(DLDO, 3),
@@ -90,9 +93,9 @@ static const struct mfd_cell lp8788_devs[] = {
 	MFD_DEV_WITH_ID(DLDO, 9),
 	MFD_DEV_WITH_ID(DLDO, 10),
 	MFD_DEV_WITH_ID(DLDO, 11),
-	MFD_DEV_WITH_ID(DLDO, 12),
 
 	/* 10 analog ldos */
+	MFD_DEV_WITH_ID(ALDO, 0),
 	MFD_DEV_WITH_ID(ALDO, 1),
 	MFD_DEV_WITH_ID(ALDO, 2),
 	MFD_DEV_WITH_ID(ALDO, 3),
@@ -102,7 +105,6 @@ static const struct mfd_cell lp8788_devs[] = {
 	MFD_DEV_WITH_ID(ALDO, 7),
 	MFD_DEV_WITH_ID(ALDO, 8),
 	MFD_DEV_WITH_ID(ALDO, 9),
-	MFD_DEV_WITH_ID(ALDO, 10),
 
 	/* ADC */
 	MFD_DEV_SIMPLE(ADC),
@@ -199,7 +201,7 @@ static int lp8788_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	if (ret)
 		return ret;
 
-	return mfd_add_devices(lp->dev, -1, lp8788_devs,
+	return mfd_add_devices(lp->dev, 0, lp8788_devs,
 			       ARRAY_SIZE(lp8788_devs), NULL, 0, NULL);
 }
 
Signature · 1 line
1.8.2.3
#4 lore raw

PATCH v2 4/5 mfd: wm831x: Fix platform device ids to avoid probe failure

In reply to Bartlomiej Zolnierkiewicz

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")
Cc Johan Hovold <[email protected]>

---
stable v3.19+

 drivers/mfd/wm831x-core.c | 184 +++++++++++++++++++++++++++-------------------
 1 file changed, 107 insertions(+), 77 deletions(-)
drivers/mfd/wm831x-core.c +107 -77
index 28366a9..68a7543 100644
@@ -1014,152 +1014,159 @@ static struct resource wm831x_wdt_resources[] = {
 static const struct mfd_cell wm8310_devs[] = {
 	{
 		.name = "wm831x-backup",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-buckv",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc1_resources),
 		.resources = wm831x_dcdc1_resources,
 	},
 	{
 		.name = "wm831x-buckv",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc2_resources),
 		.resources = wm831x_dcdc2_resources,
 	},
 	{
 		.name = "wm831x-buckp",
-		.id = 3,
+		.id = 2,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc3_resources),
 		.resources = wm831x_dcdc3_resources,
 	},
 	{
 		.name = "wm831x-boostp",
-		.id = 4,
+		.id = 3,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc4_resources),
 		.resources = wm831x_dcdc4_resources,
 	},
 	{
 		.name = "wm831x-clk",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-epe",
-		.id = 1,
+		.id = 0,
 	},
 	{
 		.name = "wm831x-epe",
-		.id = 2,
+		.id = 1,
 	},
 	{
 		.name = "wm831x-gpio",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_gpio_resources),
 		.resources = wm831x_gpio_resources,
 	},
 	{
 		.name = "wm831x-hwmon",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-isink",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_isink1_resources),
 		.resources = wm831x_isink1_resources,
 	},
 	{
 		.name = "wm831x-isink",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_isink2_resources),
 		.resources = wm831x_isink2_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_ldo1_resources),
 		.resources = wm831x_ldo1_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_ldo2_resources),
 		.resources = wm831x_ldo2_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 3,
+		.id = 2,
 		.num_resources = ARRAY_SIZE(wm831x_ldo3_resources),
 		.resources = wm831x_ldo3_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 4,
+		.id = 3,
 		.num_resources = ARRAY_SIZE(wm831x_ldo4_resources),
 		.resources = wm831x_ldo4_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 5,
+		.id = 4,
 		.num_resources = ARRAY_SIZE(wm831x_ldo5_resources),
 		.resources = wm831x_ldo5_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 6,
+		.id = 5,
 		.num_resources = ARRAY_SIZE(wm831x_ldo6_resources),
 		.resources = wm831x_ldo6_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 7,
+		.id = 6,
 		.num_resources = ARRAY_SIZE(wm831x_ldo7_resources),
 		.resources = wm831x_ldo7_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 8,
+		.id = 7,
 		.num_resources = ARRAY_SIZE(wm831x_ldo8_resources),
 		.resources = wm831x_ldo8_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 9,
+		.id = 8,
 		.num_resources = ARRAY_SIZE(wm831x_ldo9_resources),
 		.resources = wm831x_ldo9_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 10,
+		.id = 9,
 		.num_resources = ARRAY_SIZE(wm831x_ldo10_resources),
 		.resources = wm831x_ldo10_resources,
 	},
 	{
 		.name = "wm831x-alive-ldo",
-		.id = 11,
+		.id = 10,
 		.num_resources = ARRAY_SIZE(wm831x_ldo11_resources),
 		.resources = wm831x_ldo11_resources,
 	},
 	{
 		.name = "wm831x-on",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_on_resources),
 		.resources = wm831x_on_resources,
 	},
 	{
 		.name = "wm831x-power",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_power_resources),
 		.resources = wm831x_power_resources,
 	},
 	{
 		.name = "wm831x-status",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_status1_resources),
 		.resources = wm831x_status1_resources,
 	},
 	{
 		.name = "wm831x-status",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_status2_resources),
 		.resources = wm831x_status2_resources,
 	},
 	{
 		.name = "wm831x-watchdog",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_wdt_resources),
 		.resources = wm831x_wdt_resources,
 	},
@@ -1168,128 +1175,135 @@ static const struct mfd_cell wm8310_devs[] = {
 static const struct mfd_cell wm8311_devs[] = {
 	{
 		.name = "wm831x-backup",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-buckv",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc1_resources),
 		.resources = wm831x_dcdc1_resources,
 	},
 	{
 		.name = "wm831x-buckv",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc2_resources),
 		.resources = wm831x_dcdc2_resources,
 	},
 	{
 		.name = "wm831x-buckp",
-		.id = 3,
+		.id = 2,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc3_resources),
 		.resources = wm831x_dcdc3_resources,
 	},
 	{
 		.name = "wm831x-boostp",
-		.id = 4,
+		.id = 3,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc4_resources),
 		.resources = wm831x_dcdc4_resources,
 	},
 	{
 		.name = "wm831x-clk",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-epe",
-		.id = 1,
+		.id = 0,
 	},
 	{
 		.name = "wm831x-epe",
-		.id = 2,
+		.id = 1,
 	},
 	{
 		.name = "wm831x-gpio",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_gpio_resources),
 		.resources = wm831x_gpio_resources,
 	},
 	{
 		.name = "wm831x-hwmon",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-isink",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_isink1_resources),
 		.resources = wm831x_isink1_resources,
 	},
 	{
 		.name = "wm831x-isink",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_isink2_resources),
 		.resources = wm831x_isink2_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_ldo1_resources),
 		.resources = wm831x_ldo1_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_ldo2_resources),
 		.resources = wm831x_ldo2_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 3,
+		.id = 2,
 		.num_resources = ARRAY_SIZE(wm831x_ldo3_resources),
 		.resources = wm831x_ldo3_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 4,
+		.id = 3,
 		.num_resources = ARRAY_SIZE(wm831x_ldo4_resources),
 		.resources = wm831x_ldo4_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 5,
+		.id = 4,
 		.num_resources = ARRAY_SIZE(wm831x_ldo5_resources),
 		.resources = wm831x_ldo5_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 7,
+		.id = 6,
 		.num_resources = ARRAY_SIZE(wm831x_ldo7_resources),
 		.resources = wm831x_ldo7_resources,
 	},
 	{
 		.name = "wm831x-alive-ldo",
-		.id = 11,
+		.id = 10,
 		.num_resources = ARRAY_SIZE(wm831x_ldo11_resources),
 		.resources = wm831x_ldo11_resources,
 	},
 	{
 		.name = "wm831x-on",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_on_resources),
 		.resources = wm831x_on_resources,
 	},
 	{
 		.name = "wm831x-power",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_power_resources),
 		.resources = wm831x_power_resources,
 	},
 	{
 		.name = "wm831x-status",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_status1_resources),
 		.resources = wm831x_status1_resources,
 	},
 	{
 		.name = "wm831x-status",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_status2_resources),
 		.resources = wm831x_status2_resources,
 	},
 	{
 		.name = "wm831x-watchdog",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_wdt_resources),
 		.resources = wm831x_wdt_resources,
 	},
@@ -1298,152 +1312,159 @@ static const struct mfd_cell wm8311_devs[] = {
 static const struct mfd_cell wm8312_devs[] = {
 	{
 		.name = "wm831x-backup",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-buckv",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc1_resources),
 		.resources = wm831x_dcdc1_resources,
 	},
 	{
 		.name = "wm831x-buckv",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc2_resources),
 		.resources = wm831x_dcdc2_resources,
 	},
 	{
 		.name = "wm831x-buckp",
-		.id = 3,
+		.id = 2,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc3_resources),
 		.resources = wm831x_dcdc3_resources,
 	},
 	{
 		.name = "wm831x-boostp",
-		.id = 4,
+		.id = 3,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc4_resources),
 		.resources = wm831x_dcdc4_resources,
 	},
 	{
 		.name = "wm831x-clk",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-epe",
-		.id = 1,
+		.id = 0,
 	},
 	{
 		.name = "wm831x-epe",
-		.id = 2,
+		.id = 1,
 	},
 	{
 		.name = "wm831x-gpio",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_gpio_resources),
 		.resources = wm831x_gpio_resources,
 	},
 	{
 		.name = "wm831x-hwmon",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-isink",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_isink1_resources),
 		.resources = wm831x_isink1_resources,
 	},
 	{
 		.name = "wm831x-isink",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_isink2_resources),
 		.resources = wm831x_isink2_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_ldo1_resources),
 		.resources = wm831x_ldo1_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_ldo2_resources),
 		.resources = wm831x_ldo2_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 3,
+		.id = 2,
 		.num_resources = ARRAY_SIZE(wm831x_ldo3_resources),
 		.resources = wm831x_ldo3_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 4,
+		.id = 3,
 		.num_resources = ARRAY_SIZE(wm831x_ldo4_resources),
 		.resources = wm831x_ldo4_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 5,
+		.id = 4,
 		.num_resources = ARRAY_SIZE(wm831x_ldo5_resources),
 		.resources = wm831x_ldo5_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 6,
+		.id = 5,
 		.num_resources = ARRAY_SIZE(wm831x_ldo6_resources),
 		.resources = wm831x_ldo6_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 7,
+		.id = 6,
 		.num_resources = ARRAY_SIZE(wm831x_ldo7_resources),
 		.resources = wm831x_ldo7_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 8,
+		.id = 7,
 		.num_resources = ARRAY_SIZE(wm831x_ldo8_resources),
 		.resources = wm831x_ldo8_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 9,
+		.id = 8,
 		.num_resources = ARRAY_SIZE(wm831x_ldo9_resources),
 		.resources = wm831x_ldo9_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 10,
+		.id = 9,
 		.num_resources = ARRAY_SIZE(wm831x_ldo10_resources),
 		.resources = wm831x_ldo10_resources,
 	},
 	{
 		.name = "wm831x-alive-ldo",
-		.id = 11,
+		.id = 10,
 		.num_resources = ARRAY_SIZE(wm831x_ldo11_resources),
 		.resources = wm831x_ldo11_resources,
 	},
 	{
 		.name = "wm831x-on",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_on_resources),
 		.resources = wm831x_on_resources,
 	},
 	{
 		.name = "wm831x-power",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_power_resources),
 		.resources = wm831x_power_resources,
 	},
 	{
 		.name = "wm831x-status",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_status1_resources),
 		.resources = wm831x_status1_resources,
 	},
 	{
 		.name = "wm831x-status",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_status2_resources),
 		.resources = wm831x_status2_resources,
 	},
 	{
 		.name = "wm831x-watchdog",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_wdt_resources),
 		.resources = wm831x_wdt_resources,
 	},
@@ -1452,127 +1473,133 @@ static const struct mfd_cell wm8312_devs[] = {
 static const struct mfd_cell wm8320_devs[] = {
 	{
 		.name = "wm831x-backup",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-buckv",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc1_resources),
 		.resources = wm831x_dcdc1_resources,
 	},
 	{
 		.name = "wm831x-buckv",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc2_resources),
 		.resources = wm831x_dcdc2_resources,
 	},
 	{
 		.name = "wm831x-buckp",
-		.id = 3,
+		.id = 2,
 		.num_resources = ARRAY_SIZE(wm831x_dcdc3_resources),
 		.resources = wm831x_dcdc3_resources,
 	},
 	{
 		.name = "wm831x-buckp",
-		.id = 4,
+		.id = 3,
 		.num_resources = ARRAY_SIZE(wm8320_dcdc4_buck_resources),
 		.resources = wm8320_dcdc4_buck_resources,
 	},
 	{
 		.name = "wm831x-clk",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-gpio",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_gpio_resources),
 		.resources = wm831x_gpio_resources,
 	},
 	{
 		.name = "wm831x-hwmon",
+		.id = -1,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_ldo1_resources),
 		.resources = wm831x_ldo1_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_ldo2_resources),
 		.resources = wm831x_ldo2_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 3,
+		.id = 2,
 		.num_resources = ARRAY_SIZE(wm831x_ldo3_resources),
 		.resources = wm831x_ldo3_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 4,
+		.id = 3,
 		.num_resources = ARRAY_SIZE(wm831x_ldo4_resources),
 		.resources = wm831x_ldo4_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 5,
+		.id = 4,
 		.num_resources = ARRAY_SIZE(wm831x_ldo5_resources),
 		.resources = wm831x_ldo5_resources,
 	},
 	{
 		.name = "wm831x-ldo",
-		.id = 6,
+		.id = 5,
 		.num_resources = ARRAY_SIZE(wm831x_ldo6_resources),
 		.resources = wm831x_ldo6_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 7,
+		.id = 6,
 		.num_resources = ARRAY_SIZE(wm831x_ldo7_resources),
 		.resources = wm831x_ldo7_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 8,
+		.id = 7,
 		.num_resources = ARRAY_SIZE(wm831x_ldo8_resources),
 		.resources = wm831x_ldo8_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 9,
+		.id = 8,
 		.num_resources = ARRAY_SIZE(wm831x_ldo9_resources),
 		.resources = wm831x_ldo9_resources,
 	},
 	{
 		.name = "wm831x-aldo",
-		.id = 10,
+		.id = 9,
 		.num_resources = ARRAY_SIZE(wm831x_ldo10_resources),
 		.resources = wm831x_ldo10_resources,
 	},
 	{
 		.name = "wm831x-alive-ldo",
-		.id = 11,
+		.id = 10,
 		.num_resources = ARRAY_SIZE(wm831x_ldo11_resources),
 		.resources = wm831x_ldo11_resources,
 	},
 	{
 		.name = "wm831x-on",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_on_resources),
 		.resources = wm831x_on_resources,
 	},
 	{
 		.name = "wm831x-status",
-		.id = 1,
+		.id = 0,
 		.num_resources = ARRAY_SIZE(wm831x_status1_resources),
 		.resources = wm831x_status1_resources,
 	},
 	{
 		.name = "wm831x-status",
-		.id = 2,
+		.id = 1,
 		.num_resources = ARRAY_SIZE(wm831x_status2_resources),
 		.resources = wm831x_status2_resources,
 	},
 	{
 		.name = "wm831x-watchdog",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_wdt_resources),
 		.resources = wm831x_wdt_resources,
 	},
@@ -1581,6 +1608,7 @@ static const struct mfd_cell wm8320_devs[] = {
 static const struct mfd_cell touch_devs[] = {
 	{
 		.name = "wm831x-touch",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_touch_resources),
 		.resources = wm831x_touch_resources,
 	},
@@ -1589,6 +1617,7 @@ static const struct mfd_cell touch_devs[] = {
 static const struct mfd_cell rtc_devs[] = {
 	{
 		.name = "wm831x-rtc",
+		.id = -1,
 		.num_resources = ARRAY_SIZE(wm831x_rtc_resources),
 		.resources = wm831x_rtc_resources,
 	},
@@ -1597,6 +1626,7 @@ static const struct mfd_cell rtc_devs[] = {
 static const struct mfd_cell backlight_devs[] = {
 	{
 		.name = "wm831x-backlight",
+		.id = -1,
 	},
 };
 
@@ -1774,7 +1804,7 @@ int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq)
 	if (pdata && pdata->wm831x_num)
 		wm831x_num = pdata->wm831x_num * 10;
 	else
-		wm831x_num = -1;
+		wm831x_num = 0;
 
 	ret = wm831x_irq_init(wm831x, irq);
 	if (ret != 0)
Signature · 1 line
1.8.2.3
#5 lore raw

PATCH v2 5/5 mfd: da9052: Fix platform device names

In reply to Bartlomiej Zolnierkiewicz

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

In case of da9052 mfd driver the issue has already been fixed by commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision"). Unfortunately as the fix makes auto devid generation to be used for all mfd platform devices it causes unneccessary change of platform device names. Fix the driver to use the original names.

Cc Johan Hovold <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc Support Opensource <[email protected]>
---
 drivers/mfd/da9052-core.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)
drivers/mfd/da9052-core.c +16 -5
index ae498b5..77d41ad 100644
@@ -433,6 +433,10 @@ EXPORT_SYMBOL_GPL(da9052_adc_read_temp);
 static const struct mfd_cell da9052_subdev_info[] = {
 	{
 		.name = "da9052-regulator",
+		.id = 0,
+	},
+	{
+		.name = "da9052-regulator",
 		.id = 1,
 	},
 	{
@@ -484,41 +488,48 @@ static const struct mfd_cell da9052_subdev_info[] = {
 		.id = 13,
 	},
 	{
-		.name = "da9052-regulator",
-		.id = 14,
-	},
-	{
 		.name = "da9052-onkey",
+		.id = -1,
 	},
 	{
 		.name = "da9052-rtc",
+		.id = -1,
 	},
 	{
 		.name = "da9052-gpio",
+		.id = -1,
 	},
 	{
 		.name = "da9052-hwmon",
+		.id = -1,
 	},
 	{
 		.name = "da9052-leds",
+		.id = -1,
 	},
 	{
 		.name = "da9052-wled1",
+		.id = -1,
 	},
 	{
 		.name = "da9052-wled2",
+		.id = -1,
 	},
 	{
 		.name = "da9052-wled3",
+		.id = -1,
 	},
 	{
 		.name = "da9052-tsi",
+		.id = -1,
 	},
 	{
 		.name = "da9052-bat",
+		.id = -1,
 	},
 	{
 		.name = "da9052-watchdog",
+		.id = -1,
 	},
 };
 
@@ -554,7 +565,7 @@ int da9052_device_init(struct da9052 *da9052, u8 chip_id)
 		return ret;
 	}
 
-	ret = mfd_add_devices(da9052->dev, PLATFORM_DEVID_AUTO,
+	ret = mfd_add_devices(da9052->dev, 0,
 			      da9052_subdev_info,
 			      ARRAY_SIZE(da9052_subdev_info), NULL, 0, NULL);
 	if (ret) {
Signature · 1 line
1.8.2.3
#6 lore raw
In reply to Bartlomiej Zolnierkiewicz
Quoted 8 lines · Bartlomiej Zolnierkiewicz

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

First of all, thanks for finding these. I obviously overlooked this class of drivers when fixing the device-id generation.

Quoted 2 lines

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

This is however not the right fix. Instead you should be using PLATFORM_DEVID_AUTO and keep the non-zero cell-ids as is, as this will allow more than one mfd-device to be registered without resorting to hacks.

Have a look at wm831x, for example, which offsets the device id base if a wm831x_num parameter is passed in platform data (non-dt) to allow more than one device to be registered. Your patch would break this, and still not provide any other way to have multiple devices in a system.

Your changes would also break da9052, which would no longer allow more than one device to be registered, something which was already fixed once by commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision").

Thanks,
Johan

#7 lore raw

Hi,

Quoted 14 lines · Johan Hovold
Quoted 8 lines · Bartlomiej Zolnierkiewicz

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

First of all, thanks for finding these. I obviously overlooked this class of drivers when fixing the device-id generation.

Quoted 7 lines
Quoted 2 lines

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

This is however not the right fix. Instead you should be using PLATFORM_DEVID_AUTO and keep the non-zero cell-ids as is, as this will allow more than one mfd-device to be registered without resorting to hacks.

Conversion of a driver to use PLATFORM_DEVID_AUTO changes names of all platform devices registered by the driver. In my patchset I just tried to restore the broken functionality (patch that broke it went in v3.19). I thought that it was the best solution for v3.19 and v4.0-rc4 (then PLATFORM_DEVID_AUTO conversion can be done sometime later, i.e. in v4.1).

Quoted 4 lines

Have a look at wm831x, for example, which offsets the device id base if a wm831x_num parameter is passed in platform data (non-dt) to allow more than one device to be registered. Your patch would break this, and still not provide any other way to have multiple devices in a system.

OK, I see the problem with wm831x driver and will provide v3 of my patchset which would use PLATFORM_DEVID_AUTO for all drivers.

Quoted 4 lines

Your changes would also break da9052, which would no longer allow more than one device to be registered, something which was already fixed once by commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision").

OK, patch #5 should be just dropped.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

#8 lore raw
Quoted 23 lines · Bartlomiej Zolnierkiewicz
Quoted 14 lines · Johan Hovold
Quoted 8 lines · Bartlomiej Zolnierkiewicz

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

First of all, thanks for finding these. I obviously overlooked this class of drivers when fixing the device-id generation.

Quoted 7 lines
Quoted 2 lines

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

This is however not the right fix. Instead you should be using PLATFORM_DEVID_AUTO and keep the non-zero cell-ids as is, as this will allow more than one mfd-device to be registered without resorting to hacks.

Quoted 6 lines

Conversion of a driver to use PLATFORM_DEVID_AUTO changes names of all platform devices registered by the driver. In my patchset I just tried to restore the broken functionality (patch that broke it went in v3.19). I thought that it was the best solution for v3.19 and v4.0-rc4 (then PLATFORM_DEVID_AUTO conversion can be done sometime later, i.e. in v4.1).

I think you're right. We should do the conversion to PLATFORM_DEVID_AUTO later.

I'm responding to this mail with two patches for v4.0 fixing the da9052 driver, which is currently broken although not in the way suggested by your v1-series, and the other drivers you identified by doing a partial revert of commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation").

Thanks,
Johan

#9 lore raw

PATCH 1/2 mfd: da9052: fix broken regulator probe

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/mfd/da9052-core.c +4 -4
index ae498b53ee40..46e3840c7a37 100644
@@ -433,6 +433,10 @@ EXPORT_SYMBOL_GPL(da9052_adc_read_temp);
 static const struct mfd_cell da9052_subdev_info[] = {
 	{
 		.name = "da9052-regulator",
+		.id = 0,
+	},
+	{
+		.name = "da9052-regulator",
 		.id = 1,
 	},
 	{
@@ -484,10 +488,6 @@ static const struct mfd_cell da9052_subdev_info[] = {
 		.id = 13,
 	},
 	{
-		.name = "da9052-regulator",
-		.id = 14,
-	},
-	{
 		.name = "da9052-onkey",
 	},
 	{
drivers/regulator/da9052-regulator.c +3 -2
index 8a4df7a1f2ee..e628d4c2f2ae 100644
@@ -394,6 +394,7 @@ static inline struct da9052_regulator_info *find_regulator_info(u8 chip_id,
 
 static int da9052_regulator_probe(struct platform_device *pdev)
 {
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
 	struct regulator_config config = { };
 	struct da9052_regulator *regulator;
 	struct da9052 *da9052;
@@ -409,7 +410,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	regulator->da9052 = da9052;
 
 	regulator->info = find_regulator_info(regulator->da9052->chip_id,
-					      pdev->id);
+					      cell->id);
 	if (regulator->info == NULL) {
 		dev_err(&pdev->dev, "invalid regulator ID specified\n");
 		return -EINVAL;
@@ -419,7 +420,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	config.driver_data = regulator;
 	config.regmap = da9052->regmap;
 	if (pdata && pdata->regulators) {
-		config.init_data = pdata->regulators[pdev->id];
+		config.init_data = pdata->regulators[cell->id];
 	} else {
 #ifdef CONFIG_OF
 		struct device_node *nproot = da9052->dev->of_node;
Signature · 1 line
2.0.5
#10 lore raw

PATCH 2/2 mfd: core: fix platform-device name collisions

Since commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") we honour PLATFORM_DEVID_AUTO and PLATFORM_DEVID_NONE when registering mfd-devices.

Unfortunately, some mfd-drivers rely on the old behaviour of generating platform-device ids by adding the cell id also to the special value of PLATFORM_DEVID_NONE. The resulting platform ids are not only used to generate device-unique names, but are also used instead of the cell id to identify cells when probing subdevices.

These drivers should be updated to use PLATFORM_DEVID_AUTO, which would also allow more than one device to be registered without resorting to hacks (see for example wm831x), but lets fix the regression first by partially reverting the above mentioned commit with respect to PLATFORM_DEVID_NONE.

Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/mfd-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mfd/mfd-core.c +1 -1
index 2a87f69be53d..1aed3b7b8d9b 100644
@@ -128,7 +128,7 @@ static int mfd_add_device(struct device *parent, int id,
 	int platform_id;
 	int r;
 
-	if (id < 0)
+	if (id == PLATFORM_DEVID_AUTO)
 		platform_id = id;
 	else
 		platform_id = id + cell->id;
Signature · 1 line
2.0.5
author Acked-by on v1 2/2
#11 lore raw

Re: [PATCH 2/2] mfd: core: fix platform-device name collisions

Hi,

Quoted 16 lines · Johan Hovold

Since commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") we honour PLATFORM_DEVID_AUTO and PLATFORM_DEVID_NONE when registering mfd-devices.

Unfortunately, some mfd-drivers rely on the old behaviour of generating platform-device ids by adding the cell id also to the special value of PLATFORM_DEVID_NONE. The resulting platform ids are not only used to generate device-unique names, but are also used instead of the cell id to identify cells when probing subdevices.

These drivers should be updated to use PLATFORM_DEVID_AUTO, which would also allow more than one device to be registered without resorting to hacks (see for example wm831x), but lets fix the regression first by partially reverting the above mentioned commit with respect to PLATFORM_DEVID_NONE.

Quoted 4 lines
Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")
Cc stable <[email protected]> # v3.19

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Quoted 17 lines
---
 drivers/mfd/mfd-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mfd/mfd-core.c +1 -1
index 2a87f69be53d..1aed3b7b8d9b 100644
@@ -128,7 +128,7 @@ static int mfd_add_device(struct device *parent, int id,
 	int platform_id;
 	int r;
 
-	if (id < 0)
+	if (id == PLATFORM_DEVID_AUTO)
 		platform_id = id;
 	else
 		platform_id = id + cell->id;
Applied
#12 lore raw

Re: [PATCH 2/2] mfd: core: fix platform-device name collisions

In reply to Johan Hovold
Quoted 16 lines · Johan Hovold

Since commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") we honour PLATFORM_DEVID_AUTO and PLATFORM_DEVID_NONE when registering mfd-devices.

Unfortunately, some mfd-drivers rely on the old behaviour of generating platform-device ids by adding the cell id also to the special value of PLATFORM_DEVID_NONE. The resulting platform ids are not only used to generate device-unique names, but are also used instead of the cell id to identify cells when probing subdevices.

These drivers should be updated to use PLATFORM_DEVID_AUTO, which would also allow more than one device to be registered without resorting to hacks (see for example wm831x), but lets fix the regression first by partially reverting the above mentioned commit with respect to PLATFORM_DEVID_NONE.

Quoted 7 lines
Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/mfd-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

Quoted 13 lines
drivers/mfd/mfd-core.c +1 -1
index 2a87f69be53d..1aed3b7b8d9b 100644
@@ -128,7 +128,7 @@ static int mfd_add_device(struct device *parent, int id,
 	int platform_id;
 	int r;
 
-	if (id < 0)
+	if (id == PLATFORM_DEVID_AUTO)
 		platform_id = id;
 	else
 		platform_id = id + cell->id;
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author Acked-by on v1 1/2
#13 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

In reply to Johan Hovold

Hi,

Quoted 6 lines · Johan Hovold

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Quoted 6 lines

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Quoted 61 lines
---
 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/mfd/da9052-core.c +4 -4
index ae498b53ee40..46e3840c7a37 100644
@@ -433,6 +433,10 @@ EXPORT_SYMBOL_GPL(da9052_adc_read_temp);
 static const struct mfd_cell da9052_subdev_info[] = {
 	{
 		.name = "da9052-regulator",
+		.id = 0,
+	},
+	{
+		.name = "da9052-regulator",
 		.id = 1,
 	},
 	{
@@ -484,10 +488,6 @@ static const struct mfd_cell da9052_subdev_info[] = {
 		.id = 13,
 	},
 	{
-		.name = "da9052-regulator",
-		.id = 14,
-	},
-	{
 		.name = "da9052-onkey",
 	},
 	{
drivers/regulator/da9052-regulator.c +3 -2
index 8a4df7a1f2ee..e628d4c2f2ae 100644
@@ -394,6 +394,7 @@ static inline struct da9052_regulator_info *find_regulator_info(u8 chip_id,
 
 static int da9052_regulator_probe(struct platform_device *pdev)
 {
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
 	struct regulator_config config = { };
 	struct da9052_regulator *regulator;
 	struct da9052 *da9052;
@@ -409,7 +410,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	regulator->da9052 = da9052;
 
 	regulator->info = find_regulator_info(regulator->da9052->chip_id,
-					      pdev->id);
+					      cell->id);
 	if (regulator->info == NULL) {
 		dev_err(&pdev->dev, "invalid regulator ID specified\n");
 		return -EINVAL;
@@ -419,7 +420,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	config.driver_data = regulator;
 	config.regmap = da9052->regmap;
 	if (pdata && pdata->regulators) {
-		config.init_data = pdata->regulators[pdev->id];
+		config.init_data = pdata->regulators[cell->id];
 	} else {
 #ifdef CONFIG_OF
 		struct device_node *nproot = da9052->dev->of_node;
Acked-by on v1 1/2
#14 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

In reply to Johan Hovold
Quoted 9 lines · Johan Hovold

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Quoted 6 lines
Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--

Need an Ack from Mark, but looks good to me.

Quoted 58 lines
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/mfd/da9052-core.c +4 -4
index ae498b53ee40..46e3840c7a37 100644
@@ -433,6 +433,10 @@ EXPORT_SYMBOL_GPL(da9052_adc_read_temp);
 static const struct mfd_cell da9052_subdev_info[] = {
 	{
 		.name = "da9052-regulator",
+		.id = 0,
+	},
+	{
+		.name = "da9052-regulator",
 		.id = 1,
 	},
 	{
@@ -484,10 +488,6 @@ static const struct mfd_cell da9052_subdev_info[] = {
 		.id = 13,
 	},
 	{
-		.name = "da9052-regulator",
-		.id = 14,
-	},
-	{
 		.name = "da9052-onkey",
 	},
 	{
drivers/regulator/da9052-regulator.c +3 -2
index 8a4df7a1f2ee..e628d4c2f2ae 100644
@@ -394,6 +394,7 @@ static inline struct da9052_regulator_info *find_regulator_info(u8 chip_id,
 
 static int da9052_regulator_probe(struct platform_device *pdev)
 {
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
 	struct regulator_config config = { };
 	struct da9052_regulator *regulator;
 	struct da9052 *da9052;
@@ -409,7 +410,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	regulator->da9052 = da9052;
 
 	regulator->info = find_regulator_info(regulator->da9052->chip_id,
-					      pdev->id);
+					      cell->id);
 	if (regulator->info == NULL) {
 		dev_err(&pdev->dev, "invalid regulator ID specified\n");
 		return -EINVAL;
@@ -419,7 +420,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	config.driver_data = regulator;
 	config.regmap = da9052->regmap;
 	if (pdata && pdata->regulators) {
-		config.init_data = pdata->regulators[pdev->id];
+		config.init_data = pdata->regulators[cell->id];
 	} else {
 #ifdef CONFIG_OF
 		struct device_node *nproot = da9052->dev->of_node;
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#15 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Hi Mark and Lee,

Quoted 18 lines · Lee Jones
Quoted 9 lines · Johan Hovold

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Quoted 6 lines
Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
Quoted 3 lines

Need an Ack from Mark, but looks good to me.

Mark, did you have a chance to look at this one?

It fixes a regression due to an mfd commit, but not sure who of you that wants to take it.

Thanks,
Johan

#16 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 21 lines · Johan Hovold

Hi Mark and Lee,

Quoted 18 lines · Lee Jones
Quoted 15 lines · Johan Hovold

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
Quoted 8 lines
Quoted 3 lines

Need an Ack from Mark, but looks good to me.

Mark, did you have a chance to look at this one?

It fixes a regression due to an mfd commit, but not sure who of you that wants to take it.

It's now been over a month and this still hasn't been applied even though it fixes a regression.

Lee, perhaps you should take it through MFD as it was an MFD commit that broke it?

Johan

#17 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 28 lines · Johan Hovold
Quoted 27 lines · Johan Hovold

Hi Mark and Lee,

Quoted 18 lines · Lee Jones
Quoted 9 lines · Johan Hovold

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Quoted 6 lines
Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
Quoted 3 lines

Need an Ack from Mark, but looks good to me.

Mark, did you have a chance to look at this one?

Quoted 8 lines
Quoted 2 lines

It fixes a regression due to an mfd commit, but not sure who of you that wants to take it.

It's now been over a month and this still hasn't been applied even though it fixes a regression.

Lee, perhaps you should take it through MFD as it was an MFD commit that broke it?

I'm in the same situation -- can't do anything without an Ack from Mark.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#18 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 33 lines · Lee Jones
Quoted 31 lines · Johan Hovold
Quoted 29 lines · Johan Hovold

Hi Mark and Lee,

Quoted 18 lines · Lee Jones
Quoted 9 lines · Johan Hovold

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Quoted 6 lines
Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
Quoted 3 lines

Need an Ack from Mark, but looks good to me.

Mark, did you have a chance to look at this one?

It fixes a regression due to an mfd commit, but not sure who of you that wants to take it.

Quoted 7 lines
Quoted 5 lines

It's now been over a month and this still hasn't been applied even though it fixes a regression.

Lee, perhaps you should take it through MFD as it was an MFD commit that broke it?

I'm in the same situation -- can't do anything without an Ack from Mark.

I happen to know that Mark regularly deletes his mail.

Can you re-sent this complete with collected Acks please?

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
maintainer
#19 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 7 lines · Lee Jones
Quoted 1 line · Lee Jones

I'm in the same situation -- can't do anything without an Ack from Mark.

I happen to know that Mark regularly deletes his mail.

Can you re-sent this complete with collected Acks please?

If you're looking for me to review something you need to send it to me, and the chances of me looking at it are very much increased if there's a relevant subject line. I'm CCed (not even on the to list) on endless large threads and reposts of patch serieses about MFD drivers most of which are of very little relevance to me so they get ignored very easily.

#20 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 7 lines · Mark Brown
Quoted 5 lines · Lee Jones
Quoted 1 line · Lee Jones

I'm in the same situation -- can't do anything without an Ack from Mark.

I happen to know that Mark regularly deletes his mail.

Quoted 8 lines
Quoted 1 line

Can you re-sent this complete with collected Acks please?

If you're looking for me to review something you need to send it to me, and the chances of me looking at it are very much increased if there's a relevant subject line. I'm CCed (not even on the to list) on endless large threads and reposts of patch serieses about MFD drivers most of which are of very little relevance to me so they get ignored very easily.

Calm down dear, it's only a commercial.

I wasn't having a pop. Rather empathising with your situation and facilitating a resend that you're likely to see.

I'm sure Johan will do the right thing.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
maintainer
#21 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 9 lines · Lee Jones
Quoted 6 lines · Mark Brown

If you're looking for me to review something you need to send it to me, and the chances of me looking at it are very much increased if there's a relevant subject line. I'm CCed (not even on the to list) on endless large threads and reposts of patch serieses about MFD drivers most of which are of very little relevance to me so they get ignored very easily.

Quoted 6 lines

Calm down dear, it's only a commercial.

I wasn't having a pop. Rather empathising with your situation and facilitating a resend that you're likely to see.

I'm sure Johan will do the right thing.

My point is that a simple resend has a reasonable chance of getting dropped on the floor.

#22 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 12 lines · Mark Brown
Quoted 10 lines · Lee Jones
Quoted 6 lines · Mark Brown

If you're looking for me to review something you need to send it to me, and the chances of me looking at it are very much increased if there's a relevant subject line. I'm CCed (not even on the to list) on endless large threads and reposts of patch serieses about MFD drivers most of which are of very little relevance to me so they get ignored very easily.

Calm down dear, it's only a commercial.

Quoted 7 lines
Quoted 4 lines

I wasn't having a pop. Rather empathising with your situation and facilitating a resend that you're likely to see.

I'm sure Johan will do the right thing.

My point is that a simple resend has a reasonable chance of getting dropped on the floor.

As I say, I'm sure Johan will do what's required for that not to happen.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#23 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 17 lines · Lee Jones
Quoted 15 lines · Mark Brown
Quoted 13 lines · Lee Jones
Quoted 6 lines · Mark Brown

If you're looking for me to review something you need to send it to me, and the chances of me looking at it are very much increased if there's a relevant subject line. I'm CCed (not even on the to list) on endless large threads and reposts of patch serieses about MFD drivers most of which are of very little relevance to me so they get ignored very easily.

Calm down dear, it's only a commercial.

I wasn't having a pop. Rather empathising with your situation and facilitating a resend that you're likely to see.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

I'm sure Johan will do the right thing.

My point is that a simple resend has a reasonable chance of getting dropped on the floor.

As I say, I'm sure Johan will do what's required for that not to happen.

Seriously? *Me* do the right thing?

We have a regression in your subsystems (mfd/regulator) with a fix that's been sitting in both your mailboxes since March 25th.

And now that Mark finally responds to a mail after repeated reminders from both me and Lee, he still doesn't care enough to look up the original mail or at least make sure that a resend does not get lost again.

You two even work for the same organisation. Just pick up the phone or find a way to get a message across that mail filter instead of wasting everyone else's time.

I have resent the patch now. Please make sure it gets applied.

Johan

#24 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 20 lines · Johan Hovold
Quoted 19 lines · Lee Jones
Quoted 17 lines · Mark Brown
Quoted 15 lines · Lee Jones
Quoted 6 lines · Mark Brown

If you're looking for me to review something you need to send it to me, and the chances of me looking at it are very much increased if there's a relevant subject line. I'm CCed (not even on the to list) on endless large threads and reposts of patch serieses about MFD drivers most of which are of very little relevance to me so they get ignored very easily.

Calm down dear, it's only a commercial.

I wasn't having a pop. Rather empathising with your situation and facilitating a resend that you're likely to see.

I'm sure Johan will do the right thing.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

My point is that a simple resend has a reasonable chance of getting dropped on the floor.

As I say, I'm sure Johan will do what's required for that not to happen.

Seriously? *Me* do the right thing?

Yes, *you*. If a patch slips though a Maintainer's net, which does happen every so often [*], I'm sure even you are not infallible to that, a submitter must issue a RESEND (as you have now just done so).

Quoted 2 lines

We have a regression in your subsystems (mfd/regulator) with a fix that's been sitting in both your mailboxes since March 25th.

Fully aware and ready to apply once the correct process has been carried out. I get shirty when people submit MFD patches without permission, and I refuse to be a hypocrite.

Quoted 4 lines

And now that Mark finally responds to a mail after repeated reminders from both me and Lee, he still doesn't care enough to look up the original mail or at least make sure that a resend does not get lost again.

See [*] about how Mark deals with his torrent of daily mail.

Quoted 1 line

You two even work for the same organisation.

Irrelevant.

Quoted 5 lines

Just pick up the phone or
find a way to get a message across that mail filter instead of wasting everyone else's time.

I have resent the patch now. Please make sure it gets applied.

[*] Especially when you receive as much email as Mark does. To cope he has a brain-highpass-filter on the subject line and purges his inbox regularly.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#25 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 24 lines · Lee Jones
Quoted 23 lines · Johan Hovold
Quoted 22 lines · Lee Jones
Quoted 19 lines · Mark Brown
Quoted 9 lines · Lee Jones
Quoted 6 lines · Mark Brown

If you're looking for me to review something you need to send it to me, and the chances of me looking at it are very much increased if there's a relevant subject line. I'm CCed (not even on the to list) on endless large threads and reposts of patch serieses about MFD drivers most of which are of very little relevance to me so they get ignored very easily.

Quoted 6 lines

Calm down dear, it's only a commercial.

I wasn't having a pop. Rather empathising with your situation and facilitating a resend that you're likely to see.

I'm sure Johan will do the right thing.

My point is that a simple resend has a reasonable chance of getting dropped on the floor.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

As I say, I'm sure Johan will do what's required for that not to happen.

Seriously? *Me* do the right thing?

Yes, *you*. If a patch slips though a Maintainer's net, which does happen every so often [*], I'm sure even you are not infallible to that, a submitter must issue a RESEND (as you have now just done so).

As you know, five reminders asking for an ack from Mark was sent by the two of us combined without even an indication that the messages had been noted over a period of almost two months.

If Mark feels that he is getting spammed with unrelated MFD patches, then *you* and Mark need to figure out a way to get a message across when there actually is something he needs to look at.

I don't care if it's with a special [Lee-wants-Marks-ack] subject prefix, an irc message on Linaro's channels or a phone call, but it's not something that a patch submitter for MFD should need to know about (it obviously isn't even documented).

Quoted 6 lines
Quoted 2 lines

We have a regression in your subsystems (mfd/regulator) with a fix that's been sitting in both your mailboxes since March 25th.

Fully aware and ready to apply once the correct process has been carried out. I get shirty when people submit MFD patches without permission, and I refuse to be a hypocrite.

That's perfectly fine. Your subsystems intersect and you two need to figure out how you communicate. That's all.

Johan

#26 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 50 lines · Johan Hovold
Quoted 24 lines · Lee Jones
Quoted 23 lines · Johan Hovold
Quoted 22 lines · Lee Jones
Quoted 19 lines · Mark Brown
Quoted 9 lines · Lee Jones
Quoted 6 lines · Mark Brown

If you're looking for me to review something you need to send it to me, and the chances of me looking at it are very much increased if there's a relevant subject line. I'm CCed (not even on the to list) on endless large threads and reposts of patch serieses about MFD drivers most of which are of very little relevance to me so they get ignored very easily.

Quoted 6 lines

Calm down dear, it's only a commercial.

I wasn't having a pop. Rather empathising with your situation and facilitating a resend that you're likely to see.

I'm sure Johan will do the right thing.

My point is that a simple resend has a reasonable chance of getting dropped on the floor.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

As I say, I'm sure Johan will do what's required for that not to happen.

Seriously? *Me* do the right thing?

Yes, *you*. If a patch slips though a Maintainer's net, which does happen every so often [*], I'm sure even you are not infallible to that, a submitter must issue a RESEND (as you have now just done so).

As you know, five reminders asking for an ack from Mark was sent by the two of us combined without even an indication that the messages had been noted over a period of almost two months.

If Mark feels that he is getting spammed with unrelated MFD patches, then *you* and Mark need to figure out a way to get a message across when there actually is something he needs to look at.

I don't care if it's with a special [Lee-wants-Marks-ack] subject prefix, an irc message on Linaro's channels or a phone call, but it's not something that a patch submitter for MFD should need to know about (it obviously isn't even documented).

Quoted 3 lines
Quoted 2 lines

We have a regression in your subsystems (mfd/regulator) with a fix that's been sitting in both your mailboxes since March 25th.

Quoted 6 lines
Quoted 3 lines

Fully aware and ready to apply once the correct process has been carried out. I get shirty when people submit MFD patches without permission, and I refuse to be a hypocrite.

That's perfectly fine. Your subsystems intersect and you two need to figure out how you communicate. That's all.

This issue is out of the ordinary. Normally Mark is pretty good at providing me with the Acks I need. More commonly I have issues such as the one you are experiencing with non-responsive/inactive Maintainers.

In future, for yourself and anyone else who is following this thread for 'fun', if a patch crosses multiple subsystems (which I try to keep to a minimum) it's probably best to indicate that in the subject line.

mfd: regulator: <blah>

... would definitely get Marks attention. And yes, I know 'regulator' is mentioned in the subject line of the particular patch. :)

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
maintainer
#27 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 8 lines · Lee Jones

In future, for yourself and anyone else who is following this thread for 'fun', if a patch crosses multiple subsystems (which I try to keep to a minimum) it's probably best to indicate that in the subject line.

mfd: regulator: <blah>

... would definitely get Marks attention. And yes, I know 'regulator' is mentioned in the subject line of the particular patch. :)

Yeah, sadly it's not visible in the thread view in mutt here since it's too late in the subject line. That is good advice, though - the standard thing is either like you describe or "mfd/regulator: ".

maintainer
#28 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

In reply to Johan Hovold
Quoted 9 lines · Johan Hovold
Quoted 3 lines · Lee Jones

Yes, *you*. If a patch slips though a Maintainer's net, which does happen every so often [*], I'm sure even you are not infallible to that, a submitter must issue a RESEND (as you have now just done so).

As you know, five reminders asking for an ack from Mark was sent by the two of us combined without even an indication that the messages had been noted over a period of almost two months.

The reason that there was no indication that the message had been noted was that the message had in fact not been seen, nor had any of your pings; one of the big problems here is that you are sending pings instead of resending (which is something that does get frequent pushback, Lee did advise you to resend).

Quoted 3 lines

If Mark feels that he is getting spammed with unrelated MFD patches, then *you* and Mark need to figure out a way to get a message across when there actually is something he needs to look at.

Like I say, resending is the main advice here.

Quoted 4 lines

I don't care if it's with a special [Lee-wants-Marks-ack] subject prefix, an irc message on Linaro's channels or a phone call, but it's not something that a patch submitter for MFD should need to know about (it obviously isn't even documented).

Resending is something that is pretty standard. Most of the things that can result in something not getting looked at also involve no longer having a copy of the patch (so a resend will be needed anyway), and often many of the others will result in the ping not being seen either (for example it gets threaded in with the patch buried in the mailbox, or it looks like the patch is generating lots of discussion and will need a new version anyway). At best it's going to require going and finding the original mail, and they can be actively unhelpful.

A fresh copy of the patch in contrast fits naturally into the standard workflow with no extra barriers.

#29 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 22 lines · Mark Brown
Quoted 3 lines · Johan Hovold

If Mark feels that he is getting spammed with unrelated MFD patches, then *you* and Mark need to figure out a way to get a message across when there actually is something he needs to look at.

Like I say, resending is the main advice here.

Quoted 4 lines

I don't care if it's with a special [Lee-wants-Marks-ack] subject prefix, an irc message on Linaro's channels or a phone call, but it's not something that a patch submitter for MFD should need to know about (it obviously isn't even documented).

Resending is something that is pretty standard. Most of the things that can result in something not getting looked at also involve no longer having a copy of the patch (so a resend will be needed anyway), and often many of the others will result in the ping not being seen either (for example it gets threaded in with the patch buried in the mailbox, or it looks like the patch is generating lots of discussion and will need a new version anyway). At best it's going to require going and finding the original mail, and they can be actively unhelpful.

Quoted 2 lines

A fresh copy of the patch in contrast fits naturally into the standard workflow with no extra barriers.

Yes, resending is sometimes needed, but what set me off here was your comment that resending might not be enough even after you've now become aware of a several-month old regression in your subsystem.

I know you process a lot of mail, but perhaps some (further) filtering could help avoid situations like this. The patch touches drivers/regulator/ and has a stable tag for example.

Johan

maintainer
#30 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 3 lines · Johan Hovold

Yes, resending is sometimes needed, but what set me off here was your comment that resending might not be enough even after you've now become aware of a several-month old regression in your subsystem.

If you're referring to my original reply I'm afraid to disappoint you but I hadn't read far enough in the backtrace to see anything except that people wanted me to look at a patch I didn't have a copy of (I didn't even know if I'd been CCed on the original posting). I was simply trying to say that it might be worth looking at other aspects of how the patch was sent - what you got there was basically a form letter type response to contentless pings.

Quoted 3 lines

I know you process a lot of mail, but perhaps some (further) filtering could help avoid situations like this. The patch touches drivers/regulator/ and has a stable tag for example.

Neither of those is reliable enough for mechanical filtering for the things I'm doing here I'm afraid, and I especially don't think it would be a good idea people to get the idea that adding a stable tag is a good way of jumping the queue and it's not that reliable an indication of urgency (some development only issues that cause widespread breakage are much more urgent, some stable patches are for things that are definite issues with clear fixes but relatively low risk/impact).

#31 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 6 lines · Mark Brown
Quoted 3 lines · Johan Hovold

Yes, resending is sometimes needed, but what set me off here was your comment that resending might not be enough even after you've now become aware of a several-month old regression in your subsystem.

Quoted 7 lines

If you're referring to my original reply I'm afraid to disappoint you but I hadn't read far enough in the backtrace to see anything except that people wanted me to look at a patch I didn't have a copy of (I didn't even know if I'd been CCed on the original posting). I was simply trying to say that it might be worth looking at other aspects of how the patch was sent - what you got there was basically a form letter type response to contentless pings.

I might have read too much into that response, but the pings were hardly contentless (they all included the commit message and some the full patch).

Anyway, glad to see the issue resolved now. Thanks for the review.

Johan

maintainer
#32 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 2 lines · Johan Hovold

I might have read too much into that response, but the pings were hardly contentless (they all included the commit message and some the full patch).

It's the new content that is important here - if it boils down to "have you looked at this yet" that's not really adding anything and if it's a patch to be applied then it really needs to not be quoted.

#33 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 8 lines · Mark Brown
Quoted 2 lines · Johan Hovold

I might have read too much into that response, but the pings were hardly contentless (they all included the commit message and some the full patch).

It's the new content that is important here - if it boils down to "have you looked at this yet" that's not really adding anything and if it's a patch to be applied then it really needs to not be quoted.

My reminders sent directly to you explicitly mentioned it being a regression every time. That should trigger a maintainer's interest enough to look at the quoted context or ask for a resend.

How should a patch submitter know that you simply drop mails with an mfd prefix even if it's directed to you? Unless documented somewhere, that's were Lee can help through being familiar with the quirks of your work flow.

Johan

maintainer
#34 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

Quoted 3 lines · Johan Hovold

My reminders sent directly to you explicitly mentioned it being a regression every time. That should trigger a maintainer's interest enough to look at the quoted context or ask for a resend.

Sadly what I actually ended up reading was Lee's reply not one of your mails.

Quoted 3 lines

How should a patch submitter know that you simply drop mails with an mfd prefix even if it's directed to you? Unless documented somewhere, that's were Lee can help through being familiar with the quirks of your work flow.

I don't drop *all* such mails, this isn't a rules based thing but rather something that depends on a bunch of factors including how busy I am at that particular moment. MFD discussions (even more so than patches) are certainly a bit of a warning sign here but that's not the only factor and there are positives as well as negatives. For example very broad CC lists on driver specific patches usually indicate that someone just sent the mail to everyone that get_maintainers --git pulled out (which tends to generate a lot of false positives), but on the other hand something like syscon (which is broadly used) or DT bindings (which often affect subfunctions too) is more likely to be relevant.

About the only hard and fast rule for this sort of thing is that if someone wants specific people to look at a patch it's usually a good idea to send the actual patch directly to them with a relevant subject line.

#35 lore raw

PATCH RESEND 1/1 mfd: da9052: fix broken regulator probe

In reply to Lee Jones

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19

---

Resending per Lee's request to get an ack from Mark Brown.

Johan

 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/mfd/da9052-core.c +4 -4
index ae498b53ee40..46e3840c7a37 100644
@@ -433,6 +433,10 @@ EXPORT_SYMBOL_GPL(da9052_adc_read_temp);
 static const struct mfd_cell da9052_subdev_info[] = {
 	{
 		.name = "da9052-regulator",
+		.id = 0,
+	},
+	{
+		.name = "da9052-regulator",
 		.id = 1,
 	},
 	{
@@ -484,10 +488,6 @@ static const struct mfd_cell da9052_subdev_info[] = {
 		.id = 13,
 	},
 	{
-		.name = "da9052-regulator",
-		.id = 14,
-	},
-	{
 		.name = "da9052-onkey",
 	},
 	{
drivers/regulator/da9052-regulator.c +3 -2
index 8a4df7a1f2ee..e628d4c2f2ae 100644
@@ -394,6 +394,7 @@ static inline struct da9052_regulator_info *find_regulator_info(u8 chip_id,
 
 static int da9052_regulator_probe(struct platform_device *pdev)
 {
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
 	struct regulator_config config = { };
 	struct da9052_regulator *regulator;
 	struct da9052 *da9052;
@@ -409,7 +410,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	regulator->da9052 = da9052;
 
 	regulator->info = find_regulator_info(regulator->da9052->chip_id,
-					      pdev->id);
+					      cell->id);
 	if (regulator->info == NULL) {
 		dev_err(&pdev->dev, "invalid regulator ID specified\n");
 		return -EINVAL;
@@ -419,7 +420,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	config.driver_data = regulator;
 	config.regmap = da9052->regmap;
 	if (pdata && pdata->regulators) {
-		config.init_data = pdata->regulators[pdev->id];
+		config.init_data = pdata->regulators[cell->id];
 	} else {
 #ifdef CONFIG_OF
 		struct device_node *nproot = da9052->dev->of_node;
Signature · 1 line
2.3.6
maintainer Reviewed-by on v1 1/1
#36 lore raw

Re: [PATCH RESEND] mfd: da9052: fix broken regulator probe

Quoted 5 lines · Johan Hovold

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Applied
#37 lore raw

Re: [PATCH RESEND] mfd: da9052: fix broken regulator probe

In reply to Johan Hovold
Quoted 16 lines · Johan Hovold

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19

---

Quoted 8 lines

Resending per Lee's request to get an ack from Mark Brown.

Johan

 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)

Applied, thanks.

Quoted 56 lines
drivers/mfd/da9052-core.c +4 -4
index ae498b53ee40..46e3840c7a37 100644
@@ -433,6 +433,10 @@ EXPORT_SYMBOL_GPL(da9052_adc_read_temp);
 static const struct mfd_cell da9052_subdev_info[] = {
 	{
 		.name = "da9052-regulator",
+		.id = 0,
+	},
+	{
+		.name = "da9052-regulator",
 		.id = 1,
 	},
 	{
@@ -484,10 +488,6 @@ static const struct mfd_cell da9052_subdev_info[] = {
 		.id = 13,
 	},
 	{
-		.name = "da9052-regulator",
-		.id = 14,
-	},
-	{
 		.name = "da9052-onkey",
 	},
 	{
drivers/regulator/da9052-regulator.c +3 -2
index 8a4df7a1f2ee..e628d4c2f2ae 100644
@@ -394,6 +394,7 @@ static inline struct da9052_regulator_info *find_regulator_info(u8 chip_id,
 
 static int da9052_regulator_probe(struct platform_device *pdev)
 {
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
 	struct regulator_config config = { };
 	struct da9052_regulator *regulator;
 	struct da9052 *da9052;
@@ -409,7 +410,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	regulator->da9052 = da9052;
 
 	regulator->info = find_regulator_info(regulator->da9052->chip_id,
-					      pdev->id);
+					      cell->id);
 	if (regulator->info == NULL) {
 		dev_err(&pdev->dev, "invalid regulator ID specified\n");
 		return -EINVAL;
@@ -419,7 +420,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	config.driver_data = regulator;
 	config.regmap = da9052->regmap;
 	if (pdata && pdata->regulators) {
-		config.init_data = pdata->regulators[pdev->id];
+		config.init_data = pdata->regulators[cell->id];
 	} else {
 #ifdef CONFIG_OF
 		struct device_node *nproot = da9052->dev->of_node;
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#38 lore raw

Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe

In reply to Johan Hovold

FAO Mark:

Quoted 73 lines

Fix broken probe of da9052 regulators, which since commit b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision") use a non-deterministic platform-device id to retrieve static regulator information. Fortunately, adequate error handling was in place so probe would simply fail with an error message.

Update the mfd-cell ids to be zero-based and use those to identify the cells when probing the regulator devices.

Fixes b3f6c73db732 ("mfd: da9052-core: Fix platform-device id collision")
Cc stable <[email protected]> # v3.19
---
 drivers/mfd/da9052-core.c            | 8 ++++----
 drivers/regulator/da9052-regulator.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/mfd/da9052-core.c +4 -4
index ae498b53ee40..46e3840c7a37 100644
@@ -433,6 +433,10 @@ EXPORT_SYMBOL_GPL(da9052_adc_read_temp);
 static const struct mfd_cell da9052_subdev_info[] = {
 	{
 		.name = "da9052-regulator",
+		.id = 0,
+	},
+	{
+		.name = "da9052-regulator",
 		.id = 1,
 	},
 	{
@@ -484,10 +488,6 @@ static const struct mfd_cell da9052_subdev_info[] = {
 		.id = 13,
 	},
 	{
-		.name = "da9052-regulator",
-		.id = 14,
-	},
-	{
 		.name = "da9052-onkey",
 	},
 	{
drivers/regulator/da9052-regulator.c +3 -2
index 8a4df7a1f2ee..e628d4c2f2ae 100644
@@ -394,6 +394,7 @@ static inline struct da9052_regulator_info *find_regulator_info(u8 chip_id,
 
 static int da9052_regulator_probe(struct platform_device *pdev)
 {
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
 	struct regulator_config config = { };
 	struct da9052_regulator *regulator;
 	struct da9052 *da9052;
@@ -409,7 +410,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	regulator->da9052 = da9052;
 
 	regulator->info = find_regulator_info(regulator->da9052->chip_id,
-					      pdev->id);
+					      cell->id);
 	if (regulator->info == NULL) {
 		dev_err(&pdev->dev, "invalid regulator ID specified\n");
 		return -EINVAL;
@@ -419,7 +420,7 @@ static int da9052_regulator_probe(struct platform_device *pdev)
 	config.driver_data = regulator;
 	config.regmap = da9052->regmap;
 	if (pdata && pdata->regulators) {
-		config.init_data = pdata->regulators[pdev->id];
+		config.init_data = pdata->regulators[cell->id];
 	} else {
 #ifdef CONFIG_OF
 		struct device_node *nproot = da9052->dev->of_node;
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#39 lore raw
In reply to Johan Hovold

Hi,

Quoted 31 lines · Johan Hovold
Quoted 23 lines · Bartlomiej Zolnierkiewicz
Quoted 14 lines · Johan Hovold
Quoted 8 lines · Bartlomiej Zolnierkiewicz

Commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation") changed the way platform device ids are generated from mfd id base and cell ids in mfd_add_device(). Unfortunately the change in question breaks mfd drivers which are using mfd_add_devices() with mfd id base equal to -1 and non-zero cell ids (used to distinguish cells with the same name field). The result is that mfd core tries to register platform devices with the same name which obviously fails and leads to mfd device probe failure.

First of all, thanks for finding these. I obviously overlooked this class of drivers when fixing the device-id generation.

Quoted 7 lines
Quoted 2 lines

Changing mfd_add_devices() mfd id base from -1 to 0 and at the same time setting proper cell ids for all cells fixes the issue.

This is however not the right fix. Instead you should be using PLATFORM_DEVID_AUTO and keep the non-zero cell-ids as is, as this will allow more than one mfd-device to be registered without resorting to hacks.

Quoted 6 lines

Conversion of a driver to use PLATFORM_DEVID_AUTO changes names of all platform devices registered by the driver. In my patchset I just tried to restore the broken functionality (patch that broke it went in v3.19). I thought that it was the best solution for v3.19 and v4.0-rc4 (then PLATFORM_DEVID_AUTO conversion can be done sometime later, i.e. in v4.1).

Quoted 8 lines

I think you're right. We should do the conversion to PLATFORM_DEVID_AUTO later.

I'm responding to this mail with two patches for v4.0 fixing the da9052 driver, which is currently broken although not in the way suggested by your v1-series, and the other drivers you identified by doing a partial revert of commit 6e3f62f0793e ("mfd: core: Fix platform-device id generation").

Both patches look good to me. Thank you for fixing this.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics