linux-kernel - Re: [PATCH v2 2/2] gpio: add gpio-line-mux driver

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bad968a4-37e6-655e-711f-c647becbba5a@axentia.se>
Date: Mon, 27 Oct 2025 08:51:20 +0100
From: Peter Rosin <peda@...ntia.se>
To: Jonas Jelonek <jelonek.jonas@...il.com>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
 <brgl@...ev.pl>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Geert Uytterhoeven <geert+renesas@...der.be>
Cc: linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] gpio: add gpio-line-mux driver

Hi!

2025-10-27 at 00:17, Jonas Jelonek wrote:
> Add a new driver which provides a 1-to-many mapping for a single real
> GPIO using a multiplexer. Each virtual GPIO corresponds to a multiplexer
> state which, if set for the multiplexer, connects the real GPIO to the
> corresponding virtual GPIO.
> 
> For now, this doesn't support advanced features like IRQs, just normal
> IN and OUT functionality of GPIOs.
> 
> This can help in various usecases. One practical case is the special
> hardware design of the Realtek-based XS1930-10 switch from Zyxel. It
> features two SFP+ ports/cages whose signals are wired to directly to the
> switch SoC. Although Realtek SoCs are short on GPIOs, there are usually
> enough the fit the SFP signals without any hacks.
> 
> However, Zyxel did some weird design and connected RX_LOS, MOD_ABS and
> TX_FAULT of one SFP cage onto a single GPIO line controlled by a
> multiplexer (the same for the other SFP cage). The single multiplexer
> controls the lines for both SFP and depending on the state, the
> designated 'signal GPIO lines' are connected to one of the three SFP
> signals.
> 
> Because the SFP core/driver doesn't support multiplexer but needs single
> GPIOs for each of the signals, this driver fills the gap between both.
> It registers a gpio_chip, provides multiple virtual GPIOs and sets the
> backing multiplexer accordingly.
> 
> Signed-off-by: Jonas Jelonek <jelonek.jonas@...il.com>

*snip*

> +static int gpio_lmux_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct gpio_lmux *glm = (struct gpio_lmux *)gpiochip_get_data(gc);
> +	int ret;
> +
> +	if (offset > gc->ngpio)
> +		return -EINVAL;
> +
> +	guard(gpio_lmux)(glm);
> +
> +	ret = mux_control_select(glm->mux, glm->gpio_mux_states[offset]);

Consider using mux_control_select_delay() here, with some suitable
delay, to allow the mux to settle before reading the gpio line.

> +	if (ret < 0)
> +		return ret;
> +
> +	ret = gpiod_get_raw_value_cansleep(glm->shared_gpio);
> +	mux_control_deselect(glm->mux);
> +	return ret;
> +}
> +
> +static int gpio_lmux_gpio_set(struct gpio_chip *gc, unsigned int offset,
> +			      int value)
> +{
> +	struct gpio_lmux *glm = (struct gpio_lmux *)gpiochip_get_data(gc);
> +	int ret;
> +
> +	if (offset > gc->ngpio)
> +		return -EINVAL;
> +
> +	guard(gpio_lmux)(glm);
> +
> +	ret = mux_control_select(glm->mux, glm->gpio_mux_states[offset]);
> +	if (ret < 0)
> +		return ret;
> +
> +	gpiod_set_raw_value_cansleep(glm->shared_gpio, value);
> +	mux_control_deselect(glm->mux);

This .set implementation is completely broken. It you want to
set a gpio to outout high/low, you presumably want the gpio to
stay that way for at least some period of time, while whatever
else happens that relies on the gpio to be in that state. But in
this case only the mux select/deselect is protecting that gpio
state, which is bound to be inadequate for anything real.

Sure, you can probably build something trivial and see that the
gpio can be manipulated, but the second something else touches
the mux, the intended state of an output gpio line is
(potentially) clobbered.

I notice that in your target application, the sfp driver, all
uses of gpios via the mux are inputs. Input is a much easier
problem. At least as long as you do not require IRQ, if you
need IRQs you face similar problems where the mux needs to be
locked in its position for whatever period of time you can
expect IRQs.

TL;DR, this .set implementation needs to be removed, there is
simply no reasonable way to implement a muxed gpio .set in a
pure software driver. You need some hardware to preserve the
state if/when the mux is manipulated.

Cheers,
Peter

> +	return 0;
> +}