[v1,3/3] i2c: tegra: Fix suspending in active runtime PM state
diff mbox series

Message ID 20191212233428.14648-4-digetx@gmail.com
State Superseded
Headers show
Series
  • Tegra I2C: Support atomic transfers and correct suspend/resume
Related show

Commit Message

Dmitry Osipenko Dec. 12, 2019, 11:34 p.m. UTC
I noticed that sometime I2C clock is kept enabled during suspend-resume.
This happens because runtime PM defers dynamic suspension and thus it may
happen that runtime PM is in active state when system enters into suspend.
In particular I2C controller that is used for CPU's DVFS is often kept ON
during suspend because CPU's voltage scaling happens quite often.

Note: we marked runtime PM as IRQ-safe during the driver's probe in the
"Support atomic transfers" patch, thus it's okay to enforce runtime PM
suspend/resume in the NOIRQ phase which is used for the system-level
suspend/resume of the driver.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Dmitry Osipenko Dec. 12, 2019, 11:43 p.m. UTC | #1
13.12.2019 02:34, Dmitry Osipenko пишет:
> I noticed that sometime I2C clock is kept enabled during suspend-resume.
> This happens because runtime PM defers dynamic suspension and thus it may
> happen that runtime PM is in active state when system enters into suspend.
> In particular I2C controller that is used for CPU's DVFS is often kept ON
> during suspend because CPU's voltage scaling happens quite often.
> 
> Note: we marked runtime PM as IRQ-safe during the driver's probe in the
> "Support atomic transfers" patch, thus it's okay to enforce runtime PM
> suspend/resume in the NOIRQ phase which is used for the system-level
> suspend/resume of the driver.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index b3ecdd87e91f..d309a314f4d6 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -1790,9 +1790,14 @@ static int tegra_i2c_remove(struct platform_device *pdev)
>  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
>  {
>  	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
> +	int err;
>  
>  	i2c_mark_adapter_suspended(&i2c_dev->adapter);
>  
> +	err = pm_runtime_force_suspend(dev);
> +	if (err < 0)
> +		return err;
> +
>  	return 0;
>  }
>  
> @@ -1813,6 +1818,10 @@ static int __maybe_unused tegra_i2c_resume(struct device *dev)
>  	if (err)
>  		return err;
>  
> +	err = pm_runtime_force_resume(dev);
> +	if (err < 0)
> +		return err;
> +
>  	i2c_mark_adapter_resumed(&i2c_dev->adapter);
>  
>  	return 0;
> 

It just occurred to me that this patch needs to marked as fixes for the
"i2c: tegra: Move suspend handling to NOIRQ phase" patch because it
broke runtime PM enable-refcount by disabling clock/pinmux on resume
from suspend. For now I'll wait for the review comments. Please review,
thanks in advance.
Thierry Reding Dec. 13, 2019, 1:47 p.m. UTC | #2
On Fri, Dec 13, 2019 at 02:34:28AM +0300, Dmitry Osipenko wrote:
> I noticed that sometime I2C clock is kept enabled during suspend-resume.
> This happens because runtime PM defers dynamic suspension and thus it may
> happen that runtime PM is in active state when system enters into suspend.
> In particular I2C controller that is used for CPU's DVFS is often kept ON
> during suspend because CPU's voltage scaling happens quite often.
> 
> Note: we marked runtime PM as IRQ-safe during the driver's probe in the
> "Support atomic transfers" patch, thus it's okay to enforce runtime PM
> suspend/resume in the NOIRQ phase which is used for the system-level
> suspend/resume of the driver.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

I've recently discussed this with Rafael in the context of runtime PM
support in the Tegra DRM driver and my understanding is that you're not
supposed to force runtime PM suspension like this.

I had meant to send out an alternative patch to fix this, which I've
done now:

	http://patchwork.ozlabs.org/patch/1209148/

That's more in line with what Rafael and I had discussed in the other
thread and should address the issue that you're seeing as well.

Thierry

> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index b3ecdd87e91f..d309a314f4d6 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -1790,9 +1790,14 @@ static int tegra_i2c_remove(struct platform_device *pdev)
>  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
>  {
>  	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
> +	int err;
>  
>  	i2c_mark_adapter_suspended(&i2c_dev->adapter);
>  
> +	err = pm_runtime_force_suspend(dev);
> +	if (err < 0)
> +		return err;
> +
>  	return 0;
>  }
>  
> @@ -1813,6 +1818,10 @@ static int __maybe_unused tegra_i2c_resume(struct device *dev)
>  	if (err)
>  		return err;
>  
> +	err = pm_runtime_force_resume(dev);
> +	if (err < 0)
> +		return err;
> +
>  	i2c_mark_adapter_resumed(&i2c_dev->adapter);
>  
>  	return 0;
> -- 
> 2.24.0
>
Dmitry Osipenko Dec. 13, 2019, 2:04 p.m. UTC | #3
13.12.2019 16:47, Thierry Reding пишет:
> On Fri, Dec 13, 2019 at 02:34:28AM +0300, Dmitry Osipenko wrote:
>> I noticed that sometime I2C clock is kept enabled during suspend-resume.
>> This happens because runtime PM defers dynamic suspension and thus it may
>> happen that runtime PM is in active state when system enters into suspend.
>> In particular I2C controller that is used for CPU's DVFS is often kept ON
>> during suspend because CPU's voltage scaling happens quite often.
>>
>> Note: we marked runtime PM as IRQ-safe during the driver's probe in the
>> "Support atomic transfers" patch, thus it's okay to enforce runtime PM
>> suspend/resume in the NOIRQ phase which is used for the system-level
>> suspend/resume of the driver.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
> 
> I've recently discussed this with Rafael in the context of runtime PM
> support in the Tegra DRM driver and my understanding is that you're not
> supposed to force runtime PM suspension like this.
> 
> I had meant to send out an alternative patch to fix this, which I've
> done now:
> 
> 	http://patchwork.ozlabs.org/patch/1209148/
> 
> That's more in line with what Rafael and I had discussed in the other
> thread and should address the issue that you're seeing as well.

Well, either me or you are still having some misunderstanding of the
runtime PM :) To my knowledge there are a lot of drivers that enforce
suspension of the runtime PM during system's suspend, it should be a
right thing to do especially in a context of the Tegra I2C driver
because we're using asynchronous pm_runtime_put() and thus at the time
of system's suspending, the runtime PM could be ON (as I wrote in the
commit message) and then Terga's I2C driver manually disables the clock
on resume (woopsie).

By invoking pm_runtime_force_suspend() on systems's suspend, the runtime
PM executes tegra_i2c_runtime_suspend() if device is in active state. On
system resume, pm_runtime_force_resume() either keeps device in a
suspended state or resumes it, say if for userspace disabled the runtime
PM for the I2C controller.

Rafael, could you please clarify whether my patch is doing a wrong thing?

>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>> index b3ecdd87e91f..d309a314f4d6 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -1790,9 +1790,14 @@ static int tegra_i2c_remove(struct platform_device *pdev)
>>  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
>>  {
>>  	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
>> +	int err;
>>  
>>  	i2c_mark_adapter_suspended(&i2c_dev->adapter);
>>  
>> +	err = pm_runtime_force_suspend(dev);
>> +	if (err < 0)
>> +		return err;
>> +
>>  	return 0;
>>  }
>>  
>> @@ -1813,6 +1818,10 @@ static int __maybe_unused tegra_i2c_resume(struct device *dev)
>>  	if (err)
>>  		return err;
>>  
>> +	err = pm_runtime_force_resume(dev);
>> +	if (err < 0)
>> +		return err;
>> +
>>  	i2c_mark_adapter_resumed(&i2c_dev->adapter);
>>  
>>  	return 0;
>> -- 
>> 2.24.0
>>
Dmitry Osipenko Dec. 13, 2019, 2:29 p.m. UTC | #4
13.12.2019 02:34, Dmitry Osipenko пишет:
> I noticed that sometime I2C clock is kept enabled during suspend-resume.
> This happens because runtime PM defers dynamic suspension and thus it may
> happen that runtime PM is in active state when system enters into suspend.
> In particular I2C controller that is used for CPU's DVFS is often kept ON
> during suspend because CPU's voltage scaling happens quite often.
> 
> Note: we marked runtime PM as IRQ-safe during the driver's probe in the
> "Support atomic transfers" patch, thus it's okay to enforce runtime PM
> suspend/resume in the NOIRQ phase which is used for the system-level
> suspend/resume of the driver.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index b3ecdd87e91f..d309a314f4d6 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -1790,9 +1790,14 @@ static int tegra_i2c_remove(struct platform_device *pdev)
>  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
>  {
>  	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
> +	int err;
>  
>  	i2c_mark_adapter_suspended(&i2c_dev->adapter);

I'm now in a doubt that it is correct to use NOIRQ level at all for the
suspend because i2c_mark_adapter_suspended() uses mutex, thus I'm
wondering what will happen if there is an asynchronous transfer
happening during suspend..

The i2c_mark_adapter_suspended() will try to block and will never return?

> +	err = pm_runtime_force_suspend(dev);
> +	if (err < 0)
> +		return err;
> +
>  	return 0;
>  }
>  
> @@ -1813,6 +1818,10 @@ static int __maybe_unused tegra_i2c_resume(struct device *dev)
>  	if (err)
>  		return err;
>  
> +	err = pm_runtime_force_resume(dev);
> +	if (err < 0)
> +		return err;
> +
>  	i2c_mark_adapter_resumed(&i2c_dev->adapter);
>  
>  	return 0;
>
Dmitry Osipenko Dec. 13, 2019, 2:55 p.m. UTC | #5
13.12.2019 17:29, Dmitry Osipenko пишет:
> 13.12.2019 02:34, Dmitry Osipenko пишет:
>> I noticed that sometime I2C clock is kept enabled during suspend-resume.
>> This happens because runtime PM defers dynamic suspension and thus it may
>> happen that runtime PM is in active state when system enters into suspend.
>> In particular I2C controller that is used for CPU's DVFS is often kept ON
>> during suspend because CPU's voltage scaling happens quite often.
>>
>> Note: we marked runtime PM as IRQ-safe during the driver's probe in the
>> "Support atomic transfers" patch, thus it's okay to enforce runtime PM
>> suspend/resume in the NOIRQ phase which is used for the system-level
>> suspend/resume of the driver.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>> index b3ecdd87e91f..d309a314f4d6 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -1790,9 +1790,14 @@ static int tegra_i2c_remove(struct platform_device *pdev)
>>  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
>>  {
>>  	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
>> +	int err;
>>  
>>  	i2c_mark_adapter_suspended(&i2c_dev->adapter);
> 
> I'm now in a doubt that it is correct to use NOIRQ level at all for the
> suspend because i2c_mark_adapter_suspended() uses mutex, thus I'm
> wondering what will happen if there is an asynchronous transfer
> happening during suspend..
> 
> The i2c_mark_adapter_suspended() will try to block and will never return?

Moreover, the I2C interrupt should be disabled during the NOIRQ phase.
So, yes.. looks like making use of NOIRQ level wasn't a correct
decision. On the other hand, I don't think that any I2C client driver
used by Tegra SoCs in the upstream kernel could cause the problem at the
moment, so it shouldn't be critical.

BTW: Jon, please CC me next time ;) [I'll try to find a better solution
for the PCIE problem]

>> +	err = pm_runtime_force_suspend(dev);
>> +	if (err < 0)
>> +		return err;
>> +
>>  	return 0;
>>  }
>>  
>> @@ -1813,6 +1818,10 @@ static int __maybe_unused tegra_i2c_resume(struct device *dev)
>>  	if (err)
>>  		return err;
>>  
>> +	err = pm_runtime_force_resume(dev);
>> +	if (err < 0)
>> +		return err;
>> +
>>  	i2c_mark_adapter_resumed(&i2c_dev->adapter);
>>  
>>  	return 0;
>>
>
Dmitry Osipenko Dec. 13, 2019, 6:01 p.m. UTC | #6
13.12.2019 17:04, Dmitry Osipenko пишет:
> 13.12.2019 16:47, Thierry Reding пишет:
>> On Fri, Dec 13, 2019 at 02:34:28AM +0300, Dmitry Osipenko wrote:
>>> I noticed that sometime I2C clock is kept enabled during suspend-resume.
>>> This happens because runtime PM defers dynamic suspension and thus it may
>>> happen that runtime PM is in active state when system enters into suspend.
>>> In particular I2C controller that is used for CPU's DVFS is often kept ON
>>> during suspend because CPU's voltage scaling happens quite often.
>>>
>>> Note: we marked runtime PM as IRQ-safe during the driver's probe in the
>>> "Support atomic transfers" patch, thus it's okay to enforce runtime PM
>>> suspend/resume in the NOIRQ phase which is used for the system-level
>>> suspend/resume of the driver.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>
>> I've recently discussed this with Rafael in the context of runtime PM
>> support in the Tegra DRM driver and my understanding is that you're not
>> supposed to force runtime PM suspension like this.
>>
>> I had meant to send out an alternative patch to fix this, which I've
>> done now:
>>
>> 	http://patchwork.ozlabs.org/patch/1209148/
>>
>> That's more in line with what Rafael and I had discussed in the other
>> thread and should address the issue that you're seeing as well.
> 
> Well, either me or you are still having some misunderstanding of the
> runtime PM :) To my knowledge there are a lot of drivers that enforce
> suspension of the runtime PM during system's suspend, it should be a
> right thing to do especially in a context of the Tegra I2C driver
> because we're using asynchronous pm_runtime_put() and thus at the time
> of system's suspending, the runtime PM could be ON (as I wrote in the
> commit message) and then Terga's I2C driver manually disables the clock
> on resume (woopsie).

Actually, looks like it's not the asynchronous pm_runtime_put() is the
cause of suspending in active state. I see that only one of three I2C
controllers is suspended in the enabled state, maybe some child (I2C
client) device keeps it awake, will try to find out.

> By invoking pm_runtime_force_suspend() on systems's suspend, the runtime
> PM executes tegra_i2c_runtime_suspend() if device is in active state. On
> system resume, pm_runtime_force_resume() either keeps device in a
> suspended state or resumes it, say if for userspace disabled the runtime
> PM for the I2C controller.
> 
> Rafael, could you please clarify whether my patch is doing a wrong thing?
> 
>>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>>> index b3ecdd87e91f..d309a314f4d6 100644
>>> --- a/drivers/i2c/busses/i2c-tegra.c
>>> +++ b/drivers/i2c/busses/i2c-tegra.c
>>> @@ -1790,9 +1790,14 @@ static int tegra_i2c_remove(struct platform_device *pdev)
>>>  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
>>>  {
>>>  	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
>>> +	int err;
>>>  
>>>  	i2c_mark_adapter_suspended(&i2c_dev->adapter);
>>>  
>>> +	err = pm_runtime_force_suspend(dev);
>>> +	if (err < 0)
>>> +		return err;
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> @@ -1813,6 +1818,10 @@ static int __maybe_unused tegra_i2c_resume(struct device *dev)
>>>  	if (err)
>>>  		return err;
>>>  
>>> +	err = pm_runtime_force_resume(dev);
>>> +	if (err < 0)
>>> +		return err;
>>> +
>>>  	i2c_mark_adapter_resumed(&i2c_dev->adapter);
>>>  
>>>  	return 0;
>>> -- 
>>> 2.24.0
>>>
>
Dmitry Osipenko Dec. 19, 2019, 10:58 p.m. UTC | #7
13.12.2019 21:01, Dmitry Osipenko пишет:
> 13.12.2019 17:04, Dmitry Osipenko пишет:
>> 13.12.2019 16:47, Thierry Reding пишет:
>>> On Fri, Dec 13, 2019 at 02:34:28AM +0300, Dmitry Osipenko wrote:
>>>> I noticed that sometime I2C clock is kept enabled during suspend-resume.
>>>> This happens because runtime PM defers dynamic suspension and thus it may
>>>> happen that runtime PM is in active state when system enters into suspend.
>>>> In particular I2C controller that is used for CPU's DVFS is often kept ON
>>>> during suspend because CPU's voltage scaling happens quite often.
>>>>
>>>> Note: we marked runtime PM as IRQ-safe during the driver's probe in the
>>>> "Support atomic transfers" patch, thus it's okay to enforce runtime PM
>>>> suspend/resume in the NOIRQ phase which is used for the system-level
>>>> suspend/resume of the driver.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>
>>> I've recently discussed this with Rafael in the context of runtime PM
>>> support in the Tegra DRM driver and my understanding is that you're not
>>> supposed to force runtime PM suspension like this.
>>>
>>> I had meant to send out an alternative patch to fix this, which I've
>>> done now:
>>>
>>> 	http://patchwork.ozlabs.org/patch/1209148/
>>>
>>> That's more in line with what Rafael and I had discussed in the other
>>> thread and should address the issue that you're seeing as well.
>>
>> Well, either me or you are still having some misunderstanding of the
>> runtime PM :) To my knowledge there are a lot of drivers that enforce
>> suspension of the runtime PM during system's suspend, it should be a
>> right thing to do especially in a context of the Tegra I2C driver
>> because we're using asynchronous pm_runtime_put() and thus at the time
>> of system's suspending, the runtime PM could be ON (as I wrote in the
>> commit message) and then Terga's I2C driver manually disables the clock
>> on resume (woopsie).
> 
> Actually, looks like it's not the asynchronous pm_runtime_put() is the
> cause of suspending in active state. I see that only one of three I2C
> controllers is suspended in the enabled state, maybe some child (I2C
> client) device keeps it awake, will try to find out.
> 
>> By invoking pm_runtime_force_suspend() on systems's suspend, the runtime
>> PM executes tegra_i2c_runtime_suspend() if device is in active state. On
>> system resume, pm_runtime_force_resume() either keeps device in a
>> suspended state or resumes it, say if for userspace disabled the runtime
>> PM for the I2C controller.
>>
>> Rafael, could you please clarify whether my patch is doing a wrong thing?

[snip]

I'm now thinking that it will be not very worthwhile to spend time on
trying to understand why runtime PM isn't working as expected here. It
will be better to simply remove runtime PM from the I2C driver because
it is used only for clock-gating/pinmuxing and it is a very light
operation in comparison to I2C transfer performance. Thus it should be
better to avoid the runtime PM overhead by enabling/disabling the I2C
clocks before/after the transfer, I think that's what driver did before
the runtime PM addition.

Thierry / Jon / Mikko, any objections?
Dmitry Osipenko Dec. 27, 2019, 1:55 p.m. UTC | #8
13.12.2019 17:55, Dmitry Osipenko пишет:
> 13.12.2019 17:29, Dmitry Osipenko пишет:
>> 13.12.2019 02:34, Dmitry Osipenko пишет:
>>> I noticed that sometime I2C clock is kept enabled during suspend-resume.
>>> This happens because runtime PM defers dynamic suspension and thus it may
>>> happen that runtime PM is in active state when system enters into suspend.
>>> In particular I2C controller that is used for CPU's DVFS is often kept ON
>>> during suspend because CPU's voltage scaling happens quite often.
>>>
>>> Note: we marked runtime PM as IRQ-safe during the driver's probe in the
>>> "Support atomic transfers" patch, thus it's okay to enforce runtime PM
>>> suspend/resume in the NOIRQ phase which is used for the system-level
>>> suspend/resume of the driver.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  drivers/i2c/busses/i2c-tegra.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>>> index b3ecdd87e91f..d309a314f4d6 100644
>>> --- a/drivers/i2c/busses/i2c-tegra.c
>>> +++ b/drivers/i2c/busses/i2c-tegra.c
>>> @@ -1790,9 +1790,14 @@ static int tegra_i2c_remove(struct platform_device *pdev)
>>>  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
>>>  {
>>>  	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
>>> +	int err;
>>>  
>>>  	i2c_mark_adapter_suspended(&i2c_dev->adapter);
>>
>> I'm now in a doubt that it is correct to use NOIRQ level at all for the
>> suspend because i2c_mark_adapter_suspended() uses mutex, thus I'm
>> wondering what will happen if there is an asynchronous transfer
>> happening during suspend..
>>
>> The i2c_mark_adapter_suspended() will try to block and will never return?
> 
> Moreover, the I2C interrupt should be disabled during the NOIRQ phase.
> So, yes.. looks like making use of NOIRQ level wasn't a correct
> decision. On the other hand, I don't think that any I2C client driver
> used by Tegra SoCs in the upstream kernel could cause the problem at the
> moment, so it shouldn't be critical.
> 
> BTW: Jon, please CC me next time ;) [I'll try to find a better solution
> for the PCIE problem]

On a second thought, the NOIRQ shouldn't really cause any big problems
because if something executes I2C asynchronously, then the transfer
should simply timeout.

Patch
diff mbox series

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index b3ecdd87e91f..d309a314f4d6 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1790,9 +1790,14 @@  static int tegra_i2c_remove(struct platform_device *pdev)
 static int __maybe_unused tegra_i2c_suspend(struct device *dev)
 {
 	struct tegra_i2c_dev *i2c_dev = dev_get_drvdata(dev);
+	int err;
 
 	i2c_mark_adapter_suspended(&i2c_dev->adapter);
 
+	err = pm_runtime_force_suspend(dev);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 
@@ -1813,6 +1818,10 @@  static int __maybe_unused tegra_i2c_resume(struct device *dev)
 	if (err)
 		return err;
 
+	err = pm_runtime_force_resume(dev);
+	if (err < 0)
+		return err;
+
 	i2c_mark_adapter_resumed(&i2c_dev->adapter);
 
 	return 0;