[PATCH v9 07/14] USB: typec: tps6598x: Apply patch again after power resume

Abdel Alkuor posted 14 patches 2 years, 2 months ago
There is a newer version of this series
[PATCH v9 07/14] USB: typec: tps6598x: Apply patch again after power resume
Posted by Abdel Alkuor 2 years, 2 months ago
From: Abdel Alkuor <abdelalkuor@geotab.com>

TPS25750 PD controller might be powered off externally at power suspend,
after resuming PD controller power back, apply the patch again.

Signed-off-by: Abdel Alkuor <abdelalkuor@geotab.com>
---
Changes in v9:
  - No changes
Changes in v8:
  - Use device_is_compatible instead of of_device_is_compatible
Changes in v7:
  - Add driver name to commit subject
Changes in v6:
  - Check tps25750 using is_compatiable device node
Changes in v5:
  - Incorporating tps25750 into tps6598x driver
 drivers/usb/typec/tipd/core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 2598433a69cf..32e42798688f 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -1203,6 +1203,17 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct tps6598x *tps = i2c_get_clientdata(client);
+	int ret;
+
+	ret = tps6598x_check_mode(tps);
+	if (ret < 0)
+		return ret;
+
+	if (device_is_compatible(tps->dev, "ti,tps25750") && ret == TPS_MODE_PTCH) {
+		ret = tps25750_apply_patch(tps);
+		if (ret)
+			return ret;
+	}
 
 	if (tps->wakeup) {
 		disable_irq_wake(client->irq);
-- 
2.34.1
Re: [PATCH v9 07/14] USB: typec: tps6598x: Apply patch again after power resume
Posted by Heikki Krogerus 2 years, 2 months ago
On Sun, Oct 01, 2023 at 04:11:27AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <abdelalkuor@geotab.com>
> 
> TPS25750 PD controller might be powered off externally at power suspend,
> after resuming PD controller power back, apply the patch again.
> 
> Signed-off-by: Abdel Alkuor <abdelalkuor@geotab.com>

This one looks also like something that should be part of the patch 4.

My concern is that with these separated features you are creating points
into the kernel git tree where TPS25750 is enabled, but it's not fully
functional, or even broken in scenarious like this (suspend/resume).
You can't do that unless you have some really good reason.

Since all of these add only a bit of code each, I think it would be
better to just merge these into the initial patch that enabled
TPS25750 - so I belive patch 4/14.

> ---
> Changes in v9:
>   - No changes
> Changes in v8:
>   - Use device_is_compatible instead of of_device_is_compatible
> Changes in v7:
>   - Add driver name to commit subject
> Changes in v6:
>   - Check tps25750 using is_compatiable device node
> Changes in v5:
>   - Incorporating tps25750 into tps6598x driver
>  drivers/usb/typec/tipd/core.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 2598433a69cf..32e42798688f 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -1203,6 +1203,17 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct tps6598x *tps = i2c_get_clientdata(client);
> +	int ret;
> +
> +	ret = tps6598x_check_mode(tps);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (device_is_compatible(tps->dev, "ti,tps25750") && ret == TPS_MODE_PTCH) {
> +		ret = tps25750_apply_patch(tps);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	if (tps->wakeup) {
>  		disable_irq_wake(client->irq);
> -- 
> 2.34.1

-- 
heikki
Re: [PATCH v9 07/14] USB: typec: tps6598x: Apply patch again after power resume
Posted by Abdel Alkuor 2 years, 2 months ago
On Tue, Oct 03, 2023 at 09:21:08AM +0300, Heikki Krogerus wrote:
> On Sun, Oct 01, 2023 at 04:11:27AM -0400, Abdel Alkuor wrote:
> > From: Abdel Alkuor <abdelalkuor@geotab.com>
> > 
> > TPS25750 PD controller might be powered off externally at power suspend,
> > after resuming PD controller power back, apply the patch again.
> > 
> > Signed-off-by: Abdel Alkuor <abdelalkuor@geotab.com>
> 
> This one looks also like something that should be part of the patch 4.
> 
> My concern is that with these separated features you are creating points
> into the kernel git tree where TPS25750 is enabled, but it's not fully
> functional, or even broken in scenarious like this (suspend/resume).
> You can't do that unless you have some really good reason.
> 
> Since all of these add only a bit of code each, I think it would be
> better to just merge these into the initial patch that enabled
> TPS25750 - so I belive patch 4/14.
>
Makes a lot of sense. I will add part of full tps25750 support patch
> -- 
> heikki

Thanks,
Abdel