On Wed, Nov 27, 2024 at 7:45 PM A. Sverdlin alexander.sverdlin@siemens.com wrote:
From: Alexander Sverdlin alexander.sverdlin@siemens.com
The problem apparetly has been known since the conversion to raw_spin_lock() (commit 4dbada2be460 ("gpio: omap: use raw locks for locking")).
Symptom:
[ BUG: Invalid wait context ] 5.10.214
swapper/1 is trying to lock: (enable_lock){....}-{3:3}, at: clk_enable_lock other info that might help us debug this: context-{5:5} 2 locks held by swapper/1: #0: (&dev->mutex){....}-{4:4}, at: device_driver_attach #1: (&bank->lock){....}-{2:2}, at: omap_gpio_set_config stack backtrace: CPU: 0 PID: 1 Comm: swapper Not tainted 5.10.214 Hardware name: Generic AM33XX (Flattened Device Tree) unwind_backtrace show_stack __lock_acquire lock_acquire.part.0 _raw_spin_lock_irqsave clk_enable_lock clk_enable omap_gpio_set_config gpio_keys_setup_key gpio_keys_probe platform_drv_probe really_probe driver_probe_device device_driver_attach __driver_attach bus_for_each_dev bus_add_driver driver_register do_one_initcall do_initcalls kernel_init_freeable kernel_init
Reorder clk_enable()/clk_disable() calls in a way that they always happen outside of raw_spin_lock'ed sections.
Cc: stable@vger.kernel.org Fixes: 4dbada2be460 ("gpio: omap: use raw locks for locking") Signed-off-by: Alexander Sverdlin alexander.sverdlin@siemens.com
Changelog: v2: complete rework, I've totally missed the fact clk_enable()/clk_disable() cannot avoid clk_enable_lock() even if the clock is enabled; I had to extract all clk_*() calls out of raw_spin_lock'ed sections
This looks so much worse now. :(
I refuse to believe this is the only way to fix it.
Would it be possible to wrap the logic that disables the clock depending on the return value of omap_gpio_dbck_enable() in some abstraction layer? Basing the behavior on the boolean return value of a function named omap_gpio_dbck_enable() makes very little sense when looking at it now and it will make even less a year from now.
Could we add functions like omap_gpio_dbck_clk_enable() and omap_gpio_dbck_clk_disable() plus some state in the driver data set by omap_gpio_dbck_enable() which would be then read by omap_gpio_dbck_clk_disable() in order to determine whether to disable the clock or keep it going?
Bart