Skip to contentSkip to conversation

can: flexcan: add platform data for ColdFire

PATCH Superseded Angelo Dureghello 5 participants 19 messages Jun 8, 2021 – Jun 11, 2021

Subsystems (4)

v1 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

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

Add platform data object for ColdFire architecture.

---
 include/linux/platform_data/flexcan-mcf.h | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 include/linux/platform_data/flexcan-mcf.h
include/linux/platform_data/flexcan-mcf.h new file +27 -0
new file mode 100644
index 000000000000..71fe1a9df084
@@ -0,0 +1,27 @@
+/* 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
+
+struct mcf_flexcan_platform_data {
+	int stop_mode;
+	int clk_src;
+	int clock_frequency;
+	bool big_endian;
+};
+
+#endif /* _PLAT_FLEXCAN_MCF_H */
Signature · 1 line
2.31.1
#1 lore raw

PATCH 2/5 m68k: stmark2: update board setup

Add configuration for flexcan pads.

---
 arch/m68k/coldfire/stmark2.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
arch/m68k/coldfire/stmark2.c +4 -2
index 8b5af9c83244..036a6ae5f599 100644
@@ -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);
Signature · 1 line
2.31.1
#2 lore raw

PATCH 3/5 m68k: m5441x: add flexcan support

In reply to Angelo Dureghello

Add flexcan support.

---
 arch/m68k/coldfire/device.c       | 31 +++++++++++++++++++++++++++++++
 arch/m68k/coldfire/m5441x.c       |  8 ++++----
 arch/m68k/include/asm/m5441xsim.h | 19 +++++++++++++++++++
 3 files changed, 54 insertions(+), 4 deletions(-)
arch/m68k/coldfire/device.c +31 -0
index 59f7dfe50a4d..b3263283bf71 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/platform_data/flexcan-mcf.h>
 
 /*
  *	All current ColdFire parts contain from 2, 3, 4 or 10 UARTS.
@@ -581,6 +582,33 @@ 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_ERR0,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device mcf_flexcan0 = {
+	.name = "flexcan",
+	.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 +644,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 */
Signature · 1 line
2.31.1
maintainer
#3 lore raw

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

Hi Angelo,

Quoted 70 lines · Angelo Dureghello

Add flexcan support.

---
  arch/m68k/coldfire/device.c       | 31 +++++++++++++++++++++++++++++++
  arch/m68k/coldfire/m5441x.c       |  8 ++++----
  arch/m68k/include/asm/m5441xsim.h | 19 +++++++++++++++++++
  3 files changed, 54 insertions(+), 4 deletions(-)
arch/m68k/coldfire/device.c +31 -0
index 59f7dfe50a4d..b3263283bf71 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/platform_data/flexcan-mcf.h>
 
  /*
   *	All current ColdFire parts contain from 2, 3, 4 or 10 UARTS.
@@ -581,6 +582,33 @@ 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_ERR0,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device mcf_flexcan0 = {
+	.name = "flexcan",
+	.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 +644,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 +0 -0
index 1e5259a652d1..18b152edb69c 100644

arch/m68k/coldfire/m5441x.c line 23

Quoted 8 lines
arch/m68k/coldfire/m5441x.c +2 -2
@@ -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);

Just a heads up, but this will likely conflict with Arnd's clock changes, see:

    https://lkml.org/lkml/2021/6/8/774

Regards
Greg

Quoted 56 lines
arch/m68k/coldfire/m5441x.c +2 -2
  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 */
#4 lore raw

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

Hi Greg,

Quoted 28 lines · Greg Ungerer

Hi Angelo,

Quoted 15 lines · Angelo Dureghello

Add flexcan support.

---
  arch/m68k/coldfire/device.c       | 31 +++++++++++++++++++++++++++++++ arch/m68k/coldfire/m5441x.c       |  8 ++++---- arch/m68k/include/asm/m5441xsim.h | 19 +++++++++++++++++++

  3 files changed, 54 insertions(+), 4 deletions(-)
arch/m68k/coldfire/m5441x.c +0 -0
index 1e5259a652d1..18b152edb69c 100644

arch/m68k/coldfire/m5441x.c line 22

Quoted 7 lines
arch/m68k/coldfire/m5441x.c +2 -2
@@ -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);

Just a heads up, but this will likely conflict with Arnd's clock changes, see:

Quoted 2 lines

    https://lkml.org/lkml/2021/6/8/774

is this a clock naming issue ? The "mcfcan" is actually not referenced in any driver so naming it "flexcan" shouldn't be an issue.

Or i should get and enable "mcfcan" clock from the driver ?

Quoted 3 lines

Regards
Greg

Thanks,
regards,

Signature · 4 lines
Angelo Dureghello
+++ kernelspace +++
+E: angelo AT kernel-space.org
+W: www.kernel-space.org
maintainer
#5 lore raw

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

Hi Angelo,

Quoted 31 lines · Angelo Dureghello

Hi Greg,

Quoted 28 lines · Greg Ungerer

Hi Angelo,

Quoted 15 lines · Angelo Dureghello

Add flexcan support.

---
  arch/m68k/coldfire/device.c       | 31 +++++++++++++++++++++++++++++++ arch/m68k/coldfire/m5441x.c       |  8 ++++---- arch/m68k/include/asm/m5441xsim.h | 19 +++++++++++++++++++

  3 files changed, 54 insertions(+), 4 deletions(-)
arch/m68k/coldfire/m5441x.c +0 -0
index 1e5259a652d1..18b152edb69c 100644

arch/m68k/coldfire/m5441x.c line 22

Quoted 7 lines
arch/m68k/coldfire/m5441x.c +2 -2
@@ -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);

Just a heads up, but this will likely conflict with Arnd's clock changes, see:

Quoted 6 lines
Quoted 2 lines

     https://lkml.org/lkml/2021/6/8/774

is this a clock naming issue ? The "mcfcan" is actually not referenced in any driver so naming it "flexcan" shouldn't be an issue.

Yeah, no problem with that. The "mcfcan" name was really just a place holder here. Not used previously.

My comment is more that your patches and Anrd's are touching some of the same things. Depending on whose is applied first one or the other will need to be updated to reflect the others. No big deal, and easy to do in this case.

Regards
Greg

Quoted 8 lines

Or i should get and enable "mcfcan" clock from the driver ?

Quoted 3 lines

Regards
Greg

Thanks,
regards,

#6 lore raw

PATCH 4/5 can: flexcan: enable Kconfig for ColdFire

In reply to Angelo Dureghello

Enable flexcan KConfig for ColdFire m5441x cpu's.

---
 drivers/net/can/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/can/Kconfig +1 -1
index e355d3974977..7a1be9dad52d 100644
@@ -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.
 
Signature · 1 line
2.31.1
#7 lore raw

PATCH 5/5 can: flexcan: add mcf5441x support

In reply to 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.

---
 drivers/net/can/flexcan.c | 80 +++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 15 deletions(-)
drivers/net/can/flexcan.c +65 -15
index 57f3635ad8d7..af95273327cd 100644
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/flexcan-mcf.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
@@ -205,6 +206,10 @@
 
 #define FLEXCAN_TIMEOUT_US		(250)
 
+#define FLEXCAN_OFFS_INT_BOFF		1
+#define FLEXCAN_OFFS_INT_ERR		3
+#define FLEXCAN_MB_CNT_MCF		16
+
 /* FLEXCAN hardware feature flags
  *
  * Below is some version info we got:
@@ -246,6 +251,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 {
@@ -371,6 +378,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 +650,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 +1418,13 @@ 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_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 +1796,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(dev->irq + FLEXCAN_OFFS_INT_BOFF,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;
+
+		err = request_irq(dev->irq + FLEXCAN_OFFS_INT_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);
@@ -2047,7 +2081,9 @@ static int flexcan_probe(struct platform_device *pdev)
 	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;
+	bool big_endian_module;
 	u8 clk_src = 1;
 	u32 clock_freq = 0;
 
@@ -2059,11 +2095,17 @@ static int flexcan_probe(struct platform_device *pdev)
 	else if (IS_ERR(reg_xceiver))
 		return PTR_ERR(reg_xceiver);
 
-	if (pdev->dev.of_node) {
-		of_property_read_u32(pdev->dev.of_node,
-				     "clock-frequency", &clock_freq);
-		of_property_read_u8(pdev->dev.of_node,
-				    "fsl,clk-source", &clk_src);
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata) {
+		clock_freq = pdata->clock_frequency;
+		clk_src = pdata->clk_src;
+	} else {
+		if (pdev->dev.of_node) {
+			of_property_read_u32(pdev->dev.of_node,
+					     "clock-frequency", &clock_freq);
+			of_property_read_u8(pdev->dev.of_node,
+					    "fsl,clk-source", &clk_src);
+		}
 	}
 
 	if (!clock_freq) {
@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device *pdev)
 
 	devtype_data = of_device_get_match_data(&pdev->dev);
 
+	if (pdata && !devtype_data) {
+		devtype_data =
+			(struct flexcan_devtype_data *)&fsl_mcf_devtype_data;
+	}
+
 	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
 	    !(devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)) {
 		dev_err(&pdev->dev, "CAN-FD mode doesn't work with FIFO mode!\n");
@@ -2110,8 +2157,11 @@ static int flexcan_probe(struct platform_device *pdev)
 
 	priv = netdev_priv(dev);
 
-	if (of_property_read_bool(pdev->dev.of_node, "big-endian") ||
-	    devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN) {
+	big_endian_module = pdata ? pdata->big_endian :
+		of_property_read_bool(pdev->dev.of_node, "big-endian");
+
+	if (big_endian_module ||
+		(devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN)) {
 		priv->read = flexcan_read_be;
 		priv->write = flexcan_write_be;
 	} else {
Signature · 1 line
2.31.1
#8 lore raw

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

Quoted 40 lines

-----Original Message-----
From: Angelo Dureghello <[email protected]>
Sent: 2021年6月9日 4:46
To: [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Joakim Zhang <[email protected]>; Angelo Dureghello <[email protected]>
Subject: [PATCH 5/5] can: flexcan: add mcf5441x support

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.

---
 drivers/net/can/flexcan.c | 80 +++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 15 deletions(-)
drivers/net/can/flexcan.c index renamed from drivers/net/can/flexcan.c +0 -0

57f3635ad8d7..af95273327cd 100644

drivers/net/can/flexcan.c +1 -0
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/flexcan-mcf.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>

drivers/net/can/flexcan.c line 211

Quoted 6 lines
drivers/net/can/flexcan.c +3 -0
@@ -205,6 +206,10 @@
 
 #define FLEXCAN_TIMEOUT_US		(250)
 
+#define FLEXCAN_OFFS_INT_BOFF		1
+#define FLEXCAN_OFFS_INT_ERR		3
+#define FLEXCAN_MB_CNT_MCF		16

Could you rename the macro, like FLEXCAN_MCF5411X_, then user know this only for mcf5411x? And move this into header file? Why not put this also into mcf_flexcan_platform_data? The interrupt offset and the numbers of mailboxes would be various from different SoCs.

Quoted 35 lines
drivers/net/can/flexcan.c +12 -3
 /* FLEXCAN hardware feature flags
  *
  * Below is some version info we got:
@@ -246,6 +251,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 {
@@ -371,6 +378,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 +650,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;

drivers/net/can/flexcan.c static int flexcan_clks_enable(const struct · line 663

Quoted 10 lines
drivers/net/can/flexcan.c +6 -3
+	}
 
-	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);
+	}

No need do this check, it will be handled in clk_prepare_enable() / clk_disable_unprepare(). So this change is unnecessary.

Quoted 91 lines
drivers/net/can/flexcan.c +7 -2
 	return err;
 }
@@ -1401,8 +1418,13 @@ 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_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)
drivers/net/can/flexcan.c +12 -0
 		priv->tx_mb_reserved =
@@ -1774,6 +1796,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(dev->irq + FLEXCAN_OFFS_INT_BOFF,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;
+
+		err = request_irq(dev->irq + FLEXCAN_OFFS_INT_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); @@ -2047,7 +2081,9 @@

static int flexcan_probe(struct platform_device *pdev)

drivers/net/can/flexcan.c +13 -5
 	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;
+	bool big_endian_module;
 	u8 clk_src = 1;
 	u32 clock_freq = 0;
@@ -2059,11 +2095,17 @@ static int flexcan_probe(struct platform_device
 *pdev)
 	else if (IS_ERR(reg_xceiver))
 		return PTR_ERR(reg_xceiver);
 
-	if (pdev->dev.of_node) {
-		of_property_read_u32(pdev->dev.of_node,
-				     "clock-frequency", &clock_freq);
-		of_property_read_u8(pdev->dev.of_node,
-				    "fsl,clk-source", &clk_src);
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata) {
+		clock_freq = pdata->clock_frequency;
+		clk_src = pdata->clk_src;
+	} else {
+		if (pdev->dev.of_node) {
+			of_property_read_u32(pdev->dev.of_node,
+					     "clock-frequency", &clock_freq);
+			of_property_read_u8(pdev->dev.of_node,
+					    "fsl,clk-source", &clk_src);
+		}
 	}
 
drivers/net/can/flexcan.c +5 -0
 	if (!clock_freq) {
@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device
 *pdev)
 
 	devtype_data = of_device_get_match_data(&pdev->dev);
 
+	if (pdata && !devtype_data) {
+		devtype_data =
+			(struct flexcan_devtype_data *)&fsl_mcf_devtype_data;
+	}
+
 	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
 	    !(devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)) {

dev_err(&pdev->dev, "CAN-FD mode doesn't work with FIFO
mode!\n"); @@ -2110,8 +2157,11 @@ static int flexcan_probe(struct platform_device *pdev)

drivers/net/can/flexcan.c +0 -1
 	priv = netdev_priv(dev);
 
-	if (of_property_read_bool(pdev->dev.of_node, "big-endian") ||

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

Quoted 10 lines
drivers/net/can/flexcan.c +5 -2
-	    devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN) {
+	big_endian_module = pdata ? pdata->big_endian :
+		of_property_read_bool(pdev->dev.of_node, "big-endian");
+
+	if (big_endian_module ||
+		(devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN)) {
 		priv->read = flexcan_read_be;
 		priv->write = flexcan_write_be;
 	} else {
--

Best Regards,
Joakim Zhang

Quoted 1 line

2.31.1

#9 lore raw

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

Hi Joakim, Angelo,

Quoted 38 lines · Joakim Zhang
Quoted 38 lines

From: Angelo Dureghello <[email protected]>
Sent: 2021年6月9日 4:46
To: [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Joakim Zhang <[email protected]>; Angelo Dureghello <[email protected]>
Subject: [PATCH 5/5] can: flexcan: add mcf5441x support

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.

drivers/net/can/flexcan.c +5 -3
@@ -637,13 +650,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;
+     }
 
Quoted 10 lines

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

Quoted 8 lines
drivers/net/can/flexcan.c +5 -3
-     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);
+     }

No need do this check, it will be handled in clk_prepare_enable() / clk_disable_unprepare(). So this change is unnecessary.

Except that the non-CCF implementation of clk_enable() in arch/m68k/coldfire/clk.c still returns -EINVAL instead of NULL. Any plans to move to CCF? Or at least fix legacy clk_enable().

Quoted 8 lines

drivers/net/can/flexcan.c static int flexcan_probe(struct platform_device · line 2139

Quoted 8 lines
drivers/net/can/flexcan.c +3 -0
@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device
 *pdev)
 
      devtype_data = of_device_get_match_data(&pdev->dev);
 
+     if (pdata && !devtype_data) {
+             devtype_data =
+                     (struct flexcan_devtype_data *)&fsl_mcf_devtype_data;

Cast not needed?

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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
#10 lore raw

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

Hi Geert,

Quoted 59 lines · Geert Uytterhoeven

Hi Joakim, Angelo,

Quoted 38 lines · Joakim Zhang
Quoted 38 lines

From: Angelo Dureghello <[email protected]>
Sent: 2021年6月9日 4:46
To: [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Joakim Zhang <[email protected]>; Angelo Dureghello <[email protected]>
Subject: [PATCH 5/5] can: flexcan: add mcf5441x support

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.

drivers/net/can/flexcan.c +5 -3
@@ -637,13 +650,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;
+     }
 
Quoted 10 lines

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

Quoted 8 lines
drivers/net/can/flexcan.c +5 -3
-     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);
+     }

No need do this check, it will be handled in clk_prepare_enable() / clk_disable_unprepare(). So this change is unnecessary.

Except that the non-CCF implementation of clk_enable() in arch/m68k/coldfire/clk.c still returns -EINVAL instead of NULL. Any plans to move to CCF? Or at least fix legacy clk_enable().

Quoted 3 lines
Quoted 3 lines
drivers/net/can/flexcan.c +0 -0
@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device
 *pdev)
 
Quoted 8 lines
Quoted 5 lines

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

Quoted 5 lines
drivers/net/can/flexcan.c +3 -0
       devtype_data = of_device_get_match_data(&pdev->dev);
 
+     if (pdata && !devtype_data) {
+             devtype_data =
+                     (struct flexcan_devtype_data *)&fsl_mcf_devtype_data;

Cast not needed?

Thanks, fixed for v2.

Quoted 4 lines

Gr{oetje,eeting}s,

Geert

Regards,

Signature · 4 lines
Angelo Dureghello
+++ kernelspace +++
+E: angelo AT kernel-space.org
+W: www.kernel-space.org
maintainer
#11 lore raw

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

In reply to Geert Uytterhoeven

Hi Geert,

Quoted 52 lines · Geert Uytterhoeven

Hi Joakim, Angelo,

Quoted 38 lines · Joakim Zhang
Quoted 38 lines

From: Angelo Dureghello <[email protected]>
Sent: 2021年6月9日 4:46
To: [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Joakim Zhang <[email protected]>; Angelo Dureghello <[email protected]>
Subject: [PATCH 5/5] can: flexcan: add mcf5441x support

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.

drivers/net/can/flexcan.c +5 -3
@@ -637,13 +650,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;
+     }
 
Quoted 10 lines

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

Quoted 8 lines
drivers/net/can/flexcan.c +5 -3
-     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);
+     }

No need do this check, it will be handled in clk_prepare_enable() / clk_disable_unprepare(). So this change is unnecessary.

Quoted 3 lines

Except that the non-CCF implementation of clk_enable() in arch/m68k/coldfire/clk.c still returns -EINVAL instead of NULL. Any plans to move to CCF? Or at least fix legacy clk_enable().

That was a recent change, see commit c1fb1bf64bb6 ("m68k: let clk_enable() return immediately if clk is NULL").

It could just as easily just return on that NULL check.

Regards
Greg

Quoted 15 lines
Quoted 8 lines

drivers/net/can/flexcan.c static int flexcan_probe(struct platform_device · line 2139

Quoted 8 lines
drivers/net/can/flexcan.c +3 -0
@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device
 *pdev)
 
       devtype_data = of_device_get_match_data(&pdev->dev);
 
+     if (pdata && !devtype_data) {
+             devtype_data =
+                     (struct flexcan_devtype_data *)&fsl_mcf_devtype_data;

Cast not needed?

Gr{oetje,eeting}s,

Geert

#12 lore raw

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

In reply to Joakim Zhang

Hi Joakim, Geert,

thanks for the feedbacks,

Quoted 41 lines · Joakim Zhang
Quoted 41 lines

-----Original Message-----
From: Angelo Dureghello <[email protected]>
Sent: 2021年6月9日 4:46
To: [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Joakim Zhang <[email protected]>; Angelo Dureghello <[email protected]>
Subject: [PATCH 5/5] can: flexcan: add mcf5441x support

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.

---
  drivers/net/can/flexcan.c | 80 +++++++++++++++++++++++++++++++--------
  1 file changed, 65 insertions(+), 15 deletions(-)
drivers/net/can/flexcan.c index renamed from drivers/net/can/flexcan.c +0 -0

57f3635ad8d7..af95273327cd 100644

drivers/net/can/flexcan.c +1 -0
@@ -28,6 +28,7 @@
  #include <linux/of_device.h>
  #include <linux/pinctrl/consumer.h>
  #include <linux/platform_device.h>
+#include <linux/platform_data/flexcan-mcf.h>
  #include <linux/pm_runtime.h>
  #include <linux/regmap.h>
  #include <linux/regulator/consumer.h>
@@ -205,6 +206,10 @@
 
Quoted 8 lines

drivers/net/can/flexcan.c line 211

Quoted 5 lines
drivers/net/can/flexcan.c +3 -0
  #define FLEXCAN_TIMEOUT_US		(250)
 
+#define FLEXCAN_OFFS_INT_BOFF		1
+#define FLEXCAN_OFFS_INT_ERR		3
+#define FLEXCAN_MB_CNT_MCF		16

Could you rename the macro, like FLEXCAN_MCF5411X_, then user know this only for mcf5411x? And move this into header file? Why not put this also into mcf_flexcan_platform_data? The interrupt offset and the numbers of mailboxes would be various from different SoCs.

ok for me, i can name them as

FLEXCAN_MCF5411X_OFFS_INT_BOF
FLEXCAN_MCF5411X_OFFS_INT_ERR
FLEXCAN_MCF5411X_MB_CNT_MCF

and will move them in platform data header

Quoted 38 lines
Quoted 38 lines
drivers/net/can/flexcan.c +13 -3
  /* FLEXCAN hardware feature flags
   *
   * Below is some version info we got:
@@ -246,6 +251,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 {
@@ -371,6 +378,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 +650,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;
+	}
 
Quoted 11 lines

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

Quoted 8 lines
drivers/net/can/flexcan.c +5 -3
-	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);
+	}

No need do this check, it will be handled in clk_prepare_enable() / clk_disable_unprepare(). So this change is unnecessary.

as Geert pointed out, right now without this protection (that shouldn't anyway harm), probe fails:

[    1.680000] flexcan: probe of flexcan.0 failed with error -22
Quoted 90 lines
Quoted 90 lines
drivers/net/can/flexcan.c +7 -2
  	return err;
  }
@@ -1401,8 +1418,13 @@ 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_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 +12 -0
@@ -1774,6 +1796,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(dev->irq + FLEXCAN_OFFS_INT_BOFF,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;
+
+		err = request_irq(dev->irq + FLEXCAN_OFFS_INT_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); @@ -2047,7 +2081,9 @@

static int flexcan_probe(struct platform_device *pdev)
struct regulator *reg_xceiver;
struct clk *clk_ipg = NULL, *clk_per = NULL; struct flexcan_regs __iomem *regs;

drivers/net/can/flexcan.c +13 -5
+	struct mcf_flexcan_platform_data *pdata;
  	int err, irq;
+	bool big_endian_module;
  	u8 clk_src = 1;
  	u32 clock_freq = 0;
@@ -2059,11 +2095,17 @@ static int flexcan_probe(struct platform_device
 *pdev)
  	else if (IS_ERR(reg_xceiver))
  		return PTR_ERR(reg_xceiver);
 
-	if (pdev->dev.of_node) {
-		of_property_read_u32(pdev->dev.of_node,
-				     "clock-frequency", &clock_freq);
-		of_property_read_u8(pdev->dev.of_node,
-				    "fsl,clk-source", &clk_src);
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata) {
+		clock_freq = pdata->clock_frequency;
+		clk_src = pdata->clk_src;
+	} else {
+		if (pdev->dev.of_node) {
+			of_property_read_u32(pdev->dev.of_node,
+					     "clock-frequency", &clock_freq);
+			of_property_read_u8(pdev->dev.of_node,
+					    "fsl,clk-source", &clk_src);
+		}
  	}
 
  	if (!clock_freq) {
drivers/net/can/flexcan.c +5 -0
@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device
 *pdev)
 
  	devtype_data = of_device_get_match_data(&pdev->dev);
 
+	if (pdata && !devtype_data) {
+		devtype_data =
+			(struct flexcan_devtype_data *)&fsl_mcf_devtype_data;
+	}
+
  	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
  	    !(devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)) {

dev_err(&pdev->dev, "CAN-FD mode doesn't work with FIFO
mode!\n"); @@ -2110,8 +2157,11 @@ static int flexcan_probe(struct platform_device *pdev)

  	priv = netdev_priv(dev);
Quoted 16 lines

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

Quoted 11 lines
drivers/net/can/flexcan.c +5 -3
-	if (of_property_read_bool(pdev->dev.of_node, "big-endian") ||
-	    devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN) {
+	big_endian_module = pdata ? pdata->big_endian :
+		of_property_read_bool(pdev->dev.of_node, "big-endian");
+
+	if (big_endian_module ||
+		(devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN)) {
  		priv->read = flexcan_read_be;
  		priv->write = flexcan_write_be;
  	} else {
--

Best Regards,
Joakim Zhang

Quoted 1 line

2.31.1

Best Regards,

Signature · 4 lines
Angelo Dureghello
+++ kernelspace +++
+E: angelo AT kernel-space.org
+W: www.kernel-space.org
#13 lore raw

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

In reply to Angelo Dureghello
Quoted 6 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,

Please don't hard code the 2 extra interrupts as offsets, but create proper IRQ resources.

Quoted 3 lines

- implements 16 mb only,
- m68k architecture is not supporting devicetrees, so a platform data check/case has been added,

You might want to revert:

| 2c0ac9208135 can: flexcan: convert the driver to DT-only

and use it as a starting point.

Quoted 80 lines

- ColdFire is m68k, so big-endian cpu, with a little-endian flexcan module.

---
 drivers/net/can/flexcan.c | 80 +++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 15 deletions(-)
drivers/net/can/flexcan.c +23 -6
index 57f3635ad8d7..af95273327cd 100644
@@ -28,6 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/flexcan-mcf.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
@@ -205,6 +206,10 @@
 
 #define FLEXCAN_TIMEOUT_US		(250)
 
+#define FLEXCAN_OFFS_INT_BOFF		1
+#define FLEXCAN_OFFS_INT_ERR		3
+#define FLEXCAN_MB_CNT_MCF		16
+
 /* FLEXCAN hardware feature flags
  *
  * Below is some version info we got:
@@ -246,6 +251,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 {
@@ -371,6 +378,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 +650,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 +1418,13 @@ static int flexcan_rx_offload_setup(struct net_device *dev)
 		priv->mb_size = sizeof(struct flexcan_mb) + CANFD_MAX_DLEN;
 	else

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

Quoted 10 lines
drivers/net/can/flexcan.c +7 -2
 		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_MB_CNT_MCF;
+	} else {
+		priv->mb_count = (sizeof(priv->regs->mb[0]) / priv->mb_size) +
+				 (sizeof(priv->regs->mb[1]) / priv->mb_size);
+	}

nitpick: no { } needed

Quoted 7 lines
drivers/net/can/flexcan.c +0 -0
 	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
 		priv->tx_mb_reserved =
@@ -1774,6 +1796,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 1808

Quoted 10 lines
drivers/net/can/flexcan.c +10 -0
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF) {
+		err = request_irq(dev->irq + FLEXCAN_OFFS_INT_BOFF,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;
+
+		err = request_irq(dev->irq + FLEXCAN_OFFS_INT_ERR,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;

I'm missing the free_irq() in error handling and the flexcan_close() function. Please test your driver if it work still after several modprobe rmmod cycles.

Quoted 20 lines
drivers/net/can/flexcan.c +4 -1
+	}
+
 	flexcan_chip_interrupts_enable(dev);
 
 	can_led_event(dev, CAN_LED_EVENT_OPEN);
@@ -2047,7 +2081,9 @@ static int flexcan_probe(struct platform_device *pdev)
 	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;
+	bool big_endian_module;
 	u8 clk_src = 1;
 	u32 clock_freq = 0;
 
@@ -2059,11 +2095,17 @@ static int flexcan_probe(struct platform_device *pdev)
 	else if (IS_ERR(reg_xceiver))
 		return PTR_ERR(reg_xceiver);
 
-	if (pdev->dev.of_node) {

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

Quoted 10 lines
drivers/net/can/flexcan.c +6 -4
-		of_property_read_u32(pdev->dev.of_node,
-				     "clock-frequency", &clock_freq);
-		of_property_read_u8(pdev->dev.of_node,
-				    "fsl,clk-source", &clk_src);
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata) {
+		clock_freq = pdata->clock_frequency;
+		clk_src = pdata->clk_src;
+	} else {
+		if (pdev->dev.of_node) {

else if

Quoted 9 lines
drivers/net/can/flexcan.c +5 -0
+			of_property_read_u32(pdev->dev.of_node,
+					     "clock-frequency", &clock_freq);
+			of_property_read_u8(pdev->dev.of_node,
+					    "fsl,clk-source", &clk_src);
+		}
 	}
 
 	if (!clock_freq) {

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

Quoted 6 lines
drivers/net/can/flexcan.c +3 -0
@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device *pdev)
 
 	devtype_data = of_device_get_match_data(&pdev->dev);
 
+	if (pdata && !devtype_data) {
+		devtype_data =
+			(struct flexcan_devtype_data *)&fsl_mcf_devtype_data;

That looks broken, see 2c0ac9208135 how to do this properly. Please add a name "flexcan-mcf" to the id_table and use fsl_mcf_devtype_data as driver_data.

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

Quoted 13 lines
drivers/net/can/flexcan.c +4 -2
+	}
+
 	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
 	    !(devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)) {
 		dev_err(&pdev->dev, "CAN-FD mode doesn't work with FIFO mode!\n");
@@ -2110,8 +2157,11 @@ static int flexcan_probe(struct platform_device *pdev)
 
 	priv = netdev_priv(dev);
 
-	if (of_property_read_bool(pdev->dev.of_node, "big-endian") ||
-	    devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN) {
+	big_endian_module = pdata ? pdata->big_endian :
+		of_property_read_bool(pdev->dev.of_node, "big-endian");

Please add the FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN to you fsl_mcf_devtype_data if needed. There's no need to add a 3rd way of configuring this.

Quoted 10 lines
drivers/net/can/flexcan.c +3 -0
+
+	if (big_endian_module ||
+		(devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN)) {
 		priv->read = flexcan_read_be;
 		priv->write = flexcan_write_be;
 	} else {
Signature · 1 line
2.31.1

Marc

Signature · 4 lines
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 |
#14 lore raw

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

Hi Mark,

thanks for the feedbacks,

Quoted 11 lines · Marc Kleine-Budde
Quoted 6 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,

Please don't hard code the 2 extra interrupts as offsets, but create proper IRQ resources.

ok

Quoted 10 lines
Quoted 3 lines

- implements 16 mb only,
- m68k architecture is not supporting devicetrees, so a platform data check/case has been added,

You might want to revert:

| 2c0ac9208135 can: flexcan: convert the driver to DT-only

and use it as a starting point.

ok

Quoted 84 lines
Quoted 84 lines

- ColdFire is m68k, so big-endian cpu, with a little-endian flexcan module.

---
  drivers/net/can/flexcan.c | 80 +++++++++++++++++++++++++++++++--------
  1 file changed, 65 insertions(+), 15 deletions(-)
drivers/net/can/flexcan.c +24 -8
index 57f3635ad8d7..af95273327cd 100644
@@ -28,6 +28,7 @@
  #include <linux/of_device.h>
  #include <linux/pinctrl/consumer.h>
  #include <linux/platform_device.h>
+#include <linux/platform_data/flexcan-mcf.h>
  #include <linux/pm_runtime.h>
  #include <linux/regmap.h>
  #include <linux/regulator/consumer.h>
@@ -205,6 +206,10 @@
 
  #define FLEXCAN_TIMEOUT_US		(250)
 
+#define FLEXCAN_OFFS_INT_BOFF		1
+#define FLEXCAN_OFFS_INT_ERR		3
+#define FLEXCAN_MB_CNT_MCF		16
+
  /* FLEXCAN hardware feature flags
   *
   * Below is some version info we got:
@@ -246,6 +251,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 {
@@ -371,6 +378,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 +650,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 +1418,13 @@ 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);
+
Quoted 9 lines

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

Quoted 6 lines
drivers/net/can/flexcan.c +6 -0
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF) {
+		priv->mb_count = FLEXCAN_MB_CNT_MCF;
+	} else {
+		priv->mb_count = (sizeof(priv->regs->mb[0]) / priv->mb_size) +
+				 (sizeof(priv->regs->mb[1]) / priv->mb_size);
+	}

nitpick: no { } needed

ok

Quoted 7 lines
Quoted 7 lines
drivers/net/can/flexcan.c +0 -0
  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
  		priv->tx_mb_reserved =
@@ -1774,6 +1796,18 @@ static int flexcan_open(struct net_device *dev)
  	if (err)
  		goto out_can_rx_offload_disable;
 
Quoted 15 lines

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

Quoted 10 lines
drivers/net/can/flexcan.c +10 -0
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_MCF) {
+		err = request_irq(dev->irq + FLEXCAN_OFFS_INT_BOFF,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;
+
+		err = request_irq(dev->irq + FLEXCAN_OFFS_INT_ERR,
+				  flexcan_irq, IRQF_SHARED, dev->name, dev);
+		if (err)
+			goto out_can_rx_offload_disable;

I'm missing the free_irq() in error handling and the flexcan_close() function. Please test your driver if it work still after several modprobe rmmod cycles.

forgot, will fix

Quoted 19 lines
Quoted 19 lines
drivers/net/can/flexcan.c +4 -0
+	}
+
  	flexcan_chip_interrupts_enable(dev);
 
  	can_led_event(dev, CAN_LED_EVENT_OPEN);
@@ -2047,7 +2081,9 @@ static int flexcan_probe(struct platform_device *pdev)
  	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;
+	bool big_endian_module;
  	u8 clk_src = 1;
  	u32 clock_freq = 0;
 
@@ -2059,11 +2095,17 @@ static int flexcan_probe(struct platform_device *pdev)
  	else if (IS_ERR(reg_xceiver))
  		return PTR_ERR(reg_xceiver);
 
Quoted 14 lines

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

Quoted 11 lines
drivers/net/can/flexcan.c +6 -5
-	if (pdev->dev.of_node) {
-		of_property_read_u32(pdev->dev.of_node,
-				     "clock-frequency", &clock_freq);
-		of_property_read_u8(pdev->dev.of_node,
-				    "fsl,clk-source", &clk_src);
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata) {
+		clock_freq = pdata->clock_frequency;
+		clk_src = pdata->clk_src;
+	} else {
+		if (pdev->dev.of_node) {

else if

ok

Quoted 12 lines
Quoted 12 lines
drivers/net/can/flexcan.c +5 -0
+			of_property_read_u32(pdev->dev.of_node,
+					     "clock-frequency", &clock_freq);
+			of_property_read_u8(pdev->dev.of_node,
+					    "fsl,clk-source", &clk_src);
+		}
  	}
 
  	if (!clock_freq) {
@@ -2091,6 +2133,11 @@ static int flexcan_probe(struct platform_device *pdev)
 
  	devtype_data = of_device_get_match_data(&pdev->dev);
 
Quoted 7 lines

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

Quoted 3 lines
drivers/net/can/flexcan.c +3 -0
+	if (pdata && !devtype_data) {
+		devtype_data =
+			(struct flexcan_devtype_data *)&fsl_mcf_devtype_data;

That looks broken, see 2c0ac9208135 how to do this properly. Please add a name "flexcan-mcf" to the id_table and use fsl_mcf_devtype_data as driver_data.

ok

Quoted 10 lines
Quoted 10 lines
drivers/net/can/flexcan.c +2 -0
+	}
+
  	if ((devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_FD) &&
  	    !(devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)) {
  		dev_err(&pdev->dev, "CAN-FD mode doesn't work with FIFO mode!\n");
@@ -2110,8 +2157,11 @@ static int flexcan_probe(struct platform_device *pdev)
 
  	priv = netdev_priv(dev);
 
Quoted 8 lines

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

Quoted 4 lines
drivers/net/can/flexcan.c +2 -2
-	if (of_property_read_bool(pdev->dev.of_node, "big-endian") ||
-	    devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN) {
+	big_endian_module = pdata ? pdata->big_endian :
+		of_property_read_bool(pdev->dev.of_node, "big-endian");

Please add the FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN to you fsl_mcf_devtype_data if needed. There's no need to add a 3rd way of configuring this.

i think that quirk is for big endian flexcan modules. In my case, cpu is big endian but flexcan registers are little endian, thgis is why i am not using that quirk.

Quoted 14 lines
Quoted 11 lines
drivers/net/can/flexcan.c +3 -0
+
+	if (big_endian_module ||
+		(devtype_data->quirks & FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN)) {
  		priv->read = flexcan_read_be;
  		priv->write = flexcan_write_be;
  	} else {
Signature · 1 line
2.31.1

Marc

will fix all to a v2

Thanks !
Regards,

Signature · 4 lines
Angelo Dureghello
+++ kernelspace +++
+E: angelo AT kernel-space.org
+W: www.kernel-space.org
#15 lore raw
In reply to Angelo Dureghello

include/linux/platform_data/flexcan-mcf.h

Quoted 24 lines

-----Original Message-----
From: Angelo Dureghello <[email protected]>
Sent: 2021年6月9日 4:46
To: [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Joakim Zhang <[email protected]>; Angelo Dureghello <[email protected]>
Subject: [PATCH 1/5] can: flexcan: add platform data for ColdFire

Add platform data object for ColdFire architecture.

---
 include/linux/platform_data/flexcan-mcf.h | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 include/linux/platform_data/flexcan-mcf.h
include/linux/platform_data/flexcan-mcf.h new file +0 -0
new file mode 100644
index 000000000000..71fe1a9df084

Should locate CAN header file here? include/linux/can/platform/

Best Regards,
Joakim Zhang

Quoted 30 lines
include/linux/platform_data/flexcan-mcf.h new file +27 -1
@@ -0,0 +1,27 @@
+/* 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
+
+struct mcf_flexcan_platform_data {
+	int stop_mode;
+	int clk_src;
+	int clock_frequency;
+	bool big_endian;
+};
+
+#endif /* _PLAT_FLEXCAN_MCF_H */
--

2.31.1

#16 lore raw

Hi Joakim,

Quoted 18 lines · Joakim Zhang
Quoted 18 lines

-----Original Message-----
From: Angelo Dureghello <[email protected]>
Sent: 2021年6月9日 4:46
To: [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Joakim Zhang <[email protected]>; Angelo Dureghello <[email protected]>
Subject: [PATCH 1/5] can: flexcan: add platform data for ColdFire

Add platform data object for ColdFire architecture.

---
  include/linux/platform_data/flexcan-mcf.h | 27 +++++++++++++++++++++++
  1 file changed, 27 insertions(+)

create mode 100644 include/linux/platform_data/flexcan-mcf.h

Quoted 9 lines

include/linux/platform_data/flexcan-mcf.h

Quoted 6 lines
include/linux/platform_data/flexcan-mcf.h new file +0 -0
new file mode 100644
index 000000000000..71fe1a9df084

Should locate CAN header file here? include/linux/can/platform/

oh, missed that place, looks like that location is more correct, sure, can move the file there.

Quoted 24 lines

Best Regards,
Joakim Zhang

Quoted 22 lines
include/linux/platform_data/flexcan-mcf.h new file +21 -0
@@ -0,0 +1,27 @@
+/* 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
+
+struct mcf_flexcan_platform_data {
+	int stop_mode;
Quoted 9 lines
Quoted 8 lines
include/linux/platform_data/flexcan-mcf.h new file +6 -1
+	int clk_src;
+	int clock_frequency;
+	bool big_endian;
+};
+
+#endif /* _PLAT_FLEXCAN_MCF_H */
--

2.31.1

Regards

Signature · 4 lines
Angelo Dureghello
+++ kernelspace +++
+E: angelo AT kernel-space.org
+W: www.kernel-space.org
#17 lore raw
In reply to Angelo Dureghello

Hi Angelo,

Quoted 3 lines · Angelo Dureghello

Add platform data object for ColdFire architecture.

Thanks for your patch!

Quoted 14 lines
include/linux/platform_data/flexcan-mcf.h new file +11 -0
@@ -0,0 +1,27 @@
+/* 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,

include/linux/platform_data/flexcan-mcf.h line 21

Quoted 10 lines
include/linux/platform_data/flexcan-mcf.h new file +10 -0
+ * 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 {
+       int stop_mode;

Unused?

include/linux/platform_data/flexcan-mcf.h line 23

Quoted 2 lines
include/linux/platform_data/flexcan-mcf.h new file +2 -0
+       int clk_src;
+       int clock_frequency;

Might be worthwhile to match the types used in the driver (i.e. u8 and u32).

include/linux/platform_data/flexcan-mcf.h line 27

Quoted 4 lines
include/linux/platform_data/flexcan-mcf.h new file +4 -0
+       bool big_endian;
+};
+
+#endif /* _PLAT_FLEXCAN_MCF_H */

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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
#18 lore raw

Hi Geert,

Quoted 28 lines · Geert Uytterhoeven

Hi Angelo,

Quoted 3 lines · Angelo Dureghello

Add platform data object for ColdFire architecture.

Thanks for your patch!

Quoted 18 lines
include/linux/platform_data/flexcan-mcf.h new file +15 -0
@@ -0,0 +1,27 @@
+/* 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.
+ */
Quoted 8 lines

include/linux/platform_data/flexcan-mcf.h line 21

Quoted 6 lines
include/linux/platform_data/flexcan-mcf.h new file +6 -0
+
+#ifndef _PLAT_FLEXCAN_MCF_H
+#define _PLAT_FLEXCAN_MCF_H
+
+struct mcf_flexcan_platform_data {
+       int stop_mode;

Unused?

yes, unused for now, removed

Quoted 7 lines

include/linux/platform_data/flexcan-mcf.h line 23

Quoted 3 lines
include/linux/platform_data/flexcan-mcf.h new file +2 -0
+       int clk_src;
+       int clock_frequency;

Might be worthwhile to match the types used in the driver (i.e. u8 and u32).

done, thanks

Quoted 9 lines

include/linux/platform_data/flexcan-mcf.h line 27

Quoted 4 lines
include/linux/platform_data/flexcan-mcf.h new file +4 -0
+       bool big_endian;
+};
+
+#endif /* _PLAT_FLEXCAN_MCF_H */

Gr{oetje,eeting}s,

Geert

Regards,

Signature · 4 lines
Angelo Dureghello
+++ kernelspace +++
+E: angelo AT kernel-space.org
+W: www.kernel-space.org