Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

pinctrl: mcp23s08: init reg_defaults from HW at probe and switch cache type

The probe function does not guarantee that chip registers are in their
default state. Thus using reg_defaults for regmap is incorrect.

For example, the chip may have already been configured by the bootloader
before the Linux driver loads, or the mcp might not have a reset at all
and not reset a state between reboots.

In such cases, using reg_defaults leads to the cache values diverging
from the actual registers values in the chip.

Previous attempts to fix consequences of this issue were made in
'commit 3ede3f8b4b4b ("pinctrl: mcp23s08: Reset all pins to input at
probe")', but this is insufficient. The OLAT register reset is also
required. And there's still potential for new issues arising due to cache
desynchronization of other registers.

Therefore, remove reg_defaults and provide num_reg_defaults_raw. In that
case the cache defaults being initialized from hardware.

Also switch cache type to REGCACHE_MAPLE, which is aware of (in)valid
cache entries.

And remove the force reset all pins to input at probe as it is no longer
required.

Link: https://lore.kernel.org/all/20251009132651.649099-2-bigunclemax@gmail.com/
Suggested-by: Mike Looijmans <mike.looijmans@topic.nl>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Sander Vanheule <sander@svanheule.net>
Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

authored by

Maksim Kiselev and committed by
Linus Walleij
f9f4fda1 b4592884

+4 -36
+4 -36
drivers/pinctrl/pinctrl-mcp23s08.c
··· 44 44 #define MCP_GPIO 0x09 45 45 #define MCP_OLAT 0x0a 46 46 47 - static const struct reg_default mcp23x08_defaults[] = { 48 - {.reg = MCP_IODIR, .def = 0xff}, 49 - {.reg = MCP_IPOL, .def = 0x00}, 50 - {.reg = MCP_GPINTEN, .def = 0x00}, 51 - {.reg = MCP_DEFVAL, .def = 0x00}, 52 - {.reg = MCP_INTCON, .def = 0x00}, 53 - {.reg = MCP_IOCON, .def = 0x00}, 54 - {.reg = MCP_GPPU, .def = 0x00}, 55 - {.reg = MCP_OLAT, .def = 0x00}, 56 - }; 57 - 58 47 static const struct regmap_range mcp23x08_volatile_range = { 59 48 .range_min = MCP_INTF, 60 49 .range_max = MCP_GPIO, ··· 71 82 .reg_stride = 1, 72 83 .volatile_table = &mcp23x08_volatile_table, 73 84 .precious_table = &mcp23x08_precious_table, 74 - .reg_defaults = mcp23x08_defaults, 75 - .num_reg_defaults = ARRAY_SIZE(mcp23x08_defaults), 76 - .cache_type = REGCACHE_FLAT, 85 + .num_reg_defaults_raw = MCP_OLAT + 1, 86 + .cache_type = REGCACHE_MAPLE, 77 87 .max_register = MCP_OLAT, 78 88 .disable_locking = true, /* mcp->lock protects the regmap */ 79 89 }; 80 90 EXPORT_SYMBOL_GPL(mcp23x08_regmap); 81 - 82 - static const struct reg_default mcp23x17_defaults[] = { 83 - {.reg = MCP_IODIR << 1, .def = 0xffff}, 84 - {.reg = MCP_IPOL << 1, .def = 0x0000}, 85 - {.reg = MCP_GPINTEN << 1, .def = 0x0000}, 86 - {.reg = MCP_DEFVAL << 1, .def = 0x0000}, 87 - {.reg = MCP_INTCON << 1, .def = 0x0000}, 88 - {.reg = MCP_IOCON << 1, .def = 0x0000}, 89 - {.reg = MCP_GPPU << 1, .def = 0x0000}, 90 - {.reg = MCP_OLAT << 1, .def = 0x0000}, 91 - }; 92 91 93 92 static const struct regmap_range mcp23x17_volatile_range = { 94 93 .range_min = MCP_INTF << 1, ··· 106 129 .max_register = MCP_OLAT << 1, 107 130 .volatile_table = &mcp23x17_volatile_table, 108 131 .precious_table = &mcp23x17_precious_table, 109 - .reg_defaults = mcp23x17_defaults, 110 - .num_reg_defaults = ARRAY_SIZE(mcp23x17_defaults), 111 - .cache_type = REGCACHE_FLAT, 132 + .num_reg_defaults_raw = MCP_OLAT + 1, 133 + .cache_type = REGCACHE_MAPLE, 112 134 .val_format_endian = REGMAP_ENDIAN_LITTLE, 113 135 .disable_locking = true, /* mcp->lock protects the regmap */ 114 136 }; ··· 617 641 mcp->chip.owner = THIS_MODULE; 618 642 619 643 mcp->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); 620 - 621 - /* 622 - * Reset the chip - we don't really know what state it's in, so reset 623 - * all pins to input first to prevent surprises. 624 - */ 625 - ret = mcp_write(mcp, MCP_IODIR, mcp->chip.ngpio == 16 ? 0xFFFF : 0xFF); 626 - if (ret < 0) 627 - return ret; 628 644 629 645 /* verify MCP_IOCON.SEQOP = 0, so sequential reads work, 630 646 * and MCP_IOCON.HAEN = 1, so we work with all chips.