From daad134d66492a9f641163c94510549770b39657 Mon Sep 17 00:00:00 2001 From: Krzysztof Adamski Date: Mon, 22 Feb 2016 09:24:00 +0100 Subject: [PATCH 1/2] regulator: core: Request GPIO before creating sysfs entries regulator_attr_is_visible (which is a .is_visible callback of regulator_dev_group attribute_grpup) checks rdev->ena_pin to decide if "status" file should be present in sysfs. This field is set at the end of regulator_ena_gpio_request so it has to be called before device_register() otherwise this test will always fail, causing "status" file to not be visible. Since regulator_attr_is_visible also tests for is_enabled() op, this problem is only visible for regulators that does not define this callback, like regulator-fixed.c. Signed-off-by: Krzysztof Adamski Signed-off-by: Mark Brown --- drivers/regulator/core.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 744c9889f88d..3c987d76e684 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -3918,6 +3918,16 @@ regulator_register(const struct regulator_desc *regulator_desc, goto clean; } + if ((config->ena_gpio || config->ena_gpio_initialized) && + gpio_is_valid(config->ena_gpio)) { + ret = regulator_ena_gpio_request(rdev, config); + if (ret != 0) { + rdev_err(rdev, "Failed to request enable GPIO%d: %d\n", + config->ena_gpio, ret); + goto wash; + } + } + /* register with sysfs */ rdev->dev.class = ®ulator_class; rdev->dev.parent = dev; @@ -3931,16 +3941,6 @@ regulator_register(const struct regulator_desc *regulator_desc, dev_set_drvdata(&rdev->dev, rdev); - if ((config->ena_gpio || config->ena_gpio_initialized) && - gpio_is_valid(config->ena_gpio)) { - ret = regulator_ena_gpio_request(rdev, config); - if (ret != 0) { - rdev_err(rdev, "Failed to request enable GPIO%d: %d\n", - config->ena_gpio, ret); - goto wash; - } - } - /* set regulator constraints */ if (init_data) constraints = &init_data->constraints; From 32165230eb6e629d7f88e66e0bd90a201549de53 Mon Sep 17 00:00:00 2001 From: Krzysztof Adamski Date: Wed, 24 Feb 2016 11:52:50 +0100 Subject: [PATCH 2/2] regulator: core: fix crash in error path of regulator_register This problem was introduced by: commit daad134d6649 ("regulator: core: Request GPIO before creating sysfs entries") The error path was not updated correctly after moving GPIO registration code and in case regulator_ena_gpio_free failed, device_unregister() was called even though device_register() was not yet called. This problem breaks the boot at least on all Tegra 32-bit devices. It will also crash each device that specifices GPIO that is unavaiable at regulator_register call. Here's error log I've got when forced GPIO to be invalid: [ 1.116612] usb-otg-vbus-reg: Failed to request enable GPIO10: -22 [ 1.122794] Unable to handle kernel NULL pointer dereference at virtual address 00000044 [ 1.130894] pgd = c0004000 [ 1.133598] [00000044] *pgd=00000000 [ 1.137205] Internal error: Oops: 5 [#1] SMP ARM and here's backtrace from KDB: Exception stack(0xef11fbd0 to 0xef11fc18) fbc0: 00000000 c0738a14 00000000 00000000 fbe0: c0b2a0b0 00000000 00000000 c0738a14 c0b5fdf8 00000001 ef7f6074 ef11fc4c fc00: ef11fc50 ef11fc20 c02a8344 c02a7f1c 60000013 ffffffff [] (__dabt_svc) from [] (kernfs_find_ns+0x18/0xf8) [] (kernfs_find_ns) from [] (kernfs_find_and_get_ns+0x40/0x58) [] (kernfs_find_and_get_ns) from [] (sysfs_unmerge_group+0x28/0x68) [] (sysfs_unmerge_group) from [] (dpm_sysfs_remove+0x30/0x5c) [] (dpm_sysfs_remove) from [] (device_del+0x48/0x1f4) [] (device_del) from [] (device_unregister+0x30/0x6c) [] (device_unregister) from [] (regulator_register+0x6d0/0xdac) [] (regulator_register) from [] (devm_regulator_register+0x50/0x84) [] (devm_regulator_register) from [] (reg_fixed_voltage_probe+0x25c/0x3c0) [] (reg_fixed_voltage_probe) from [] (platform_drv_probe+0x60/0xb0) [] (platform_drv_probe) from [] (driver_probe_device+0x24c/0x440) [] (driver_probe_device) from [] (__device_attach_driver+0xc0/0x120) [] (__device_attach_driver) from [] (bus_for_each_drv+0x6c/0x98) [] (bus_for_each_drv) from [] (__device_attach+0xac/0x138) [] (__device_attach) from [] (device_initial_probe+0x1c/0x20) [] (device_initial_probe) from [] (bus_probe_device+0x94/0x9c) [] (bus_probe_device) from [] (deferred_probe_work_func+0x80/0xcc) [] (deferred_probe_work_func) from [] (process_one_work+0x158/0x454) [] (process_one_work) from [] (worker_thread+0x38/0x510) [] (worker_thread) from [] (kthread+0xe8/0x104) [] (kthread) from [] (ret_from_fork+0x14/0x3c) Signed-off-by: Krzysztof Adamski Reported-by: Jon Hunter Acked-by: Jon Hunter Tested-by: Jon Hunter Signed-off-by: Mark Brown --- drivers/regulator/core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 6ee9ba43fa8f..055f8c1a83a8 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -3919,7 +3919,7 @@ regulator_register(const struct regulator_desc *regulator_desc, if (ret != 0) { rdev_err(rdev, "Failed to request enable GPIO%d: %d\n", config->ena_gpio, ret); - goto wash; + goto clean; } } @@ -3931,7 +3931,7 @@ regulator_register(const struct regulator_desc *regulator_desc, ret = device_register(&rdev->dev); if (ret != 0) { put_device(&rdev->dev); - goto clean; + goto wash; } dev_set_drvdata(&rdev->dev, rdev); @@ -3974,13 +3974,13 @@ unset_supplies: scrub: regulator_ena_gpio_free(rdev); - -wash: device_unregister(&rdev->dev); /* device core frees rdev */ rdev = ERR_PTR(ret); goto out; +wash: + regulator_ena_gpio_free(rdev); clean: kfree(rdev); rdev = ERR_PTR(ret);