On Thu, Nov 21, 2024 at 11:34:29AM +0900, Joe Hattori wrote:
> An fwnode_handle and usb_role_switch are obtained with an incremented
> refcount in anx7411_typec_port_probe(), however the refcounts are not
> decremented in the error path. The fwnode_handle is also not decremented
> in the .remove() function. Therefore, call fwnode_handle_put() and
> usb_role_switch_put() accordingly.
>
> Fixes: fe6d8a9c8e64 ("usb: typec: anx7411: Add Analogix PD ANX7411 support")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Joe Hattori <joe@xxxxxxxxxxxxxxxxxxxxx>
You don't need to check for the port nor the role_sw, but never mind:
Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> Changes in v2:
> - Call usb_role_switch_put()
> - Remove the port in anx7411_port_unregister().
> ---
> drivers/usb/typec/anx7411.c | 47 +++++++++++++++++++++++--------------
> 1 file changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/usb/typec/anx7411.c b/drivers/usb/typec/anx7411.c
> index d1e7c487ddfb..95607efb9f7e 100644
> --- a/drivers/usb/typec/anx7411.c
> +++ b/drivers/usb/typec/anx7411.c
> @@ -1021,6 +1021,16 @@ static void anx7411_port_unregister_altmodes(struct typec_altmode **adev)
> }
> }
>
> +static void anx7411_port_unregister(struct typec_params *typecp)
> +{
> + fwnode_handle_put(typecp->caps.fwnode);
> + anx7411_port_unregister_altmodes(typecp->port_amode);
> + if (typecp->port)
> + typec_unregister_port(typecp->port);
> + if (typecp->role_sw)
> + usb_role_switch_put(typecp->role_sw);
> +}
> +
> static int anx7411_usb_mux_set(struct typec_mux_dev *mux,
> struct typec_mux_state *state)
> {
> @@ -1154,34 +1164,34 @@ static int anx7411_typec_port_probe(struct anx7411_data *ctx,
> ret = fwnode_property_read_string(fwnode, "power-role", &buf);
> if (ret) {
> dev_err(dev, "power-role not found: %d\n", ret);
> - return ret;
> + goto put_fwnode;
> }
>
> ret = typec_find_port_power_role(buf);
> if (ret < 0)
> - return ret;
> + goto put_fwnode;
> cap->type = ret;
>
> ret = fwnode_property_read_string(fwnode, "data-role", &buf);
> if (ret) {
> dev_err(dev, "data-role not found: %d\n", ret);
> - return ret;
> + goto put_fwnode;
> }
>
> ret = typec_find_port_data_role(buf);
> if (ret < 0)
> - return ret;
> + goto put_fwnode;
> cap->data = ret;
>
> ret = fwnode_property_read_string(fwnode, "try-power-role", &buf);
> if (ret) {
> dev_err(dev, "try-power-role not found: %d\n", ret);
> - return ret;
> + goto put_fwnode;
> }
>
> ret = typec_find_power_role(buf);
> if (ret < 0)
> - return ret;
> + goto put_fwnode;
> cap->prefer_role = ret;
>
> /* Get source pdos */
> @@ -1193,7 +1203,7 @@ static int anx7411_typec_port_probe(struct anx7411_data *ctx,
> typecp->src_pdo_nr);
> if (ret < 0) {
> dev_err(dev, "source cap validate failed: %d\n", ret);
> - return -EINVAL;
> + goto put_fwnode;
> }
>
> typecp->caps_flags |= HAS_SOURCE_CAP;
> @@ -1207,7 +1217,7 @@ static int anx7411_typec_port_probe(struct anx7411_data *ctx,
> typecp->sink_pdo_nr);
> if (ret < 0) {
> dev_err(dev, "sink cap validate failed: %d\n", ret);
> - return -EINVAL;
> + goto put_fwnode;
> }
>
> for (i = 0; i < typecp->sink_pdo_nr; i++) {
> @@ -1251,13 +1261,21 @@ static int anx7411_typec_port_probe(struct anx7411_data *ctx,
> ret = PTR_ERR(ctx->typec.port);
> ctx->typec.port = NULL;
> dev_err(dev, "Failed to register type c port %d\n", ret);
> - return ret;
> + goto put_usb_role_switch;
> }
>
> typec_port_register_altmodes(ctx->typec.port, NULL, ctx,
> ctx->typec.port_amode,
> MAX_ALTMODE);
> return 0;
> +
> +put_usb_role_switch:
> + if (ctx->typec.role_sw)
> + usb_role_switch_put(ctx->typec.role_sw);
> +put_fwnode:
> + fwnode_handle_put(fwnode);
> +
> + return ret;
> }
>
> static int anx7411_typec_check_connection(struct anx7411_data *ctx)
> @@ -1523,8 +1541,7 @@ static int anx7411_i2c_probe(struct i2c_client *client)
> destroy_workqueue(plat->workqueue);
>
> free_typec_port:
> - typec_unregister_port(plat->typec.port);
> - anx7411_port_unregister_altmodes(plat->typec.port_amode);
> + anx7411_port_unregister(&plat->typec);
>
> free_typec_switch:
> anx7411_unregister_switch(plat);
> @@ -1548,17 +1565,11 @@ static void anx7411_i2c_remove(struct i2c_client *client)
>
> i2c_unregister_device(plat->spi_client);
>
> - if (plat->typec.role_sw)
> - usb_role_switch_put(plat->typec.role_sw);
> -
> anx7411_unregister_mux(plat);
>
> anx7411_unregister_switch(plat);
>
> - if (plat->typec.port)
> - typec_unregister_port(plat->typec.port);
> -
> - anx7411_port_unregister_altmodes(plat->typec.port_amode);
> + anx7411_port_unregister(&plat->typec);
> }
>
> static const struct i2c_device_id anx7411_id[] = {
> --
> 2.34.1
--
heikki
