We can't just return if devm_tps6598_psy_register fails since previous
resources are not devres managed and have yet to be cleaned up.
Fixes: 10eb0b6ac63a ("usb: typec: tps6598x: Export some power supply properties")
Signed-off-by: Sven Peter <[email protected]>
---
drivers/usb/typec/tipd/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 748ff4f6b5f6..ebc786d728e2 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -827,7 +827,7 @@ static int tps6598x_probe(struct i2c_client *client)
ret = devm_tps6598_psy_register(tps);
if (ret)
- return ret;
+ goto err_role_put;
tps->port = typec_register_port(&client->dev, &typec_cap);
if (IS_ERR(tps->port)) {
--
2.25.1
The err_role_put error path always calls fwnode_handle_put to release
the fwnode. This path can be reached after probe itself has already
released that fwnode though. Fix that by moving fwnode_handle_put in the
happy path to the very end.
Fixes: 18a6c866bb19 ("usb: typec: tps6598x: Add USB role switching logic")
Signed-off-by: Sven Peter <[email protected]>
---
drivers/usb/typec/tipd/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index ebc786d728e2..824e573af570 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -834,7 +834,6 @@ static int tps6598x_probe(struct i2c_client *client)
ret = PTR_ERR(tps->port);
goto err_role_put;
}
- fwnode_handle_put(fwnode);
if (tps->status & TPS_STATUS_PLUG_PRESENT) {
ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &tps->pwr_status);
@@ -858,6 +857,7 @@ static int tps6598x_probe(struct i2c_client *client)
}
i2c_set_clientdata(client, tps);
+ fwnode_handle_put(fwnode);
return 0;
--
2.25.1
typec_unregister_port is only called for some error paths after
typec_register_port was successful. Ensure it's called in all
cases.
Fixes: 92440202a880 ("usb: typec: tipd: Only update power status on IRQ")
Signed-off-by: Sven Peter <[email protected]>
---
drivers/usb/typec/tipd/core.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 824e573af570..c35501a92b4d 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -839,7 +839,7 @@ static int tps6598x_probe(struct i2c_client *client)
ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &tps->pwr_status);
if (ret < 0) {
dev_err(tps->dev, "failed to read power status: %d\n", ret);
- goto err_role_put;
+ goto err_unregister_port;
}
ret = tps6598x_connect(tps);
if (ret)
@@ -852,8 +852,7 @@ static int tps6598x_probe(struct i2c_client *client)
dev_name(&client->dev), tps);
if (ret) {
tps6598x_disconnect(tps, 0);
- typec_unregister_port(tps->port);
- goto err_role_put;
+ goto err_unregister_port;
}
i2c_set_clientdata(client, tps);
@@ -861,6 +860,8 @@ static int tps6598x_probe(struct i2c_client *client)
return 0;
+err_unregister_port:
+ typec_unregister_port(tps->port);
err_role_put:
usb_role_switch_put(tps->role_sw);
err_fwnode_put:
--
2.25.1
While the code currently correctly calls tps6598x_disconnect before jumping
to the error cleanup label it's inconsistent compared to all the other cleanup
actions and prone to introduce bugs if any more resources are added.
Signed-off-by: Sven Peter <[email protected]>
---
drivers/usb/typec/tipd/core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index c35501a92b4d..22ff212e05e6 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -850,16 +850,16 @@ static int tps6598x_probe(struct i2c_client *client)
irq_handler,
IRQF_SHARED | IRQF_ONESHOT,
dev_name(&client->dev), tps);
- if (ret) {
- tps6598x_disconnect(tps, 0);
- goto err_unregister_port;
- }
+ if (ret)
+ goto err_disconnect;
i2c_set_clientdata(client, tps);
fwnode_handle_put(fwnode);
return 0;
+err_disconnect:
+ tps6598x_disconnect(tps, 0);
err_unregister_port:
typec_unregister_port(tps->port);
err_role_put:
--
2.25.1
On Mon, Nov 14, 2022 at 06:44:47PM +0100, Sven Peter wrote:
> The err_role_put error path always calls fwnode_handle_put to release
> the fwnode. This path can be reached after probe itself has already
> released that fwnode though. Fix that by moving fwnode_handle_put in the
> happy path to the very end.
>
> Fixes: 18a6c866bb19 ("usb: typec: tps6598x: Add USB role switching logic")
> Signed-off-by: Sven Peter <[email protected]>
This looks like stable material as well.
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index ebc786d728e2..824e573af570 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -834,7 +834,6 @@ static int tps6598x_probe(struct i2c_client *client)
> ret = PTR_ERR(tps->port);
> goto err_role_put;
> }
> - fwnode_handle_put(fwnode);
>
> if (tps->status & TPS_STATUS_PLUG_PRESENT) {
> ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &tps->pwr_status);
> @@ -858,6 +857,7 @@ static int tps6598x_probe(struct i2c_client *client)
> }
>
> i2c_set_clientdata(client, tps);
> + fwnode_handle_put(fwnode);
>
> return 0;
>
thanks,
--
heikki
On Mon, Nov 14, 2022 at 06:44:49PM +0100, Sven Peter wrote:
> While the code currently correctly calls tps6598x_disconnect before jumping
> to the error cleanup label it's inconsistent compared to all the other cleanup
> actions and prone to introduce bugs if any more resources are added.
>
> Signed-off-by: Sven Peter <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index c35501a92b4d..22ff212e05e6 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -850,16 +850,16 @@ static int tps6598x_probe(struct i2c_client *client)
> irq_handler,
> IRQF_SHARED | IRQF_ONESHOT,
> dev_name(&client->dev), tps);
> - if (ret) {
> - tps6598x_disconnect(tps, 0);
> - goto err_unregister_port;
> - }
> + if (ret)
> + goto err_disconnect;
>
> i2c_set_clientdata(client, tps);
> fwnode_handle_put(fwnode);
>
> return 0;
>
> +err_disconnect:
> + tps6598x_disconnect(tps, 0);
> err_unregister_port:
> typec_unregister_port(tps->port);
> err_role_put:
thanks,
--
heikki
On Mon, Nov 14, 2022 at 06:44:46PM +0100, Sven Peter wrote:
> We can't just return if devm_tps6598_psy_register fails since previous
> resources are not devres managed and have yet to be cleaned up.
>
> Fixes: 10eb0b6ac63a ("usb: typec: tps6598x: Export some power supply properties")
> Signed-off-by: Sven Peter <[email protected]>
I think this should also automatically go to the stable tree.
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 748ff4f6b5f6..ebc786d728e2 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -827,7 +827,7 @@ static int tps6598x_probe(struct i2c_client *client)
>
> ret = devm_tps6598_psy_register(tps);
> if (ret)
> - return ret;
> + goto err_role_put;
>
> tps->port = typec_register_port(&client->dev, &typec_cap);
> if (IS_ERR(tps->port)) {
thanks,
--
heikki
On Mon, Nov 14, 2022 at 06:44:48PM +0100, Sven Peter wrote:
> typec_unregister_port is only called for some error paths after
> typec_register_port was successful. Ensure it's called in all
> cases.
>
> Fixes: 92440202a880 ("usb: typec: tipd: Only update power status on IRQ")
> Signed-off-by: Sven Peter <[email protected]>
Also direct stable material.
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> drivers/usb/typec/tipd/core.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 824e573af570..c35501a92b4d 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -839,7 +839,7 @@ static int tps6598x_probe(struct i2c_client *client)
> ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &tps->pwr_status);
> if (ret < 0) {
> dev_err(tps->dev, "failed to read power status: %d\n", ret);
> - goto err_role_put;
> + goto err_unregister_port;
> }
> ret = tps6598x_connect(tps);
> if (ret)
> @@ -852,8 +852,7 @@ static int tps6598x_probe(struct i2c_client *client)
> dev_name(&client->dev), tps);
> if (ret) {
> tps6598x_disconnect(tps, 0);
> - typec_unregister_port(tps->port);
> - goto err_role_put;
> + goto err_unregister_port;
> }
>
> i2c_set_clientdata(client, tps);
> @@ -861,6 +860,8 @@ static int tps6598x_probe(struct i2c_client *client)
>
> return 0;
>
> +err_unregister_port:
> + typec_unregister_port(tps->port);
> err_role_put:
> usb_role_switch_put(tps->role_sw);
> err_fwnode_put:
thanks,
--
heikki