| Message ID | 20201019155737.26577-9-qiangqing.zhang@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | can: flexcan: add stop mode support for i.MX8QM | expand |
On 10/19/20 5:57 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.MX8QM, stop mode request is controlled by System Controller Unit(SCU) > firmware, this patch introduces FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW quirk > for this function. > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > --- > drivers/net/can/flexcan.c | 131 +++++++++++++++++++++++++++++++++----- > 1 file changed, 114 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > index c0cdee904ca7..8ad5065c918f 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> > @@ -203,6 +205,8 @@ > > #define FLEXCAN_TIMEOUT_US (250) > > +#define FLEXCAN_IMX_SC_R_CAN(x) (IMX_SC_R_CAN_0 + (x)) Why not move it into the appropriate svc header file? > + > /* FLEXCAN hardware feature flags > * > * Below is some version info we got: > @@ -242,6 +246,8 @@ > #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9) > /* support memory detection and correction */ > #define FLEXCAN_QUIRK_SUPPORT_ECC BIT(10) > +/* Setup stop mode with SCU firmware to support wakeup */ > +#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11) > > /* Structure of the message buffer */ > struct flexcan_mb { > @@ -347,6 +353,7 @@ struct flexcan_priv { > u8 mb_count; > u8 mb_size; > u8 clk_src; /* clock source of CAN Protocol Engine */ > + u8 can_idx; > > u64 rx_mask; > u64 tx_mask; > @@ -358,6 +365,9 @@ struct flexcan_priv { > struct regulator *reg_xceiver; > struct flexcan_stop_mode stm; > > + /* IPC handle when setup 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 +397,7 @@ 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_SCFW, > }; > > static struct flexcan_devtype_data fsl_imx8mp_devtype_data = { > @@ -546,18 +556,42 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable) > priv->write(reg_mcr, ®s->mcr); > } > > +static int flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool enabled) > +{ > + u8 idx = priv->can_idx; > + u32 rsrc_id, val; > + > + rsrc_id = FLEXCAN_IMX_SC_R_CAN(idx); > + > + if (enabled) > + val = 1; > + else > + val = 0; > + > + /* stop mode request via scu firmware */ > + return 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; > u32 reg_mcr; > + int ret; > > reg_mcr = priv->read(®s->mcr); > reg_mcr |= FLEXCAN_MCR_SLF_WAK; > priv->write(reg_mcr, ®s->mcr); > > /* enable stop request */ > - regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, > - 1 << priv->stm.req_bit, 1 << priv->stm.req_bit); > + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) { > + ret = flexcan_stop_mode_enable_scfw(priv, true); > + if (ret < 0) > + return ret; > + } 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); > } > @@ -566,10 +600,17 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv) > { > struct flexcan_regs __iomem *regs = priv->regs; > u32 reg_mcr; > + int ret; > > /* 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_SETUP_STOP_MODE_SCFW) { > + ret = flexcan_stop_mode_enable_scfw(priv, false); > + if (ret < 0) > + return ret; > + } else { > + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, > + 1 << priv->stm.req_bit, 0); > + } > > reg_mcr = priv->read(®s->mcr); > reg_mcr &= ~FLEXCAN_MCR_SLF_WAK; > @@ -1838,7 +1879,7 @@ static void unregister_flexcandev(struct net_device *dev) > unregister_candev(dev); > } > > -static int flexcan_setup_stop_mode(struct platform_device *pdev) > +static int flexcan_setup_stop_mode_gpr(struct platform_device *pdev) > { > struct net_device *dev = platform_get_drvdata(pdev); > struct device_node *np = pdev->dev.of_node; > @@ -1883,11 +1924,6 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev) > "gpr %s req_gpr=0x02%x req_bit=%u\n", > gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_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: > @@ -1895,6 +1931,64 @@ 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; > + u8 can_idx; > + int ret; > + > + ret = of_property_read_u8(pdev->dev.of_node, "fsl,scu-index", &can_idx); > + if (ret < 0) { > + dev_dbg(&pdev->dev, "failed to get scu index\n"); > + return ret; > + } > + > + priv = netdev_priv(dev); > + priv->can_idx = can_idx; > + > + /* this function could be defered probe, return -EPROBE_DEFER */ > + ret = imx_scu_get_handle(&priv->sc_ipc_handle); > + if (ret < 0) { > + dev_dbg(&pdev->dev, "get ipc handle used by SCU failed\n"); > + return ret; > + } Don't print an error message on -EPROBE_DEFER. > + > + return 0; > +} > + > +/* flexcan_setup_stop_mode - Setup stop mode for wakeup > + * > + * Return: = 0 setup stop mode successfully or doesn't support this feature > + * < 0 fail to setup stop mode (could be defered probe) > + */ > +static int flexcan_setup_stop_mode(struct platform_device *pdev) > +{ > + struct net_device *dev = platform_get_drvdata(pdev); > + struct flexcan_priv *priv; > + int ret; > + > + priv = netdev_priv(dev); > + > + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) > + 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 > + /* return 0 directly if doesn't support stop mode feature */ > + return 0; > + > + if (ret) > + return ret; > + > + 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; > +} > + > 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, }, > @@ -2040,17 +2134,19 @@ static int flexcan_probe(struct platform_device *pdev) > goto failed_register; > } > > + err = flexcan_setup_stop_mode(pdev); > + if (err < 0) { > + dev_err(&pdev->dev, "setup stop mode failed\n"); > + goto failed_canregister; > + } > + > of_can_transceiver(dev); > devm_can_led_init(dev); > > - if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) { > - err = flexcan_setup_stop_mode(pdev); > - if (err) > - dev_dbg(&pdev->dev, "failed to setup stop-mode\n"); > - } > - > return 0; > > + failed_canregister: > + unregister_flexcandev(dev); > failed_register: > pm_runtime_put_noidle(&pdev->dev); > pm_runtime_disable(&pdev->dev); > @@ -2062,6 +2158,7 @@ static int flexcan_remove(struct platform_device *pdev) > { > struct net_device *dev = platform_get_drvdata(pdev); > > + device_set_wakeup_enable(&pdev->dev, false); Please make this a seperate patch, as it fixes a bug. > unregister_flexcandev(dev); > pm_runtime_disable(&pdev->dev); > free_candev(dev); > 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 |
Hi Marc, > -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: 2020年10月19日 16:16 > To: Joakim Zhang <qiangqing.zhang@nxp.com>; robh+dt@kernel.org; > shawnguo@kernel.org; s.hauer@pengutronix.de > Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; Ying Liu > <victor.liu@nxp.com>; linux-can@vger.kernel.org; Pankaj Bansal > <pankaj.bansal@nxp.com>; netdev@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH V2 8/8] can: flexcan: add CAN wakeup function for > i.MX8QM > > On 10/19/20 5:57 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.MX8QM, stop mode request is controlled by System Controller > > Unit(SCU) firmware, this patch introduces > > FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW quirk for this function. > > > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > > --- > > drivers/net/can/flexcan.c | 131 > > +++++++++++++++++++++++++++++++++----- > > 1 file changed, 114 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > > index c0cdee904ca7..8ad5065c918f 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> > > @@ -203,6 +205,8 @@ > > > > #define FLEXCAN_TIMEOUT_US (250) > > > > +#define FLEXCAN_IMX_SC_R_CAN(x) (IMX_SC_R_CAN_0 + (x)) > > Why not move it into the appropriate svc header file? Sorry, not quite understand. Which file do you mean the appropriate svc header file? Is it include/dt-bindings/firmware/imx/rsrc.h? After glancing the header file under include/linux/firmware/imx, have not found the appropriate svc header. Could you please explain more? > > + > > /* FLEXCAN hardware feature flags > > * > > * Below is some version info we got: > > @@ -242,6 +246,8 @@ > > #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9) > > /* support memory detection and correction */ #define > > FLEXCAN_QUIRK_SUPPORT_ECC BIT(10) > > +/* Setup stop mode with SCU firmware to support wakeup */ #define > > +FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11) > > > > /* Structure of the message buffer */ struct flexcan_mb { @@ -347,6 > > +353,7 @@ struct flexcan_priv { > > u8 mb_count; > > u8 mb_size; > > u8 clk_src; /* clock source of CAN Protocol Engine */ > > + u8 can_idx; > > > > u64 rx_mask; > > u64 tx_mask; > > @@ -358,6 +365,9 @@ struct flexcan_priv { > > struct regulator *reg_xceiver; > > struct flexcan_stop_mode stm; > > > > + /* IPC handle when setup 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 +397,7 @@ > > 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_SCFW, > > }; > > > > static struct flexcan_devtype_data fsl_imx8mp_devtype_data = { @@ > > -546,18 +556,42 @@ static void flexcan_enable_wakeup_irq(struct > flexcan_priv *priv, bool enable) > > priv->write(reg_mcr, ®s->mcr); > > } > > > > +static int flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, > > +bool enabled) { > > + u8 idx = priv->can_idx; > > + u32 rsrc_id, val; > > + > > + rsrc_id = FLEXCAN_IMX_SC_R_CAN(idx); > > + > > + if (enabled) > > + val = 1; > > + else > > + val = 0; > > + > > + /* stop mode request via scu firmware */ > > + return 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; > > u32 reg_mcr; > > + int ret; > > > > reg_mcr = priv->read(®s->mcr); > > reg_mcr |= FLEXCAN_MCR_SLF_WAK; > > priv->write(reg_mcr, ®s->mcr); > > > > /* enable stop request */ > > - regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, > > - 1 << priv->stm.req_bit, 1 << priv->stm.req_bit); > > + if (priv->devtype_data->quirks & > FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) { > > + ret = flexcan_stop_mode_enable_scfw(priv, true); > > + if (ret < 0) > > + return ret; > > + } 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); > > } > > @@ -566,10 +600,17 @@ static inline int flexcan_exit_stop_mode(struct > > flexcan_priv *priv) { > > struct flexcan_regs __iomem *regs = priv->regs; > > u32 reg_mcr; > > + int ret; > > > > /* 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_SETUP_STOP_MODE_SCFW) { > > + ret = flexcan_stop_mode_enable_scfw(priv, false); > > + if (ret < 0) > > + return ret; > > + } else { > > + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, > > + 1 << priv->stm.req_bit, 0); > > + } > > > > reg_mcr = priv->read(®s->mcr); > > reg_mcr &= ~FLEXCAN_MCR_SLF_WAK; > > @@ -1838,7 +1879,7 @@ static void unregister_flexcandev(struct > net_device *dev) > > unregister_candev(dev); > > } > > > > -static int flexcan_setup_stop_mode(struct platform_device *pdev) > > +static int flexcan_setup_stop_mode_gpr(struct platform_device *pdev) > > { > > struct net_device *dev = platform_get_drvdata(pdev); > > struct device_node *np = pdev->dev.of_node; @@ -1883,11 +1924,6 @@ > > static int flexcan_setup_stop_mode(struct platform_device *pdev) > > "gpr %s req_gpr=0x02%x req_bit=%u\n", > > gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_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: > > @@ -1895,6 +1931,64 @@ 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; > > + u8 can_idx; > > + int ret; > > + > > + ret = of_property_read_u8(pdev->dev.of_node, "fsl,scu-index", &can_idx); > > + if (ret < 0) { > > + dev_dbg(&pdev->dev, "failed to get scu index\n"); > > + return ret; > > + } > > + > > + priv = netdev_priv(dev); > > + priv->can_idx = can_idx; > > + > > + /* this function could be defered probe, return -EPROBE_DEFER */ > > + ret = imx_scu_get_handle(&priv->sc_ipc_handle); > > + if (ret < 0) { > > + dev_dbg(&pdev->dev, "get ipc handle used by SCU failed\n"); > > + return ret; > > + } > > Don't print an error message on -EPROBE_DEFER. Ok, make sense, thanks. > > + > > + return 0; > > +} > > + > > +/* flexcan_setup_stop_mode - Setup stop mode for wakeup > > + * > > + * Return: = 0 setup stop mode successfully or doesn't support this feature > > + * < 0 fail to setup stop mode (could be defered probe) > > + */ > > +static int flexcan_setup_stop_mode(struct platform_device *pdev) { > > + struct net_device *dev = platform_get_drvdata(pdev); > > + struct flexcan_priv *priv; > > + int ret; > > + > > + priv = netdev_priv(dev); > > + > > + if (priv->devtype_data->quirks & > FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) > > + 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 > > + /* return 0 directly if doesn't support stop mode feature */ > > + return 0; > > + > > + if (ret) > > + return ret; > > + > > + 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; > > +} > > + > > 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, }, @@ -2040,17 +2134,19 @@ static int > flexcan_probe(struct platform_device *pdev) > > goto failed_register; > > } > > > > + err = flexcan_setup_stop_mode(pdev); > > + if (err < 0) { > > + dev_err(&pdev->dev, "setup stop mode failed\n"); > > + goto failed_canregister; > > + } > > + > > of_can_transceiver(dev); > > devm_can_led_init(dev); > > > > - if (priv->devtype_data->quirks & > FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) { > > - err = flexcan_setup_stop_mode(pdev); > > - if (err) > > - dev_dbg(&pdev->dev, "failed to setup stop-mode\n"); > > - } > > - > > return 0; > > > > + failed_canregister: > > + unregister_flexcandev(dev); > > failed_register: > > pm_runtime_put_noidle(&pdev->dev); > > pm_runtime_disable(&pdev->dev); > > @@ -2062,6 +2158,7 @@ static int flexcan_remove(struct platform_device > > *pdev) { > > struct net_device *dev = platform_get_drvdata(pdev); > > > > + device_set_wakeup_enable(&pdev->dev, false); > > Please make this a seperate patch, as it fixes a bug. Ok. Best Regards, Joakim Zh > > unregister_flexcandev(dev); > > pm_runtime_disable(&pdev->dev); > > free_candev(dev); > > > 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 |
On 10/19/20 10:39 AM, Joakim Zhang wrote: >>> +#define FLEXCAN_IMX_SC_R_CAN(x) (IMX_SC_R_CAN_0 + (x)) >> >> Why not move it into the appropriate svc header file? > > Sorry, not quite understand. Which file do you mean the appropriate svc > header file? Is it include/dt-bindings/firmware/imx/rsrc.h? yes, I meant that: > include/dt-bindings/firmware/imx/rsrc.h:111:#define IMX_SC_R_CAN_0 105 > After glancing the header file under include/linux/firmware/imx, have not > found the appropriate svc header. Could you please explain more? 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 |
Hi Marc, > -----Original Message----- > From: Marc Kleine-Budde <mkl@pengutronix.de> > Sent: 2020年10月19日 16:42 > To: Joakim Zhang <qiangqing.zhang@nxp.com>; robh+dt@kernel.org; > shawnguo@kernel.org; s.hauer@pengutronix.de > Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>; Ying Liu > <victor.liu@nxp.com>; linux-can@vger.kernel.org; Pankaj Bansal > <pankaj.bansal@nxp.com>; netdev@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH V2 8/8] can: flexcan: add CAN wakeup function for > i.MX8QM > > On 10/19/20 10:39 AM, Joakim Zhang wrote: > >>> +#define FLEXCAN_IMX_SC_R_CAN(x) (IMX_SC_R_CAN_0 + (x)) > >> > >> Why not move it into the appropriate svc header file? > > > > Sorry, not quite understand. Which file do you mean the appropriate > > svc header file? Is it include/dt-bindings/firmware/imx/rsrc.h? > > yes, I meant that: > > > include/dt-bindings/firmware/imx/rsrc.h:111:#define IMX_SC_R_CAN_0 > 105 As I can see in rsrc.h file, it just list each resource sequentially, and there is a note in the comments: "Note items from list should never be changed or removed (only added to at the end of the list)." So the driver author doesn't want any scu users to change these resource macro. If we only do below change for CAN, but keep other devices unchanged, It would be very strange. And I think this code change could not be accepted. There may be another consideration, now we only has 3 CAN instances, how can we handle if later SoCs have more CAN instances, and they still want to reuse this header file. This is also reason I prefer to use these defined macros directly in flexcan driver. --- a/include/dt-bindings/firmware/imx/rsrc.h +++ b/include/dt-bindings/firmware/imx/rsrc.h @@ -108,9 +108,7 @@ #define IMX_SC_R_ADC_1 102 #define IMX_SC_R_FTM_0 103 #define IMX_SC_R_FTM_1 104 -#define IMX_SC_R_CAN_0 105 -#define IMX_SC_R_CAN_1 106 -#define IMX_SC_R_CAN_2 107 +#define IMX_SC_R_CAN(x) (105 + (x)) #define IMX_SC_R_DMA_1_CH0 108 #define IMX_SC_R_DMA_1_CH1 109 #define IMX_SC_R_DMA_1_CH2 110 Add @Aisheng Dong, could above code changes can be accepted by you? Best Regards, Joakim Zhang
On 10/19/20 11:03 AM, Joakim Zhang wrote: >>> include/dt-bindings/firmware/imx/rsrc.h:111:#define IMX_SC_R_CAN_0 >> 105 > > As I can see in rsrc.h file, it just list each resource sequentially, and there is a note in the comments: > "Note items from list should never be changed or removed (only added to at > the end of the list)." So don't do it! Do not remove the CAN_0...2 :) Only add IMX_SC_R_CAN(x) > So the driver author doesn't want any scu users to > change these resource macro. If we only do below change for CAN, but keep > other devices unchanged, It would be very strange. And I think this code > change could not be accepted. There may be another consideration, now we only > has 3 CAN instances, how can we handle if later SoCs have more CAN instances, > and they still want to reuse this header file. This is also reason I prefer > to use these defined macros directly in flexcan driver. > > --- a/include/dt-bindings/firmware/imx/rsrc.h > +++ b/include/dt-bindings/firmware/imx/rsrc.h > @@ -108,9 +108,7 @@ > #define IMX_SC_R_ADC_1 102 > #define IMX_SC_R_FTM_0 103 > #define IMX_SC_R_FTM_1 104 > -#define IMX_SC_R_CAN_0 105 > -#define IMX_SC_R_CAN_1 106 > -#define IMX_SC_R_CAN_2 107 > +#define IMX_SC_R_CAN(x) (105 + (x)) > #define IMX_SC_R_DMA_1_CH0 108 > #define IMX_SC_R_DMA_1_CH1 109 > #define IMX_SC_R_DMA_1_CH2 110 > > Add @Aisheng Dong, could above code changes can be accepted by you? 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 |
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index c0cdee904ca7..8ad5065c918f 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> @@ -203,6 +205,8 @@ #define FLEXCAN_TIMEOUT_US (250) +#define FLEXCAN_IMX_SC_R_CAN(x) (IMX_SC_R_CAN_0 + (x)) + /* FLEXCAN hardware feature flags * * Below is some version info we got: @@ -242,6 +246,8 @@ #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9) /* support memory detection and correction */ #define FLEXCAN_QUIRK_SUPPORT_ECC BIT(10) +/* Setup stop mode with SCU firmware to support wakeup */ +#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11) /* Structure of the message buffer */ struct flexcan_mb { @@ -347,6 +353,7 @@ struct flexcan_priv { u8 mb_count; u8 mb_size; u8 clk_src; /* clock source of CAN Protocol Engine */ + u8 can_idx; u64 rx_mask; u64 tx_mask; @@ -358,6 +365,9 @@ struct flexcan_priv { struct regulator *reg_xceiver; struct flexcan_stop_mode stm; + /* IPC handle when setup 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 +397,7 @@ 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_SCFW, }; static struct flexcan_devtype_data fsl_imx8mp_devtype_data = { @@ -546,18 +556,42 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable) priv->write(reg_mcr, ®s->mcr); } +static int flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool enabled) +{ + u8 idx = priv->can_idx; + u32 rsrc_id, val; + + rsrc_id = FLEXCAN_IMX_SC_R_CAN(idx); + + if (enabled) + val = 1; + else + val = 0; + + /* stop mode request via scu firmware */ + return 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; u32 reg_mcr; + int ret; reg_mcr = priv->read(®s->mcr); reg_mcr |= FLEXCAN_MCR_SLF_WAK; priv->write(reg_mcr, ®s->mcr); /* enable stop request */ - regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, - 1 << priv->stm.req_bit, 1 << priv->stm.req_bit); + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) { + ret = flexcan_stop_mode_enable_scfw(priv, true); + if (ret < 0) + return ret; + } 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); } @@ -566,10 +600,17 @@ static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv) { struct flexcan_regs __iomem *regs = priv->regs; u32 reg_mcr; + int ret; /* 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_SETUP_STOP_MODE_SCFW) { + ret = flexcan_stop_mode_enable_scfw(priv, false); + if (ret < 0) + return ret; + } else { + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, + 1 << priv->stm.req_bit, 0); + } reg_mcr = priv->read(®s->mcr); reg_mcr &= ~FLEXCAN_MCR_SLF_WAK; @@ -1838,7 +1879,7 @@ static void unregister_flexcandev(struct net_device *dev) unregister_candev(dev); } -static int flexcan_setup_stop_mode(struct platform_device *pdev) +static int flexcan_setup_stop_mode_gpr(struct platform_device *pdev) { struct net_device *dev = platform_get_drvdata(pdev); struct device_node *np = pdev->dev.of_node; @@ -1883,11 +1924,6 @@ static int flexcan_setup_stop_mode(struct platform_device *pdev) "gpr %s req_gpr=0x02%x req_bit=%u\n", gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_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: @@ -1895,6 +1931,64 @@ 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; + u8 can_idx; + int ret; + + ret = of_property_read_u8(pdev->dev.of_node, "fsl,scu-index", &can_idx); + if (ret < 0) { + dev_dbg(&pdev->dev, "failed to get scu index\n"); + return ret; + } + + priv = netdev_priv(dev); + priv->can_idx = can_idx; + + /* this function could be defered probe, return -EPROBE_DEFER */ + ret = imx_scu_get_handle(&priv->sc_ipc_handle); + if (ret < 0) { + dev_dbg(&pdev->dev, "get ipc handle used by SCU failed\n"); + return ret; + } + + return 0; +} + +/* flexcan_setup_stop_mode - Setup stop mode for wakeup + * + * Return: = 0 setup stop mode successfully or doesn't support this feature + * < 0 fail to setup stop mode (could be defered probe) + */ +static int flexcan_setup_stop_mode(struct platform_device *pdev) +{ + struct net_device *dev = platform_get_drvdata(pdev); + struct flexcan_priv *priv; + int ret; + + priv = netdev_priv(dev); + + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) + 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 + /* return 0 directly if doesn't support stop mode feature */ + return 0; + + if (ret) + return ret; + + 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; +} + 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, }, @@ -2040,17 +2134,19 @@ static int flexcan_probe(struct platform_device *pdev) goto failed_register; } + err = flexcan_setup_stop_mode(pdev); + if (err < 0) { + dev_err(&pdev->dev, "setup stop mode failed\n"); + goto failed_canregister; + } + of_can_transceiver(dev); devm_can_led_init(dev); - if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) { - err = flexcan_setup_stop_mode(pdev); - if (err) - dev_dbg(&pdev->dev, "failed to setup stop-mode\n"); - } - return 0; + failed_canregister: + unregister_flexcandev(dev); failed_register: pm_runtime_put_noidle(&pdev->dev); pm_runtime_disable(&pdev->dev); @@ -2062,6 +2158,7 @@ static int flexcan_remove(struct platform_device *pdev) { struct net_device *dev = platform_get_drvdata(pdev); + device_set_wakeup_enable(&pdev->dev, false); unregister_flexcandev(dev); pm_runtime_disable(&pdev->dev); free_candev(dev);
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.MX8QM, stop mode request is controlled by System Controller Unit(SCU) firmware, this patch introduces FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW quirk for this function. Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> --- drivers/net/can/flexcan.c | 131 +++++++++++++++++++++++++++++++++----- 1 file changed, 114 insertions(+), 17 deletions(-)