On 11/21/18 4:34 AM, Aisheng DONG wrote: >> -----Original Message----- >> From: Joakim Zhang > [...] >> >> If wakeup is enabled, enter stop mode, else enter disabled mode.Self wake can >> only work on stop mode. >> >> Starting from IMX6, the flexcan stop mode control bits is SoC specific, move it >> out of IP driver and parse it from devicetree. >> >> Signed-off-by: Dong Aisheng <aisheng.dong@xxxxxxx> >> Signed-off-by: Joakim Zhang <qiangqing.zhang@xxxxxxx> >> --- >> drivers/net/can/flexcan.c | 175 >> +++++++++++++++++++++++++++++++++++--- >> 1 file changed, 163 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index >> 3813f6708201..d21d0db8c83e 100644 >> --- a/drivers/net/can/flexcan.c >> +++ b/drivers/net/can/flexcan.c >> @@ -19,12 +19,17 @@ >> #include <linux/delay.h> >> #include <linux/interrupt.h> >> #include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/list.h> > > This two lines seems not neccesary > >> +#include <linux/mfd/syscon.h> >> +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h> >> #include <linux/module.h> >> #include <linux/of.h> >> #include <linux/of_device.h> >> #include <linux/platform_device.h> >> #include <linux/pm_runtime.h> >> #include <linux/regulator/consumer.h> >> +#include <linux/regmap.h> >> >> #define DRV_NAME "flexcan" >> >> @@ -132,7 +137,8 @@ >> (FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE) #define >> FLEXCAN_ESR_ALL_INT \ >> (FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \ >> - FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT) >> + FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \ >> + FLEXCAN_ESR_WAK_INT) >> >> /* FLEXCAN interrupt flag register (IFLAG) bits */ >> /* Errata ERR005829 step7: Reserve first valid MB */ @@ -191,6 +197,7 @@ >> #define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP BIT(5) /* Use timestamp >> based offloading */ >> #define FLEXCAN_QUIRK_BROKEN_PERR_STATE BIT(6) /* No interrupt for >> error passive */ >> #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN BIT(7) /* default to BE >> register access */ >> +#define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8) /* Setup stop mode to >> support wakeup */ >> >> /* Structure of the message buffer */ >> struct flexcan_mb { >> @@ -255,6 +262,14 @@ struct flexcan_devtype_data { >> u32 quirks; /* quirks needed for different IP cores */ >> }; >> >> +struct flexcan_stop_mode { >> + struct regmap *gpr; >> + u8 req_gpr; >> + u8 req_bit; >> + u8 ack_gpr; >> + u8 ack_bit; >> +}; >> + >> struct flexcan_priv { >> struct can_priv can; >> struct can_rx_offload offload; >> @@ -272,6 +287,7 @@ struct flexcan_priv { >> struct clk *clk_per; >> const struct flexcan_devtype_data *devtype_data; >> struct regulator *reg_xceiver; >> + struct flexcan_stop_mode stm; >> >> /* Read and Write APIs */ >> u32 (*read)(void __iomem *addr); >> @@ -295,7 +311,8 @@ static const struct flexcan_devtype_data >> fsl_imx28_devtype_data = { >> >> static const struct flexcan_devtype_data fsl_imx6q_devtype_data = { >> .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | >> FLEXCAN_QUIRK_ENABLE_EACEN_RRS | >> - FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | >> FLEXCAN_QUIRK_BROKEN_PERR_STATE, >> + FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | >> FLEXCAN_QUIRK_BROKEN_PERR_STATE | >> + FLEXCAN_QUIRK_SETUP_STOP_MODE, >> }; >> >> static const struct flexcan_devtype_data fsl_vf610_devtype_data = { @@ >> -392,6 +409,40 @@ static void flexcan_clks_disable(const struct flexcan_priv >> *priv) >> clk_disable_unprepare(priv->clk_per); >> } >> >> +static void flexcan_wake_mask_enable(struct flexcan_priv *priv) { >> + struct flexcan_regs __iomem *regs = priv->regs; >> + u32 reg_mcr; >> + >> + reg_mcr = priv->read(®s->mcr); >> + reg_mcr |= FLEXCAN_MCR_WAK_MSK; >> + priv->write(reg_mcr, ®s->mcr); >> +} >> + >> +static void flexcan_wake_mask_disable(struct flexcan_priv *priv) { >> + struct flexcan_regs __iomem *regs = priv->regs; >> + u32 reg_mcr; >> + >> + reg_mcr = priv->read(®s->mcr); >> + reg_mcr &= ~FLEXCAN_MCR_WAK_MSK; >> + priv->write(reg_mcr, ®s->mcr); >> +} >> + > > Please combine those two functions into one, also improve the name a bit. > e.g. > flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable) > >> +static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv) { >> + /* enable stop request */ >> + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr, >> + 1 << priv->stm.req_bit, 1 << priv->stm.req_bit); } >> + >> +static inline void flexcan_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 int flexcan_transceiver_enable(const struct flexcan_priv *priv) { >> if (!priv->reg_xceiver) >> @@ -955,6 +1006,10 @@ static int flexcan_chip_start(struct net_device *dev) >> reg_mcr |= FLEXCAN_MCR_FEN | >> FLEXCAN_MCR_MAXMB(priv->tx_mb_idx); >> } >> + >> + /* enable self wakeup */ >> + reg_mcr |= FLEXCAN_MCR_SLF_WAK; >> + > > Should we check before setting as we already have a SETUP_STOP_MODE quirk? > >> netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr); >> priv->write(reg_mcr, ®s->mcr); >> >> @@ -1240,6 +1295,60 @@ static void unregister_flexcandev(struct net_device >> *dev) >> unregister_candev(dev); >> } >> >> +static int flexcan_setup_stop_mode(struct platform_device *pdev) { >> + struct net_device *dev = platform_get_drvdata(pdev); >> + struct device_node *np = pdev->dev.of_node; >> + struct device_node *node; > > Probably gpr_np is better. > >> + struct flexcan_priv *priv; >> + bool wakeup = true; >> + phandle phandle; >> + u32 out_val[5]; >> + int ret; >> + >> + if (!np) >> + return -EINVAL; >> + >> + /* stop mode property format is: >> + * <&gpr req_gpr req_bit ack_gpr ack_bit>. >> + */ >> + ret = of_property_read_u32_array(np, "fsl,stop-mode", out_val, 5); >> + if (ret) { >> + dev_dbg(&pdev->dev, "no stop-mode property\n"); >> + return ret; >> + } >> + phandle = *out_val; >> + >> + node = of_find_node_by_phandle(phandle); >> + if (!node) { >> + dev_dbg(&pdev->dev, "could not find gpr node by phandle\n"); >> + return PTR_ERR(node); >> + } >> + >> + priv = netdev_priv(dev); >> + priv->stm.gpr = syscon_node_to_regmap(node); >> + if (IS_ERR(priv->stm.gpr)) { >> + dev_dbg(&pdev->dev, "could not find gpr regmap\n"); >> + return PTR_ERR(priv->stm.gpr); >> + } >> + >> + of_node_put(node); >> + >> + priv->stm.req_gpr = out_val[1]; >> + priv->stm.req_bit = out_val[2]; >> + priv->stm.ack_gpr = out_val[3]; >> + priv->stm.ack_bit = out_val[4]; >> + >> + dev_dbg(&pdev->dev, "gpr %s req_gpr 0x%x req_bit %u ack_gpr 0x%x >> ack_bit %u\n", >> + node->full_name, priv->stm.req_gpr, >> + priv->stm.req_bit, priv->stm.ack_gpr, >> + priv->stm.ack_bit); >> + >> + device_set_wakeup_capable(&pdev->dev, wakeup); > > If 'wakeup' variable is not changeable, then not need use it. > >> + >> + return 0; >> +} >> + >> static const struct of_device_id flexcan_of_match[] = { >> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, }, >> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, }, >> @@ -1400,6 +1509,12 @@ 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"); >> + } >> + >> pm_runtime_put(&pdev->dev); >> >> dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n", @@ >> -1437,10 +1552,18 @@ static int __maybe_unused flexcan_suspend(struct >> device *device) >> int err = 0; >> >> if (netif_running(dev)) { >> - err = flexcan_chip_disable(priv); >> - if (err) >> - return err; >> - err = pm_runtime_force_suspend(device); >> + /* if wakeup is enabled, enter stop mode >> + * else enter disabled mode. >> + */ >> + if (device_may_wakeup(device)) { >> + enable_irq_wake(dev->irq); >> + flexcan_enter_stop_mode(priv); >> + } else { >> + err = flexcan_chip_disable(priv); >> + if (err) >> + return err; >> + err = pm_runtime_force_suspend(device); > > This is mixed with runtime pm. > I would suggestion you first upstream wakeup support. > Then runtime pm later separately. In fact this patch is based on the patch "can: flexcan: Implement CAN Runtime PM", which is still under review, too. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Attachment:
signature.asc
Description: OpenPGP digital signature