[PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[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 <angelo@kernel-space.org>
---
 include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 include/linux/can/platform/mcf5441x.h

diff --git a/include/linux/can/platform/mcf5441x.h b/include/linux/can/platform/mcf5441x.h
new file mode 100644
index 000000000000..6866f0f60caf
--- /dev/null
+++ b/include/linux/can/platform/mcf5441x.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Flexcan options for ColdFire family
+ *
+ * Copyright (C) 2021  Angelo Dureghello <angelo@kernel-space.org>
+ *
+ * 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
+
+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 */
-- 
2.31.1

[PATCH v2 2/5] m68k: stmark2: update board setup

[Angelo Dureghello]

Add configuration for flexcan pads.

---
Changes for v2:
none

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 arch/m68k/coldfire/stmark2.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/m68k/coldfire/stmark2.c b/arch/m68k/coldfire/stmark2.c
index 8b5af9c83244..036a6ae5f599 100644
--- a/arch/m68k/coldfire/stmark2.c
+++ b/arch/m68k/coldfire/stmark2.c
@@ -111,7 +111,9 @@ static int __init init_stmark2(void)
 	__raw_writeb(0x00, MCFGPIO_PAR_BE);
 	__raw_writeb(0x00, MCFGPIO_PAR_FBCTL);
 	__raw_writeb(0x00, MCFGPIO_PAR_CS);
-	__raw_writeb(0x00, MCFGPIO_PAR_CANI2C);
+
+	/* CAN pads */
+	__raw_writeb(0x50, MCFGPIO_PAR_CANI2C);
 
 	platform_add_devices(stmark2_devices, ARRAY_SIZE(stmark2_devices));
 
@@ -121,4 +123,4 @@ static int __init init_stmark2(void)
 	return 0;
 }
 
-late_initcall(init_stmark2);
+device_initcall(init_stmark2);
-- 
2.31.1

[PATCH v2 3/5] m68k: m5441x: add flexcan support

[Angelo Dureghello]

Add flexcan support.

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

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 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(-)

diff --git a/arch/m68k/coldfire/device.c b/arch/m68k/coldfire/device.c
index 59f7dfe50a4d..1b2eaee6d6f6 100644
--- a/arch/m68k/coldfire/device.c
+++ b/arch/m68k/coldfire/device.c
@@ -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
 };
 
 /*
diff --git a/arch/m68k/coldfire/m5441x.c b/arch/m68k/coldfire/m5441x.c
index 1e5259a652d1..18b152edb69c 100644
--- a/arch/m68k/coldfire/m5441x.c
+++ b/arch/m68k/coldfire/m5441x.c
@@ -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 */
diff --git a/arch/m68k/include/asm/m5441xsim.h b/arch/m68k/include/asm/m5441xsim.h
index e091e36d3464..f48cf63bd782 100644
--- a/arch/m68k/include/asm/m5441xsim.h
+++ b/arch/m68k/include/asm/m5441xsim.h
@@ -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 */
-- 
2.31.1

[PATCH v2 4/5] can: flexcan: enable Kconfig for ColdFire

[Angelo Dureghello]

Enable flexcan KConfig for ColdFire m5441x cpu's.

Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
 drivers/net/can/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index e355d3974977..7a1be9dad52d 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -97,7 +97,7 @@ config CAN_AT91
 
 config CAN_FLEXCAN
 	tristate "Support for Freescale FLEXCAN based chips"
-	depends on OF && HAS_IOMEM
+	depends on (OF || M5441x) && HAS_IOMEM
 	help
 	  Say Y here if you want to support for Freescale FlexCAN.
 
-- 
2.31.1

[PATCH v2 5/5] can: flexcan: add mcf5441x support

[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 <angelo@kernel-space.org>
---
 drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 57f3635ad8d7..2188dc36a010 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -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;
 
-	err = clk_prepare_enable(priv->clk_ipg);
-	if (err)
-		return err;
+	if (priv->clk_ipg) {
+		err = clk_prepare_enable(priv->clk_ipg);
+		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;
 
+	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;
+
+		err = request_irq(priv->irq_err,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;
+	}
+
 	flexcan_chip_interrupts_enable(dev);
 
 	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);
+	}
+
 	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)
 
 	if (pdev->dev.of_node) {
 		of_property_read_u32(pdev->dev.of_node,
-				     "clock-frequency", &clock_freq);
+				"clock-frequency", &clock_freq);
 		of_property_read_u8(pdev->dev.of_node,
-				    "fsl,clk-source", &clk_src);
+				"fsl,clk-source", &clk_src);
+	} 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;
 
+	if (devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF) {
+		priv->irq_boff = platform_get_irq(pdev, 1);
+		if (priv->irq_boff <= 0)
+			return -ENODEV;
+		priv->irq_err = platform_get_irq(pdev, 2);
+		if (priv->irq_err <= 0)
+			return -ENODEV;
+	}
+
 	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);
-- 
2.31.1

Re: [PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[Greg Ungerer]

Hi Angelo,

On 17/6/21 9:16 am, Angelo Dureghello wrote:
> 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 <angelo@kernel-space.org>
> ---
>   include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
>   create mode 100644 include/linux/can/platform/mcf5441x.h
> 
> diff --git a/include/linux/can/platform/mcf5441x.h b/include/linux/can/platform/mcf5441x.h
> new file mode 100644
> index 000000000000..6866f0f60caf
> --- /dev/null
> +++ b/include/linux/can/platform/mcf5441x.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Flexcan options for ColdFire family
> + *
> + * Copyright (C) 2021  Angelo Dureghello <angelo@kernel-space.org>
> + *
> + * 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
> +
> +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

Re: [PATCH v2 3/5] m68k: m5441x: add flexcan support

[Greg Ungerer]

Hi Angelo,

On 17/6/21 9:16 am, Angelo Dureghello wrote:
> Add flexcan support.
> 
> ---
> Changes for v2:
> - add irq resources for ERR and BOFF interrutps
> 
> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>

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



> ---
>   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(-)
> 
> diff --git a/arch/m68k/coldfire/device.c b/arch/m68k/coldfire/device.c
> index 59f7dfe50a4d..1b2eaee6d6f6 100644
> --- a/arch/m68k/coldfire/device.c
> +++ b/arch/m68k/coldfire/device.c
> @@ -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
>   };
>   
>   /*
> diff --git a/arch/m68k/coldfire/m5441x.c b/arch/m68k/coldfire/m5441x.c
> index 1e5259a652d1..18b152edb69c 100644
> --- a/arch/m68k/coldfire/m5441x.c
> +++ b/arch/m68k/coldfire/m5441x.c
> @@ -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 */
> diff --git a/arch/m68k/include/asm/m5441xsim.h b/arch/m68k/include/asm/m5441xsim.h
> index e091e36d3464..f48cf63bd782 100644
> --- a/arch/m68k/include/asm/m5441xsim.h
> +++ b/arch/m68k/include/asm/m5441xsim.h
> @@ -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 */
> 

Re: [PATCH v2 3/5] m68k: m5441x: add flexcan support

[Angelo Dureghello]

Hi Greg,

On 17/06/21 2:18 AM, Greg Ungerer wrote:
> Hi Angelo,
> 
> On 17/6/21 9:16 am, Angelo Dureghello wrote:
>> Add flexcan support.
>>
>> ---
>> Changes for v2:
>> - add irq resources for ERR and BOFF interrutps
>>
>> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
> 
> 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.
>
Oh ok, thanks.

> Regards
> Greg
> 
> 

Regards,
-- 
Angelo Dureghello

Re: [PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[Angelo Dureghello]

Hi Greg,

On 17/06/21 2:17 AM, Greg Ungerer wrote:
> Hi Angelo,
> 
> On 17/6/21 9:16 am, Angelo Dureghello wrote:
>> 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 <angelo@kernel-space.org>
>> ---
>>   include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>   create mode 100644 include/linux/can/platform/mcf5441x.h
>>
>> diff --git a/include/linux/can/platform/mcf5441x.h 
>> b/include/linux/can/platform/mcf5441x.h
>> new file mode 100644
>> index 000000000000..6866f0f60caf
>> --- /dev/null
>> +++ b/include/linux/can/platform/mcf5441x.h
>> @@ -0,0 +1,28 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Flexcan options for ColdFire family
>> + *
>> + * Copyright (C) 2021  Angelo Dureghello <angelo@kernel-space.org>
>> + *
>> + * 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
>> +
>> +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?


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.

> 
> Regards
> Greg

Thanks !

Regards,
angelo
> 

Re: [PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[Geert Uytterhoeven]

Hi Angelo,

On Thu, Jun 17, 2021 at 1:17 AM Angelo Dureghello
<angelo@kernel-space.org> wrote:
> 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 <angelo@kernel-space.org>

Thanks for your patch!

> --- /dev/null
> +++ b/include/linux/can/platform/mcf5441x.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Flexcan options for ColdFire family
> + *
> + * Copyright (C) 2021  Angelo Dureghello <angelo@kernel-space.org>
> + *
> + * 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
> +
> +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.

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 3063 bytes --]

On 6/17/21 9:23 AM, Angelo Dureghello wrote:
> Hi Greg,
> 
> On 17/06/21 2:17 AM, Greg Ungerer wrote:
>> Hi Angelo,
>>
>> On 17/6/21 9:16 am, Angelo Dureghello wrote:
>>> 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 <angelo@kernel-space.org>
>>> ---
>>>   include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++
>>>   1 file changed, 28 insertions(+)
>>>   create mode 100644 include/linux/can/platform/mcf5441x.h
>>>
>>> diff --git a/include/linux/can/platform/mcf5441x.h 
>>> b/include/linux/can/platform/mcf5441x.h
>>> new file mode 100644
>>> index 000000000000..6866f0f60caf
>>> --- /dev/null
>>> +++ b/include/linux/can/platform/mcf5441x.h
>>> @@ -0,0 +1,28 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Flexcan options for ColdFire family
>>> + *
>>> + * Copyright (C) 2021  Angelo Dureghello <angelo@kernel-space.org>
>>> + *
>>> + * 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
>>> +
>>> +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?
> 
> 
> 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.

The platform data is not mcf specific, it only holds the information which would
otherwise be part of the device tree (and is not accessible via other frameworks).

From my point of view, it makes sense to name it flexcan.h and place it under
include/linux/can/platform.

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 |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[Angelo Dureghello]

Hi Geert,

On 17/06/21 10:28 AM, Geert Uytterhoeven wrote:
> Hi Angelo,
> 
> On Thu, Jun 17, 2021 at 1:17 AM Angelo Dureghello
> <angelo@kernel-space.org> wrote:
>> 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 <angelo@kernel-space.org>
> 
> Thanks for your patch!
> 
>> --- /dev/null
>> +++ b/include/linux/can/platform/mcf5441x.h
>> @@ -0,0 +1,28 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Flexcan options for ColdFire family
>> + *
>> + * Copyright (C) 2021  Angelo Dureghello <angelo@kernel-space.org>
>> + *
>> + * 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
>> +
>> +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.

sure, never thought to this, thanks,
> 
>> +};
>> +
>> +#define FLEXCAN_MCF5411X_MB_CNT_MCF    16
>> +
>> +#endif /* _PLAT_FLEXCAN_MCF_H */
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

Regards,
angelo

Re: [PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]

On 17.06.2021 09:23:13, Angelo Dureghello wrote:
> > 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?
> 
> 
> 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,

If there are other flexcan IP cores with small differences compared to
this one, you better give this IP core a popper name. See Patch 3:

| +static struct platform_device mcf_flexcan0 = { 
| +	.name = "flexcan-mcf",

what about naming it "flexcan-mcf5441x"? Or is this too specific? As
this is not a DT compatible we can change this later if needed.

| +	.id = 0, 
| +	.num_resources = ARRAY_SIZE(mcf_flexcan0_resource), 
| +	.resource = mcf_flexcan0_resource, 
| +	.dev.platform_data = &mcf_flexcan_info, 
| +}; 

> Will rename it to mcf_flexcan.h.

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 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 5/5] can: flexcan: add mcf5441x support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 8580 bytes --]

On 17.06.2021 01:16:52, Angelo Dureghello wrote:
> 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.

> ---
> 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 <angelo@kernel-space.org>
> ---
>  drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 84 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 57f3635ad8d7..2188dc36a010 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -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;
>  
> -	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()...

> +		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;
>  
> +	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.

> +
> +		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.

> +	}
> +
>  	flexcan_chip_interrupts_enable(dev);
>  
>  	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

> +
>  	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)
>  
>  	if (pdev->dev.of_node) {
>  		of_property_read_u32(pdev->dev.of_node,
> -				     "clock-frequency", &clock_freq);
> +				"clock-frequency", &clock_freq);

unrelated change

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

unrelated change

> +	} 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;
>  
> +	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.

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

Please add proper error handling.

> +	}
> +
>  	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);
> -- 
> 2.31.1
> 
> 

regards,
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 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 5/5] can: flexcan: add mcf5441x support

[Angelo Dureghello]

Hi Mark,

thanks for the review, will fix all.


> regards,
> Marc
> 

Regards,
angelo

Re: [PATCH v2 4/5] can: flexcan: enable Kconfig for ColdFire

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

On 17.06.2021 01:16:51, Angelo Dureghello wrote:
> Enable flexcan KConfig for ColdFire m5441x cpu's.
> 
> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
> ---
>  drivers/net/can/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index e355d3974977..7a1be9dad52d 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -97,7 +97,7 @@ config CAN_AT91
>  
>  config CAN_FLEXCAN
>  	tristate "Support for Freescale FLEXCAN based chips"
> -	depends on OF && HAS_IOMEM
> +	depends on (OF || M5441x) && HAS_IOMEM

I think it's OK to only remove the OF.

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

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 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 5/5] can: flexcan: add mcf5441x support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]

On 17.06.2021 01:16:52, Angelo Dureghello wrote:
> 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 <angelo@kernel-space.org>
> ---
>  drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 84 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 57f3635ad8d7..2188dc36a010 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -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)

Can you split this into QUIRK_NR_IRQ_3 and QUIRK_NR_MB_16.

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 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/5] can: flexcan: add platform data for ColdFire

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]

On 17.06.2021 01:16:48, Angelo Dureghello wrote:
> 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 <angelo@kernel-space.org>
> ---
>  include/linux/can/platform/mcf5441x.h | 28 +++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 include/linux/can/platform/mcf5441x.h
> 
> diff --git a/include/linux/can/platform/mcf5441x.h b/include/linux/can/platform/mcf5441x.h
> new file mode 100644
> index 000000000000..6866f0f60caf
> --- /dev/null
> +++ b/include/linux/can/platform/mcf5441x.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Flexcan options for ColdFire family
> + *
> + * Copyright (C) 2021  Angelo Dureghello <angelo@kernel-space.org>
> + *
> + * 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
> +
> +struct mcf_flexcan_platform_data {
> +	u8 clk_src;
> +	u32 clock_frequency;
> +	bool big_endian;

Please remove the big_endian, you don't handle it in the flexcan driver
anyways.

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 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 5/5] can: flexcan: add mcf5441x support

[Angelo Dureghello]

Hi Mark,

On 17/06/21 3:00 PM, Marc Kleine-Budde wrote:
> On 17.06.2021 01:16:52, Angelo Dureghello wrote:
>> 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.
> 

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.

>> ---
>> 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 <angelo@kernel-space.org>
>> ---
>>   drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 84 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> index 57f3635ad8d7..2188dc36a010 100644
>> --- a/drivers/net/can/flexcan.c
>> +++ b/drivers/net/can/flexcan.c
>> @@ -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;
>>   
>> -	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()...
> 

tested, it's ok with NULL.

>> +		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;
>>   
>> +	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.

done

> 
>> +
>> +		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.

done

> 
>> +	}
>> +
>>   	flexcan_chip_interrupts_enable(dev);
>>   
>>   	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
> 

done

>> +
>>   	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)
>>   
>>   	if (pdev->dev.of_node) {
>>   		of_property_read_u32(pdev->dev.of_node,
>> -				     "clock-frequency", &clock_freq);
>> +				"clock-frequency", &clock_freq);
> 
> unrelated change

removed

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

removed

> 
>> +	} 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;
>>   
>> +	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.

done

> 
>> +		priv->irq_err = platform_get_irq(pdev, 2);
>> +		if (priv->irq_err <= 0)
>> +			return -ENODEV;
> 
> Please add proper error handling.

done

> 
>> +	}
>> +
>>   	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);
>> -- 
>> 2.31.1
>>
>>
> 
> regards,
> Marc
> 

Regards,
angelo

Re: [PATCH v2 5/5] can: flexcan: add mcf5441x support

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1502 bytes --]

On 18.06.2021 00:14:32, Angelo Dureghello wrote:
> On 17/06/21 3:00 PM, Marc Kleine-Budde wrote:
> > On 17.06.2021 01:16:52, Angelo Dureghello wrote:
> > > 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.
> 
> 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.

The normal datasheets don't list the IP version. If you don't work for a
company integrating the flexcan core into a SoC, you probably don't have
access to the "low level" datasheets or the people who can look into
them. So just add a ? for details you don't know.

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 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 5/5] can: flexcan: add mcf5441x support

[Angelo Dureghello]

Hi Marc,

On 17/06/21 9:38 PM, Marc Kleine-Budde wrote:
> On 17.06.2021 01:16:52, Angelo Dureghello wrote:
>> 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 <angelo@kernel-space.org>
>> ---
>>   drivers/net/can/flexcan.c | 95 ++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 84 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> index 57f3635ad8d7..2188dc36a010 100644
>> --- a/drivers/net/can/flexcan.c
>> +++ b/drivers/net/can/flexcan.c
>> @@ -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)
> 
> Can you split this into QUIRK_NR_IRQ_3 and QUIRK_NR_MB_16.
> 

done

> Marc
> 

-- 
Angelo Dureghello
+++ kernelspace +++
+E: angelo AT kernel-space.org
+W: www.kernel-space.org