Hi Bartosz!
On Tue, 2024-11-26 at 21:37 +0100, Bartosz Golaszewski wrote:
@@ -647,6 +656,13 @@ static void omap_gpio_irq_shutdown(struct irq_data *d) unsigned long flags; unsigned offset = d->hwirq;
+ /* + * Enable the clock here so that the nested clk_disable() in the + * following omap_clear_gpio_debounce() is lockless + */ + if (bank->dbck_flag) + clk_enable(bank->dbck);
But this looks like a functional change. You effectively bump the clock enable count but don't add a corresponding clk_disable() in the affected path. Is the clock ever actually disabled then?
Am I not getting something?
Actually I though I enable and disable them in the very same function, so for the first enable above...
raw_spin_lock_irqsave(&bank->lock, flags); bank->irq_usage &= ~(BIT(offset)); omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE); @@ -656,6 +672,9 @@ static void omap_gpio_irq_shutdown(struct irq_data *d) omap_clear_gpio_debounce(bank, offset); omap_disable_gpio_module(bank, offset); raw_spin_unlock_irqrestore(&bank->lock, flags);
+ if (bank->dbck_flag) + clk_disable(bank->dbck);
^^^^^^^^^^^^^^^^^^^^^^^^
this would be the corresponding disable.
}
static void omap_gpio_irq_bus_lock(struct irq_data *data) @@ -827,6 +846,13 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset) struct gpio_bank *bank = gpiochip_get_data(chip); unsigned long flags;
+ /* + * Enable the clock here so that the nested clk_disable() in the + * following omap_clear_gpio_debounce() is lockless + */ + if (bank->dbck_flag) + clk_enable(bank->dbck);
^^^^^^^^^^^^^^^^^^^^^^ And for this second enable....
raw_spin_lock_irqsave(&bank->lock, flags); bank->mod_usage &= ~(BIT(offset)); if (!LINE_USED(bank->irq_usage, offset)) { @@ -836,6 +862,9 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset) omap_disable_gpio_module(bank, offset); raw_spin_unlock_irqrestore(&bank->lock, flags);
+ if (bank->dbck_flag) + clk_disable(bank->dbck);
^^^^^^^^^^^^^^^^^^^^^^^ ... this would be the corresponding 2nd disable.
pm_runtime_put(chip->parent); }
Aren't they paired from your PoV?