Re: [PATCH 2/2] usb: typec: fsa4480: Report switch & mux events to codec

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: typec: fsa4480: leak of codec switch and mux in fsa4480_remove()
--

commit ec3a8ea75d1b120f5c00db8c4658dd7e1e5f7c65
Author: Luca Weiss <luca.weiss@fairphone.com>

usb: typec: fsa4480: Report switch & mux events to codec

This commit updates the fsa4480 driver to acquire a Type-C mux and
orientation switch for the connected audio codec and forward switch and
mux state updates to it. It also enables the SENSE and MIC bits when
entering audio accessory mode.

> diff --git a/drivers/usb/typec/mux/fsa4480.c b/drivers/usb/typec/mux/fsa4480.c
> index bea0c1deec94d..d992a7e1887c8 100644
> --- a/drivers/usb/typec/mux/fsa4480.c
> +++ b/drivers/usb/typec/mux/fsa4480.c

[ ... ]

> @@ -287,6 +299,19 @@ static int fsa4480_probe(struct i2c_client *client)
>  		FIELD_GET(FSA4480_DEVICE_ID_REV_ID, val),
>  		FIELD_GET(FSA4480_DEVICE_ID_VENDOR_ID, val));
>  
> +	/* Get Codec's MUX & Switch devices */
> +	fsa->codec = fwnode_typec_mux_get(dev->fwnode);
> +	if (IS_ERR(fsa->codec))
> +		return dev_err_probe(dev, PTR_ERR(fsa->codec),
> +				     "failed to acquire codec mode-switch\n");
> +
> +	fsa->codec_switch = fwnode_typec_switch_get(dev->fwnode);
> +	if (IS_ERR(fsa->codec_switch)) {
> +		ret = dev_err_probe(dev, PTR_ERR(fsa->codec_switch),
> +				    "failed to acquire codec orientation-switch\n");
> +		goto err_put_mux;
> +	}
> +
>  	/* Safe mode */
>  	fsa->cur_enable = FSA4480_ENABLE_DEVICE | FSA4480_ENABLE_USB;
>  	fsa->mode = TYPEC_STATE_SAFE;
> @@ -310,8 +335,11 @@ static int fsa4480_probe(struct i2c_client *client)
>  	sw_desc.set = fsa4480_switch_set;
>  
>  	fsa->sw = typec_switch_register(dev, &sw_desc);
> -	if (IS_ERR(fsa->sw))
> -		return dev_err_probe(dev, PTR_ERR(fsa->sw), "failed to register typec switch\n");
> +	if (IS_ERR(fsa->sw)) {
> +		ret = dev_err_probe(dev, PTR_ERR(fsa->sw),
> +				    "failed to register typec switch\n");
> +		goto err_put_switch;
> +	}
>  
>  	mux_desc.drvdata = fsa;
>  	mux_desc.fwnode = dev_fwnode(dev);
> @@ -319,12 +347,24 @@ static int fsa4480_probe(struct i2c_client *client)
>  
>  	fsa->mux = typec_mux_register(dev, &mux_desc);
>  	if (IS_ERR(fsa->mux)) {
> -		typec_switch_unregister(fsa->sw);
> -		return dev_err_probe(dev, PTR_ERR(fsa->mux), "failed to register typec mux\n");
> +		ret = dev_err_probe(dev, PTR_ERR(fsa->mux),
> +				    "failed to register typec mux\n");
> +		goto err_unregister_switch;
>  	}
>  
>  	i2c_set_clientdata(client, fsa);
>  	return 0;
> +
> +err_unregister_switch:
> +	typec_switch_unregister(fsa->sw);
> +
> +err_put_switch:
> +	typec_switch_put(fsa->codec_switch);
> +
> +err_put_mux:
> +	typec_mux_put(fsa->codec);
> +
> +	return ret;
>  }
>  
>  static void fsa4480_remove(struct i2c_client *client)

[Severity: Medium]
Does fsa4480_remove() leak fsa->codec_switch and fsa->codec when the
device is unbound or the driver module is unloaded?

In fsa4480_probe(), fwnode_typec_mux_get() and fwnode_typec_switch_get()
allocate the wrapper structures and take device and module references on
the codec provider, which are not managed by devres. While the error path
in fsa4480_probe() cleans them up, fsa4480_remove() still only unregisters
fsa->mux and fsa->sw:

static void fsa4480_remove(struct i2c_client *client)
{
	struct fsa4480 *fsa = i2c_get_clientdata(client);

	typec_mux_unregister(fsa->mux);
	typec_switch_unregister(fsa->sw);
}

Should fsa4480_remove() also call typec_switch_put(fsa->codec_switch) and
typec_mux_put(fsa->codec)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018225906.1265299-1-val@packett.cool?part=2