2022-11-16 08:50:10

by Clark Wang

[permalink] [raw]
Subject: [PATCH] i2c: imx: add irqf_no_suspend flag

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 <[email protected]>
---
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



2022-11-16 10:08:14

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH] i2c: imx: add irqf_no_suspend flag

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 <[email protected]>

Acked-by: Oleksij Rempel <[email protected]>

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

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-12-02 00:32:03

by Wolfram Sang

[permalink] [raw]
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 <[email protected]>
>
> Acked-by: Oleksij Rempel <[email protected]>

Is this really happening? The driver already implements
master_xfer_atomic, so I'd suspect it gets called instead?


Attachments:
(No filename) (566.00 B)
signature.asc (849.00 B)
Download all attachments

2022-12-09 03:37:46

by Clark Wang

[permalink] [raw]
Subject: RE: [PATCH] i2c: imx: add irqf_no_suspend flag

Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang <[email protected]>
> Sent: 2022??12??2?? 7:11
> To: Oleksij Rempel <[email protected]>
> Cc: Clark Wang <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>;
> [email protected]; [email protected];
> [email protected]
> 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 <[email protected]>
> >
> > Acked-by: Oleksij Rempel <[email protected]>
>
> 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