2015-08-06 1:02 GMT+09:00 Srinivas Kandagatla <srinivas.kandaga...@linaro.org>: > A recursive lockdep warning occurs if you call regulator_set_voltage() > on a load switches that are modelled as regulators with a parent supply as > there is no nesting annotation for the rdev->mutex. > To avoid this warning, use the unlocked version of the get_voltage(). > > wiithout this patch kernel throws up below warning: > > ============================================= > [ INFO: possible recursive locking detected ] > 4.2.0-rc5-dirty #132 Not tainted > --------------------------------------------- > swapper/0/1 is trying to acquire lock: > (&rdev->mutex){+.+.+.}, at: [<c066b6e4>] regulator_get_voltage+0x44/0x68 > > but task is already holding lock: > (&rdev->mutex){+.+.+.}, at: [<c066d718>] regulator_set_voltage+0x50/0x168 > > other info that might help us debug this: > Possible unsafe locking scenario: > > CPU0 > ---- > lock(&rdev->mutex); > lock(&rdev->mutex); > > *** DEADLOCK *** > > May be due to missing lock nesting notation > > 3 locks held by swapper/0/1: > #0: (&dev->mutex){......}, at: [<c0756e34>] __driver_attach+0x58/0xa8 > #1: (&dev->mutex){......}, at: [<c0756e44>] __driver_attach+0x68/0xa8 > #2: (&rdev->mutex){+.+.+.}, at: [<c066d718>] > regulator_set_voltage+0x50/0x168 > > stack backtrace: > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc5-dirty #132 > Hardware name: Qualcomm (Flattened Device Tree) > [<c021af98>] (unwind_backtrace) from [<c021536c>] (show_stack+0x20/0x24) > [<c021536c>] (show_stack) from [<c0ba6698>] (dump_stack+0x8c/0xc0) > [<c0ba6698>] (dump_stack) from [<c02a8604>] (__lock_acquire+0x1bf4/0x1ec0) > [<c02a8604>] (__lock_acquire) from [<c02a9020>] (lock_acquire+0xe0/0x300) > [<c02a9020>] (lock_acquire) from [<c0baa7dc>] (mutex_lock_nested+0x88/0x45c) > [<c0baa7dc>] (mutex_lock_nested) from [<c066b6e4>] > (regulator_get_voltage+0x44/0x68) > [<c066b6e4>] (regulator_get_voltage) from [<c066b5c8>] > (_regulator_get_voltage+0xb8/0x190) > [<c066b5c8>] (_regulator_get_voltage) from [<c066d7f4>] > (regulator_set_voltage+0x12c/0x168) > [<c066d7f4>] (regulator_set_voltage) from [<c09af5b4>] > (mmci_sig_volt_switch+0x6c/0x110) > [<c09af5b4>] (mmci_sig_volt_switch) from [<c099d8a4>] > (mmc_power_up+0x78/0x114) > [<c099d8a4>] (mmc_power_up) from [<c099e69c>] (mmc_start_host+0x54/0x7c) > [<c099e69c>] (mmc_start_host) from [<c099f95c>] (mmc_add_host+0x6c/0x90) > [<c099f95c>] (mmc_add_host) from [<c09b014c>] (mmci_probe+0x5ac/0x854) > [<c09b014c>] (mmci_probe) from [<c063f5e4>] (amba_probe+0xdc/0x158) > [<c063f5e4>] (amba_probe) from [<c0756d38>] (driver_probe_device+0x1dc/0x280) > [<c0756d38>] (driver_probe_device) from [<c0756e80>] > (__driver_attach+0xa4/0xa8) > [<c0756e80>] (__driver_attach) from [<c0755120>] (bus_for_each_dev+0x64/0x98) > [<c0755120>] (bus_for_each_dev) from [<c0756608>] (driver_attach+0x2c/0x30) > [<c0756608>] (driver_attach) from [<c075634c>] (bus_add_driver+0xf8/0x204) > [<c075634c>] (bus_add_driver) from [<c0757fb8>] (driver_register+0x88/0x104) > [<c0757fb8>] (driver_register) from [<c063f418>] > (amba_driver_register+0x50/0x64) > [<c063f418>] (amba_driver_register) from [<c113e904>] > (mmci_driver_init+0x14/0x1c) > [<c113e904>] (mmci_driver_init) from [<c020adb4>] > (do_one_initcall+0x90/0x1e4) > [<c020adb4>] (do_one_initcall) from [<c10e4ea8>] > (kernel_init_freeable+0x12c/0x1f4) > [<c10e4ea8>] (kernel_init_freeable) from [<c0b9ec78>] (kernel_init+0x1c/0xfc) > [<c0b9ec78>] (kernel_init) from [<c02114d8>] (ret_from_fork+0x14/0x3c) > > Reported-by: Nicolas Dechesne <nicolas.deche...@linaro.org> > Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > --- > drivers/regulator/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index 3c3137a..7717b04 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -2919,7 +2919,7 @@ static int _regulator_get_voltage(struct regulator_dev > *rdev) > } else if (rdev->desc->fixed_uV && (rdev->desc->n_voltages == 1)) { > ret = rdev->desc->fixed_uV; > } else if (rdev->supply) { > - ret = regulator_get_voltage(rdev->supply); > + ret = _regulator_get_voltage(rdev->supply->rdev);
Is the 'rdev' and 'rdev->supply' same regulators? If not then you are just hiding false warning by removing locks thus introducing real issue... Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/