lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5af22765-b428-468a-8cc4-cddc561f4a50@topic.nl>
Date: Mon, 6 Oct 2025 10:04:09 +0200
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: bigunclemax@...il.com
CC: Linus Walleij <linus.walleij@...aro.org>, linux-gpio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] pinctrl: mcp23s08: Reset all output latches to default
 at probe

On 06-10-2025 09:49, bigunclemax@...il.com wrote:
> From: "Maksim Kiselev" <bigunclemax@...il.com>
>
> It appears that resetting only the direction register is not sufficient,
> it's also necessary to reset the OLAT register to its default values.
>
> Otherwise, the following situation can occur:
>
> If a pin was configured as OUT=1 before driver probe(Ex: IODIR=1,IOLAT=1),
> then after loading the MCP driver, the cache will be populated from
> reg_defaults with IOLAT=0 (while the actual value in the chip is 1).
> A subsequent setting OUT=0 will fail because
> mcp_update_bits(mcp, MCP_OLAT, ...) calls regmap_update_bits(),
> which will check that the value to be set (0) matches the cached value (0)
> and thus skip writing actual value to the MCP chip.

Maybe it's be better to fix the underlying issue: This driver should not be 
using a pre-populated regmap cache. Unless it performs a hard reset, the 
driver has no way of knowing what the initial values are, it should just read 
them from the chip.

>
> To avoid this, the OLAT register must be explicitly reset at probe.
>
> Fixes: 3ede3f8b4b4b ("pinctrl: mcp23s08: Reset all pins to input at probe")
> Signed-off-by: Maksim Kiselev <bigunclemax@...il.com>
> ---
>   drivers/pinctrl/pinctrl-mcp23s08.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 78ff7930649d..23af441aa468 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -622,6 +622,11 @@ int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>   	if (ret < 0)
>   		return ret;
>   
> +	/* Also reset all out latches to default values */
> +	ret = mcp_write(mcp, MCP_OLAT, 0x0);
> +	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.
>   	 */

Mike.




Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ