can: flexcan: add platform data for ColdFire

Add flexcan support.

---
Changes for v2:
- add irq resources for ERR and BOFF interrutps

Signed-off-by Angelo Dureghello <[email protected]>

---
  arch/m68k/coldfire/device.c       | 42 +++++++++++++++++++++++++++++++
  arch/m68k/coldfire/m5441x.c       |  8 +++---
  arch/m68k/include/asm/m5441xsim.h | 19 ++++++++++++++
  3 files changed, 65 insertions(+), 4 deletions(-)

arch/m68k/coldfire/device.c +42 -0

index 59f7dfe50a4d..1b2eaee6d6f6 100644
@@ -23,6 +23,7 @@
  #include <linux/platform_data/edma.h>
  #include <linux/platform_data/dma-mcf-edma.h>
  #include <linux/platform_data/mmc-esdhc-mcf.h>
+#include <linux/can/platform/mcf5441x.h>
 
  /*
   *	All current ColdFire parts contain from 2, 3, 4 or 10 UARTS.

@@ -581,6 +582,44 @@ static struct platform_device mcf_esdhc = {
  };
  #endif /* MCFSDHC_BASE */
 
+#if IS_ENABLED(CONFIG_CAN)
+static struct mcf_flexcan_platform_data mcf_flexcan_info = {
+	.clk_src = 1,
+	.clock_frequency = 120000000,
+};
+
+static struct resource mcf_flexcan0_resource[] = {
+	{
+		.start = MCFFLEXCAN_BASE0,
+		.end = MCFFLEXCAN_BASE0 + MCFFLEXCAN_SIZE,
+		.flags = IORESOURCE_MEM,
+	},
+	{
+		.start = MCF_IRQ_IFL0,
+		.end = MCF_IRQ_IFL0,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = MCF_IRQ_BOFF0,
+		.end = MCF_IRQ_BOFF0,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = MCF_IRQ_ERR0,
+		.end = MCF_IRQ_ERR0,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device mcf_flexcan0 = {
+	.name = "flexcan-mcf",
+	.id = 0,
+	.num_resources = ARRAY_SIZE(mcf_flexcan0_resource),
+	.resource = mcf_flexcan0_resource,
+	.dev.platform_data = &mcf_flexcan_info,
+};
+#endif /* IS_ENABLED(CONFIG_CAN) */
+
  static struct platform_device *mcf_devices[] __initdata = {
  	&mcf_uart,
  #if IS_ENABLED(CONFIG_FEC)

@@ -616,6 +655,9 @@ static struct platform_device *mcf_devices[] __initdata = {
  #ifdef MCFSDHC_BASE
  	&mcf_esdhc,
  #endif
+#if IS_ENABLED(CONFIG_CAN)
+	&mcf_flexcan0,
+#endif
  };
 
  /*

arch/m68k/coldfire/m5441x.c +4 -4

index 1e5259a652d1..18b152edb69c 100644
@@ -18,8 +18,8 @@
  #include <asm/mcfclk.h>
 
  DEFINE_CLK(0, "flexbus", 2, MCF_CLK);
-DEFINE_CLK(0, "mcfcan.0", 8, MCF_CLK);
-DEFINE_CLK(0, "mcfcan.1", 9, MCF_CLK);
+DEFINE_CLK(0, "flexcan.0", 8, MCF_CLK);
+DEFINE_CLK(0, "flexcan.1", 9, MCF_CLK);
  DEFINE_CLK(0, "imx1-i2c.1", 14, MCF_CLK);
  DEFINE_CLK(0, "mcfdspi.1", 15, MCF_CLK);
  DEFINE_CLK(0, "edma", 17, MCF_CLK);

@@ -146,6 +146,8 @@ struct clk *mcf_clks[] = {
 
  static struct clk * const enable_clks[] __initconst = {
  	/* make sure these clocks are enabled */
+	&__clk_0_8, /* flexcan.0 */
+	&__clk_0_9, /* flexcan.1 */
  	&__clk_0_15, /* dspi.1 */
  	&__clk_0_17, /* eDMA */
  	&__clk_0_18, /* intc0 */

@@ -166,8 +168,6 @@ static struct clk * const enable_clks[] __initconst = {
  	&__clk_1_37, /* gpio */
  };
  static struct clk * const disable_clks[] __initconst = {
-	&__clk_0_8, /* can.0 */
-	&__clk_0_9, /* can.1 */
  	&__clk_0_14, /* i2c.1 */
  	&__clk_0_22, /* i2c.0 */
  	&__clk_0_23, /* dspi.0 */

arch/m68k/include/asm/m5441xsim.h +19 -0

index e091e36d3464..f48cf63bd782 100644
@@ -73,6 +73,12 @@
  #define MCFINT0_FECENTC1	55
 
  /* on interrupt controller 1 */
+#define MCFINT1_FLEXCAN0_IFL	0
+#define MCFINT1_FLEXCAN0_BOFF	1
+#define MCFINT1_FLEXCAN0_ERR	3
+#define MCFINT1_FLEXCAN1_IFL	4
+#define MCFINT1_FLEXCAN1_BOFF	5
+#define MCFINT1_FLEXCAN1_ERR	7
  #define MCFINT1_UART4		48
  #define MCFINT1_UART5		49
  #define MCFINT1_UART6		50

@@ -314,4 +320,17 @@
  #define MCF_IRQ_SDHC		(MCFINT2_VECBASE + MCFINT2_SDHC)
  #define MCFSDHC_CLK		(MCFSDHC_BASE + 0x2c)
 
+/*
+ * Flexcan module
+ */
+#define MCFFLEXCAN_BASE0	0xfc020000
+#define MCFFLEXCAN_BASE1	0xfc024000
+#define MCFFLEXCAN_SIZE		0x4000
+#define MCF_IRQ_IFL0		(MCFINT1_VECBASE + MCFINT1_FLEXCAN0_IFL)
+#define MCF_IRQ_BOFF0		(MCFINT1_VECBASE + MCFINT1_FLEXCAN0_BOFF)
+#define MCF_IRQ_ERR0		(MCFINT1_VECBASE + MCFINT1_FLEXCAN0_ERR)
+#define MCF_IRQ_IFL1		(MCFINT1_VECBASE + MCFINT1_FLEXCAN1_IFL)
+#define MCF_IRQ_BOFF1		(MCFINT1_VECBASE + MCFINT1_FLEXCAN1_BOFF)
+#define MCF_IRQ_ERR1		(MCFINT1_VECBASE + MCFINT1_FLEXCAN1_ERR)
+
  #endif /* m5441xsim_h */

Hi Angelo,

Quoted 6 lines · Angelo Dureghello

Add flexcan support.

---
Changes for v2:
- add irq resources for ERR and BOFF interrutps

Quoted 1 line

Signed-off-by Angelo Dureghello <[email protected]>

You should put your signed-off-by above the first "---". Tools like "git am" drop everything below the first "---" and before the patch proper. So it ends up losing your signed-off-by.

Regards
Greg

Enable flexcan KConfig for ColdFire m5441x cpu's.

Signed-off-by Angelo Dureghello <[email protected]>

---
 drivers/net/can/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/can/Kconfig +0 -0

index e355d3974977..7a1be9dad52d 100644

drivers/net/can/Kconfig +0 -0

 	help
 	  Say Y here if you want to support for Freescale FlexCAN.
 

Signature · 1 line

2.31.1

Add flexcan support for NXP ColdFire mcf5441x family.

This flexcan module is quite similar to imx6 flexcan module, but with some exceptions:

- 3 separate interrupt sources, MB, BOFF and ERR,
- implements 16 mb only,
- m68k architecture is not supporting devicetrees, so a platform data check/case has been added,
- ColdFire is m68k, so big-endian cpu, with a little-endian flexcan module.

---
Changes for v2:
- re-add platform data handling restarting from 2c0ac9208135
- re-add flexcan_id_table, as from 2c0ac9208135
- usinig irq resources for ERR and BOFF interrupts
- add missing free_irq() for interrupts
- minor syntax fixes

Signed-off-by Angelo Dureghello <[email protected]>

---
 drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 11 deletions(-)

drivers/net/can/flexcan.c +12 -0

index 57f3635ad8d7..2188dc36a010 100644
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/can/platform/mcf5441x.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>

@@ -246,6 +247,8 @@
 #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)
+/* Setup for flexcan module as in mcf, 16 mb, 3 separate interrupts  */
+#define FLEXCAN_QUIRK_SETUP_MCF BIT(12)
 
 /* Structure of the message buffer */
 struct flexcan_mb {

@@ -363,6 +366,9 @@ struct flexcan_priv {
 	struct regulator *reg_xceiver;
 	struct flexcan_stop_mode stm;
 
+	int irq_boff;
+	int irq_err;
+
 	/* IPC handle when setup stop mode by System Controller firmware(scfw) */
 	struct imx_sc_ipc *sc_ipc_handle;
 

@@ -371,6 +377,12 @@ struct flexcan_priv {
 	void (*write)(u32 val, void __iomem *addr);
 };
 
+static const struct flexcan_devtype_data fsl_mcf_devtype_data = {
+	.quirks = FLEXCAN_QUIRK_BROKEN_WERR_STATE |
+		FLEXCAN_QUIRK_BROKEN_PERR_STATE |
+		FLEXCAN_QUIRK_SETUP_MCF,
+};
+
 static const struct flexcan_devtype_data fsl_p1010_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_BROKEN_WERR_STATE |
 		FLEXCAN_QUIRK_BROKEN_PERR_STATE |

drivers/net/can/flexcan.c +14 -5

+		if (err)
+			return err;
+	}
 
-	err = clk_prepare_enable(priv->clk_per);
-	if (err)
-		clk_disable_unprepare(priv->clk_ipg);
+	if (priv->clk_per) {
+		err = clk_prepare_enable(priv->clk_per);
+		if (err)
+			clk_disable_unprepare(priv->clk_ipg);
+	}
 
 	return err;
 }

@@ -1401,8 +1417,12 @@ static int flexcan_rx_offload_setup(struct net_device *dev)
 		priv->mb_size = sizeof(struct flexcan_mb) + CANFD_MAX_DLEN;
 	else
 		priv->mb_size = sizeof(struct flexcan_mb) + CAN_MAX_DLEN;
-	priv->mb_count = (sizeof(priv->regs->mb[0]) / priv->mb_size) +
-			 (sizeof(priv->regs->mb[1]) / priv->mb_size);
+
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF)
+		priv->mb_count = FLEXCAN_MCF5411X_MB_CNT_MCF;
+	else
+		priv->mb_count = (sizeof(priv->regs->mb[0]) / priv->mb_size) +
+				 (sizeof(priv->regs->mb[1]) / priv->mb_size);
 
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
 		priv->tx_mb_reserved =

drivers/net/can/flexcan.c +2 -0

+	}
+
 	flexcan_chip_interrupts_enable(dev);
 
 	can_led_event(dev, CAN_LED_EVENT_OPEN);

drivers/net/can/flexcan.c +13 -0

+
 	can_rx_offload_disable(&priv->offload);
 	flexcan_chip_stop_disable_on_error(dev);
 

@@ -2039,14 +2077,26 @@ static const struct of_device_id flexcan_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, flexcan_of_match);
 
+static const struct platform_device_id flexcan_id_table[] = {
+	{
+		.name = "flexcan-mcf",
+		.driver_data = (kernel_ulong_t)&fsl_mcf_devtype_data,
+	}, {
+		/* sentinel */
+	},
+};
+MODULE_DEVICE_TABLE(platform, flexcan_id_table);
+
 static int flexcan_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id;
 	const struct flexcan_devtype_data *devtype_data;
 	struct net_device *dev;
 	struct flexcan_priv *priv;
 	struct regulator *reg_xceiver;
 	struct clk *clk_ipg = NULL, *clk_per = NULL;
 	struct flexcan_regs __iomem *regs;
+	struct mcf_flexcan_platform_data *pdata;
 	int err, irq;
 	u8 clk_src = 1;
 	u32 clock_freq = 0;

drivers/net/can/flexcan.c +14 -1

+	} else {
+		pdata = dev_get_platdata(&pdev->dev);
+		if (pdata) {
+			clock_freq = pdata->clock_frequency;
+			clk_src = pdata->clk_src;
+		}
 	}
 
 	if (!clock_freq) {

@@ -2089,7 +2145,14 @@ static int flexcan_probe(struct platform_device *pdev)
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
-	devtype_data = of_device_get_match_data(&pdev->dev);
+	of_id = of_match_device(flexcan_of_match, &pdev->dev);
+	if (of_id)
+		devtype_data = of_id->data;
+	else if (platform_get_device_id(pdev)->driver_data)
+		devtype_data = (struct flexcan_devtype_data *)
+			platform_get_device_id(pdev)->driver_data;
+	else
+		return -ENODEV;
 
 	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
 	    !(devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)) {

drivers/net/can/flexcan.c +3 -0

+	}
+
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) {
 		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD |
 			CAN_CTRLMODE_FD_NON_ISO;

@@ -2322,6 +2394,7 @@ static struct platform_driver flexcan_driver = {
 	},
 	.probe = flexcan_probe,
 	.remove = flexcan_remove,
+	.id_table = flexcan_id_table,
 };
 
 module_platform_driver(flexcan_driver);

Signature · 1 line

2.31.1

regards,
Marc

Quoted 11 lines · Angelo Dureghello

Add flexcan support for NXP ColdFire mcf5441x family.

This flexcan module is quite similar to imx6 flexcan module, but with some exceptions:

- 3 separate interrupt sources, MB, BOFF and ERR,
- implements 16 mb only,
- m68k architecture is not supporting devicetrees, so a platform data check/case has been added,
- ColdFire is m68k, so big-endian cpu, with a little-endian flexcan module.

Please add the mcg5441x to the "FLEXCAN hardware feature flags" table in the driver, and add a new column for the number of mailboxes.

Quoted 61 lines

---
Changes for v2:
- re-add platform data handling restarting from 2c0ac9208135
- re-add flexcan_id_table, as from 2c0ac9208135
- usinig irq resources for ERR and BOFF interrupts
- add missing free_irq() for interrupts
- minor syntax fixes

Signed-off-by Angelo Dureghello <[email protected]>

---
  drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
  1 file changed, 84 insertions(+), 11 deletions(-)

drivers/net/can/flexcan.c +12 -0

index 57f3635ad8d7..2188dc36a010 100644
@@ -28,6 +28,7 @@
  #include <linux/of_device.h>
  #include <linux/pinctrl/consumer.h>
  #include <linux/platform_device.h>
+#include <linux/can/platform/mcf5441x.h>
  #include <linux/pm_runtime.h>
  #include <linux/regmap.h>
  #include <linux/regulator/consumer.h>

@@ -246,6 +247,8 @@
  #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)
+/* Setup for flexcan module as in mcf, 16 mb, 3 separate interrupts  */
+#define FLEXCAN_QUIRK_SETUP_MCF BIT(12)
 
  /* Structure of the message buffer */
  struct flexcan_mb {

@@ -363,6 +366,9 @@ struct flexcan_priv {
  	struct regulator *reg_xceiver;
  	struct flexcan_stop_mode stm;
 
+	int irq_boff;
+	int irq_err;
+
  	/* IPC handle when setup stop mode by System Controller firmware(scfw) */
  	struct imx_sc_ipc *sc_ipc_handle;
 

@@ -371,6 +377,12 @@ struct flexcan_priv {
  	void (*write)(u32 val, void __iomem *addr);
  };
 
+static const struct flexcan_devtype_data fsl_mcf_devtype_data = {
+	.quirks = FLEXCAN_QUIRK_BROKEN_WERR_STATE |
+		FLEXCAN_QUIRK_BROKEN_PERR_STATE |
+		FLEXCAN_QUIRK_SETUP_MCF,
+};
+
  static const struct flexcan_devtype_data fsl_p1010_devtype_data = {
  	.quirks = FLEXCAN_QUIRK_BROKEN_WERR_STATE |
  		FLEXCAN_QUIRK_BROKEN_PERR_STATE |

@@ -637,13 +649,17 @@ static int flexcan_clks_enable(const struct flexcan_priv *priv)
  {
  	int err;
 

drivers/net/can/flexcan.c static int flexcan_clks_enable(const struct flexcan_priv *priv) · line 653

Quoted 5 lines

drivers/net/can/flexcan.c +2 -3

-	err = clk_prepare_enable(priv->clk_ipg);
-	if (err)
-		return err;
+	if (priv->clk_ipg) {
+		err = clk_prepare_enable(priv->clk_ipg);

Does the coldfire clk_disable_unprepare() work with NULL pointers? I'm asking for flexcan_clks_disable()...

Quoted 34 lines

drivers/net/can/flexcan.c +14 -5

+		if (err)
+			return err;
+	}
 
-	err = clk_prepare_enable(priv->clk_per);
-	if (err)
-		clk_disable_unprepare(priv->clk_ipg);
+	if (priv->clk_per) {
+		err = clk_prepare_enable(priv->clk_per);
+		if (err)
+			clk_disable_unprepare(priv->clk_ipg);
+	}
 
  	return err;
  }

@@ -1401,8 +1417,12 @@ static int flexcan_rx_offload_setup(struct net_device *dev)
  		priv->mb_size = sizeof(struct flexcan_mb) + CANFD_MAX_DLEN;
  	else
  		priv->mb_size = sizeof(struct flexcan_mb) + CAN_MAX_DLEN;
-	priv->mb_count = (sizeof(priv->regs->mb[0]) / priv->mb_size) +
-			 (sizeof(priv->regs->mb[1]) / priv->mb_size);
+
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF)
+		priv->mb_count = FLEXCAN_MCF5411X_MB_CNT_MCF;
+	else
+		priv->mb_count = (sizeof(priv->regs->mb[0]) / priv->mb_size) +
+				 (sizeof(priv->regs->mb[1]) / priv->mb_size);
 
  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
  		priv->tx_mb_reserved =

@@ -1774,6 +1794,18 @@ static int flexcan_open(struct net_device *dev)
  	if (err)
  		goto out_can_rx_offload_disable;
 

drivers/net/can/flexcan.c static int flexcan_open(struct net_device *dev) · line 1801

Quoted 5 lines

drivers/net/can/flexcan.c +5 -0

+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF) {
+		err = request_irq(priv->irq_boff,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;

Please add proper error handling.

drivers/net/can/flexcan.c static int flexcan_open(struct net_device *dev) · line 1806

Quoted 6 lines

drivers/net/can/flexcan.c +5 -0

+
+		err = request_irq(priv->irq_err,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;

Please add proper error handling.

Quoted 5 lines

drivers/net/can/flexcan.c +2 -0

+	}
+
  	flexcan_chip_interrupts_enable(dev);
 

drivers/net/can/flexcan.c static int flexcan_close(struct net_device *dev) · line 1843

Quoted 10 lines

drivers/net/can/flexcan.c +5 -0

  	can_led_event(dev, CAN_LED_EVENT_OPEN);

@@ -1804,6 +1836,12 @@ static int flexcan_close(struct net_device *dev)
  	netif_stop_queue(dev);
  	flexcan_chip_interrupts_disable(dev);
  	free_irq(dev->irq, dev);
+
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF) {
+		free_irq(priv->irq_boff, dev);
+		free_irq(priv->irq_err, dev);
+	}

please free in the opposite order of requesting

Quoted 33 lines

drivers/net/can/flexcan.c +13 -0

+
  	can_rx_offload_disable(&priv->offload);
  	flexcan_chip_stop_disable_on_error(dev);
 

@@ -2039,14 +2077,26 @@ static const struct of_device_id flexcan_of_match[] = {
  };
  MODULE_DEVICE_TABLE(of, flexcan_of_match);
 
+static const struct platform_device_id flexcan_id_table[] = {
+	{
+		.name = "flexcan-mcf",
+		.driver_data = (kernel_ulong_t)&fsl_mcf_devtype_data,
+	}, {
+		/* sentinel */
+	},
+};
+MODULE_DEVICE_TABLE(platform, flexcan_id_table);
+
  static int flexcan_probe(struct platform_device *pdev)
  {
+	const struct of_device_id *of_id;
  	const struct flexcan_devtype_data *devtype_data;
  	struct net_device *dev;
  	struct flexcan_priv *priv;
  	struct regulator *reg_xceiver;
  	struct clk *clk_ipg = NULL, *clk_per = NULL;
  	struct flexcan_regs __iomem *regs;
+	struct mcf_flexcan_platform_data *pdata;
  	int err, irq;
  	u8 clk_src = 1;
  	u32 clock_freq = 0;

@@ -2061,9 +2111,15 @@ static int flexcan_probe(struct platform_device *pdev)
 

drivers/net/can/flexcan.c static int flexcan_probe(struct platform_device *pdev) · line 2114

Quoted 4 lines

drivers/net/can/flexcan.c +1 -1

  	if (pdev->dev.of_node) {
  		of_property_read_u32(pdev->dev.of_node,
-				     "clock-frequency", &clock_freq);
+				"clock-frequency", &clock_freq);

unrelated change

drivers/net/can/flexcan.c static int flexcan_probe(struct platform_device *pdev) · line 2116

Quoted 4 lines

drivers/net/can/flexcan.c +1 -1

  		of_property_read_u8(pdev->dev.of_node,
-				    "fsl,clk-source", &clk_src);
+				"fsl,clk-source", &clk_src);

unrelated change

Quoted 30 lines

drivers/net/can/flexcan.c +14 -1

+	} else {
+		pdata = dev_get_platdata(&pdev->dev);
+		if (pdata) {
+			clock_freq = pdata->clock_frequency;
+			clk_src = pdata->clk_src;
+		}
  	}
 
  	if (!clock_freq) {

@@ -2089,7 +2145,14 @@ static int flexcan_probe(struct platform_device *pdev)
  	if (IS_ERR(regs))
  		return PTR_ERR(regs);
 
-	devtype_data = of_device_get_match_data(&pdev->dev);
+	of_id = of_match_device(flexcan_of_match, &pdev->dev);
+	if (of_id)
+		devtype_data = of_id->data;
+	else if (platform_get_device_id(pdev)->driver_data)
+		devtype_data = (struct flexcan_devtype_data *)
+			platform_get_device_id(pdev)->driver_data;
+	else
+		return -ENODEV;
 
  	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
  	    !(devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)) {

@@ -2133,6 +2196,15 @@ static int flexcan_probe(struct platform_device *pdev)
  	priv->devtype_data = devtype_data;
  	priv->reg_xceiver = reg_xceiver;
 

drivers/net/can/flexcan.c static int flexcan_probe(struct platform_device *pdev) · line 2202

Quoted 4 lines

drivers/net/can/flexcan.c +4 -0

+	if (devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF) {
+		priv->irq_boff = platform_get_irq(pdev, 1);
+		if (priv->irq_boff <= 0)
+			return -ENODEV;

Please add proper error handling.

drivers/net/can/flexcan.c static int flexcan_probe(struct platform_device *pdev) · line 2205

Quoted 4 lines

drivers/net/can/flexcan.c +3 -0

+		priv->irq_err = platform_get_irq(pdev, 2);
+		if (priv->irq_err <= 0)
+			return -ENODEV;

Please add proper error handling.

Quoted 13 lines

drivers/net/can/flexcan.c +3 -0

+	}
+
  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) {
  		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD |
  			CAN_CTRLMODE_FD_NON_ISO;

@@ -2322,6 +2394,7 @@ static struct platform_driver flexcan_driver = {
  	},
  	.probe = flexcan_probe,
  	.remove = flexcan_remove,
+	.id_table = flexcan_id_table,
  };
 

Quoted 5 lines

drivers/net/can/flexcan.c +0 -0

  module_platform_driver(flexcan_driver);

Signature · 1 line

2.31.1

regards,
Marc

Quoted 13 lines · Marc Kleine-Budde

Quoted 11 lines · Angelo Dureghello

Add flexcan support for NXP ColdFire mcf5441x family.

This flexcan module is quite similar to imx6 flexcan module, but with some exceptions:

- 3 separate interrupt sources, MB, BOFF and ERR,
- implements 16 mb only,
- m68k architecture is not supporting devicetrees, so a platform data check/case has been added,
- ColdFire is m68k, so big-endian cpu, with a little-endian flexcan module.

Quoted 2 lines

Please add the mcg5441x to the "FLEXCAN hardware feature flags" table in the driver, and add a new column for the number of mailboxes.

need a little help here: where can i find IP version and FlexCAN version ? I am 90% sure mcf5441x is FlexCAN2 (no FD). No trace of IP version in the datasheets.

Add flexcan support for NXP ColdFire mcf5441x family.

This flexcan module is quite similar to imx6 flexcan module, but with some exceptions:

- 3 separate interrupt sources, MB, BOFF and ERR,
- implements 16 mb only,
- m68k architecture is not supporting devicetrees, so a platform data check/case has been added,
- ColdFire is m68k, so big-endian cpu, with a little-endian flexcan module.

---
Changes for v2:
- re-add platform data handling restarting from 2c0ac9208135
- re-add flexcan_id_table, as from 2c0ac9208135
- usinig irq resources for ERR and BOFF interrupts
- add missing free_irq() for interrupts
- minor syntax fixes

Signed-off-by Angelo Dureghello <[email protected]>

---
 drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 11 deletions(-)

drivers/net/can/flexcan.c +1 -0

index 57f3635ad8d7..2188dc36a010 100644
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/can/platform/mcf5441x.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>

Quoted 37 lines · Angelo Dureghello

Add flexcan support for NXP ColdFire mcf5441x family.

This flexcan module is quite similar to imx6 flexcan module, but with some exceptions:

- 3 separate interrupt sources, MB, BOFF and ERR,
- implements 16 mb only,
- m68k architecture is not supporting devicetrees, so a platform data check/case has been added,
- ColdFire is m68k, so big-endian cpu, with a little-endian flexcan module.

---
Changes for v2:
- re-add platform data handling restarting from 2c0ac9208135
- re-add flexcan_id_table, as from 2c0ac9208135
- usinig irq resources for ERR and BOFF interrupts
- add missing free_irq() for interrupts
- minor syntax fixes

Signed-off-by Angelo Dureghello <[email protected]>

---
  drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
  1 file changed, 84 insertions(+), 11 deletions(-)

drivers/net/can/flexcan.c +1 -0

index 57f3635ad8d7..2188dc36a010 100644
@@ -28,6 +28,7 @@
  #include <linux/of_device.h>
  #include <linux/pinctrl/consumer.h>
  #include <linux/platform_device.h>
+#include <linux/can/platform/mcf5441x.h>
  #include <linux/pm_runtime.h>
  #include <linux/regmap.h>
  #include <linux/regulator/consumer.h>

drivers/net/can/flexcan.c line 251

Quoted 6 lines

drivers/net/can/flexcan.c +2 -0

@@ -246,6 +247,8 @@
  #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)
+/* Setup for flexcan module as in mcf, 16 mb, 3 separate interrupts  */
+#define FLEXCAN_QUIRK_SETUP_MCF BIT(12)

Can you split this into QUIRK_NR_IRQ_3 and QUIRK_NR_MB_16.

Marc

Add platform data object for ColdFire architecture.

---
Changes for v2:
- move header file in more proper location
- remove irq defines
- change variable types to match driver types

Signed-off-by Angelo Dureghello <[email protected]>

---
  include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++
  1 file changed, 28 insertions(+)

create mode 100644 include/linux/can/platform/mcf5441x.h

include/linux/can/platform/mcf5441x.h new file +18 -0

new file mode 100644
index 000000000000..6866f0f60caf
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Flexcan options for ColdFire family
+ *
+ * Copyright (C) 2021  Angelo Dureghello <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _PLAT_FLEXCAN_MCF_H
+#define _PLAT_FLEXCAN_MCF_H

Hi Angelo,

Quoted 38 lines · Angelo Dureghello

Add platform data object for ColdFire architecture.

---
Changes for v2:
- move header file in more proper location
- remove irq defines
- change variable types to match driver types

Signed-off-by Angelo Dureghello <[email protected]>

---
  include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++

  1 file changed, 28 insertions(+)

  create mode 100644 include/linux/can/platform/mcf5441x.h

include/linux/can/platform/mcf5441x.h new file +18 -0

new file mode 100644
index 000000000000..6866f0f60caf
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Flexcan options for ColdFire family
+ *
+ * Copyright (C) 2021  Angelo Dureghello <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _PLAT_FLEXCAN_MCF_H
+#define _PLAT_FLEXCAN_MCF_H

include/linux/can/platform/mcf5441x.h line 28

Quoted 10 lines

include/linux/can/platform/mcf5441x.h new file +10 -0

+
+struct mcf_flexcan_platform_data {
+    u8 clk_src;
+    u32 clock_frequency;
+    bool big_endian;
+};
+
+#define FLEXCAN_MCF5411X_MB_CNT_MCF    16
+
+#endif /* _PLAT_FLEXCAN_MCF_H */

Is this actually specific to the ColdFire M5411x SoC?
I thought a number of other ColdFire parts also have canbus support. Are they not all the same underlying hardware block?

So should this be more generic, say flexcan.h or mcf_flexcan.h or something like that?

Regards
Greg

Hi Greg,

Quoted 57 lines · Greg Ungerer

Hi Angelo,

Quoted 39 lines · Angelo Dureghello

Add platform data object for ColdFire architecture.

---
Changes for v2:
- move header file in more proper location
- remove irq defines
- change variable types to match driver types

Signed-off-by Angelo Dureghello <[email protected]>

---
  include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++

  1 file changed, 28 insertions(+)

  create mode 100644 include/linux/can/platform/mcf5441x.h

include/linux/can/platform/mcf5441x.h new file +18 -0

new file mode 100644
index 000000000000..6866f0f60caf
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Flexcan options for ColdFire family
+ *
+ * Copyright (C) 2021  Angelo Dureghello <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _PLAT_FLEXCAN_MCF_H
+#define _PLAT_FLEXCAN_MCF_H

include/linux/can/platform/mcf5441x.h line 28

Quoted 10 lines

include/linux/can/platform/mcf5441x.h new file +10 -0

+
+struct mcf_flexcan_platform_data {
+    u8 clk_src;
+    u32 clock_frequency;
+    bool big_endian;
+};
+
+#define FLEXCAN_MCF5411X_MB_CNT_MCF    16
+
+#endif /* _PLAT_FLEXCAN_MCF_H */

Is this actually specific to the ColdFire M5411x SoC?
I thought a number of other ColdFire parts also have canbus support. Are they not all the same underlying hardware block?

Quoted 2 lines

So should this be more generic, say flexcan.h or mcf_flexcan.h or something like that?

yes, the file can be reused for all mcf, they have similar hardware blocks (flexcan) but with small differences, like i.e. number of buffers, so the define guards are named generic (MCF), with specific defines as needed,

Will rename it to mcf_flexcan.h.

Quoted 4 lines

Is this actually specific to the ColdFire M5411x SoC?
I thought a number of other ColdFire parts also have canbus support. Are they not all the same underlying hardware block?

Quoted 2 lines

So should this be more generic, say flexcan.h or mcf_flexcan.h or something like that?

yes, the file can be reused for all mcf, they have similar hardware blocks (flexcan) but with small differences, like i.e. number of buffers, so the define guards are named generic (MCF), with specific defines as needed,

Will rename it to mcf_flexcan.h.

Add platform data object for ColdFire architecture.

---
Changes for v2:
- move header file in more proper location
- remove irq defines
- change variable types to match driver types

Signed-off-by Angelo Dureghello <[email protected]>

include/linux/can/platform/mcf5441x.h new file +13 -0

@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Flexcan options for ColdFire family
+ *
+ * Copyright (C) 2021  Angelo Dureghello <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

Hi Angelo,

Quoted 9 lines · Angelo Dureghello

Add platform data object for ColdFire architecture.

---
Changes for v2:
- move header file in more proper location
- remove irq defines
- change variable types to match driver types

Signed-off-by Angelo Dureghello <[email protected]>

Thanks for your patch!

Quoted 21 lines

include/linux/can/platform/mcf5441x.h new file +18 -0

@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Flexcan options for ColdFire family
+ *
+ * Copyright (C) 2021  Angelo Dureghello <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _PLAT_FLEXCAN_MCF_H
+#define _PLAT_FLEXCAN_MCF_H

include/linux/can/platform/mcf5441x.h line 23

Quoted 5 lines

include/linux/can/platform/mcf5441x.h new file +5 -0

+
+struct mcf_flexcan_platform_data {
+       u8 clk_src;
+       u32 clock_frequency;
+       bool big_endian;

Please sort the members according to decreasing size, to avoid holes.

include/linux/can/platform/mcf5441x.h line 28

Quoted 6 lines

include/linux/can/platform/mcf5441x.h new file +5 -0

+};
+
+#define FLEXCAN_MCF5411X_MB_CNT_MCF    16
+
+#endif /* _PLAT_FLEXCAN_MCF_H */

Gr{oetje,eeting}s,

Geert

Add platform data object for ColdFire architecture.

---
Changes for v2:
- move header file in more proper location
- remove irq defines
- change variable types to match driver types

Signed-off-by Angelo Dureghello <[email protected]>

---
 include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 include/linux/can/platform/mcf5441x.h

include/linux/can/platform/mcf5441x.h new file +13 -0

new file mode 100644
index 000000000000..6866f0f60caf
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Flexcan options for ColdFire family
+ *
+ * Copyright (C) 2021  Angelo Dureghello <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the