From: bigunclemax@gmail.com
Cc: bigunclemax@gmail.com, Mike Looijmans <mike.looijmans@topic.nl>,
Linus Walleij <linus.walleij@linaro.org>,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2] pinctrl: mcp23s08: delete regmap reg_defaults to avoid cache sync issues
Date: Thu, 9 Oct 2025 16:26:47 +0300 [thread overview]
Message-ID: <20251009132651.649099-2-bigunclemax@gmail.com> (raw)
From: Maksim Kiselev <bigunclemax@gmail.com>
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 entirely to eliminate the root cause
of these problems.
Also remove the force reset all pins to input at probe as it is no longer
required.
Link: https://lore.kernel.org/all/20251006074934.27180-1-bigunclemax@gmail.com/
Suggested-by: Mike Looijmans <mike.looijmans@topic.nl>
Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
---
drivers/pinctrl/pinctrl-mcp23s08.c | 34 ------------------------------
1 file changed, 34 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 78ff7930649d2..0b329661b5978 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -44,17 +44,6 @@
#define MCP_GPIO 0x09
#define MCP_OLAT 0x0a
-static const struct reg_default mcp23x08_defaults[] = {
- {.reg = MCP_IODIR, .def = 0xff},
- {.reg = MCP_IPOL, .def = 0x00},
- {.reg = MCP_GPINTEN, .def = 0x00},
- {.reg = MCP_DEFVAL, .def = 0x00},
- {.reg = MCP_INTCON, .def = 0x00},
- {.reg = MCP_IOCON, .def = 0x00},
- {.reg = MCP_GPPU, .def = 0x00},
- {.reg = MCP_OLAT, .def = 0x00},
-};
-
static const struct regmap_range mcp23x08_volatile_range = {
.range_min = MCP_INTF,
.range_max = MCP_GPIO,
@@ -82,25 +71,12 @@ const struct regmap_config mcp23x08_regmap = {
.reg_stride = 1,
.volatile_table = &mcp23x08_volatile_table,
.precious_table = &mcp23x08_precious_table,
- .reg_defaults = mcp23x08_defaults,
- .num_reg_defaults = ARRAY_SIZE(mcp23x08_defaults),
.cache_type = REGCACHE_FLAT,
.max_register = MCP_OLAT,
.disable_locking = true, /* mcp->lock protects the regmap */
};
EXPORT_SYMBOL_GPL(mcp23x08_regmap);
-static const struct reg_default mcp23x17_defaults[] = {
- {.reg = MCP_IODIR << 1, .def = 0xffff},
- {.reg = MCP_IPOL << 1, .def = 0x0000},
- {.reg = MCP_GPINTEN << 1, .def = 0x0000},
- {.reg = MCP_DEFVAL << 1, .def = 0x0000},
- {.reg = MCP_INTCON << 1, .def = 0x0000},
- {.reg = MCP_IOCON << 1, .def = 0x0000},
- {.reg = MCP_GPPU << 1, .def = 0x0000},
- {.reg = MCP_OLAT << 1, .def = 0x0000},
-};
-
static const struct regmap_range mcp23x17_volatile_range = {
.range_min = MCP_INTF << 1,
.range_max = MCP_GPIO << 1,
@@ -129,8 +105,6 @@ const struct regmap_config mcp23x17_regmap = {
.max_register = MCP_OLAT << 1,
.volatile_table = &mcp23x17_volatile_table,
.precious_table = &mcp23x17_precious_table,
- .reg_defaults = mcp23x17_defaults,
- .num_reg_defaults = ARRAY_SIZE(mcp23x17_defaults),
.cache_type = REGCACHE_FLAT,
.val_format_endian = REGMAP_ENDIAN_LITTLE,
.disable_locking = true, /* mcp->lock protects the regmap */
@@ -614,14 +588,6 @@ int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
mcp->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
- /*
- * Reset the chip - we don't really know what state it's in, so reset
- * all pins to input first to prevent surprises.
- */
- ret = mcp_write(mcp, MCP_IODIR, mcp->chip.ngpio == 16 ? 0xFFFF : 0xFF);
- if (ret < 0)
- return ret;
-
/* verify MCP_IOCON.SEQOP = 0, so sequential reads work,
* and MCP_IOCON.HAEN = 1, so we work with all chips.
*/
--
2.48.1
next reply other threads:[~2025-10-09 13:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-09 13:26 bigunclemax [this message]
2025-10-13 13:22 ` [PATCH v2] pinctrl: mcp23s08: delete regmap reg_defaults to avoid cache sync issues Linus Walleij
2025-10-20 19:40 ` Sander Vanheule
2025-10-22 14:45 ` Mike Looijmans
2025-10-24 6:54 ` Mike Looijmans
-- strict thread matches above, loose matches on Subject: below --
2025-10-18 19:31 Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251009132651.649099-2-bigunclemax@gmail.com \
--to=bigunclemax@gmail.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mike.looijmans@topic.nl \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).