Re: [PATCH] usb: typec: mux: Fix typec_switch_match()

[Thorsten Leemhuis]

On 8/17/26 20:22, Marek Vasut wrote:
> The fwnode_typec_switch_get() sporadically returns NULL instead of an
> -EPROBE_DEFER for orientation-switch described in DT.

Hi Marek! Just to make sure, have you seen the patch submission
"usb: typec: mux: initialize orientation switch array"?
https://lore.kernel.org/all/20260804083434.20885-1-i@xxxxx/

It has a fixes tag for the commit you mention and reads: ""Commit
a53b4f9c51a9 ("usb: typec: mux: avoid duplicated orientation switches")
started using the orientation switch result array as state for duplicate
detection, but left the array uninitialized.

The first match therefore scans indeterminate stack contents before any
result has been stored. [...]""

Not my area of expertise, but it sounds related, that's why I wanted to
ensure you are aware of it. If it's something else: sorry!

Ciao, Thorsten

> This makes it
> impossible to discern whether the DT does describe an orientation-switch
> which did not probe yet, or whether the DT does not describe the switch.
> This happens with gpio-sbu-mux connected to an I2C GPIO expander.
> 
> The class_find_device() on typec_switch_match() may return NULL in case
> the mux did not probe just yet early on boot. The sw_devs[] array can be
> empty on boot as well. If these two conditions occur, then the conditional
> if (to_typec_switch_dev(dev) == sw_devs[i]) evaluates to true and the match
> function returns NULL, which propagates to fwnode_typec_switch_get() which
> makes it look as if the orientation-switch was not described in DT.
> 
> This is incorrect, because the mux driver will probe a bit later on, but
> at that point, the caller of fwnode_typec_switch_get() already got the
> NULL return value. The NULL return value also does not trigger IS_ERR(),
> therefore the caller driver interprets this as if the orientation-switch
> is not described in DT, and does not return -EPROBE_DEFER to try again,
> even if it should.
> 
> Fix this by checking the class_find_device() return value, and return
> -EPROBE_DEFER if it is NULL right away. If the return value is not NULL,
> perform the deduplication test, and if that test passes, consider the
> return value to be already non-NULL.
> 
> Fixes: a53b4f9c51a9 ("usb: typec: mux: avoid duplicated orientation switches")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Marek Vasut <marex@xxxxxxxxxxxx>
> ---
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Cc: Jens Glathe <jens.glathe@xxxxxxxxxxxxxxxxxxxxxx>
> Cc: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> Cc: kernel@xxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: linux-usb@xxxxxxxxxxxxxxx
> ---
> NOTE: A similar change was reverted in
>       f576c75f95a5 ("Revert "usb: typec: mux: avoid duplicated mux switches"")
>       Maybe the orientation switch commit also needs a revert ?
>       Or the mux switch revert can be undone and fixed using this NULL check ?
> ---
>  drivers/usb/typec/mux.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
> index 9b908c46bd7df..2bc7e8edb3cbd 100644
> --- a/drivers/usb/typec/mux.c
> +++ b/drivers/usb/typec/mux.c
> @@ -56,17 +56,19 @@ static void *typec_switch_match(const struct fwnode_handle *fwnode,
>  	 * function "defers probe" for now.
>  	 */
>  	dev = class_find_device(&typec_mux_class, NULL, fwnode,
>  				switch_fwnode_match);
> +	if (!dev)
> +		return ERR_PTR(-EPROBE_DEFER);
>  
>  	/* Skip duplicates */
>  	for (i = 0; i < TYPEC_MUX_MAX_DEVS; i++)
>  		if (to_typec_switch_dev(dev) == sw_devs[i]) {
>  			put_device(dev);
>  			return NULL;
>  		}
>  
> -	return dev ? to_typec_switch_dev(dev) : ERR_PTR(-EPROBE_DEFER);
> +	return to_typec_switch_dev(dev);
>  }
>  
>  /**
>   * fwnode_typec_switch_get - Find USB Type-C orientation switch