| Message ID | 20130529084642.GA18273@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 29, 2013 at 10:46:42AM +0200, Sebastian Andrzej Siewior wrote: > Without this, devm_regmap_init_mmio() creates & uses a simple > spin_lock() and this should be enough. Within the probe function the > registers are read and written in process context. Later they are > accessed from the ISR and lockdep complains because now the lock is > taken suddenly with IRQs enabled. Currently I don't see any other way to > keep lockdep quiet than doing this. This is not a good place to make this change, there's nothing specific to the device here and in actual fact there's already a change in the works from Lars-Peter Clausen converting the spinlock to always use spin_lock_irqsave() so it should be resolved soon. The thread was on lkml in the past few days.
On 05/29/2013 01:12 PM, Mark Brown wrote: > On Wed, May 29, 2013 at 10:46:42AM +0200, Sebastian Andrzej Siewior wrote: >> Without this, devm_regmap_init_mmio() creates & uses a simple >> spin_lock() and this should be enough. Within the probe function the >> registers are read and written in process context. Later they are >> accessed from the ISR and lockdep complains because now the lock is >> taken suddenly with IRQs enabled. Currently I don't see any other way to >> keep lockdep quiet than doing this. > > This is not a good place to make this change, there's nothing specific > to the device here and in actual fact there's already a change in the > works from Lars-Peter Clausen converting the spinlock to always use > spin_lock_irqsave() so it should be resolved soon. The thread was on > lkml in the past few days. Yes, this patch shouldn't be necessary in next/master anymore. For the record the patch can be found here: https://patchwork.kernel.org/patch/2609721/ - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/29/2013 01:25 PM, Lars-Peter Clausen wrote: >> This is not a good place to make this change, there's nothing specific >> to the device here and in actual fact there's already a change in the >> works from Lars-Peter Clausen converting the spinlock to always use >> spin_lock_irqsave() so it should be resolved soon. The thread was on >> lkml in the past few days. > > Yes, this patch shouldn't be necessary in next/master anymore. For the > record the patch can be found here: https://patchwork.kernel.org/patch/2609721/ Ah good good good. So this one can dropped. Thank you. > - Lars Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c index 5e4076f..bd982e5 100644 --- a/drivers/mfd/ti_am335x_tscadc.c +++ b/drivers/mfd/ti_am335x_tscadc.c @@ -42,11 +42,27 @@ static void tscadc_writel(struct ti_tscadc_dev *tsadc, unsigned int reg, regmap_write(tsadc->regmap_tscadc, reg, val); } +static void tscadc_lock_spinlock(void *__map) +{ + struct ti_tscadc_dev *tsadc = __map; + + spin_lock_irqsave(&tsadc->reg_map_lock, tsadc->reg_map_flags); +} + +static void tscadc_unlock_spinlock(void *__map) +{ + struct ti_tscadc_dev *tsadc = __map; + + spin_unlock_irqrestore(&tsadc->reg_map_lock, tsadc->reg_map_flags); +} + static const struct regmap_config tscadc_regmap_config = { .name = "ti_tscadc", .reg_bits = 32, .reg_stride = 4, .val_bits = 32, + .lock = tscadc_lock_spinlock, + .unlock = tscadc_unlock_spinlock, }; static void tscadc_idle_config(struct ti_tscadc_dev *config) @@ -69,6 +85,7 @@ static int ti_tscadc_probe(struct platform_device *pdev) int err, ctrl; int clk_value, clock_rate; int tsc_wires = 0, adc_channels = 0, total_channels; + struct regmap_config tscadc_regmap_cfg = tscadc_regmap_config; if (!pdev->dev.of_node) { dev_err(&pdev->dev, "Could not find valid DT data.\n"); @@ -110,6 +127,7 @@ static int ti_tscadc_probe(struct platform_device *pdev) } else tscadc->irq = err; + spin_lock_init(&tscadc->reg_map_lock); res = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), pdev->name); if (!res) { @@ -124,8 +142,9 @@ static int ti_tscadc_probe(struct platform_device *pdev) return -ENOMEM; } + tscadc_regmap_cfg.lock_arg = tscadc; tscadc->regmap_tscadc = devm_regmap_init_mmio(&pdev->dev, - tscadc->tscadc_base, &tscadc_regmap_config); + tscadc->tscadc_base, &tscadc_regmap_cfg); if (IS_ERR(tscadc->regmap_tscadc)) { dev_err(&pdev->dev, "regmap init failed\n"); err = PTR_ERR(tscadc->regmap_tscadc); diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h index c985262..284e482 100644 --- a/include/linux/mfd/ti_am335x_tscadc.h +++ b/include/linux/mfd/ti_am335x_tscadc.h @@ -129,6 +129,8 @@ #define TSCADC_CELLS 2 struct ti_tscadc_dev { + spinlock_t reg_map_lock; + unsigned long reg_map_flags; struct device *dev; struct regmap *regmap_tscadc; void __iomem *tscadc_base;
Without this, devm_regmap_init_mmio() creates & uses a simple spin_lock() and this should be enough. Within the probe function the registers are read and written in process context. Later they are accessed from the ISR and lockdep complains because now the lock is taken suddenly with IRQs enabled. Currently I don't see any other way to keep lockdep quiet than doing this. Cc: Mark Brown <broonie@kernel.org> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- v1..v2: pass a ti_tscadc_dev via lock arg drivers/mfd/ti_am335x_tscadc.c | 21 ++++++++++++++++++++- include/linux/mfd/ti_am335x_tscadc.h | 2 ++ 2 files changed, 22 insertions(+), 1 deletion(-)