Re: [PATCH 1/3] can: flexcan: add auto stop mode for IMX93 to support wakeup - Marc Kleine-Budde

From: Marc Kleine-Budde <mkl@pengutronix.de>
To: haibo.chen@nxp.com
Cc: wg@grandegger.com, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de,
	festevam@gmail.com, linux-imx@nxp.com, linux-can@vger.kernel.org,
	netdev@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 1/3] can: flexcan: add auto stop mode for IMX93 to support wakeup
Date: Wed, 30 Nov 2022 11:54:17 +0100	[thread overview]
Message-ID: <20221130105417.yhljwl5dtuu5f2du@pengutronix.de> (raw)
In-Reply-To: <1669116752-4260-1-git-send-email-haibo.chen@nxp.com>

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

On 22.11.2022 19:32:30, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> IMX93 do not contain a GPR to config the stop mode, it will set
> the flexcan into stop mode automatically once the ARM core go
> into low power mode (WFI instruct) and gate off the flexcan
> related clock automatically. But to let these logic work as
> expect, before ARM core go into low power mode, need to make
> sure the flexcan related clock keep on.
> 
> To support stop mode and wakeup feature on imx93, this patch
> add a new fsl_imx93_devtype_data to separate from imx8mp.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/net/can/flexcan/flexcan-core.c | 37 +++++++++++++++++++++++---
>  drivers/net/can/flexcan/flexcan.h      |  2 ++
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 9bdadd716f4e..0aeff34e5ae1 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c

[...]

> @@ -2299,8 +2322,16 @@ static int __maybe_unused flexcan_noirq_suspend(struct device *device)
>  	if (netif_running(dev)) {
>  		int err;
>  
> -		if (device_may_wakeup(device))
> +		if (device_may_wakeup(device)) {
>  			flexcan_enable_wakeup_irq(priv, true);
> +			/* For auto stop mode, need to keep the clock on before
> +			 * system go into low power mode. After system go into
> +			 * low power mode, hardware will config the flexcan into
> +			 * stop mode, and gate off the clock automatically.
> +			 */
> +			if (priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE)
> +				return 0;
> +		}

With this change the flexcan_noirq_resume() is not symmetrical any more:
pm_runtime_force_suspend() is not called for mx93, but
pm_runtime_force_resume() is called.

>  
>  		err = pm_runtime_force_suspend(device);
>  		if (err)

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 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]