[V2,1/4] can: flexcan: fix deadlock when using self wakeup
diff mbox series

Message ID 20191127055334.1476-2-qiangqing.zhang@nxp.com
State Awaiting Upstream
Delegated to: David Miller
Headers show
Series
  • can: flexcan: fixes for stop mode
Related show

Commit Message

Joakim Zhang Nov. 27, 2019, 5:56 a.m. UTC
From: Sean Nyekjaer <sean@geanix.com>

When suspending, when there is still can traffic on the interfaces the
flexcan immediately wakes the platform again. As it should :-). But it
throws this error msg:
[ 3169.378661] PM: noirq suspend of devices failed

On the way down to suspend the interface that throws the error message does
call flexcan_suspend but fails to call flexcan_noirq_suspend. That means the
flexcan_enter_stop_mode is called, but on the way out of suspend the driver
only calls flexcan_resume and skips flexcan_noirq_resume, thus it doesn't call
flexcan_exit_stop_mode. This leaves the flexcan in stop mode, and with the
current driver it can't recover from this even with a soft reboot, it requires
a hard reboot.

This patch can fix deadlock when using self wakeup, it happenes to be
able to fix another issue that frames out-of-order in first IRQ handler
run after wakeup.

In wakeup case, after system resume, frames received out-of-order in
first IRQ handler, the problem is wakeup latency from frame reception to
IRQ handler is much bigger than the counter overflow. This means it's
impossible to sort the CAN frames by timestamp. The reason is that controller
exits stop mode during noirq resume, then it can receive the frame immediately.
If noirq reusme stage consumes much time, it will extend interrupt response
time. So exit stop mode during resume stage instead of noirq resume can
fix this issue.

Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
Signed-off-by: Sean Nyekjaer <sean@geanix.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
------
ChangeLog:
	V1->V2: no change.
---
 drivers/net/can/flexcan.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

Comments

Sean Nyekjaer Nov. 27, 2019, 9:58 a.m. UTC | #1
On 27/11/2019 06.56, Joakim Zhang wrote:
> From: Sean Nyekjaer <sean@geanix.com>
> 
> When suspending, when there is still can traffic on the interfaces the
> flexcan immediately wakes the platform again. As it should :-). But it
> throws this error msg:
> [ 3169.378661] PM: noirq suspend of devices failed
> 
> On the way down to suspend the interface that throws the error message does
> call flexcan_suspend but fails to call flexcan_noirq_suspend. That means the
> flexcan_enter_stop_mode is called, but on the way out of suspend the driver
> only calls flexcan_resume and skips flexcan_noirq_resume, thus it doesn't call
> flexcan_exit_stop_mode. This leaves the flexcan in stop mode, and with the
> current driver it can't recover from this even with a soft reboot, it requires
> a hard reboot.
> 
> This patch can fix deadlock when using self wakeup, it happenes to be
> able to fix another issue that frames out-of-order in first IRQ handler
> run after wakeup.
> 
> In wakeup case, after system resume, frames received out-of-order in
> first IRQ handler, the problem is wakeup latency from frame reception to
> IRQ handler is much bigger than the counter overflow. This means it's
> impossible to sort the CAN frames by timestamp. The reason is that controller
> exits stop mode during noirq resume, then it can receive the frame immediately.
> If noirq reusme stage consumes much time, it will extend interrupt response
> time. So exit stop mode during resume stage instead of noirq resume can
> fix this issue.
> 
> Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Tested-by: Sean Nyekjaer <sean@geanix.com>
> ------
> ChangeLog:
> 	V1->V2: no change.
> ---
>   drivers/net/can/flexcan.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 2efa06119f68..2297663cacb2 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -134,8 +134,7 @@
>   	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)
>   #define FLEXCAN_ESR_ALL_INT \
>   	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> -	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> -	 FLEXCAN_ESR_WAK_INT)
> +	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
>   
>   /* FLEXCAN interrupt flag register (IFLAG) bits */
>   /* Errata ERR005829 step7: Reserve first valid MB */
> @@ -960,6 +959,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>   
>   	reg_esr = priv->read(&regs->esr);
>   
> +	/* ACK wakeup interrupt */
> +	if (reg_esr & FLEXCAN_ESR_WAK_INT) {
> +		handled = IRQ_HANDLED;
> +		priv->write(reg_esr & FLEXCAN_ESR_WAK_INT, &regs->esr);
> +	}
> +
>   	/* ACK all bus error and state change IRQ sources */
>   	if (reg_esr & FLEXCAN_ESR_ALL_INT) {
>   		handled = IRQ_HANDLED;
> @@ -1722,6 +1727,9 @@ static int __maybe_unused flexcan_resume(struct device *device)
>   		netif_start_queue(dev);
>   		if (device_may_wakeup(device)) {
>   			disable_irq_wake(dev->irq);
> +			err = flexcan_exit_stop_mode(priv);
> +			if (err)
> +				return err;
>   		} else {
>   			err = pm_runtime_force_resume(device);
>   			if (err)
> @@ -1767,14 +1775,9 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
>   {
>   	struct net_device *dev = dev_get_drvdata(device);
>   	struct flexcan_priv *priv = netdev_priv(dev);
> -	int err;
>   
> -	if (netif_running(dev) && device_may_wakeup(device)) {
> +	if (netif_running(dev) && device_may_wakeup(device))
>   		flexcan_enable_wakeup_irq(priv, false);
> -		err = flexcan_exit_stop_mode(priv);
> -		if (err)
> -			return err;
> -	}
>   
>   	return 0;
>   }
>
Marc Kleine-Budde Dec. 3, 2019, 5:25 p.m. UTC | #2
On 11/27/19 6:56 AM, Joakim Zhang wrote:
> From: Sean Nyekjaer <sean@geanix.com>
> 
> When suspending, when there is still can traffic on the interfaces the
> flexcan immediately wakes the platform again. As it should :-). But it
> throws this error msg:
> [ 3169.378661] PM: noirq suspend of devices failed
> 
> On the way down to suspend the interface that throws the error message does
> call flexcan_suspend but fails to call flexcan_noirq_suspend. That means the
> flexcan_enter_stop_mode is called, but on the way out of suspend the driver
> only calls flexcan_resume and skips flexcan_noirq_resume, thus it doesn't call
> flexcan_exit_stop_mode. This leaves the flexcan in stop mode, and with the
> current driver it can't recover from this even with a soft reboot, it requires
> a hard reboot.
> 
> This patch can fix deadlock when using self wakeup, it happenes to be
> able to fix another issue that frames out-of-order in first IRQ handler
> run after wakeup.
> 
> In wakeup case, after system resume, frames received out-of-order in
> first IRQ handler, the problem is wakeup latency from frame reception to
> IRQ handler is much bigger than the counter overflow. This means it's
> impossible to sort the CAN frames by timestamp. The reason is that controller
> exits stop mode during noirq resume, then it can receive the frame immediately.
> If noirq reusme stage consumes much time, it will extend interrupt response
> time. So exit stop mode during resume stage instead of noirq resume can
> fix this issue.
> 
> Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ------
> ChangeLog:
> 	V1->V2: no change.
> ---
>  drivers/net/can/flexcan.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 2efa06119f68..2297663cacb2 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -134,8 +134,7 @@
>  	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)
>  #define FLEXCAN_ESR_ALL_INT \
>  	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> -	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> -	 FLEXCAN_ESR_WAK_INT)
> +	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)

Why do you remove the FLEXCAN_ESR_WAK_INT from the FLEXCAN_ESR_ALL_INT?

>  
>  /* FLEXCAN interrupt flag register (IFLAG) bits */
>  /* Errata ERR005829 step7: Reserve first valid MB */
> @@ -960,6 +959,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  
>  	reg_esr = priv->read(&regs->esr);
>  
> +	/* ACK wakeup interrupt */
> +	if (reg_esr & FLEXCAN_ESR_WAK_INT) {
> +		handled = IRQ_HANDLED;
> +		priv->write(reg_esr & FLEXCAN_ESR_WAK_INT, &regs->esr);
> +	}
> +

If FLEXCAN_ESR_WAK_INT stays in FLEXCAN_ESR_ALL_INT, you don't need that
explicit ACK here.

Marc
Marc Kleine-Budde Dec. 3, 2019, 5:42 p.m. UTC | #3
On 12/3/19 6:25 PM, Marc Kleine-Budde wrote:
> On 11/27/19 6:56 AM, Joakim Zhang wrote:
>> From: Sean Nyekjaer <sean@geanix.com>
>>
>> When suspending, when there is still can traffic on the interfaces the
>> flexcan immediately wakes the platform again. As it should :-). But it
>> throws this error msg:
>> [ 3169.378661] PM: noirq suspend of devices failed
>>
>> On the way down to suspend the interface that throws the error message does
>> call flexcan_suspend but fails to call flexcan_noirq_suspend. That means the
>> flexcan_enter_stop_mode is called, but on the way out of suspend the driver
>> only calls flexcan_resume and skips flexcan_noirq_resume, thus it doesn't call
>> flexcan_exit_stop_mode. This leaves the flexcan in stop mode, and with the
>> current driver it can't recover from this even with a soft reboot, it requires
>> a hard reboot.
>>
>> This patch can fix deadlock when using self wakeup, it happenes to be
>> able to fix another issue that frames out-of-order in first IRQ handler
>> run after wakeup.
>>
>> In wakeup case, after system resume, frames received out-of-order in
>> first IRQ handler, the problem is wakeup latency from frame reception to
>> IRQ handler is much bigger than the counter overflow. This means it's
>> impossible to sort the CAN frames by timestamp. The reason is that controller
>> exits stop mode during noirq resume, then it can receive the frame immediately.
>> If noirq reusme stage consumes much time, it will extend interrupt response
>> time. So exit stop mode during resume stage instead of noirq resume can
>> fix this issue.
>>
>> Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
>> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
>> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
>> ------
>> ChangeLog:
>> 	V1->V2: no change.
>> ---
>>  drivers/net/can/flexcan.c | 19 +++++++++++--------
>>  1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> index 2efa06119f68..2297663cacb2 100644
>> --- a/drivers/net/can/flexcan.c
>> +++ b/drivers/net/can/flexcan.c
>> @@ -134,8 +134,7 @@
>>  	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)
>>  #define FLEXCAN_ESR_ALL_INT \
>>  	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
>> -	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
>> -	 FLEXCAN_ESR_WAK_INT)
>> +	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
> 
> Why do you remove the FLEXCAN_ESR_WAK_INT from the FLEXCAN_ESR_ALL_INT?
> 
>>  
>>  /* FLEXCAN interrupt flag register (IFLAG) bits */
>>  /* Errata ERR005829 step7: Reserve first valid MB */
>> @@ -960,6 +959,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>>  
>>  	reg_esr = priv->read(&regs->esr);
>>  
>> +	/* ACK wakeup interrupt */
>> +	if (reg_esr & FLEXCAN_ESR_WAK_INT) {
>> +		handled = IRQ_HANDLED;
>> +		priv->write(reg_esr & FLEXCAN_ESR_WAK_INT, &regs->esr);
>> +	}
>> +
> 
> If FLEXCAN_ESR_WAK_INT stays in FLEXCAN_ESR_ALL_INT, you don't need that
> explicit ACK here.

Otherwise this patch is OK. With this patch the flexcan_suspend() and
flexcan_resume() look finally symmetric. \o/

Marc
Joakim Zhang Dec. 4, 2019, 1:58 a.m. UTC | #4
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年12月4日 1:25
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; sean@geanix.com;
> linux-can@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH V2 1/4] can: flexcan: fix deadlock when using self wakeup

[...]
> >  drivers/net/can/flexcan.c | 19 +++++++++++--------
> >  1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 2efa06119f68..2297663cacb2 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -134,8 +134,7 @@
> >  	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)  #define
> > FLEXCAN_ESR_ALL_INT \
> >  	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> > -	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> > -	 FLEXCAN_ESR_WAK_INT)
> > +	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
> 
> Why do you remove the FLEXCAN_ESR_WAK_INT from the
> FLEXCAN_ESR_ALL_INT?
> 
> >
> >  /* FLEXCAN interrupt flag register (IFLAG) bits */
> >  /* Errata ERR005829 step7: Reserve first valid MB */ @@ -960,6
> > +959,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> >
> >  	reg_esr = priv->read(&regs->esr);
> >
> > +	/* ACK wakeup interrupt */
> > +	if (reg_esr & FLEXCAN_ESR_WAK_INT) {
> > +		handled = IRQ_HANDLED;
> > +		priv->write(reg_esr & FLEXCAN_ESR_WAK_INT, &regs->esr);
> > +	}
> > +
> 
> If FLEXCAN_ESR_WAK_INT stays in FLEXCAN_ESR_ALL_INT, you don't need
> that explicit ACK here.

Hi Marc,

I remove the FLEXCAN_ESR_WAK_INT from the FLEXCAN_ESR_ALL_INT since FLEXCAN_ESR_ALL_INT is for
all bus error and state change IRQ sources, wakeup interrupt does not belong to these. If you think this does
not need, I can remove this change.

Best Regards,
Joakim Zhang
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://www.pengutronix.de  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |
Marc Kleine-Budde Dec. 4, 2019, 8:31 a.m. UTC | #5
On 12/4/19 2:58 AM, Joakim Zhang wrote:
> [...]
>>>  drivers/net/can/flexcan.c | 19 +++++++++++--------
>>>  1 file changed, 11 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>> index 2efa06119f68..2297663cacb2 100644
>>> --- a/drivers/net/can/flexcan.c
>>> +++ b/drivers/net/can/flexcan.c
>>> @@ -134,8 +134,7 @@
>>>  	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)  #define
>>> FLEXCAN_ESR_ALL_INT \
>>>  	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
>>> -	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
>>> -	 FLEXCAN_ESR_WAK_INT)
>>> +	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
>>
>> Why do you remove the FLEXCAN_ESR_WAK_INT from the
>> FLEXCAN_ESR_ALL_INT?
>>
>>>
>>>  /* FLEXCAN interrupt flag register (IFLAG) bits */
>>>  /* Errata ERR005829 step7: Reserve first valid MB */ @@ -960,6
>>> +959,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>>>
>>>  	reg_esr = priv->read(&regs->esr);
>>>
>>> +	/* ACK wakeup interrupt */
>>> +	if (reg_esr & FLEXCAN_ESR_WAK_INT) {
>>> +		handled = IRQ_HANDLED;
>>> +		priv->write(reg_esr & FLEXCAN_ESR_WAK_INT, &regs->esr);
>>> +	}
>>> +
>>
>> If FLEXCAN_ESR_WAK_INT stays in FLEXCAN_ESR_ALL_INT, you don't need
>> that explicit ACK here.
> 
> Hi Marc,
> 
> I remove the FLEXCAN_ESR_WAK_INT from the FLEXCAN_ESR_ALL_INT since
> FLEXCAN_ESR_ALL_INT is for all bus error and state change IRQ
> sources, wakeup interrupt does not belong to these. If you think this
> does not need, I can remove this change.

I see, makes sense.

Make this a separate patch. Move the FLEXCAN_ESR_WAK_INT from the
FLEXCAN_ESR_ALL_INT, but add it to the existing ack of the interrupts.
Like this:

> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index b6f675a5e2d9..74f622b40b61 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -960,10 +960,10 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  
>         reg_esr = priv->read(&regs->esr);
>  
> -       /* ACK all bus error and state change IRQ sources */
> -       if (reg_esr & FLEXCAN_ESR_ALL_INT) {
> +       /* ACK all bus error, state change and wake IRQ sources */
> +       if (reg_esr & (FLEXCAN_ESR_ALL_INT | FLEXCAN_ESR_WAK_INT)) {
>                 handled = IRQ_HANDLED;
> -               priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
> +               priv->write(reg_esr & (FLEXCAN_ESR_ALL_INT | FLEXCAN_ESR_WAK_INT), &regs->esr);
>         }
>  
>         /* state change interrupt or broken error state quirk fix is enabled */

Marc
Joakim Zhang Dec. 4, 2019, 8:35 a.m. UTC | #6
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年12月4日 16:31
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; sean@geanix.com;
> linux-can@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH V2 1/4] can: flexcan: fix deadlock when using self wakeup
> 
> On 12/4/19 2:58 AM, Joakim Zhang wrote:
> > [...]
> >>>  drivers/net/can/flexcan.c | 19 +++++++++++--------
> >>>  1 file changed, 11 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> >>> index 2efa06119f68..2297663cacb2 100644
> >>> --- a/drivers/net/can/flexcan.c
> >>> +++ b/drivers/net/can/flexcan.c
> >>> @@ -134,8 +134,7 @@
> >>>  	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)  #define
> >>> FLEXCAN_ESR_ALL_INT \
> >>>  	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> >>> -	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> >>> -	 FLEXCAN_ESR_WAK_INT)
> >>> +	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
> >>
> >> Why do you remove the FLEXCAN_ESR_WAK_INT from the
> >> FLEXCAN_ESR_ALL_INT?
> >>
> >>>
> >>>  /* FLEXCAN interrupt flag register (IFLAG) bits */
> >>>  /* Errata ERR005829 step7: Reserve first valid MB */ @@ -960,6
> >>> +959,12 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> >>>
> >>>  	reg_esr = priv->read(&regs->esr);
> >>>
> >>> +	/* ACK wakeup interrupt */
> >>> +	if (reg_esr & FLEXCAN_ESR_WAK_INT) {
> >>> +		handled = IRQ_HANDLED;
> >>> +		priv->write(reg_esr & FLEXCAN_ESR_WAK_INT, &regs->esr);
> >>> +	}
> >>> +
> >>
> >> If FLEXCAN_ESR_WAK_INT stays in FLEXCAN_ESR_ALL_INT, you don't need
> >> that explicit ACK here.
> >
> > Hi Marc,
> >
> > I remove the FLEXCAN_ESR_WAK_INT from the FLEXCAN_ESR_ALL_INT since
> > FLEXCAN_ESR_ALL_INT is for all bus error and state change IRQ sources,
> > wakeup interrupt does not belong to these. If you think this does not
> > need, I can remove this change.
> 
> I see, makes sense.
> 
> Make this a separate patch. Move the FLEXCAN_ESR_WAK_INT from the
> FLEXCAN_ESR_ALL_INT, but add it to the existing ack of the interrupts.
> Like this:
> 
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index b6f675a5e2d9..74f622b40b61 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -960,10 +960,10 @@ static irqreturn_t flexcan_irq(int irq, void
> > *dev_id)
> >
> >         reg_esr = priv->read(&regs->esr);
> >
> > -       /* ACK all bus error and state change IRQ sources */
> > -       if (reg_esr & FLEXCAN_ESR_ALL_INT) {
> > +       /* ACK all bus error, state change and wake IRQ sources */
> > +       if (reg_esr & (FLEXCAN_ESR_ALL_INT | FLEXCAN_ESR_WAK_INT)) {
> >                 handled = IRQ_HANDLED;
> > -               priv->write(reg_esr & FLEXCAN_ESR_ALL_INT,
> &regs->esr);
> > +               priv->write(reg_esr & (FLEXCAN_ESR_ALL_INT |
> > + FLEXCAN_ESR_WAK_INT), &regs->esr);
> >         }
> >
> >         /* state change interrupt or broken error state quirk fix is
> > enabled */

Got it! Thanks.

Best Regards,
Joakim Zhang
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://www.pengutronix.de  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Patch
diff mbox series

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 2efa06119f68..2297663cacb2 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -134,8 +134,7 @@ 
 	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)
 #define FLEXCAN_ESR_ALL_INT \
 	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
-	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
-	 FLEXCAN_ESR_WAK_INT)
+	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
 
 /* FLEXCAN interrupt flag register (IFLAG) bits */
 /* Errata ERR005829 step7: Reserve first valid MB */
@@ -960,6 +959,12 @@  static irqreturn_t flexcan_irq(int irq, void *dev_id)
 
 	reg_esr = priv->read(&regs->esr);
 
+	/* ACK wakeup interrupt */
+	if (reg_esr & FLEXCAN_ESR_WAK_INT) {
+		handled = IRQ_HANDLED;
+		priv->write(reg_esr & FLEXCAN_ESR_WAK_INT, &regs->esr);
+	}
+
 	/* ACK all bus error and state change IRQ sources */
 	if (reg_esr & FLEXCAN_ESR_ALL_INT) {
 		handled = IRQ_HANDLED;
@@ -1722,6 +1727,9 @@  static int __maybe_unused flexcan_resume(struct device *device)
 		netif_start_queue(dev);
 		if (device_may_wakeup(device)) {
 			disable_irq_wake(dev->irq);
+			err = flexcan_exit_stop_mode(priv);
+			if (err)
+				return err;
 		} else {
 			err = pm_runtime_force_resume(device);
 			if (err)
@@ -1767,14 +1775,9 @@  static int __maybe_unused flexcan_noirq_resume(struct device *device)
 {
 	struct net_device *dev = dev_get_drvdata(device);
 	struct flexcan_priv *priv = netdev_priv(dev);
-	int err;
 
-	if (netif_running(dev) && device_may_wakeup(device)) {
+	if (netif_running(dev) && device_may_wakeup(device))
 		flexcan_enable_wakeup_irq(priv, false);
-		err = flexcan_exit_stop_mode(priv);
-		if (err)
-			return err;
-	}
 
 	return 0;
 }