mfd: Fix platform device ids to avoid probe failure
Subsystems (5)
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
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).
---
stable v3.19+
drivers/mfd/max8997.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/mfd/max8997.c +8 -8
@@ -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
PATCH v2 2/5 mfd: da9055: Fix platform device ids to avoid probe failure
In reply to Bartlomiej ZolnierkiewiczCommit 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.
---
stable v3.19+
drivers/mfd/da9055-core.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/mfd/da9055-core.c +13 -8
@@ -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
PATCH v2 3/5 mfd: lp8788: Fix platform device ids to avoid probe failure
In reply to Bartlomiej ZolnierkiewiczCommit 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.
---
stable v3.19+
drivers/mfd/lp8788.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/mfd/lp8788.c +6 -4
@@ -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
PATCH v2 4/5 mfd: wm831x: Fix platform device ids to avoid probe failure
In reply to Bartlomiej ZolnierkiewiczCommit 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.
---
stable v3.19+
drivers/mfd/wm831x-core.c | 184 +++++++++++++++++++++++++++------------------- 1 file changed, 107 insertions(+), 77 deletions(-)
drivers/mfd/wm831x-core.c +107 -77
@@ -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
PATCH v2 5/5 mfd: da9052: Fix platform device names
In reply to Bartlomiej ZolnierkiewiczCommit 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.
--- drivers/mfd/da9052-core.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
drivers/mfd/da9052-core.c +16 -5
@@ -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
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 inmfd_add_device(). Unfortunately the change in question breaks mfd drivers which are usingmfd_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
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 inmfd_add_device(). Unfortunately the change in question breaks mfd drivers which are usingmfd_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_AUTOand 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_numparameter 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
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 inmfd_add_device(). Unfortunately the change in question breaks mfd drivers which are usingmfd_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_AUTOand 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_AUTOchanges 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 (thenPLATFORM_DEVID_AUTOconversion 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
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.
--- 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
@@ -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
@@ -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
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.
--- drivers/mfd/mfd-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mfd/mfd-core.c +1 -1
@@ -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
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 honourPLATFORM_DEVID_AUTOandPLATFORM_DEVID_NONEwhen 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 toPLATFORM_DEVID_NONE.
Quoted 4 lines
Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")Cc stable <[email protected]> # v3.19Reported-by Bartlomiej Zolnierkiewicz <[email protected]>Signed-off-by Johan Hovold <[email protected]>
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@@ -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;
Lee Jones
AppliedRe: [PATCH 2/2] mfd: core: fix platform-device name collisions
In reply to Johan HovoldQuoted 16 lines · Johan Hovold
Since commit
6e3f62f0793e("mfd: core: Fix platform-device id generation") we honourPLATFORM_DEVID_AUTOandPLATFORM_DEVID_NONEwhen 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 toPLATFORM_DEVID_NONE.
Quoted 7 lines
Fixes 6e3f62f0793e ("mfd: core: Fix platform-device id generation")Cc stable <[email protected]> # v3.19Reported-by Bartlomiej Zolnierkiewicz <[email protected]>Signed-off-by Johan Hovold <[email protected]>--- 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@@ -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
Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe
In reply to Johan HovoldHi,
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.19Signed-off-by Johan Hovold <[email protected]>
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@@ -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@@ -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;
Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe
In reply to Johan HovoldQuoted 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.19Signed-off-by Johan Hovold <[email protected]>--- 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@@ -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@@ -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
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.19Signed-off-by Johan Hovold <[email protected]>--- 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.
Acked-by Lee Jones <[email protected]>
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
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.19Signed-off-by Johan Hovold <[email protected]>--- 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.
Acked-by Lee Jones <[email protected]>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
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.19Signed-off-by Johan Hovold <[email protected]>--- 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.
Acked-by Lee Jones <[email protected]>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
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.19Signed-off-by Johan Hovold <[email protected]>--- 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.
Acked-by Lee Jones <[email protected]>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
Mark Brown
maintainerRe: [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.
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
Mark Brown
maintainerRe: [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.
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
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
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
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
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
Mark Brown
maintainerRe: [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: ".
Mark Brown
maintainerRe: [PATCH 1/2] mfd: da9052: fix broken regulator probe
In reply to Johan HovoldQuoted 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.
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
Mark Brown
maintainerRe: [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).
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
Mark Brown
maintainerRe: [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.
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
Mark Brown
maintainerRe: [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.
PATCH RESEND 1/1 mfd: da9052: fix broken regulator probe
In reply to Lee JonesFix 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.
---
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
@@ -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
@@ -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
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.
Lee Jones
AppliedRe: [PATCH RESEND] mfd: da9052: fix broken regulator probe
In reply to Johan HovoldQuoted 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.19Signed-off-by Johan Hovold <[email protected]>Acked-by Lee Jones <[email protected]>---
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@@ -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@@ -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
Re: [PATCH 1/2] mfd: da9052: fix broken regulator probe
In reply to Johan HovoldFAO 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.19Signed-off-by Johan Hovold <[email protected]>--- 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@@ -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@@ -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
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 inmfd_add_device(). Unfortunately the change in question breaks mfd drivers which are usingmfd_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_AUTOand 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_AUTOchanges 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 (thenPLATFORM_DEVID_AUTOconversion 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_AUTOlater.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