diff mbox series

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

Message ID 20231001081134.37101-8-alkuor@gmail.com
State New
Headers
Series Add TPS25750 USB type-C PD controller support |

Commit Message

Abdel Alkuor Oct. 1, 2023, 8:11 a.m. UTC 
  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(+)

Comments

Heikki Krogerus Oct. 3, 2023, 6:21 a.m. UTC | #1 
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
Abdel Alkuor Oct. 3, 2023, 11:14 a.m. UTC | #2 
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
diff mbox series

Patch

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);