diff mbox series

[linux-can-next/flexcan,3/4] can: flexcan: add CAN wakeup function for i.MX8

Message ID 20200925151028.11004-4-qiangqing.zhang@nxp.com
State Awaiting Upstream
Delegated to: David Miller
Headers show
Series patch set for flexcan | expand

Commit Message

Joakim Zhang Sept. 25, 2020, 3:10 p.m. UTC
The System Controller Firmware (SCFW) is a low-level system function
which runs on a dedicated Cortex-M core to provide power, clock, and
resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
(QM, QP), and i.MX8QX (QXP, DX).

SCU driver manages the IPC interface between host CPU and the
SCU firmware running on M4.

For i.MX8, stop mode request is controlled by System Controller Unit(SCU)
firmware.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 81 ++++++++++++++++++++++++++++++++-------
 1 file changed, 68 insertions(+), 13 deletions(-)

Comments

Marc Kleine-Budde Sept. 25, 2020, 9:44 a.m. UTC | #1
On 9/25/20 5:10 PM, Joakim Zhang wrote:
> The System Controller Firmware (SCFW) is a low-level system function
> which runs on a dedicated Cortex-M core to provide power, clock, and
> resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> (QM, QP), and i.MX8QX (QXP, DX).
> 
> SCU driver manages the IPC interface between host CPU and the
> SCU firmware running on M4.
> 
> For i.MX8, stop mode request is controlled by System Controller Unit(SCU)
> firmware.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---

Just for reference, this fails to build with:

ERROR: modpost: "imx_scu_get_handle" [drivers/net/can/flexcan.ko] undefined!
ERROR: modpost: "imx_sc_misc_set_control" [drivers/net/can/flexcan.ko] undefined!

Marc
Joakim Zhang Sept. 25, 2020, 9:49 a.m. UTC | #2
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2020年9月25日 17:44
> To: 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 linux-can-next/flexcan 3/4] can: flexcan: add CAN wakeup
> function for i.MX8
> 
> On 9/25/20 5:10 PM, Joakim Zhang wrote:
> > The System Controller Firmware (SCFW) is a low-level system function
> > which runs on a dedicated Cortex-M core to provide power, clock, and
> > resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> > (QM, QP), and i.MX8QX (QXP, DX).
> >
> > SCU driver manages the IPC interface between host CPU and the SCU
> > firmware running on M4.
> >
> > For i.MX8, stop mode request is controlled by System Controller
> > Unit(SCU) firmware.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> 
> Just for reference, this fails to build with:
> 
> ERROR: modpost: "imx_scu_get_handle" [drivers/net/can/flexcan.ko]
> undefined!
> ERROR: modpost: "imx_sc_misc_set_control" [drivers/net/can/flexcan.ko]
> undefined!

Yes, I build at my side, due to scu symbols have not export on non-scu systems.

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 Sept. 29, 2020, 10:48 a.m. UTC | #3
On 9/25/20 5:10 PM, Joakim Zhang wrote:
> The System Controller Firmware (SCFW) is a low-level system function
> which runs on a dedicated Cortex-M core to provide power, clock, and
> resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> (QM, QP), and i.MX8QX (QXP, DX).
> 
> SCU driver manages the IPC interface between host CPU and the
> SCU firmware running on M4.
> 
> For i.MX8, stop mode request is controlled by System Controller Unit(SCU)
> firmware.

As you mentioned in the other mail, some functions are missing from the
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  drivers/net/can/flexcan.c | 81 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 68 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 8c8753f77764..41b52cb56f93 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -9,6 +9,7 @@
>  //
>  // Based on code originally by Andrey Volkov <avolkov@varma-el.com>
>  
> +#include <dt-bindings/firmware/imx/rsrc.h>
>  #include <linux/bitfield.h>
>  #include <linux/can.h>
>  #include <linux/can/dev.h>
> @@ -17,6 +18,7 @@
>  #include <linux/can/rx-offload.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/firmware/imx/sci.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/mfd/syscon.h>
> @@ -240,6 +242,8 @@
>  #define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)

rename this into "FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR"

>  /* Support CAN-FD mode */
>  #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
> +/* Use System Controller Firmware */
> +#define FLEXCAN_QUIRK_USE_SCFW BIT(10)

...and this into FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW

>  
>  /* Structure of the message buffer */
>  struct flexcan_mb {
> @@ -358,6 +362,9 @@ struct flexcan_priv {
>  	struct regulator *reg_xceiver;
>  	struct flexcan_stop_mode stm;
>  
> +	/* IPC handle when enable stop mode by System Controller firmware(scfw) */
> +	struct imx_sc_ipc *sc_ipc_handle;
> +
>  	/* Read and Write APIs */
>  	u32 (*read)(void __iomem *addr);
>  	void (*write)(u32 val, void __iomem *addr);
> @@ -387,7 +394,8 @@ static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
>  static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
>  	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
>  		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> -		FLEXCAN_QUIRK_SUPPORT_FD,
> +		FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE |
> +		FLEXCAN_QUIRK_USE_SCFW,
>  };
>  
>  static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
> @@ -546,6 +554,25 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable)
>  	priv->write(reg_mcr, &regs->mcr);
>  }
>  
> +static void flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool enabled)
> +{
> +	struct device_node *np = priv->dev->of_node;
> +	u32 rsrc_id, val;
> +	int idx;
> +
> +	idx = of_alias_get_id(np, "can");
> +	if (idx == 0)
> +		rsrc_id = IMX_SC_R_CAN_0;
> +	else if (idx == 1)
> +		rsrc_id = IMX_SC_R_CAN_1;
> +	else
> +		rsrc_id = IMX_SC_R_CAN_2;

This looks too fragile to me. Better add a property to the DT which indicates
the index.

> +
> +	val = enabled ? 1 : 0;

Please use an if() here.

> +	/* stop mode request */
> +	imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id, IMX_SC_C_IPG_STOP, val);
> +}
> +
>  static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
>  {
>  	struct flexcan_regs __iomem *regs = priv->regs;
> @@ -555,9 +582,12 @@ static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
>  	reg_mcr |= FLEXCAN_MCR_SLF_WAK;
>  	priv->write(reg_mcr, &regs->mcr);
>  
> -	/* enable stop request */
> -	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> -			   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
> +	 /* enable stop request */
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
> +		flexcan_stop_mode_enable_scfw(priv, true);

error handling?

> +	else
> +		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +				   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
>  
>  	return flexcan_low_power_enter_ack(priv);
>  }
> @@ -568,8 +598,11 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
>  	u32 reg_mcr;
>  
>  	/* remove stop request */
> -	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> -			   1 << priv->stm.req_bit, 0);
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
> +		flexcan_stop_mode_enable_scfw(priv, false);

error handling?

> +	else
> +		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +				   1 << priv->stm.req_bit, 0);
>  
>  	reg_mcr = priv->read(&regs->mcr);
>  	reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
> @@ -1927,11 +1960,6 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
>  		gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_bit,
>  		priv->stm.ack_gpr, priv->stm.ack_bit);
>  
> -	device_set_wakeup_capable(&pdev->dev, true);
> -
> -	if (of_property_read_bool(np, "wakeup-source"))
> -		device_set_wakeup_enable(&pdev->dev, true);
> -
>  	return 0;
>  
>  out_put_node:
> @@ -1939,6 +1967,23 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +static int flexcan_setup_stop_mode_scfw(struct platform_device *pdev)
> +{
> +	struct net_device *dev = platform_get_drvdata(pdev);
> +	struct flexcan_priv *priv;
> +	int ret;
> +
> +	priv = netdev_priv(dev);
> +
> +	ret = imx_scu_get_handle(&priv->sc_ipc_handle);

this function can return -EPROBE_DEFER

https://elixir.bootlin.com/linux/latest/source/drivers/firmware/imx/imx-scu.c#L97

> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "get ipc handle used by SCU failed\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct of_device_id flexcan_of_match[] = {
>  	{ .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
>  	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
> @@ -2088,9 +2133,19 @@ 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);

what about renaming the flexcan_setup_stop_mode() to
flexcan_setup_stop_mode_gpr() and moving this below into a function called
flexcan_setup_stop_mode().

> -		if (err)
> +		if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
> +			err = flexcan_setup_stop_mode_scfw(pdev);
> +		else
> +			err = flexcan_setup_stop_mode(pdev);
> +
> +		if (err) {
>  			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
> +		} else {
> +			device_set_wakeup_capable(&pdev->dev, true);
> +
> +			if (of_property_read_bool(pdev->dev.of_node, "wakeup-source"))
> +				device_set_wakeup_enable(&pdev->dev, true);
> +		}
>  	}
>  
>  	return 0;
> 

Marc
diff mbox series

Patch

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8c8753f77764..41b52cb56f93 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -9,6 +9,7 @@ 
 //
 // Based on code originally by Andrey Volkov <avolkov@varma-el.com>
 
+#include <dt-bindings/firmware/imx/rsrc.h>
 #include <linux/bitfield.h>
 #include <linux/can.h>
 #include <linux/can/dev.h>
@@ -17,6 +18,7 @@ 
 #include <linux/can/rx-offload.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/firmware/imx/sci.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
@@ -240,6 +242,8 @@ 
 #define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)
 /* Support CAN-FD mode */
 #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
+/* Use System Controller Firmware */
+#define FLEXCAN_QUIRK_USE_SCFW BIT(10)
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -358,6 +362,9 @@  struct flexcan_priv {
 	struct regulator *reg_xceiver;
 	struct flexcan_stop_mode stm;
 
+	/* IPC handle when enable stop mode by System Controller firmware(scfw) */
+	struct imx_sc_ipc *sc_ipc_handle;
+
 	/* Read and Write APIs */
 	u32 (*read)(void __iomem *addr);
 	void (*write)(u32 val, void __iomem *addr);
@@ -387,7 +394,8 @@  static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
 static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
 		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
-		FLEXCAN_QUIRK_SUPPORT_FD,
+		FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE |
+		FLEXCAN_QUIRK_USE_SCFW,
 };
 
 static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
@@ -546,6 +554,25 @@  static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable)
 	priv->write(reg_mcr, &regs->mcr);
 }
 
+static void flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool enabled)
+{
+	struct device_node *np = priv->dev->of_node;
+	u32 rsrc_id, val;
+	int idx;
+
+	idx = of_alias_get_id(np, "can");
+	if (idx == 0)
+		rsrc_id = IMX_SC_R_CAN_0;
+	else if (idx == 1)
+		rsrc_id = IMX_SC_R_CAN_1;
+	else
+		rsrc_id = IMX_SC_R_CAN_2;
+
+	val = enabled ? 1 : 0;
+	/* stop mode request */
+	imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id, IMX_SC_C_IPG_STOP, val);
+}
+
 static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
 {
 	struct flexcan_regs __iomem *regs = priv->regs;
@@ -555,9 +582,12 @@  static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
 	reg_mcr |= FLEXCAN_MCR_SLF_WAK;
 	priv->write(reg_mcr, &regs->mcr);
 
-	/* enable stop request */
-	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
-			   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+	 /* enable stop request */
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
+		flexcan_stop_mode_enable_scfw(priv, true);
+	else
+		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+				   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
 
 	return flexcan_low_power_enter_ack(priv);
 }
@@ -568,8 +598,11 @@  static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
 	u32 reg_mcr;
 
 	/* remove stop request */
-	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
-			   1 << priv->stm.req_bit, 0);
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
+		flexcan_stop_mode_enable_scfw(priv, false);
+	else
+		regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+				   1 << priv->stm.req_bit, 0);
 
 	reg_mcr = priv->read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
@@ -1927,11 +1960,6 @@  static int flexcan_setup_stop_mode(struct platform_device *pdev)
 		gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_bit,
 		priv->stm.ack_gpr, priv->stm.ack_bit);
 
-	device_set_wakeup_capable(&pdev->dev, true);
-
-	if (of_property_read_bool(np, "wakeup-source"))
-		device_set_wakeup_enable(&pdev->dev, true);
-
 	return 0;
 
 out_put_node:
@@ -1939,6 +1967,23 @@  static int flexcan_setup_stop_mode(struct platform_device *pdev)
 	return ret;
 }
 
+static int flexcan_setup_stop_mode_scfw(struct platform_device *pdev)
+{
+	struct net_device *dev = platform_get_drvdata(pdev);
+	struct flexcan_priv *priv;
+	int ret;
+
+	priv = netdev_priv(dev);
+
+	ret = imx_scu_get_handle(&priv->sc_ipc_handle);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "get ipc handle used by SCU failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static const struct of_device_id flexcan_of_match[] = {
 	{ .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },
 	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
@@ -2088,9 +2133,19 @@  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)
+		if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_SCFW)
+			err = flexcan_setup_stop_mode_scfw(pdev);
+		else
+			err = flexcan_setup_stop_mode(pdev);
+
+		if (err) {
 			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
+		} else {
+			device_set_wakeup_capable(&pdev->dev, true);
+
+			if (of_property_read_bool(pdev->dev.of_node, "wakeup-source"))
+				device_set_wakeup_enable(&pdev->dev, true);
+		}
 	}
 
 	return 0;