netdev - RE: [PATCH net-next 11/39] can: flexcan: add auto stop mode for IMX93 to support wakeup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DB7PR04MB4010740EC36BE814CE5C416B90E39@DB7PR04MB4010.eurprd04.prod.outlook.com>
Date:   Tue, 13 Dec 2022 02:22:02 +0000
From:   Bough Chen <haibo.chen@....com>
To:     Marc Kleine-Budde <mkl@...gutronix.de>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>
Subject: RE: [PATCH net-next 11/39] can: flexcan: add auto stop mode for IMX93
 to support wakeup

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@...gutronix.de>
> Sent: 2022年12月12日 19:30
> To: netdev@...r.kernel.org
> Cc: davem@...emloft.net; kuba@...nel.org; linux-can@...r.kernel.org;
> kernel@...gutronix.de; Bough Chen <haibo.chen@....com>; Marc
> Kleine-Budde <mkl@...gutronix.de>
> Subject: [PATCH net-next 11/39] can: flexcan: add auto stop mode for IMX93 to
> support wakeup
> 
> From: Haibo Chen <haibo.chen@....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@....com>
> Link:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kern
> el.org%2Fall%2F1669116752-4260-1-git-send-email-haibo.chen%40nxp.com&a
> mp;data=05%7C01%7Chaibo.chen%40nxp.com%7C57db640faf1a4fe6ac9e08da
> dc34556c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638064414
> 581423735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=
> yc1AspfoVq%2BHKokRFxf06vcaqqP%2FtZUnpMnTaXL97y8%3D&amp;reserved=
> 0
> Signed-off-by: Marc Kleine-Budde <mkl@...gutronix.de>

Hi Marc,

Thanks for sending this patch.
But this patch has one runtime PM unbalance issue, I will fix this issue and send again these days.

Best Regards
Haibo Chen
> ---
>  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
> @@ -345,6 +345,15 @@ static struct flexcan_devtype_data
> fsl_imx8mp_devtype_data = {
>  		FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
>  };
> 
> +static struct flexcan_devtype_data fsl_imx93_devtype_data = {
> +	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> +		FLEXCAN_QUIRK_DISABLE_MECR |
> FLEXCAN_QUIRK_USE_RX_MAILBOX |
> +		FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> FLEXCAN_QUIRK_AUTO_STOP_MODE |
> +		FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC |
> +		FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
> +		FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR,
> +};
> +
>  static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
>  	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
>  		FLEXCAN_QUIRK_DISABLE_MECR |
> FLEXCAN_QUIRK_USE_RX_MAILBOX | @@ -532,9 +541,14 @@ static inline int
> flexcan_enter_stop_mode(struct flexcan_priv *priv)
>  		ret = flexcan_stop_mode_enable_scfw(priv, true);
>  		if (ret < 0)
>  			return ret;
> -	} else {
> +	} else if (priv->devtype_data.quirks &
> +FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
>  		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
>  				   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
> +	} else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE)
> {
> +		/* For the auto stop mode, software do nothing, hardware will cover
> +		 * all the operation automatically after system go into low power
> mode.
> +		 */
> +		return 0;
>  	}
> 
>  	return flexcan_low_power_enter_ack(priv);
> @@ -551,7 +565,7 @@ static inline int flexcan_exit_stop_mode(struct
> flexcan_priv *priv)
>  		ret = flexcan_stop_mode_enable_scfw(priv, false);
>  		if (ret < 0)
>  			return ret;
> -	} else {
> +	} else if (priv->devtype_data.quirks &
> +FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
>  		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
>  				   1 << priv->stm.req_bit, 0);
>  	}
> @@ -560,6 +574,12 @@ static inline int flexcan_exit_stop_mode(struct
> flexcan_priv *priv)
>  	reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
>  	priv->write(reg_mcr, &regs->mcr);
> 
> +	/* For the auto stop mode, hardware will exist stop mode
> +	 * automatically after system go out of low power mode.
> +	 */
> +	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE)
> +		return 0;
> +
>  	return flexcan_low_power_exit_ack(priv);  }
> 
> @@ -1974,6 +1994,8 @@ static int flexcan_setup_stop_mode(struct
> platform_device *pdev)
>  		ret = flexcan_setup_stop_mode_scfw(pdev);
>  	else if (priv->devtype_data.quirks &
> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR)
>  		ret = flexcan_setup_stop_mode_gpr(pdev);
> +	else if (priv->devtype_data.quirks & FLEXCAN_QUIRK_AUTO_STOP_MODE)
> +		ret = 0;
>  	else
>  		/* return 0 directly if doesn't support stop mode feature */
>  		return 0;
> @@ -1992,6 +2014,7 @@ static int flexcan_setup_stop_mode(struct
> platform_device *pdev)  static const struct of_device_id flexcan_of_match[] =
> {
>  	{ .compatible = "fsl,imx8qm-flexcan", .data =
> &fsl_imx8qm_devtype_data, },
>  	{ .compatible = "fsl,imx8mp-flexcan", .data =
> &fsl_imx8mp_devtype_data, },
> +	{ .compatible = "fsl,imx93-flexcan", .data = &fsl_imx93_devtype_data,
> +},
>  	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>  	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
>  	{ .compatible = "fsl,imx53-flexcan", .data = &fsl_imx25_devtype_data, },
> @@ -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;
> +		}
> 
>  		err = pm_runtime_force_suspend(device);
>  		if (err)
> diff --git a/drivers/net/can/flexcan/flexcan.h
> b/drivers/net/can/flexcan/flexcan.h
> index 025c3417031f..91402977780b 100644
> --- a/drivers/net/can/flexcan/flexcan.h
> +++ b/drivers/net/can/flexcan/flexcan.h
> @@ -68,6 +68,8 @@
>  #define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(15)
>  /* Device supports RX via FIFO */
>  #define FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(16)
> +/* auto enter stop mode to support wakeup */ #define
> +FLEXCAN_QUIRK_AUTO_STOP_MODE BIT(17)
> 
>  struct flexcan_devtype_data {
>  	u32 quirks;		/* quirks needed for different IP cores */
> --
> 2.35.1
>