Skip to content

Commit a37eecb

Browse files
Evgenii Shatokhinlinusw
authored andcommitted
pinctrl: mcp23s08: Fix sleeping in atomic context due to regmap locking
If a device uses MCP23xxx IO expander to receive IRQs, the following bug can happen: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:283 in_atomic(): 1, irqs_disabled(): 1, non_block: 0, ... preempt_count: 1, expected: 0 ... Call Trace: ... __might_resched+0x104/0x10e __might_sleep+0x3e/0x62 mutex_lock+0x20/0x4c regmap_lock_mutex+0x10/0x18 regmap_update_bits_base+0x2c/0x66 mcp23s08_irq_set_type+0x1ae/0x1d6 __irq_set_trigger+0x56/0x172 __setup_irq+0x1e6/0x646 request_threaded_irq+0xb6/0x160 ... We observed the problem while experimenting with a touchscreen driver which used MCP23017 IO expander (I2C). The regmap in the pinctrl-mcp23s08 driver uses a mutex for protection from concurrent accesses, which is the default for regmaps without .fast_io, .disable_locking, etc. mcp23s08_irq_set_type() calls regmap_update_bits_base(), and the latter locks the mutex. However, __setup_irq() locks desc->lock spinlock before calling these functions. As a result, the system tries to lock the mutex whole holding the spinlock. It seems, the internal regmap locks are not needed in this driver at all. mcp->lock seems to protect the regmap from concurrent accesses already, except, probably, in mcp_pinconf_get/set. mcp23s08_irq_set_type() and mcp23s08_irq_mask/unmask() are called under chip_bus_lock(), which calls mcp23s08_irq_bus_lock(). The latter takes mcp->lock and enables regmap caching, so that the potentially slow I2C accesses are deferred until chip_bus_unlock(). The accesses to the regmap from mcp23s08_probe_one() do not need additional locking. In all remaining places where the regmap is accessed, except mcp_pinconf_get/set(), the driver already takes mcp->lock. This patch adds locking in mcp_pinconf_get/set() and disables internal locking in the regmap config. Among other things, it fixes the sleeping in atomic context described above. Fixes: 8f38910 ("pinctrl: mcp23s08: switch to regmap caching") Cc: stable@vger.kernel.org Signed-off-by: Evgenii Shatokhin <e.shatokhin@yadro.com> Link: https://lore.kernel.org/20241209074659.1442898-1-e.shatokhin@yadro.com Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
1 parent b77bd3b commit a37eecb

File tree

1 file changed

+6
-0
lines changed

1 file changed

+6
-0
lines changed

drivers/pinctrl/pinctrl-mcp23s08.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ const struct regmap_config mcp23x08_regmap = {
8686
.num_reg_defaults = ARRAY_SIZE(mcp23x08_defaults),
8787
.cache_type = REGCACHE_FLAT,
8888
.max_register = MCP_OLAT,
89+
.disable_locking = true, /* mcp->lock protects the regmap */
8990
};
9091
EXPORT_SYMBOL_GPL(mcp23x08_regmap);
9192

@@ -132,6 +133,7 @@ const struct regmap_config mcp23x17_regmap = {
132133
.num_reg_defaults = ARRAY_SIZE(mcp23x17_defaults),
133134
.cache_type = REGCACHE_FLAT,
134135
.val_format_endian = REGMAP_ENDIAN_LITTLE,
136+
.disable_locking = true, /* mcp->lock protects the regmap */
135137
};
136138
EXPORT_SYMBOL_GPL(mcp23x17_regmap);
137139

@@ -228,7 +230,9 @@ static int mcp_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
228230

229231
switch (param) {
230232
case PIN_CONFIG_BIAS_PULL_UP:
233+
mutex_lock(&mcp->lock);
231234
ret = mcp_read(mcp, MCP_GPPU, &data);
235+
mutex_unlock(&mcp->lock);
232236
if (ret < 0)
233237
return ret;
234238
status = (data & BIT(pin)) ? 1 : 0;
@@ -257,7 +261,9 @@ static int mcp_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
257261

258262
switch (param) {
259263
case PIN_CONFIG_BIAS_PULL_UP:
264+
mutex_lock(&mcp->lock);
260265
ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg);
266+
mutex_unlock(&mcp->lock);
261267
break;
262268
default:
263269
dev_dbg(mcp->dev, "Invalid config param %04x\n", param);

0 commit comments

Comments
 (0)