| Message ID | 20201104170423.23436-2-digetx@gmail.com |
|---|---|
| State | Accepted |
| Commit | 37174f3341306eaea7b7f4f5cc624e0040305a98 |
| Headers | show |
| Series | None | expand |
On Wed, Nov 4, 2020 at 6:04 PM Dmitry Osipenko <digetx@gmail.com> wrote: > > Use raw_spinlock in order to fix spurious messages about invalid context > when spinlock debugging is enabled. This happens because there is a legit > nested raw_spinlock->spinlock locking usage within IRQ-related code. IRQ > core uses raw spinlock and then Tegra GPIO driver uses a nested spinlock. > The debug code can't recognize and handle this case, hence we need to use > raw spinlock in the GPIO driver. > > [ BUG: Invalid wait context ] > ... > (dump_stack) from (__lock_acquire) > (__lock_acquire) from (lock_acquire) > (lock_acquire) from (_raw_spin_lock_irqsave) > (_raw_spin_lock_irqsave) from (tegra_gpio_irq_set_type) > (tegra_gpio_irq_set_type) from (__irq_set_trigger) > (__irq_set_trigger) from (__setup_irq) > (__setup_irq) from (request_threaded_irq) > (request_threaded_irq) from (devm_request_threaded_irq) > (devm_request_threaded_irq) from (elants_i2c_probe) > (elants_i2c_probe) from (i2c_device_probe) > ... > > Tested-by: Peter Geis <pgwipeout@gmail.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > > Changelog: > > v2: - Only lvl_lock is converted to raw_spinlock. The dbc_lock doesn't > relate to IRQ and doesn't need the conversion. > > - Improved commit message by clarifying that IRQ core uses raw > spinlock. > > - Added clarifying comment to the code for the lvl_lock. > > drivers/gpio/gpio-tegra.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > index 98fc78739ebf..e19ebff6018c 100644 > --- a/drivers/gpio/gpio-tegra.c > +++ b/drivers/gpio/gpio-tegra.c > @@ -61,8 +61,16 @@ struct tegra_gpio_info; > struct tegra_gpio_bank { > unsigned int bank; > unsigned int irq; > - spinlock_t lvl_lock[4]; > - spinlock_t dbc_lock[4]; /* Lock for updating debounce count register */ > + > + /* > + * IRQ-core code uses raw locking, and thus, nested locking also > + * should be raw in order not to trip spinlock debug warnings. > + */ > + raw_spinlock_t lvl_lock[4]; > + > + /* Lock for updating debounce count register */ > + spinlock_t dbc_lock[4]; > + > #ifdef CONFIG_PM_SLEEP > u32 cnf[4]; > u32 out[4]; > @@ -334,14 +342,14 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) > return -EINVAL; > } > > - spin_lock_irqsave(&bank->lvl_lock[port], flags); > + raw_spin_lock_irqsave(&bank->lvl_lock[port], flags); > > val = tegra_gpio_readl(tgi, GPIO_INT_LVL(tgi, gpio)); > val &= ~(GPIO_INT_LVL_MASK << GPIO_BIT(gpio)); > val |= lvl_type << GPIO_BIT(gpio); > tegra_gpio_writel(tgi, val, GPIO_INT_LVL(tgi, gpio)); > > - spin_unlock_irqrestore(&bank->lvl_lock[port], flags); > + raw_spin_unlock_irqrestore(&bank->lvl_lock[port], flags); > > tegra_gpio_mask_write(tgi, GPIO_MSK_OE(tgi, gpio), gpio, 0); > tegra_gpio_enable(tgi, gpio); > @@ -675,7 +683,7 @@ static int tegra_gpio_probe(struct platform_device *pdev) > tegra_gpio_irq_handler, bank); > > for (j = 0; j < 4; j++) { > - spin_lock_init(&bank->lvl_lock[j]); > + raw_spin_lock_init(&bank->lvl_lock[j]); > spin_lock_init(&bank->dbc_lock[j]); > } > } > -- > 2.27.0 > Patch applied, thanks! Bartosz
diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 98fc78739ebf..e19ebff6018c 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -61,8 +61,16 @@ struct tegra_gpio_info; struct tegra_gpio_bank { unsigned int bank; unsigned int irq; - spinlock_t lvl_lock[4]; - spinlock_t dbc_lock[4]; /* Lock for updating debounce count register */ + + /* + * IRQ-core code uses raw locking, and thus, nested locking also + * should be raw in order not to trip spinlock debug warnings. + */ + raw_spinlock_t lvl_lock[4]; + + /* Lock for updating debounce count register */ + spinlock_t dbc_lock[4]; + #ifdef CONFIG_PM_SLEEP u32 cnf[4]; u32 out[4]; @@ -334,14 +342,14 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) return -EINVAL; } - spin_lock_irqsave(&bank->lvl_lock[port], flags); + raw_spin_lock_irqsave(&bank->lvl_lock[port], flags); val = tegra_gpio_readl(tgi, GPIO_INT_LVL(tgi, gpio)); val &= ~(GPIO_INT_LVL_MASK << GPIO_BIT(gpio)); val |= lvl_type << GPIO_BIT(gpio); tegra_gpio_writel(tgi, val, GPIO_INT_LVL(tgi, gpio)); - spin_unlock_irqrestore(&bank->lvl_lock[port], flags); + raw_spin_unlock_irqrestore(&bank->lvl_lock[port], flags); tegra_gpio_mask_write(tgi, GPIO_MSK_OE(tgi, gpio), gpio, 0); tegra_gpio_enable(tgi, gpio); @@ -675,7 +683,7 @@ static int tegra_gpio_probe(struct platform_device *pdev) tegra_gpio_irq_handler, bank); for (j = 0; j < 4; j++) { - spin_lock_init(&bank->lvl_lock[j]); + raw_spin_lock_init(&bank->lvl_lock[j]); spin_lock_init(&bank->dbc_lock[j]); } }