mirror of
https://github.com/torvalds/linux.git
synced 2026-11-06 21:58:01 +01:00
pinctrl: stm32: program the EXTI mux from .alloc instead of .activate
stm32_gpio_domain_activate() writes the EXTI interrupt multiplexer through a regmap obtained from the generic syscon driver. The irq core calls .activate from __setup_irq() with the raw desc->lock held, so this happens in a raw atomic section. regmap-mmio sets fast_io, and syscon does not ask for a raw spinlock, so the regmap is protected by a spinlock_t. On PREEMPT_RT that is a sleeping lock, which must not be taken there. lockdep reports it as soon as a GPIO interrupt is requested: [ BUG: Invalid wait context ] 7.1.13 #7 Not tainted ----------------------------- kworker/u8:1/29 is trying to lock: (syscon:117:(&syscon_config)->lock){....}-{3:3}, at: regmap_lock_spinlock other info that might help us debug this: context-{5:5} 5 locks held by kworker/u8:1/29: #0: ((wq_completion)async){+.+.}-{0:0}, at: process_one_work #1: ((work_completion)(&entry->work)){+.+.}-{0:0}, at: process_one_work #2: (&dev->mutex){....}-{4:4}, at: __device_attach_async_helper #3: (&desc->request_mutex){+.+.}-{4:4}, at: __setup_irq #4: (&irq_desc_lock_class){-...}-{2:2}, at: __setup_irq The driver was already aware of running in atomic context there: it uses the _in_atomic() hwspinlock primitives around the very same access. The syscon lock is the one lock in that section it does not control. Program the mux from .alloc instead, which runs in a sleepable context. That callback already owns this resource: it reserves the mux line in pctl->irqmux_map under irqmux_lock, so writing the value it just claimed is a natural fit, and the value only depends on the bank. Nothing requires the mux to be reprogrammed at interrupt startup time: the domain has no .deactivate, .free() only releases the irqmux_map bit without touching the registers. Keep the access inside the existing irqmux_lock section rather than after it. hwspin_lock_timeout_in_atomic() deliberately skips the local lock that the other hwspinlock modes take, so its caller is responsible for disabling preemption and for excluding other local contexts; every other hwspinlock user in this driver relies on bank->lock for that. irqmux_lock plays the same role here, and it also turns the mux reservation and the mux write into a single critical section. Nesting the regmap spinlock_t inside irqmux_lock, itself a spinlock_t, is what the wait-context checker expects. Note that this bounds the semaphore hold time on !PREEMPT_RT only. On PREEMPT_RT spinlock_t disables neither interrupts nor preemption, so the hardware semaphore can be held across a preemptible section and a coprocessor contending for it may poll for an unbounded time. That is not introduced here: bank->lock is a spinlock_t too, so every existing hwspinlock user in this driver behaves the same way on PREEMPT_RT. Removing that exposure requires the EXTI mux regmap to be raw-spinlock based, which the syscon core does not currently offer, and is left for a separate change. While moving the code, release the mux reservation when the hwspinlock cannot be taken, which the .activate callback had no way of doing. Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Closes: https://lore.kernel.org/all/20220202174430.pf37tt6lua2op3gc@pengutronix.de/ Signed-off-by: Ju Nan <junan76@163.com> Signed-off-by: Linus Walleij <linusw@kernel.org>
This commit is contained in:
parent
544b3f8042
commit
f33a24cc01
|
|
@ -600,30 +600,6 @@ static int stm32_gpio_domain_translate(struct irq_domain *d,
|
|||
return 0;
|
||||
}
|
||||
|
||||
static int stm32_gpio_domain_activate(struct irq_domain *d,
|
||||
struct irq_data *irq_data, bool reserve)
|
||||
{
|
||||
struct stm32_gpio_bank *bank = d->host_data;
|
||||
struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
|
||||
int ret = 0;
|
||||
|
||||
if (pctl->hwlock) {
|
||||
ret = hwspin_lock_timeout_in_atomic(pctl->hwlock,
|
||||
HWSPNLCK_TIMEOUT_MS);
|
||||
if (ret) {
|
||||
dev_err(pctl->dev, "Can't get hwspinlock\n");
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
|
||||
regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->bank_ioport_nr);
|
||||
|
||||
if (pctl->hwlock)
|
||||
hwspin_unlock_in_atomic(pctl->hwlock);
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
static int stm32_gpio_domain_alloc(struct irq_domain *d,
|
||||
unsigned int virq,
|
||||
unsigned int nr_irqs, void *data)
|
||||
|
|
@ -645,10 +621,27 @@ static int stm32_gpio_domain_alloc(struct irq_domain *d,
|
|||
if (pctl->irqmux_map & BIT(hwirq)) {
|
||||
dev_err(pctl->dev, "irq line %ld already requested.\n", hwirq);
|
||||
ret = -EBUSY;
|
||||
} else {
|
||||
pctl->irqmux_map |= BIT(hwirq);
|
||||
goto unlock;
|
||||
}
|
||||
|
||||
pctl->irqmux_map |= BIT(hwirq);
|
||||
|
||||
if (pctl->hwlock) {
|
||||
ret = hwspin_lock_timeout_in_atomic(pctl->hwlock,
|
||||
HWSPNLCK_TIMEOUT_MS);
|
||||
if (ret) {
|
||||
dev_err(pctl->dev, "Can't get hwspinlock\n");
|
||||
pctl->irqmux_map &= ~BIT(hwirq);
|
||||
goto unlock;
|
||||
}
|
||||
}
|
||||
|
||||
regmap_field_write(pctl->irqmux[hwirq], bank->bank_ioport_nr);
|
||||
|
||||
if (pctl->hwlock)
|
||||
hwspin_unlock_in_atomic(pctl->hwlock);
|
||||
|
||||
unlock:
|
||||
spin_unlock_irqrestore(&pctl->irqmux_lock, flags);
|
||||
if (ret)
|
||||
return ret;
|
||||
|
|
@ -683,7 +676,6 @@ static const struct irq_domain_ops stm32_gpio_domain_ops = {
|
|||
.translate = stm32_gpio_domain_translate,
|
||||
.alloc = stm32_gpio_domain_alloc,
|
||||
.free = stm32_gpio_domain_free,
|
||||
.activate = stm32_gpio_domain_activate,
|
||||
};
|
||||
|
||||
/* Pinctrl functions */
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user