diff mbox series

[V2,2/4] can: flexcan: try to exit stop mode during probe stage

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

Commit Message

Joakim Zhang Nov. 27, 2019, 5:56 a.m. UTC
CAN controller could be stucked in stop mode once it enters stop mode
when suspend, and then it fails to exit stop mode when resume. Only code
reset can get CAN out of stop mode, so add stop mode remove request
during probe stage for other methods(soft reset from chip level,
unbind/bind driver, etc) to let CAN active again. MCR[LPMACK] will be
checked when enable CAN in register_flexcandev().

Suggested-by: Sean Nyekjaer <sean@geanix.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
------
ChangeLog:
	V1->V2: new add.
---
 drivers/net/can/flexcan.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

Comments

Sean Nyekjaer Nov. 27, 2019, 9:58 a.m. UTC | #1
On 27/11/2019 06.56, Joakim Zhang wrote:
> CAN controller could be stucked in stop mode once it enters stop mode
> when suspend, and then it fails to exit stop mode when resume. Only code
> reset can get CAN out of stop mode, so add stop mode remove request
> during probe stage for other methods(soft reset from chip level,
> unbind/bind driver, etc) to let CAN active again. MCR[LPMACK] will be
> checked when enable CAN in register_flexcandev().
> 
> Suggested-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: new add.
> ---
>   drivers/net/can/flexcan.c | 28 ++++++++++++++++++++++------
>   1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 2297663cacb2..5d5ed28d3005 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -449,6 +449,13 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
>   	return 0;
>   }
>   
> +static void flexcan_try_exit_stop_mode(struct flexcan_priv *priv)
> +{
> +	/* remove stop request */
> +	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +			   1 << priv->stm.req_bit, 0);
> +}
> +
>   static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
>   {
>   	struct flexcan_regs __iomem *regs = priv->regs;
> @@ -1649,6 +1656,21 @@ static int flexcan_probe(struct platform_device *pdev)
>   	priv->devtype_data = devtype_data;
>   	priv->reg_xceiver = reg_xceiver;
>   
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
> +		err = flexcan_setup_stop_mode(pdev);
> +		if (err)
> +			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
> +
> +		/* CAN controller could be stucked in stop mode once it enters
> +		 * stop mode when suspend, and then it fails to exit stop
> +		 * mode when resume. Only code reset can get CAN out of stop
> +		 * mode, so add stop mode remove request here for other methods
> +		 * (soft reset, bind, etc) to let CAN active again. MCR[LPMACK]
> +		 * will be checked when enable CAN in register_flexcandev().
> +		 */
> +		flexcan_try_exit_stop_mode(priv);
> +	}
> +
>   	pm_runtime_get_noresume(&pdev->dev);
>   	pm_runtime_set_active(&pdev->dev);
>   	pm_runtime_enable(&pdev->dev);
> @@ -1661,12 +1683,6 @@ static int flexcan_probe(struct platform_device *pdev)
>   
>   	devm_can_led_init(dev);
>   
> -	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
> -		err = flexcan_setup_stop_mode(pdev);
> -		if (err)
> -			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
> -	}
> -
>   	return 0;
>   
>    failed_register:
>
Marc Kleine-Budde Dec. 3, 2019, 6:14 p.m. UTC | #2
On 11/27/19 6:56 AM, Joakim Zhang wrote:
> CAN controller could be stucked in stop mode once it enters stop mode
                          ^^^^^^^ stuck
> when suspend, and then it fails to exit stop mode when resume.

How can this happen?

> Only code reset can get CAN out of stop mode,

What is "code reset"?

> so add stop mode remove request during probe stage for other
> methods(soft reset from chip level, unbind/bind driver, etc) to let
        ^^^ please add a space
> CAN active again.

Can you rephrase the sentence after "so add stop mode remove request
during probe stage". I'm not completely sure what you want to tell.

> MCR[LPMACK] will be checked when enable CAN in 
> register_flexcandev().
> 
> Suggested-by: Sean Nyekjaer <sean@geanix.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ------
> ChangeLog:
> 	V1->V2: new add.
> ---
>  drivers/net/can/flexcan.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 2297663cacb2..5d5ed28d3005 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -449,6 +449,13 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
>  	return 0;
>  }
>  
> +static void flexcan_try_exit_stop_mode(struct flexcan_priv *priv)
> +{
> +	/* remove stop request */
> +	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +			   1 << priv->stm.req_bit, 0);
> +}
> +
>  static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
>  {
>  	struct flexcan_regs __iomem *regs = priv->regs;
> @@ -1649,6 +1656,21 @@ static int flexcan_probe(struct platform_device *pdev)
>  	priv->devtype_data = devtype_data;
>  	priv->reg_xceiver = reg_xceiver;
>  
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
> +		err = flexcan_setup_stop_mode(pdev);
> +		if (err)
> +			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
> +
> +		/* CAN controller could be stucked in stop mode once it enters
> +		 * stop mode when suspend, and then it fails to exit stop
> +		 * mode when resume. Only code reset can get CAN out of stop
> +		 * mode, so add stop mode remove request here for other methods
> +		 * (soft reset, bind, etc) to let CAN active again. MCR[LPMACK]
> +		 * will be checked when enable CAN in register_flexcandev().
> +		 */
> +		flexcan_try_exit_stop_mode(priv);
> +	}
> +
>  	pm_runtime_get_noresume(&pdev->dev);
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
> @@ -1661,12 +1683,6 @@ static int flexcan_probe(struct platform_device *pdev)
>  
>  	devm_can_led_init(dev);
>  
> -	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
> -		err = flexcan_setup_stop_mode(pdev);
> -		if (err)
> -			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
> -	}
> -
>  	return 0;
>  
>   failed_register:
> 

Marc
Joakim Zhang Dec. 4, 2019, 2:22 a.m. UTC | #3
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年12月4日 2:15
> 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 2/4] can: flexcan: try to exit stop mode during probe
> stage
> 
> On 11/27/19 6:56 AM, Joakim Zhang wrote:
> > CAN controller could be stucked in stop mode once it enters stop mode
>                           ^^^^^^^ stuck
> > when suspend, and then it fails to exit stop mode when resume.
> 
> How can this happen?

I am also confused how can this happen, as I asked Sean, only CAN enter stop mode when suspend, then system hang, it could let CAN
stuck in stop mode. However, Sean said this indeed happen at his side, @sean@geanix.com, could you explain how this happen in details?

> > Only code reset can get CAN out of stop mode,
> 
> What is "code reset"?

As I know, "code reset" is to press the POWER KEY from the board. At my side, reboot command from OS also can get CAN out of stop mode.
Below is experiment I did:
	Firstly, do a hacking to let CAN stuck into stop mode, then:
	(1) press power on/off key, get CAN out of stop mode;
	(2) reboot command from console, get CAN out of stop mode;
	(3) unbind/bind driver, cannot get CAN out of stop mode;  
	(4) remod/insmod module, cannot get CAN out of stop mode;

> > so add stop mode remove request during probe stage for other
> > methods(soft reset from chip level, unbind/bind driver, etc) to let
>         ^^^ please add a space
> > CAN active again.
> 
> Can you rephrase the sentence after "so add stop mode remove request during
> probe stage". I'm not completely sure what you want to tell.

Sure.

Best Regards,
Joakim Zhang
> > MCR[LPMACK] will be checked when enable CAN in register_flexcandev().
> >
> > Suggested-by: Sean Nyekjaer <sean@geanix.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>

[...]

> 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:45 a.m. UTC | #4
On 12/4/19 3:22 AM, Joakim Zhang wrote:
> 
>> -----Original Message-----
>> From: Marc Kleine-Budde <mkl@pengutronix.de>
>> Sent: 2019年12月4日 2:15
>> 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 2/4] can: flexcan: try to exit stop mode during probe
>> stage
>>
>> On 11/27/19 6:56 AM, Joakim Zhang wrote:
>>> CAN controller could be stucked in stop mode once it enters stop mode
>>                           ^^^^^^^ stuck
>>> when suspend, and then it fails to exit stop mode when resume.
>>
>> How can this happen?
> 
> I am also confused how can this happen, as I asked Sean, only CAN
> enter stop mode when suspend, then system hang,
How do you recover the system when suspended?

> it could let CAN 
> stuck in stop mode. However, Sean said this indeed happen at his
> side, @sean@geanix.com, could you explain how this happen in
> details?
That would be good.

>>> Only code reset can get CAN out of stop mode,
>>
>> What is "code reset"?
> 
> As I know, "code reset" is to press the POWER KEY from the board. At
> my side, reboot command from OS also can get CAN out of stop mode.
Do you mean "cold reset", also known as Power-On-Reset, POR or power cycle?

What does pressing the POWER KEY do? A power cycle of the system or
toggling the reset line of the imx?

We need to describe in detail, as not everyone has the same board as
you, and these boards might not even have a power key :)

> Below is experiment I did:
> 	Firstly, do a hacking to let CAN stuck into stop mode, then:

You mean you put the CAN into stop mode without keeping track in the CAN
driver that the CAN-IP is in stop mode, e.g. by hacking the driver.

Then you try several methods to recover:

> 	(1) press power on/off key, get CAN out of stop mode;
> 	(2) reboot command from console, get CAN out of stop mode;
> 	(3) unbind/bind driver, cannot get CAN out of stop mode;  
> 	(4) remod/insmod module, cannot get CAN out of stop mode;

(2) resets the complete imx, including the CAN-IP core, (1) probably, too.

(3) and (4) fail to recover the CAN core, as the IP core is still
powered off by some upstream component. So the question why this happens
in the first place is IMHO as important as trying to wake up the core. I
think if we discover this situation (CAN Core is in stop-mode in probe)
we should print a warning message, but try to recover.

>>> so add stop mode remove request during probe stage for other
>>> methods(soft reset from chip level, unbind/bind driver, etc) to let
>>         ^^^ please add a space
>>> CAN active again.
>>
>> Can you rephrase the sentence after "so add stop mode remove request during
>> probe stage". I'm not completely sure what you want to tell.
> 
> Sure.

tnx,
Marc
Joakim Zhang Dec. 4, 2019, 9:58 a.m. UTC | #5
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年12月4日 16:45
> 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 2/4] can: flexcan: try to exit stop mode during probe
> stage
> 
> On 12/4/19 3:22 AM, Joakim Zhang wrote:
> >
> >> -----Original Message-----
> >> From: Marc Kleine-Budde <mkl@pengutronix.de>
> >> Sent: 2019年12月4日 2:15
> >> 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 2/4] can: flexcan: try to exit stop mode
> >> during probe stage
> >>
> >> On 11/27/19 6:56 AM, Joakim Zhang wrote:
> >>> CAN controller could be stucked in stop mode once it enters stop
> >>> mode
> >>                           ^^^^^^^ stuck
> >>> when suspend, and then it fails to exit stop mode when resume.
> >>
> >> How can this happen?
> >
> > I am also confused how can this happen, as I asked Sean, only CAN
> > enter stop mode when suspend, then system hang,
> How do you recover the system when suspended?
RTC wakeup or TTY wakeup.

> > it could let CAN
> > stuck in stop mode. However, Sean said this indeed happen at his side,
> > @sean@geanix.com, could you explain how this happen in details?
> That would be good.
>
> >>> Only code reset can get CAN out of stop mode,
> >>
> >> What is "code reset"?
> >
> > As I know, "code reset" is to press the POWER KEY from the board. At
> > my side, reboot command from OS also can get CAN out of stop mode.
> Do you mean "cold reset", also known as Power-On-Reset, POR or power
> cycle?
Should be Power-On-Reset.

> What does pressing the POWER KEY do? A power cycle of the system or
> toggling the reset line of the imx?
I think it toggles the reset line of imx. I am so sorry that I am not familiar with system reset :(.
 
> We need to describe in detail, as not everyone has the same board as you, and
> these boards might not even have a power key :)
Yes.

> > Below is experiment I did:
> > 	Firstly, do a hacking to let CAN stuck into stop mode, then:
> 
> You mean you put the CAN into stop mode without keeping track in the CAN
> driver that the CAN-IP is in stop mode, e.g. by hacking the driver.
Yes, you can add flexcan_enter_stop_mode() at the last of driver probe. After probe, CAN has been stuck in stop mode.
Or you can enable CAN wakeup, then comment out flexcan_exit_stop_mode() in flexcan_resume(), do suspend then wakeup system, CAN has been stuck in stop mode.

> Then you try several methods to recover:
> 
> > 	(1) press power on/off key, get CAN out of stop mode;
> > 	(2) reboot command from console, get CAN out of stop mode;
> > 	(3) unbind/bind driver, cannot get CAN out of stop mode;
> > 	(4) remod/insmod module, cannot get CAN out of stop mode;
> 
> (2) resets the complete imx, including the CAN-IP core, (1) probably, too.
Yes, since stop mode enter/exit request at a chip level, need reset completely, such as a "code reset", would get CAN out stop mode.
"Soft reset" cannot get CAN out of stop mode.

> (3) and (4) fail to recover the CAN core, as the IP core is still powered off by
> some upstream component. So the question why this happens in the first place
> is IMHO as important as trying to wake up the core. I think if we discover this
> situation (CAN Core is in stop-mode in probe) we should print a warning
> message, but try to recover.
We really need figure out why CAN could be stuck in stop mode. As I know, enter stop mode in flexcan_suspend(), and then exit stop mode in flexcan_resume(), it could be impossible.
Hope Sean can explain it in details, then we can discuss how to fix it more reasonable.

Thanks Marc.

Best Regards,
Joakim Zhang
> >>> so add stop mode remove request during probe stage for other
> >>> methods(soft reset from chip level, unbind/bind driver, etc) to let
> >>         ^^^ please add a space
> >>> CAN active again.
> >>
> >> Can you rephrase the sentence after "so add stop mode remove request
> >> during probe stage". I'm not completely sure what you want to tell.
> >
> > Sure.
> 
> tnx,
> 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 |
Joakim Zhang Dec. 5, 2019, 8:46 a.m. UTC | #6
Hi Sean,

Could you please answer Marc's concern in details when you are free? Then we can discuss how to fix it more reasonable. Thanks.
 
Best Regards,
Joakim Zhang

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年12月4日 16:45
> 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 2/4] can: flexcan: try to exit stop mode during probe
> stage
> 
> On 12/4/19 3:22 AM, Joakim Zhang wrote:
> >
> >> -----Original Message-----
> >> From: Marc Kleine-Budde <mkl@pengutronix.de>
> >> Sent: 2019年12月4日 2:15
> >> 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 2/4] can: flexcan: try to exit stop mode
> >> during probe stage
> >>
> >> On 11/27/19 6:56 AM, Joakim Zhang wrote:
> >>> CAN controller could be stucked in stop mode once it enters stop
> >>> mode
> >>                           ^^^^^^^ stuck
> >>> when suspend, and then it fails to exit stop mode when resume.
> >>
> >> How can this happen?
> >
> > I am also confused how can this happen, as I asked Sean, only CAN
> > enter stop mode when suspend, then system hang,
> How do you recover the system when suspended?
> 
> > it could let CAN
> > stuck in stop mode. However, Sean said this indeed happen at his side,
> > @sean@geanix.com, could you explain how this happen in details?
> That would be good.
> 
> >>> Only code reset can get CAN out of stop mode,
> >>
> >> What is "code reset"?
> >
> > As I know, "code reset" is to press the POWER KEY from the board. At
> > my side, reboot command from OS also can get CAN out of stop mode.
> Do you mean "cold reset", also known as Power-On-Reset, POR or power
> cycle?
> 
> What does pressing the POWER KEY do? A power cycle of the system or
> toggling the reset line of the imx?
> 
> We need to describe in detail, as not everyone has the same board as you, and
> these boards might not even have a power key :)
> 
> > Below is experiment I did:
> > 	Firstly, do a hacking to let CAN stuck into stop mode, then:
> 
> You mean you put the CAN into stop mode without keeping track in the CAN
> driver that the CAN-IP is in stop mode, e.g. by hacking the driver.
> 
> Then you try several methods to recover:
> 
> > 	(1) press power on/off key, get CAN out of stop mode;
> > 	(2) reboot command from console, get CAN out of stop mode;
> > 	(3) unbind/bind driver, cannot get CAN out of stop mode;
> > 	(4) remod/insmod module, cannot get CAN out of stop mode;
> 
> (2) resets the complete imx, including the CAN-IP core, (1) probably, too.
> 
> (3) and (4) fail to recover the CAN core, as the IP core is still powered off by
> some upstream component. So the question why this happens in the first place
> is IMHO as important as trying to wake up the core. I think if we discover this
> situation (CAN Core is in stop-mode in probe) we should print a warning
> message, but try to recover.
> 
> >>> so add stop mode remove request during probe stage for other
> >>> methods(soft reset from chip level, unbind/bind driver, etc) to let
> >>         ^^^ please add a space
> >>> CAN active again.
> >>
> >> Can you rephrase the sentence after "so add stop mode remove request
> >> during probe stage". I'm not completely sure what you want to tell.
> >
> > Sure.
> 
> tnx,
> 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 |
Sean Nyekjaer Dec. 5, 2019, 9:21 a.m. UTC | #7
On 04/12/2019 09.45, Marc Kleine-Budde wrote:
> On 12/4/19 3:22 AM, Joakim Zhang wrote:
>>
>>> -----Original Message-----
>>> From: Marc Kleine-Budde <mkl@pengutronix.de>
>>> Sent: 2019年12月4日 2:15
>>> 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 2/4] can: flexcan: try to exit stop mode during probe
>>> stage
>>>
>>> On 11/27/19 6:56 AM, Joakim Zhang wrote:
>>>> CAN controller could be stucked in stop mode once it enters stop mode
>>>                            ^^^^^^^ stuck
>>>> when suspend, and then it fails to exit stop mode when resume.
>>>
>>> How can this happen?
>>
>> I am also confused how can this happen, as I asked Sean, only CAN
>> enter stop mode when suspend, then system hang,
> How do you recover the system when suspended?
> 
>> it could let CAN
>> stuck in stop mode. However, Sean said this indeed happen at his
>> side, @sean@geanix.com, could you explain how this happen in
>> details?
> That would be good.
> 
>>>> Only code reset can get CAN out of stop mode,
>>>
>>> What is "code reset"?
>>
>> As I know, "code reset" is to press the POWER KEY from the board. At
>> my side, reboot command from OS also can get CAN out of stop mode.
> Do you mean "cold reset", also known as Power-On-Reset, POR or power cycle?
> 
> What does pressing the POWER KEY do? A power cycle of the system or
> toggling the reset line of the imx?
> 
> We need to describe in detail, as not everyone has the same board as
> you, and these boards might not even have a power key :)
> 
>> Below is experiment I did:
>> 	Firstly, do a hacking to let CAN stuck into stop mode, then:
> 
> You mean you put the CAN into stop mode without keeping track in the CAN
> driver that the CAN-IP is in stop mode, e.g. by hacking the driver.
> 
> Then you try several methods to recover:
> 
>> 	(1) press power on/off key, get CAN out of stop mode;
>> 	(2) reboot command from console, get CAN out of stop mode;
>> 	(3) unbind/bind driver, cannot get CAN out of stop mode;
>> 	(4) remod/insmod module, cannot get CAN out of stop mode;
> 
> (2) resets the complete imx, including the CAN-IP core, (1) probably, too.
No, if the CAN-IP core is in stop-mode it will stay that way even after 
a reboot from the console.
At least it's what we are seeing in the field.

This could be because we are missing a wire from the watchdog out to the 
RESETBMCU/PWRON on the PMIC.
But i guess a check for if the CAN-Ip is in stop-mode doesn't hurt 
anything :)

/Sean
Joakim Zhang Dec. 5, 2019, 11:04 a.m. UTC | #8
> -----Original Message-----
> From: Sean Nyekjaer <sean@geanix.com>
> Sent: 2019年12月5日 17:22
> To: Marc Kleine-Budde <mkl@pengutronix.de>; Joakim Zhang
> <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH V2 2/4] can: flexcan: try to exit stop mode during probe
> stage
> 
> 
> 
> On 04/12/2019 09.45, Marc Kleine-Budde wrote:
> > On 12/4/19 3:22 AM, Joakim Zhang wrote:
> >>
> >>> -----Original Message-----
> >>> From: Marc Kleine-Budde <mkl@pengutronix.de>
> >>> Sent: 2019年12月4日 2:15
> >>> 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 2/4] can: flexcan: try to exit stop mode
> >>> during probe stage
> >>>
> >>> On 11/27/19 6:56 AM, Joakim Zhang wrote:
> >>>> CAN controller could be stucked in stop mode once it enters stop
> >>>> mode
> >>>                            ^^^^^^^ stuck
> >>>> when suspend, and then it fails to exit stop mode when resume.
> >>>
> >>> How can this happen?
> >>
> >> I am also confused how can this happen, as I asked Sean, only CAN
> >> enter stop mode when suspend, then system hang,
> > How do you recover the system when suspended?
> >
> >> it could let CAN
> >> stuck in stop mode. However, Sean said this indeed happen at his
> >> side, @sean@geanix.com, could you explain how this happen in details?
> > That would be good.
> >
> >>>> Only code reset can get CAN out of stop mode,
> >>>
> >>> What is "code reset"?
> >>
> >> As I know, "code reset" is to press the POWER KEY from the board. At
> >> my side, reboot command from OS also can get CAN out of stop mode.
> > Do you mean "cold reset", also known as Power-On-Reset, POR or power
> cycle?
> >
> > What does pressing the POWER KEY do? A power cycle of the system or
> > toggling the reset line of the imx?
> >
> > We need to describe in detail, as not everyone has the same board as
> > you, and these boards might not even have a power key :)
> >
> >> Below is experiment I did:
> >> 	Firstly, do a hacking to let CAN stuck into stop mode, then:
> >
> > You mean you put the CAN into stop mode without keeping track in the
> > CAN driver that the CAN-IP is in stop mode, e.g. by hacking the driver.
> >
> > Then you try several methods to recover:
> >
> >> 	(1) press power on/off key, get CAN out of stop mode;
> >> 	(2) reboot command from console, get CAN out of stop mode;
> >> 	(3) unbind/bind driver, cannot get CAN out of stop mode;
> >> 	(4) remod/insmod module, cannot get CAN out of stop mode;
> >
> > (2) resets the complete imx, including the CAN-IP core, (1) probably, too.
> No, if the CAN-IP core is in stop-mode it will stay that way even after a reboot
> from the console.
> At least it's what we are seeing in the field.
> 
> This could be because we are missing a wire from the watchdog out to the
> RESETBMCU/PWRON on the PMIC.
> But i guess a check for if the CAN-Ip is in stop-mode doesn't hurt anything :)
Hi Sean,

At my side, both Power-On-Reset and reboot from console can get CAN-IP out of stop mode, HW is i.MX7D-SDB/i.MX8QXP-mek.
I think HW design could make difference.

We more care about how does CAN-IP stuck in stop mode, could you please explain in details? We want figure out the root cause.

> /Sean
Sean Nyekjaer Dec. 5, 2019, 11:11 a.m. UTC | #9
On 05/12/2019 12.04, Joakim Zhang wrote:
> Hi Sean,
> 
> At my side, both Power-On-Reset and reboot from console can get CAN-IP out of stop mode, HW is i.MX7D-SDB/i.MX8QXP-mek.
> I think HW design could make difference.
> 
> We more care about how does CAN-IP stuck in stop mode, could you please explain in details? We want figure out the root cause.

When running only with the first stop mode commit:
de3578c198c6 ("can: flexcan: add self wakeup support")
And there is incoming traffic on both CAN lines.
I happens when going into suspend.
Then the CAN-IP is stuck in stop mode..

I'm on this custom i.MX6ull board.
I have another custom board also with the i.MX6ull where the watchdog 
pins are wired to the PMIC.
Will try to test on that next week

/Sean
Joakim Zhang Dec. 5, 2019, 11:19 a.m. UTC | #10
> -----Original Message-----
> From: Sean Nyekjaer <sean@geanix.com>
> Sent: 2019年12月5日 19:12
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; Marc Kleine-Budde
> <mkl@pengutronix.de>; linux-can@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH V2 2/4] can: flexcan: try to exit stop mode during probe
> stage
> 
> 
> 
> On 05/12/2019 12.04, Joakim Zhang wrote:
> > Hi Sean,
> >
> > At my side, both Power-On-Reset and reboot from console can get CAN-IP out
> of stop mode, HW is i.MX7D-SDB/i.MX8QXP-mek.
> > I think HW design could make difference.
> >
> > We more care about how does CAN-IP stuck in stop mode, could you please
> explain in details? We want figure out the root cause.
> 
> When running only with the first stop mode commit:
> de3578c198c6 ("can: flexcan: add self wakeup support") And there is incoming
> traffic on both CAN lines.
> I happens when going into suspend.
> Then the CAN-IP is stuck in stop mode..

That means with below patch then CAN-IP could not been stuck in stop mode, right?
	can: flexcan: fix deadlock when using self wakeup

If yes, I think we don't need check stop mode in probe stage, since issue has disappeared automatically.

> I'm on this custom i.MX6ull board.
> I have another custom board also with the i.MX6ull where the watchdog pins
> are wired to the PMIC.
> Will try to test on that next week

Okay, thanks.

Best Regards,
Joakim Zhang
> /Sean
Sean Nyekjaer Dec. 5, 2019, 11:32 a.m. UTC | #11
On 05/12/2019 12.19, Joakim Zhang wrote:
> 
>> -----Original Message-----
>> From: Sean Nyekjaer <sean@geanix.com>
>> Sent: 2019年12月5日 19:12
>> To: Joakim Zhang <qiangqing.zhang@nxp.com>; Marc Kleine-Budde
>> <mkl@pengutronix.de>; linux-can@vger.kernel.org
>> Cc: dl-linux-imx <linux-imx@nxp.com>; netdev@vger.kernel.org
>> Subject: Re: [PATCH V2 2/4] can: flexcan: try to exit stop mode during probe
>> stage
>>
>>
>>
>> On 05/12/2019 12.04, Joakim Zhang wrote:
>>> Hi Sean,
>>>
>>> At my side, both Power-On-Reset and reboot from console can get CAN-IP out
>> of stop mode, HW is i.MX7D-SDB/i.MX8QXP-mek.
>>> I think HW design could make difference.
>>>
>>> We more care about how does CAN-IP stuck in stop mode, could you please
>> explain in details? We want figure out the root cause.
>>
>> When running only with the first stop mode commit:
>> de3578c198c6 ("can: flexcan: add self wakeup support") And there is incoming
>> traffic on both CAN lines.
>> I happens when going into suspend.
>> Then the CAN-IP is stuck in stop mode..
> 
> That means with below patch then CAN-IP could not been stuck in stop mode, right?
> 	can: flexcan: fix deadlock when using self wakeup
Yes :)
> 
> If yes, I think we don't need check stop mode in probe stage, since issue has disappeared automatically.

If one have devices deployed where:
"can: flexcan: fix deadlock when using self wakeup" isn't applied.
They could have devices stuck in stop-mode.

That's what i meant by this patch doesn't do any harm to have the check 
included.

/Sean
Marc Kleine-Budde Dec. 7, 2019, 8:32 p.m. UTC | #12
On 12/5/19 12:32 PM, Sean Nyekjaer wrote:
>> If yes, I think we don't need check stop mode in probe stage, since
>> issue has disappeared automatically.
> 
> If one have devices deployed where:
> "can: flexcan: fix deadlock when using self wakeup" isn't applied.
> They could have devices stuck in stop-mode.

Ok.

But both patches:

    can: flexcan: fix deadlock when using self wakeup
    can: flexcan: try to exit stop mode during probe stage

are not yet mainline, so if "can: flexcan: fix deadlock when using self
wakeup" fixes the problem and goes into stable we don't need "can:
flexcan: try to exit stop mode during probe stage", right?

> That's what i meant by this patch doesn't do any harm to have the check 
> included.

I don't want to have code in the driver that serves no purpose.

regards,
Marc
Sean Nyekjaer Dec. 8, 2019, 10:47 a.m. UTC | #13
On 07/12/2019 21.32, Marc Kleine-Budde wrote:
> On 12/5/19 12:32 PM, Sean Nyekjaer wrote:
>>> If yes, I think we don't need check stop mode in probe stage, since
>>> issue has disappeared automatically.
>>
>> If one have devices deployed where:
>> "can: flexcan: fix deadlock when using self wakeup" isn't applied.
>> They could have devices stuck in stop-mode.
> 
> Ok.
> 
> But both patches:
> 
>      can: flexcan: fix deadlock when using self wakeup
>      can: flexcan: try to exit stop mode during probe stage
> 
> are not yet mainline, so if "can: flexcan: fix deadlock when using self
> wakeup" fixes the problem and goes into stable we don't need "can:
> flexcan: try to exit stop mode during probe stage", right?
> 
>> That's what i meant by this patch doesn't do any harm to have the check
>> included.
> 
> I don't want to have code in the driver that serves no purpose.
> 

Fine with me, I'm continuing to have the patch included in my tree until 
all devices are upgraded with:
	can: flexcan: fix deadlock when using self wakeup

/Sean

> regards,
> Marc
>
Marc Kleine-Budde Dec. 8, 2019, 10:56 a.m. UTC | #14
On 12/8/19 11:47 AM, Sean Nyekjaer wrote:
>> But both patches:
>>
>>      can: flexcan: fix deadlock when using self wakeup
>>      can: flexcan: try to exit stop mode during probe stage
>>
>> are not yet mainline, so if "can: flexcan: fix deadlock when using self
>> wakeup" fixes the problem and goes into stable we don't need "can:
>> flexcan: try to exit stop mode during probe stage", right?
>>
>>> That's what i meant by this patch doesn't do any harm to have the check
>>> included.
>>
>> I don't want to have code in the driver that serves no purpose.
> 
> Fine with me, I'm continuing to have the patch included in my tree until 
> all devices are upgraded with:
> 	can: flexcan: fix deadlock when using self wakeup

That makes sende in your usecase, as you already have "can: flexcan: try
to exit stop mode during probe stage" in your tree.

regards,
Marc
diff mbox series

Patch

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 2297663cacb2..5d5ed28d3005 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -449,6 +449,13 @@  static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
 	return 0;
 }
 
+static void flexcan_try_exit_stop_mode(struct flexcan_priv *priv)
+{
+	/* remove stop request */
+	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+			   1 << priv->stm.req_bit, 0);
+}
+
 static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
 {
 	struct flexcan_regs __iomem *regs = priv->regs;
@@ -1649,6 +1656,21 @@  static int flexcan_probe(struct platform_device *pdev)
 	priv->devtype_data = devtype_data;
 	priv->reg_xceiver = reg_xceiver;
 
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
+		err = flexcan_setup_stop_mode(pdev);
+		if (err)
+			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
+
+		/* CAN controller could be stucked in stop mode once it enters
+		 * stop mode when suspend, and then it fails to exit stop
+		 * mode when resume. Only code reset can get CAN out of stop
+		 * mode, so add stop mode remove request here for other methods
+		 * (soft reset, bind, etc) to let CAN active again. MCR[LPMACK]
+		 * will be checked when enable CAN in register_flexcandev().
+		 */
+		flexcan_try_exit_stop_mode(priv);
+	}
+
 	pm_runtime_get_noresume(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
@@ -1661,12 +1683,6 @@  static int flexcan_probe(struct platform_device *pdev)
 
 	devm_can_led_init(dev);
 
-	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
-		err = flexcan_setup_stop_mode(pdev);
-		if (err)
-			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
-	}
-
 	return 0;
 
  failed_register: