diff mbox series

i2c: imx: add irqf_no_suspend flag

Message ID 20221116074431.513214-1-xiaoning.wang@nxp.com
State New
Headers
Series i2c: imx: add irqf_no_suspend flag |

Commit Message

Clark Wang Nov. 16, 2022, 7:44 a.m. UTC 
  The i2c irq is masked when user starts an i2c transfer process
during noirq suspend stage. As a result, i2c transfer fails.
To solve the problem, IRQF_NO_SUSPEND is added to i2c bus.

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
 drivers/i2c/busses/i2c-imx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Oleksij Rempel Nov. 16, 2022, 9:02 a.m. UTC | #1 
On Wed, Nov 16, 2022 at 03:44:31PM +0800, Clark Wang wrote:
> The i2c irq is masked when user starts an i2c transfer process
> during noirq suspend stage. As a result, i2c transfer fails.
> To solve the problem, IRQF_NO_SUSPEND is added to i2c bus.
> 
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>

Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>

> ---
>  drivers/i2c/busses/i2c-imx.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 1ce0cf7a323f..ba49b2f7a1d1 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1510,7 +1510,8 @@ static int i2c_imx_probe(struct platform_device *pdev)
>  		goto rpm_disable;
>  
>  	/* Request IRQ */
> -	ret = request_threaded_irq(irq, i2c_imx_isr, NULL, IRQF_SHARED,
> +	ret = request_threaded_irq(irq, i2c_imx_isr, NULL,
> +				   IRQF_SHARED | IRQF_NO_SUSPEND,
>  				   pdev->name, i2c_imx);
>  	if (ret) {
>  		dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> -- 
> 2.34.1
> 
>
Wolfram Sang Dec. 1, 2022, 11:11 p.m. UTC | #2 
On Wed, Nov 16, 2022 at 10:02:49AM +0100, Oleksij Rempel wrote:
> On Wed, Nov 16, 2022 at 03:44:31PM +0800, Clark Wang wrote:
> > The i2c irq is masked when user starts an i2c transfer process
> > during noirq suspend stage. As a result, i2c transfer fails.
> > To solve the problem, IRQF_NO_SUSPEND is added to i2c bus.
> > 
> > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> 
> Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>

Is this really happening? The driver already implements
master_xfer_atomic, so I'd suspect it gets called instead?
Clark Wang Dec. 9, 2022, 2:37 a.m. UTC | #3 
Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang <wsa@kernel.org>
> Sent: 2022年12月2日 7:11
> To: Oleksij Rempel <o.rempel@pengutronix.de>
> Cc: Clark Wang <xiaoning.wang@nxp.com>; linux@rempel-privat.de;
> kernel@pengutronix.de; shawnguo@kernel.org; s.hauer@pengutronix.de;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> linux-i2c@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] i2c: imx: add irqf_no_suspend flag
> 
> On Wed, Nov 16, 2022 at 10:02:49AM +0100, Oleksij Rempel wrote:
> > On Wed, Nov 16, 2022 at 03:44:31PM +0800, Clark Wang wrote:
> > > The i2c irq is masked when user starts an i2c transfer process
> > > during noirq suspend stage. As a result, i2c transfer fails.
> > > To solve the problem, IRQF_NO_SUSPEND is added to i2c bus.
> > >
> > > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> >
> > Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> Is this really happening? The driver already implements master_xfer_atomic,
> so I'd suspect it gets called instead?

Yes, you are right!

For the atomic API, I have a question. Will this api be used only in the noirq phase? We have a case that is currently bothering us.

Case description: Use the typec device interrupt pin to wake up the suspend system. We used ptn5110 for typec device. It's an i2c device, configure it via i2c bus.

We found that when the system is in the resume process of wakeup, because the typec interrupt is not disabled during suspend, once the noirq phase is over, it will immediately call i2c xfer to read and write ptn5110 to handle that interrupt. At this time, even resume_early has not been called, that is, the runtime pm of the i2c controller has not been enabled.
We made a workaround to check whether the runtime pm is enabled in i2c xfer. If it is not enabled, temporarily enable it, and call pm_runtime_disable at the end of i2c xfer. However, sometimes the resume_early will be called when the runtime pm is temporarily enabled in this workaround, resulting in an unbalanced enabling of the runtime pm of i2c controller.

Do you think this is a problem? Is the I2C atomic API helpful for this case?

Thank you very much!

Best Regards,
Clark Wang
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 1ce0cf7a323f..ba49b2f7a1d1 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1510,7 +1510,8 @@  static int i2c_imx_probe(struct platform_device *pdev)
 		goto rpm_disable;
 
 	/* Request IRQ */
-	ret = request_threaded_irq(irq, i2c_imx_isr, NULL, IRQF_SHARED,
+	ret = request_threaded_irq(irq, i2c_imx_isr, NULL,
+				   IRQF_SHARED | IRQF_NO_SUSPEND,
 				   pdev->name, i2c_imx);
 	if (ret) {
 		dev_err(&pdev->dev, "can't claim irq %d\n", irq);