diff mbox series

[v2,4/4] usb: typec: ucsi: stm32g0: add support for power management

Message ID 20220711120122.25804-5-fabrice.gasnier@foss.st.com (mailing list archive)
State Superseded
Headers show
Series usb: typec: ucsi: add support for stm32g0 | expand

Commit Message

Fabrice Gasnier July 11, 2022, 12:01 p.m. UTC 
Type-C connector can be used as a wakeup source (typically to detect
changes on the port, attach or detach...).
Add suspend / resume routines to enable wake irqs, and signal a wakeup
event in case the IRQ has fired while in suspend.
The i2c core is doing the necessary initialization when the "wakeup-source"
flag is provided.
Note: the interrupt handler shouldn't be called before the i2c bus resumes.
So, the interrupts are disabled during suspend period, and re-enabled
upon resume, to avoid i2c transfer while suspended, from the irq handler.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/usb/typec/ucsi/ucsi_stm32g0.c | 52 +++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

Comments

Heikki Krogerus July 12, 2022, 8:56 a.m. UTC | #1 
Hi,

Mon, Jul 11, 2022 at 02:01:22PM +0200, Fabrice Gasnier kirjoitti:
> Type-C connector can be used as a wakeup source (typically to detect
> changes on the port, attach or detach...).
> Add suspend / resume routines to enable wake irqs, and signal a wakeup
> event in case the IRQ has fired while in suspend.
> The i2c core is doing the necessary initialization when the "wakeup-source"
> flag is provided.
> Note: the interrupt handler shouldn't be called before the i2c bus resumes.
> So, the interrupts are disabled during suspend period, and re-enabled
> upon resume, to avoid i2c transfer while suspended, from the irq handler.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Does this really need a separate patch? Does the support depend on the
second patch somehow?

If not, then just merge this into the first patch. That
g0->in_bootloader check you can add in the second patch.

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/ucsi/ucsi_stm32g0.c | 52 +++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_stm32g0.c b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
> index b1d891c9a92c0..061551d464f12 100644
> --- a/drivers/usb/typec/ucsi/ucsi_stm32g0.c
> +++ b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
> @@ -66,6 +66,8 @@ struct ucsi_stm32g0 {
>  	unsigned long flags;
>  	const char *fw_name;
>  	struct ucsi *ucsi;
> +	bool suspended;
> +	bool wakeup_event;
>  };
>  
>  /*
> @@ -416,6 +418,9 @@ static irqreturn_t ucsi_stm32g0_irq_handler(int irq, void *data)
>  	u32 cci;
>  	int ret;
>  
> +	if (g0->suspended)
> +		g0->wakeup_event = true;
> +
>  	ret = ucsi_stm32g0_read(g0->ucsi, UCSI_CCI, &cci, sizeof(cci));
>  	if (ret)
>  		return IRQ_NONE;
> @@ -696,6 +701,52 @@ static int ucsi_stm32g0_remove(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static int ucsi_stm32g0_suspend(struct device *dev)
> +{
> +	struct ucsi_stm32g0 *g0 = dev_get_drvdata(dev);
> +	struct i2c_client *client = g0->client;
> +
> +	if (g0->in_bootloader)
> +		return 0;
> +
> +	/* Keep the interrupt disabled until the i2c bus has been resumed */
> +	disable_irq(client->irq);
> +
> +	g0->suspended = true;
> +	g0->wakeup_event = false;
> +
> +	if (device_may_wakeup(dev) || device_wakeup_path(dev))
> +		enable_irq_wake(client->irq);
> +
> +	return 0;
> +}
> +
> +static int ucsi_stm32g0_resume(struct device *dev)
> +{
> +	struct ucsi_stm32g0 *g0 = dev_get_drvdata(dev);
> +	struct i2c_client *client = g0->client;
> +
> +	if (g0->in_bootloader)
> +		return 0;
> +
> +	if (device_may_wakeup(dev) || device_wakeup_path(dev))
> +		disable_irq_wake(client->irq);
> +
> +	enable_irq(client->irq);
> +
> +	/* Enforce any pending handler gets called to signal a wakeup_event */
> +	synchronize_irq(client->irq);
> +
> +	if (g0->wakeup_event)
> +		pm_wakeup_event(g0->dev, 0);
> +
> +	g0->suspended = false;
> +
> +	return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(ucsi_stm32g0_pm_ops, ucsi_stm32g0_suspend, ucsi_stm32g0_resume);
> +
>  static const struct of_device_id __maybe_unused ucsi_stm32g0_typec_of_match[] = {
>  	{ .compatible = "st,stm32g0-typec" },
>  	{},
> @@ -712,6 +763,7 @@ static struct i2c_driver ucsi_stm32g0_i2c_driver = {
>  	.driver = {
>  		.name = "ucsi-stm32g0-i2c",
>  		.of_match_table = of_match_ptr(ucsi_stm32g0_typec_of_match),
> +		.pm = pm_sleep_ptr(&ucsi_stm32g0_pm_ops),
>  	},
>  	.probe = ucsi_stm32g0_probe,
>  	.remove = ucsi_stm32g0_remove,
> -- 
> 2.25.1
Fabrice Gasnier July 12, 2022, 10:01 a.m. UTC | #2 
On 7/12/22 10:56, Heikki Krogerus wrote:
> Hi,
> 
> Mon, Jul 11, 2022 at 02:01:22PM +0200, Fabrice Gasnier kirjoitti:
>> Type-C connector can be used as a wakeup source (typically to detect
>> changes on the port, attach or detach...).
>> Add suspend / resume routines to enable wake irqs, and signal a wakeup
>> event in case the IRQ has fired while in suspend.
>> The i2c core is doing the necessary initialization when the "wakeup-source"
>> flag is provided.
>> Note: the interrupt handler shouldn't be called before the i2c bus resumes.
>> So, the interrupts are disabled during suspend period, and re-enabled
>> upon resume, to avoid i2c transfer while suspended, from the irq handler.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> 
> Does this really need a separate patch? Does the support depend on the
> second patch somehow?

Hi Heikki,

There's no dependency. I did a separate patch mainly to ease the review.

> 
> If not, then just merge this into the first patch. That
> g0->in_bootloader check you can add in the second patch.

Ok, I'll merge this into the first patches as you suggest, and add your
Acked-by.

Thanks for reviewing,
Best Regards,
Fabrice

> 
> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
>> ---
>>  drivers/usb/typec/ucsi/ucsi_stm32g0.c | 52 +++++++++++++++++++++++++++
>>  1 file changed, 52 insertions(+)
>>
>> diff --git a/drivers/usb/typec/ucsi/ucsi_stm32g0.c b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
>> index b1d891c9a92c0..061551d464f12 100644
>> --- a/drivers/usb/typec/ucsi/ucsi_stm32g0.c
>> +++ b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
>> @@ -66,6 +66,8 @@ struct ucsi_stm32g0 {
>>  	unsigned long flags;
>>  	const char *fw_name;
>>  	struct ucsi *ucsi;
>> +	bool suspended;
>> +	bool wakeup_event;
>>  };
>>  
>>  /*
>> @@ -416,6 +418,9 @@ static irqreturn_t ucsi_stm32g0_irq_handler(int irq, void *data)
>>  	u32 cci;
>>  	int ret;
>>  
>> +	if (g0->suspended)
>> +		g0->wakeup_event = true;
>> +
>>  	ret = ucsi_stm32g0_read(g0->ucsi, UCSI_CCI, &cci, sizeof(cci));
>>  	if (ret)
>>  		return IRQ_NONE;
>> @@ -696,6 +701,52 @@ static int ucsi_stm32g0_remove(struct i2c_client *client)
>>  	return 0;
>>  }
>>  
>> +static int ucsi_stm32g0_suspend(struct device *dev)
>> +{
>> +	struct ucsi_stm32g0 *g0 = dev_get_drvdata(dev);
>> +	struct i2c_client *client = g0->client;
>> +
>> +	if (g0->in_bootloader)
>> +		return 0;
>> +
>> +	/* Keep the interrupt disabled until the i2c bus has been resumed */
>> +	disable_irq(client->irq);
>> +
>> +	g0->suspended = true;
>> +	g0->wakeup_event = false;
>> +
>> +	if (device_may_wakeup(dev) || device_wakeup_path(dev))
>> +		enable_irq_wake(client->irq);
>> +
>> +	return 0;
>> +}
>> +
>> +static int ucsi_stm32g0_resume(struct device *dev)
>> +{
>> +	struct ucsi_stm32g0 *g0 = dev_get_drvdata(dev);
>> +	struct i2c_client *client = g0->client;
>> +
>> +	if (g0->in_bootloader)
>> +		return 0;
>> +
>> +	if (device_may_wakeup(dev) || device_wakeup_path(dev))
>> +		disable_irq_wake(client->irq);
>> +
>> +	enable_irq(client->irq);
>> +
>> +	/* Enforce any pending handler gets called to signal a wakeup_event */
>> +	synchronize_irq(client->irq);
>> +
>> +	if (g0->wakeup_event)
>> +		pm_wakeup_event(g0->dev, 0);
>> +
>> +	g0->suspended = false;
>> +
>> +	return 0;
>> +}
>> +
>> +static DEFINE_SIMPLE_DEV_PM_OPS(ucsi_stm32g0_pm_ops, ucsi_stm32g0_suspend, ucsi_stm32g0_resume);
>> +
>>  static const struct of_device_id __maybe_unused ucsi_stm32g0_typec_of_match[] = {
>>  	{ .compatible = "st,stm32g0-typec" },
>>  	{},
>> @@ -712,6 +763,7 @@ static struct i2c_driver ucsi_stm32g0_i2c_driver = {
>>  	.driver = {
>>  		.name = "ucsi-stm32g0-i2c",
>>  		.of_match_table = of_match_ptr(ucsi_stm32g0_typec_of_match),
>> +		.pm = pm_sleep_ptr(&ucsi_stm32g0_pm_ops),
>>  	},
>>  	.probe = ucsi_stm32g0_probe,
>>  	.remove = ucsi_stm32g0_remove,
>> -- 
>> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/usb/typec/ucsi/ucsi_stm32g0.c b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
index b1d891c9a92c0..061551d464f12 100644
--- a/drivers/usb/typec/ucsi/ucsi_stm32g0.c
+++ b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
@@ -66,6 +66,8 @@  struct ucsi_stm32g0 {
 	unsigned long flags;
 	const char *fw_name;
 	struct ucsi *ucsi;
+	bool suspended;
+	bool wakeup_event;
 };
 
 /*
@@ -416,6 +418,9 @@  static irqreturn_t ucsi_stm32g0_irq_handler(int irq, void *data)
 	u32 cci;
 	int ret;
 
+	if (g0->suspended)
+		g0->wakeup_event = true;
+
 	ret = ucsi_stm32g0_read(g0->ucsi, UCSI_CCI, &cci, sizeof(cci));
 	if (ret)
 		return IRQ_NONE;
@@ -696,6 +701,52 @@  static int ucsi_stm32g0_remove(struct i2c_client *client)
 	return 0;
 }
 
+static int ucsi_stm32g0_suspend(struct device *dev)
+{
+	struct ucsi_stm32g0 *g0 = dev_get_drvdata(dev);
+	struct i2c_client *client = g0->client;
+
+	if (g0->in_bootloader)
+		return 0;
+
+	/* Keep the interrupt disabled until the i2c bus has been resumed */
+	disable_irq(client->irq);
+
+	g0->suspended = true;
+	g0->wakeup_event = false;
+
+	if (device_may_wakeup(dev) || device_wakeup_path(dev))
+		enable_irq_wake(client->irq);
+
+	return 0;
+}
+
+static int ucsi_stm32g0_resume(struct device *dev)
+{
+	struct ucsi_stm32g0 *g0 = dev_get_drvdata(dev);
+	struct i2c_client *client = g0->client;
+
+	if (g0->in_bootloader)
+		return 0;
+
+	if (device_may_wakeup(dev) || device_wakeup_path(dev))
+		disable_irq_wake(client->irq);
+
+	enable_irq(client->irq);
+
+	/* Enforce any pending handler gets called to signal a wakeup_event */
+	synchronize_irq(client->irq);
+
+	if (g0->wakeup_event)
+		pm_wakeup_event(g0->dev, 0);
+
+	g0->suspended = false;
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ucsi_stm32g0_pm_ops, ucsi_stm32g0_suspend, ucsi_stm32g0_resume);
+
 static const struct of_device_id __maybe_unused ucsi_stm32g0_typec_of_match[] = {
 	{ .compatible = "st,stm32g0-typec" },
 	{},
@@ -712,6 +763,7 @@  static struct i2c_driver ucsi_stm32g0_i2c_driver = {
 	.driver = {
 		.name = "ucsi-stm32g0-i2c",
 		.of_match_table = of_match_ptr(ucsi_stm32g0_typec_of_match),
+		.pm = pm_sleep_ptr(&ucsi_stm32g0_pm_ops),
 	},
 	.probe = ucsi_stm32g0_probe,
 	.remove = ucsi_stm32g0_remove,