All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Add flexcan support for LS1021A SoCs
@ 2015-05-14 11:33 ` Bhupesh Sharma
  0 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: arnd, linux-can, mkl
  Cc: bhupesh.linux, bhupesh.sharma, Sakar.Arora, linux-arm-kernel

This patchset add the support for FlexCAN module present
on LS1021A SoCs.

LS1021A-Rev1 SoC has a broken RX FIFO mode, thereby mandating
the use of message buffers for frame reception.

LS1021A-Rev2 SoC has this issue fixed.

Bhupesh Sharma (3):
  doc/bindings: Add 'endianess' optional-property for FlexCAN
    controller
  can: flexcan: Add ls1021a flexcan device entry
  can: flexcan: Add support for non RX-FIFO mode

Sakar Arora (2):
  arm/dts: Add nodes for flexcan devices present on LS1021A SoC
  can: flexcan: Remodel FlexCAN register r/w APIs for BE instances

 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    4 +
 arch/arm/boot/dts/ls1021a-qds.dts                  |    8 +
 arch/arm/boot/dts/ls1021a.dtsi                     |   18 +
 drivers/net/can/flexcan.c                          |  400 +++++++++++++-------
 4 files changed, 299 insertions(+), 131 deletions(-)

-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 0/5] Add flexcan support for LS1021A SoCs
@ 2015-05-14 11:33 ` Bhupesh Sharma
  0 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset add the support for FlexCAN module present
on LS1021A SoCs.

LS1021A-Rev1 SoC has a broken RX FIFO mode, thereby mandating
the use of message buffers for frame reception.

LS1021A-Rev2 SoC has this issue fixed.

Bhupesh Sharma (3):
  doc/bindings: Add 'endianess' optional-property for FlexCAN
    controller
  can: flexcan: Add ls1021a flexcan device entry
  can: flexcan: Add support for non RX-FIFO mode

Sakar Arora (2):
  arm/dts: Add nodes for flexcan devices present on LS1021A SoC
  can: flexcan: Remodel FlexCAN register r/w APIs for BE instances

 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    4 +
 arch/arm/boot/dts/ls1021a-qds.dts                  |    8 +
 arch/arm/boot/dts/ls1021a.dtsi                     |   18 +
 drivers/net/can/flexcan.c                          |  400 +++++++++++++-------
 4 files changed, 299 insertions(+), 131 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 1/5] doc/bindings: Add 'endianess' optional-property for FlexCAN controller
  2015-05-14 11:33 ` Bhupesh Sharma
@ 2015-05-14 11:33   ` Bhupesh Sharma
  -1 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: arnd, linux-can, mkl
  Cc: bhupesh.sharma, bhupesh.linux, linux-arm-kernel, Sakar.Arora

This patch adds 'endianess' as the optional-property for
describing the FlexCAN controller present on various FSL platforms.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index 56d6cc3..0d5e668 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -18,6 +18,10 @@ Optional properties:
 
 - xceiver-supply: Regulator that powers the CAN transceiver
 
+- little-endian: If the FlexCAN IP on this SoC is little-endian, use
+                 this property. By default it is assumed that the IP
+                 is big-endian
+
 Example:
 
 	can@1c000 {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 1/5] doc/bindings: Add 'endianess' optional-property for FlexCAN controller
@ 2015-05-14 11:33   ` Bhupesh Sharma
  0 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds 'endianess' as the optional-property for
describing the FlexCAN controller present on various FSL platforms.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index 56d6cc3..0d5e668 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -18,6 +18,10 @@ Optional properties:
 
 - xceiver-supply: Regulator that powers the CAN transceiver
 
+- little-endian: If the FlexCAN IP on this SoC is little-endian, use
+                 this property. By default it is assumed that the IP
+                 is big-endian
+
 Example:
 
 	can at 1c000 {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 2/5] arm/dts: Add nodes for flexcan devices present on LS1021A SoC
  2015-05-14 11:33 ` Bhupesh Sharma
@ 2015-05-14 11:33   ` Bhupesh Sharma
  -1 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: arnd, linux-can, mkl
  Cc: bhupesh.linux, bhupesh.sharma, Sakar.Arora, linux-arm-kernel

From: Sakar Arora <Sakar.Arora@freescale.com>

This patch adds the device nodes for flexcan controller(s) present
on LS1021A SoC.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
---
 arch/arm/boot/dts/ls1021a-qds.dts |    8 ++++++++
 arch/arm/boot/dts/ls1021a.dtsi    |   18 ++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/ls1021a-qds.dts b/arch/arm/boot/dts/ls1021a-qds.dts
index 9c5e16b..1641a04 100644
--- a/arch/arm/boot/dts/ls1021a-qds.dts
+++ b/arch/arm/boot/dts/ls1021a-qds.dts
@@ -238,3 +238,11 @@
 &uart1 {
 	status = "okay";
 };
+
+&can0 {
+	status = "okay";
+};
+
+&can1 {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index c70bb27..3fb7b52 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -332,6 +332,24 @@
 			status = "disabled";
 		};
 
+		can0: can@2a70000 {
+			compatible = "fsl,ls1021a-flexcan";
+			reg = <0x0 0x2a70000 0x0 0x1000>;
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&platform_clk 1>, <&platform_clk 1>;;
+			clock-names = "ipg", "per";
+			little-endian;
+		};
+
+		can1: can@2a80000 {
+			compatible = "fsl,ls1021a-flexcan";
+			reg = <0x0 0x2a80000 0x0 0x1000>;
+			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&platform_clk 1>, <&platform_clk 1>;;
+			clock-names = "ipg", "per";
+			little-endian;
+		};
+
 		wdog0: watchdog@2ad0000 {
 			compatible = "fsl,imx21-wdt";
 			reg = <0x0 0x2ad0000 0x0 0x10000>;
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 2/5] arm/dts: Add nodes for flexcan devices present on LS1021A SoC
@ 2015-05-14 11:33   ` Bhupesh Sharma
  0 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

From: Sakar Arora <Sakar.Arora@freescale.com>

This patch adds the device nodes for flexcan controller(s) present
on LS1021A SoC.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
---
 arch/arm/boot/dts/ls1021a-qds.dts |    8 ++++++++
 arch/arm/boot/dts/ls1021a.dtsi    |   18 ++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/ls1021a-qds.dts b/arch/arm/boot/dts/ls1021a-qds.dts
index 9c5e16b..1641a04 100644
--- a/arch/arm/boot/dts/ls1021a-qds.dts
+++ b/arch/arm/boot/dts/ls1021a-qds.dts
@@ -238,3 +238,11 @@
 &uart1 {
 	status = "okay";
 };
+
+&can0 {
+	status = "okay";
+};
+
+&can1 {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index c70bb27..3fb7b52 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -332,6 +332,24 @@
 			status = "disabled";
 		};
 
+		can0: can at 2a70000 {
+			compatible = "fsl,ls1021a-flexcan";
+			reg = <0x0 0x2a70000 0x0 0x1000>;
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&platform_clk 1>, <&platform_clk 1>;;
+			clock-names = "ipg", "per";
+			little-endian;
+		};
+
+		can1: can at 2a80000 {
+			compatible = "fsl,ls1021a-flexcan";
+			reg = <0x0 0x2a80000 0x0 0x1000>;
+			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&platform_clk 1>, <&platform_clk 1>;;
+			clock-names = "ipg", "per";
+			little-endian;
+		};
+
 		wdog0: watchdog at 2ad0000 {
 			compatible = "fsl,imx21-wdt";
 			reg = <0x0 0x2ad0000 0x0 0x10000>;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry
  2015-05-14 11:33 ` Bhupesh Sharma
@ 2015-05-14 11:33   ` Bhupesh Sharma
  -1 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: arnd, linux-can, mkl
  Cc: bhupesh.linux, bhupesh.sharma, Sakar.Arora, linux-arm-kernel

This patch adds ls1021a flexcan device entry to the flexcan driver code.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
---
 drivers/net/can/flexcan.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index ad0a7e8..deaa24e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -267,10 +267,26 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data;
 static struct flexcan_devtype_data fsl_imx6q_devtype_data = {
 	.features = FLEXCAN_HAS_V10_FEATURES,
 };
+
 static struct flexcan_devtype_data fsl_vf610_devtype_data = {
 	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES,
 };
 
+/* LS1021A-Rev1 has a broken RX-FIFO support. So only legacy RX message-buffers
+ * work here.
+ */
+static struct flexcan_devtype_data fsl_ls1021a_devtype_data = {
+	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES |
+		    FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT,
+};
+
+/* LS1021A-Rev2 has functional RX-FIFO mode, so no need to fall back to
+ * the legacy mode.
+ */
+static struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
+	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES,
+};
+
 static const struct can_bittiming_const flexcan_bittiming_const = {
 	.name = DRV_NAME,
 	.tseg1_min = 4,
@@ -1139,6 +1155,10 @@ static void unregister_flexcandev(struct net_device *dev)
 static const struct of_device_id flexcan_of_match[] = {
 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
+	{ .compatible = "fsl,ls1021a-flexcan",
+	  .data = &fsl_ls1021a_devtype_data, },
+	{ .compatible = "fsl,ls1021ar2-flexcan",
+	  .data = &fsl_ls1021a_r2_devtype_data, },
 	{ .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
 	{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, },
 	{ /* sentinel */ },
-- 
1.7.9.5



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry
@ 2015-05-14 11:33   ` Bhupesh Sharma
  0 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds ls1021a flexcan device entry to the flexcan driver code.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
---
 drivers/net/can/flexcan.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index ad0a7e8..deaa24e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -267,10 +267,26 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data;
 static struct flexcan_devtype_data fsl_imx6q_devtype_data = {
 	.features = FLEXCAN_HAS_V10_FEATURES,
 };
+
 static struct flexcan_devtype_data fsl_vf610_devtype_data = {
 	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES,
 };
 
+/* LS1021A-Rev1 has a broken RX-FIFO support. So only legacy RX message-buffers
+ * work here.
+ */
+static struct flexcan_devtype_data fsl_ls1021a_devtype_data = {
+	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES |
+		    FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT,
+};
+
+/* LS1021A-Rev2 has functional RX-FIFO mode, so no need to fall back to
+ * the legacy mode.
+ */
+static struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
+	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES,
+};
+
 static const struct can_bittiming_const flexcan_bittiming_const = {
 	.name = DRV_NAME,
 	.tseg1_min = 4,
@@ -1139,6 +1155,10 @@ static void unregister_flexcandev(struct net_device *dev)
 static const struct of_device_id flexcan_of_match[] = {
 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
+	{ .compatible = "fsl,ls1021a-flexcan",
+	  .data = &fsl_ls1021a_devtype_data, },
+	{ .compatible = "fsl,ls1021ar2-flexcan",
+	  .data = &fsl_ls1021a_r2_devtype_data, },
 	{ .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
 	{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, },
 	{ /* sentinel */ },
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances
  2015-05-14 11:33 ` Bhupesh Sharma
@ 2015-05-14 11:33   ` Bhupesh Sharma
  -1 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: arnd, linux-can, mkl
  Cc: bhupesh.sharma, bhupesh.linux, linux-arm-kernel, Sakar.Arora

From: Sakar Arora <Sakar.Arora@freescale.com>

The FlexCAN IP on certain SoCs like (Freescale's LS1021A) is modelled
in a big-endian fashion, i.e. the registers and the message buffers are
organized in a BE way.

More details about the LS1021A SoC can be seen here:
http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=LS1021A&nodeId=018rH325E4017B#

This patch ensures that the register read/write APIs are remodelled to
address such cases, while ensuring that existing platforms (where the
FlexCAN IP was modelled in LE way) do not break.

Tested on LS1021A-QDS board.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
---
 drivers/net/can/flexcan.c |  208 ++++++++++++++++++++++++++-------------------
 1 file changed, 119 insertions(+), 89 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index deaa24e..b0222ae 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -258,6 +258,10 @@ struct flexcan_priv {
 	struct flexcan_platform_data *pdata;
 	const struct flexcan_devtype_data *devtype_data;
 	struct regulator *reg_xceiver;
+
+	/* Read and Write APIs */
+	u32 (*read)(void __iomem *addr);
+	void (*write)(u32 val, void __iomem *addr);
 };
 
 static struct flexcan_devtype_data fsl_p1010_devtype_data = {
@@ -300,32 +304,38 @@ static const struct can_bittiming_const flexcan_bittiming_const = {
 };
 
 /*
- * Abstract off the read/write for arm versus ppc. This
- * assumes that PPC uses big-endian registers and everything
- * else uses little-endian registers, independent of CPU
- * endianess.
+ * FlexCAN module is essentially modelled as a little-endian IP in most
+ * SoCs, i.e the registers as well as the message buffer areas are
+ * implemented in a little-endian fashion.
+ *
+ * However there are some SoCs (e.g. LS1021A) which implement the FlexCAN
+ * module in a big-endian fashion (i.e the registers as well as the
+ * message buffer areas are implemented in a big-endian way).
+ *
+ * In addition, the FlexCAN module can be found on SoCs having ARM or
+ * PPC cores. So, we need to abstract off the register read/write
+ * functions, ensuring that these cater to all the combinations of module
+ * endianness and underlying CPU endianness.
  */
-#if defined(CONFIG_PPC)
-static inline u32 flexcan_read(void __iomem *addr)
+static inline u32 flexcan_read_le(void __iomem *addr)
 {
-	return in_be32(addr);
+	return ioread32(addr);
 }
 
-static inline void flexcan_write(u32 val, void __iomem *addr)
+static inline void flexcan_write_le(u32 val, void __iomem *addr)
 {
-	out_be32(addr, val);
+	iowrite32(val, addr);
 }
-#else
-static inline u32 flexcan_read(void __iomem *addr)
+
+static inline u32 flexcan_read_be(void __iomem *addr)
 {
-	return readl(addr);
+	return ioread32be(addr);
 }
 
-static inline void flexcan_write(u32 val, void __iomem *addr)
+static inline void flexcan_write_be(u32 val, void __iomem *addr)
 {
-	writel(val, addr);
+	iowrite32be(val, addr);
 }
-#endif
 
 static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
 {
@@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct flexcan_priv *priv)
 	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
 	u32 reg;
 
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg &= ~FLEXCAN_MCR_MDIS;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
-	while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
+	while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
 		udelay(10);
 
-	if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
+	if (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
 		return -ETIMEDOUT;
 
 	return 0;
@@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct flexcan_priv *priv)
 	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
 	u32 reg;
 
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg |= FLEXCAN_MCR_MDIS;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
-	while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
+	while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
 		udelay(10);
 
-	if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
+	if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
 		return -ETIMEDOUT;
 
 	return 0;
@@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv)
 	unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate;
 	u32 reg;
 
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg |= FLEXCAN_MCR_HALT;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
-	while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+	while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
 		udelay(100);
 
-	if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+	if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
 		return -ETIMEDOUT;
 
 	return 0;
@@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv)
 	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
 	u32 reg;
 
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg &= ~FLEXCAN_MCR_HALT;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
-	while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+	while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
 		udelay(10);
 
-	if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
+	if (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
 		return -ETIMEDOUT;
 
 	return 0;
@@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv)
 	struct flexcan_regs __iomem *regs = priv->base;
 	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
 
-	flexcan_write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
-	while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST))
+	priv->write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
+	while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST))
 		udelay(10);
 
-	if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
+	if (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
 		return -ETIMEDOUT;
 
 	return 0;
 }
 
-
 static int __flexcan_get_berr_counter(const struct net_device *dev,
 				      struct can_berr_counter *bec)
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
-	u32 reg = flexcan_read(&regs->ecr);
+	u32 reg = priv->read(&regs->ecr);
 
 	bec->txerr = (reg >> 0) & 0xff;
 	bec->rxerr = (reg >> 8) & 0xff;
@@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	if (cf->can_dlc > 0) {
 		u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
-		flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
+		priv->write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
 	}
 	if (cf->can_dlc > 3) {
 		u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
-		flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
+		priv->write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
 	}
 
 	can_put_echo_skb(skb, dev, 0);
 
-	flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
-	flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+	priv->write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
+	priv->write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 
 	/* Errata ERR005829 step8:
 	 * Write twice INACTIVE(0x8) code to first MB.
 	 */
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
 
 	return NETDEV_TX_OK;
@@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct net_device *dev,
 	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
 	u32 reg_ctrl, reg_id;
 
-	reg_ctrl = flexcan_read(&mb->can_ctrl);
-	reg_id = flexcan_read(&mb->can_id);
+	reg_ctrl = priv->read(&mb->can_ctrl);
+	reg_id = priv->read(&mb->can_id);
 	if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
 		cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
 	else
@@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct net_device *dev,
 		cf->can_id |= CAN_RTR_FLAG;
 	cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
 
-	*(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
-	*(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
+	*(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0]));
+	*(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1]));
 
 	/* mark as read */
-	flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
-	flexcan_read(&regs->timer);
+	priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+	priv->read(&regs->timer);
 }
 
 static int flexcan_read_frame(struct net_device *dev)
@@ -699,17 +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	 * The error bits are cleared on read,
 	 * use saved value from irq handler.
 	 */
-	reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
+	reg_esr = priv->read(&regs->esr) | priv->reg_esr;
 
 	/* handle state changes */
 	work_done += flexcan_poll_state(dev, reg_esr);
 
 	/* handle RX-FIFO */
-	reg_iflag1 = flexcan_read(&regs->iflag1);
+	reg_iflag1 = priv->read(&regs->iflag1);
 	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
 	       work_done < quota) {
 		work_done += flexcan_read_frame(dev);
-		reg_iflag1 = flexcan_read(&regs->iflag1);
+		reg_iflag1 = priv->read(&regs->iflag1);
 	}
 
 	/* report bus errors */
@@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	if (work_done < quota) {
 		napi_complete(napi);
 		/* enable IRQs */
-		flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
-		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
+		priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+		priv->write(priv->reg_ctrl_default, &regs->ctrl);
 	}
 
 	return work_done;
@@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
 
-	reg_iflag1 = flexcan_read(&regs->iflag1);
-	reg_esr = flexcan_read(&regs->esr);
+	reg_iflag1 = priv->read(&regs->iflag1);
+	reg_esr = priv->read(&regs->esr);
 	/* ACK all bus error and state change IRQ sources */
 	if (reg_esr & FLEXCAN_ESR_ALL_INT)
-		flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
+		priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
 
 	/*
 	 * schedule NAPI in case of:
@@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		 * save them for later use.
 		 */
 		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
-		flexcan_write(FLEXCAN_IFLAG_DEFAULT &
+		priv->write(FLEXCAN_IFLAG_DEFAULT &
 			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
-		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
+		priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 		       &regs->ctrl);
 		napi_schedule(&priv->napi);
 	}
 
 	/* FIFO overflow */
 	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
-		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
+		priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
 		dev->stats.rx_over_errors++;
 		dev->stats.rx_errors++;
 	}
@@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		stats->tx_packets++;
 		can_led_event(dev, CAN_LED_EVENT_TX);
 		/* after sending a RTR frame mailbox is in RX mode */
-		flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+		priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 			      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
-		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
+		priv->write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
 		netif_wake_queue(dev);
 	}
 
@@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg;
 
-	reg = flexcan_read(&regs->ctrl);
+	reg = priv->read(&regs->ctrl);
 	reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
 		 FLEXCAN_CTRL_RJW(0x3) |
 		 FLEXCAN_CTRL_PSEG1(0x7) |
@@ -814,11 +823,11 @@ static void flexcan_set_bittiming(struct net_device *dev)
 		reg |= FLEXCAN_CTRL_SMP;
 
 	netdev_info(dev, "writing ctrl=0x%08x\n", reg);
-	flexcan_write(reg, &regs->ctrl);
+	priv->write(reg, &regs->ctrl);
 
 	/* print chip status */
 	netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
-		   flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
+		   priv->read(&regs->mcr), priv->read(&regs->ctrl));
 }
 
 /*
@@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * disable local echo
 	 *
 	 */
-	reg_mcr = flexcan_read(&regs->mcr);
+	reg_mcr = priv->read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
 		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
 		FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
 	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
-	flexcan_write(reg_mcr, &regs->mcr);
+	priv->write(reg_mcr, &regs->mcr);
 
 	/*
 	 * CTRL
@@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * enable bus off interrupt
 	 * (== FLEXCAN_CTRL_ERR_STATE)
 	 */
-	reg_ctrl = flexcan_read(&regs->ctrl);
+	reg_ctrl = priv->read(&regs->ctrl);
 	reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
 	reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
 		FLEXCAN_CTRL_ERR_STATE;
@@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device *dev)
 	/* save for later use */
 	priv->reg_ctrl_default = reg_ctrl;
 	netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
-	flexcan_write(reg_ctrl, &regs->ctrl);
+	priv->write(reg_ctrl, &regs->ctrl);
 
 	/* clear and invalidate all mailboxes first */
 	for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) {
-		flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
+		priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
 			      &regs->cantxfg[i].can_ctrl);
 	}
 
 	/* Errata ERR005829: mark first TX mailbox as INACTIVE */
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
 
 	/* mark TX mailbox as INACTIVE */
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 
 	/* acceptance mask/acceptance code (accept everything) */
-	flexcan_write(0x0, &regs->rxgmask);
-	flexcan_write(0x0, &regs->rx14mask);
-	flexcan_write(0x0, &regs->rx15mask);
+	priv->write(0x0, &regs->rxgmask);
+	priv->write(0x0, &regs->rx14mask);
+	priv->write(0x0, &regs->rx15mask);
 
 	if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
-		flexcan_write(0x0, &regs->rxfgmask);
+		priv->write(0x0, &regs->rxfgmask);
 
 	/*
 	 * On Vybrid, disable memory error detection interrupts
@@ -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device *dev)
 		 * and Correction of Memory Errors" to write to
 		 * MECR register
 		 */
-		reg_crl2 = flexcan_read(&regs->crl2);
+		reg_crl2 = priv->read(&regs->crl2);
 		reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
-		flexcan_write(reg_crl2, &regs->crl2);
+		priv->write(reg_crl2, &regs->crl2);
 
-		reg_mecr = flexcan_read(&regs->mecr);
+		reg_mecr = priv->read(&regs->mecr);
 		reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
-		flexcan_write(reg_mecr, &regs->mecr);
+		priv->write(reg_mecr, &regs->mecr);
 		reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK |
 				FLEXCAN_MECR_FANCEI_MSK);
-		flexcan_write(reg_mecr, &regs->mecr);
+		priv->write(reg_mecr, &regs->mecr);
 	}
 
 	err = flexcan_transceiver_enable(priv);
@@ -958,11 +967,11 @@ static int flexcan_chip_start(struct net_device *dev)
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	/* enable FIFO interrupts */
-	flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+	priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
 
 	/* print chip status */
 	netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
-		   flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
+		   priv->read(&regs->mcr), priv->read(&regs->ctrl));
 
 	return 0;
 
@@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device *dev)
 	flexcan_chip_disable(priv);
 
 	/* Disable all interrupts */
-	flexcan_write(0, &regs->imask1);
-	flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
+	priv->write(0, &regs->imask1);
+	priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 		      &regs->ctrl);
 
 	flexcan_transceiver_disable(priv);
@@ -1108,26 +1117,26 @@ static int register_flexcandev(struct net_device *dev)
 	err = flexcan_chip_disable(priv);
 	if (err)
 		goto out_disable_per;
-	reg = flexcan_read(&regs->ctrl);
+	reg = priv->read(&regs->ctrl);
 	reg |= FLEXCAN_CTRL_CLK_SRC;
-	flexcan_write(reg, &regs->ctrl);
+	priv->write(reg, &regs->ctrl);
 
 	err = flexcan_chip_enable(priv);
 	if (err)
 		goto out_chip_disable;
 
 	/* set freeze, halt and activate FIFO, restrict register access */
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
 	/*
 	 * Currently we only support newer versions of this core
 	 * featuring a RX FIFO. Older cores found on some Coldfire
 	 * derivates are not yet supported.
 	 */
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	if (!(reg & FLEXCAN_MCR_FEN)) {
 		netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
 		err = -ENODEV;
@@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int err, irq;
 	u32 clock_freq = 0;
+	/* Default case for most ARM based FSL SoC having BE FlexCAN IP */
+	bool core_is_little = true, module_is_little = false;
 
 	reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
 	if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER)
@@ -1237,6 +1248,25 @@ static int flexcan_probe(struct platform_device *pdev)
 	dev->flags |= IFF_ECHO;
 
 	priv = netdev_priv(dev);
+
+	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+		core_is_little = false;
+
+	if (of_property_read_bool(dev->dev.of_node, "little-endian"))
+		module_is_little = true;
+
+	if ((core_is_little && module_is_little) ||
+	    (!core_is_little && !module_is_little)) {
+		priv->read = flexcan_read_le;
+		priv->write = flexcan_write_le;
+	}
+
+	if ((!core_is_little && module_is_little) ||
+	    (core_is_little && !module_is_little)) {
+		priv->read = flexcan_read_be;
+		priv->write = flexcan_write_be;
+	}
+
 	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances
@ 2015-05-14 11:33   ` Bhupesh Sharma
  0 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

From: Sakar Arora <Sakar.Arora@freescale.com>

The FlexCAN IP on certain SoCs like (Freescale's LS1021A) is modelled
in a big-endian fashion, i.e. the registers and the message buffers are
organized in a BE way.

More details about the LS1021A SoC can be seen here:
http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=LS1021A&nodeId=018rH325E4017B#

This patch ensures that the register read/write APIs are remodelled to
address such cases, while ensuring that existing platforms (where the
FlexCAN IP was modelled in LE way) do not break.

Tested on LS1021A-QDS board.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
---
 drivers/net/can/flexcan.c |  208 ++++++++++++++++++++++++++-------------------
 1 file changed, 119 insertions(+), 89 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index deaa24e..b0222ae 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -258,6 +258,10 @@ struct flexcan_priv {
 	struct flexcan_platform_data *pdata;
 	const struct flexcan_devtype_data *devtype_data;
 	struct regulator *reg_xceiver;
+
+	/* Read and Write APIs */
+	u32 (*read)(void __iomem *addr);
+	void (*write)(u32 val, void __iomem *addr);
 };
 
 static struct flexcan_devtype_data fsl_p1010_devtype_data = {
@@ -300,32 +304,38 @@ static const struct can_bittiming_const flexcan_bittiming_const = {
 };
 
 /*
- * Abstract off the read/write for arm versus ppc. This
- * assumes that PPC uses big-endian registers and everything
- * else uses little-endian registers, independent of CPU
- * endianess.
+ * FlexCAN module is essentially modelled as a little-endian IP in most
+ * SoCs, i.e the registers as well as the message buffer areas are
+ * implemented in a little-endian fashion.
+ *
+ * However there are some SoCs (e.g. LS1021A) which implement the FlexCAN
+ * module in a big-endian fashion (i.e the registers as well as the
+ * message buffer areas are implemented in a big-endian way).
+ *
+ * In addition, the FlexCAN module can be found on SoCs having ARM or
+ * PPC cores. So, we need to abstract off the register read/write
+ * functions, ensuring that these cater to all the combinations of module
+ * endianness and underlying CPU endianness.
  */
-#if defined(CONFIG_PPC)
-static inline u32 flexcan_read(void __iomem *addr)
+static inline u32 flexcan_read_le(void __iomem *addr)
 {
-	return in_be32(addr);
+	return ioread32(addr);
 }
 
-static inline void flexcan_write(u32 val, void __iomem *addr)
+static inline void flexcan_write_le(u32 val, void __iomem *addr)
 {
-	out_be32(addr, val);
+	iowrite32(val, addr);
 }
-#else
-static inline u32 flexcan_read(void __iomem *addr)
+
+static inline u32 flexcan_read_be(void __iomem *addr)
 {
-	return readl(addr);
+	return ioread32be(addr);
 }
 
-static inline void flexcan_write(u32 val, void __iomem *addr)
+static inline void flexcan_write_be(u32 val, void __iomem *addr)
 {
-	writel(val, addr);
+	iowrite32be(val, addr);
 }
-#endif
 
 static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
 {
@@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct flexcan_priv *priv)
 	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
 	u32 reg;
 
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg &= ~FLEXCAN_MCR_MDIS;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
-	while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
+	while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
 		udelay(10);
 
-	if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
+	if (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
 		return -ETIMEDOUT;
 
 	return 0;
@@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct flexcan_priv *priv)
 	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
 	u32 reg;
 
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg |= FLEXCAN_MCR_MDIS;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
-	while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
+	while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
 		udelay(10);
 
-	if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
+	if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
 		return -ETIMEDOUT;
 
 	return 0;
@@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv)
 	unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate;
 	u32 reg;
 
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg |= FLEXCAN_MCR_HALT;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
-	while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+	while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
 		udelay(100);
 
-	if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+	if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
 		return -ETIMEDOUT;
 
 	return 0;
@@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv)
 	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
 	u32 reg;
 
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg &= ~FLEXCAN_MCR_HALT;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
-	while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
+	while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
 		udelay(10);
 
-	if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
+	if (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
 		return -ETIMEDOUT;
 
 	return 0;
@@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv)
 	struct flexcan_regs __iomem *regs = priv->base;
 	unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
 
-	flexcan_write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
-	while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST))
+	priv->write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
+	while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST))
 		udelay(10);
 
-	if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
+	if (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
 		return -ETIMEDOUT;
 
 	return 0;
 }
 
-
 static int __flexcan_get_berr_counter(const struct net_device *dev,
 				      struct can_berr_counter *bec)
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
-	u32 reg = flexcan_read(&regs->ecr);
+	u32 reg = priv->read(&regs->ecr);
 
 	bec->txerr = (reg >> 0) & 0xff;
 	bec->rxerr = (reg >> 8) & 0xff;
@@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	if (cf->can_dlc > 0) {
 		u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
-		flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
+		priv->write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
 	}
 	if (cf->can_dlc > 3) {
 		u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
-		flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
+		priv->write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
 	}
 
 	can_put_echo_skb(skb, dev, 0);
 
-	flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
-	flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+	priv->write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
+	priv->write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 
 	/* Errata ERR005829 step8:
 	 * Write twice INACTIVE(0x8) code to first MB.
 	 */
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
 
 	return NETDEV_TX_OK;
@@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct net_device *dev,
 	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
 	u32 reg_ctrl, reg_id;
 
-	reg_ctrl = flexcan_read(&mb->can_ctrl);
-	reg_id = flexcan_read(&mb->can_id);
+	reg_ctrl = priv->read(&mb->can_ctrl);
+	reg_id = priv->read(&mb->can_id);
 	if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
 		cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
 	else
@@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct net_device *dev,
 		cf->can_id |= CAN_RTR_FLAG;
 	cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
 
-	*(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
-	*(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
+	*(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0]));
+	*(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1]));
 
 	/* mark as read */
-	flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
-	flexcan_read(&regs->timer);
+	priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+	priv->read(&regs->timer);
 }
 
 static int flexcan_read_frame(struct net_device *dev)
@@ -699,17 +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	 * The error bits are cleared on read,
 	 * use saved value from irq handler.
 	 */
-	reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
+	reg_esr = priv->read(&regs->esr) | priv->reg_esr;
 
 	/* handle state changes */
 	work_done += flexcan_poll_state(dev, reg_esr);
 
 	/* handle RX-FIFO */
-	reg_iflag1 = flexcan_read(&regs->iflag1);
+	reg_iflag1 = priv->read(&regs->iflag1);
 	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
 	       work_done < quota) {
 		work_done += flexcan_read_frame(dev);
-		reg_iflag1 = flexcan_read(&regs->iflag1);
+		reg_iflag1 = priv->read(&regs->iflag1);
 	}
 
 	/* report bus errors */
@@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	if (work_done < quota) {
 		napi_complete(napi);
 		/* enable IRQs */
-		flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
-		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
+		priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+		priv->write(priv->reg_ctrl_default, &regs->ctrl);
 	}
 
 	return work_done;
@@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
 
-	reg_iflag1 = flexcan_read(&regs->iflag1);
-	reg_esr = flexcan_read(&regs->esr);
+	reg_iflag1 = priv->read(&regs->iflag1);
+	reg_esr = priv->read(&regs->esr);
 	/* ACK all bus error and state change IRQ sources */
 	if (reg_esr & FLEXCAN_ESR_ALL_INT)
-		flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
+		priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
 
 	/*
 	 * schedule NAPI in case of:
@@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		 * save them for later use.
 		 */
 		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
-		flexcan_write(FLEXCAN_IFLAG_DEFAULT &
+		priv->write(FLEXCAN_IFLAG_DEFAULT &
 			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
-		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
+		priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 		       &regs->ctrl);
 		napi_schedule(&priv->napi);
 	}
 
 	/* FIFO overflow */
 	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
-		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
+		priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
 		dev->stats.rx_over_errors++;
 		dev->stats.rx_errors++;
 	}
@@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		stats->tx_packets++;
 		can_led_event(dev, CAN_LED_EVENT_TX);
 		/* after sending a RTR frame mailbox is in RX mode */
-		flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+		priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 			      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
-		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
+		priv->write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
 		netif_wake_queue(dev);
 	}
 
@@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg;
 
-	reg = flexcan_read(&regs->ctrl);
+	reg = priv->read(&regs->ctrl);
 	reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
 		 FLEXCAN_CTRL_RJW(0x3) |
 		 FLEXCAN_CTRL_PSEG1(0x7) |
@@ -814,11 +823,11 @@ static void flexcan_set_bittiming(struct net_device *dev)
 		reg |= FLEXCAN_CTRL_SMP;
 
 	netdev_info(dev, "writing ctrl=0x%08x\n", reg);
-	flexcan_write(reg, &regs->ctrl);
+	priv->write(reg, &regs->ctrl);
 
 	/* print chip status */
 	netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
-		   flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
+		   priv->read(&regs->mcr), priv->read(&regs->ctrl));
 }
 
 /*
@@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * disable local echo
 	 *
 	 */
-	reg_mcr = flexcan_read(&regs->mcr);
+	reg_mcr = priv->read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
 		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
 		FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
 	netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
-	flexcan_write(reg_mcr, &regs->mcr);
+	priv->write(reg_mcr, &regs->mcr);
 
 	/*
 	 * CTRL
@@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * enable bus off interrupt
 	 * (== FLEXCAN_CTRL_ERR_STATE)
 	 */
-	reg_ctrl = flexcan_read(&regs->ctrl);
+	reg_ctrl = priv->read(&regs->ctrl);
 	reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
 	reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
 		FLEXCAN_CTRL_ERR_STATE;
@@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device *dev)
 	/* save for later use */
 	priv->reg_ctrl_default = reg_ctrl;
 	netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
-	flexcan_write(reg_ctrl, &regs->ctrl);
+	priv->write(reg_ctrl, &regs->ctrl);
 
 	/* clear and invalidate all mailboxes first */
 	for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) {
-		flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
+		priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
 			      &regs->cantxfg[i].can_ctrl);
 	}
 
 	/* Errata ERR005829: mark first TX mailbox as INACTIVE */
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
 
 	/* mark TX mailbox as INACTIVE */
-	flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
+	priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
 		      &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 
 	/* acceptance mask/acceptance code (accept everything) */
-	flexcan_write(0x0, &regs->rxgmask);
-	flexcan_write(0x0, &regs->rx14mask);
-	flexcan_write(0x0, &regs->rx15mask);
+	priv->write(0x0, &regs->rxgmask);
+	priv->write(0x0, &regs->rx14mask);
+	priv->write(0x0, &regs->rx15mask);
 
 	if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
-		flexcan_write(0x0, &regs->rxfgmask);
+		priv->write(0x0, &regs->rxfgmask);
 
 	/*
 	 * On Vybrid, disable memory error detection interrupts
@@ -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device *dev)
 		 * and Correction of Memory Errors" to write to
 		 * MECR register
 		 */
-		reg_crl2 = flexcan_read(&regs->crl2);
+		reg_crl2 = priv->read(&regs->crl2);
 		reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
-		flexcan_write(reg_crl2, &regs->crl2);
+		priv->write(reg_crl2, &regs->crl2);
 
-		reg_mecr = flexcan_read(&regs->mecr);
+		reg_mecr = priv->read(&regs->mecr);
 		reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
-		flexcan_write(reg_mecr, &regs->mecr);
+		priv->write(reg_mecr, &regs->mecr);
 		reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK |
 				FLEXCAN_MECR_FANCEI_MSK);
-		flexcan_write(reg_mecr, &regs->mecr);
+		priv->write(reg_mecr, &regs->mecr);
 	}
 
 	err = flexcan_transceiver_enable(priv);
@@ -958,11 +967,11 @@ static int flexcan_chip_start(struct net_device *dev)
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	/* enable FIFO interrupts */
-	flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+	priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
 
 	/* print chip status */
 	netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
-		   flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
+		   priv->read(&regs->mcr), priv->read(&regs->ctrl));
 
 	return 0;
 
@@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device *dev)
 	flexcan_chip_disable(priv);
 
 	/* Disable all interrupts */
-	flexcan_write(0, &regs->imask1);
-	flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
+	priv->write(0, &regs->imask1);
+	priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 		      &regs->ctrl);
 
 	flexcan_transceiver_disable(priv);
@@ -1108,26 +1117,26 @@ static int register_flexcandev(struct net_device *dev)
 	err = flexcan_chip_disable(priv);
 	if (err)
 		goto out_disable_per;
-	reg = flexcan_read(&regs->ctrl);
+	reg = priv->read(&regs->ctrl);
 	reg |= FLEXCAN_CTRL_CLK_SRC;
-	flexcan_write(reg, &regs->ctrl);
+	priv->write(reg, &regs->ctrl);
 
 	err = flexcan_chip_enable(priv);
 	if (err)
 		goto out_chip_disable;
 
 	/* set freeze, halt and activate FIFO, restrict register access */
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
-	flexcan_write(reg, &regs->mcr);
+	priv->write(reg, &regs->mcr);
 
 	/*
 	 * Currently we only support newer versions of this core
 	 * featuring a RX FIFO. Older cores found on some Coldfire
 	 * derivates are not yet supported.
 	 */
-	reg = flexcan_read(&regs->mcr);
+	reg = priv->read(&regs->mcr);
 	if (!(reg & FLEXCAN_MCR_FEN)) {
 		netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
 		err = -ENODEV;
@@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int err, irq;
 	u32 clock_freq = 0;
+	/* Default case for most ARM based FSL SoC having BE FlexCAN IP */
+	bool core_is_little = true, module_is_little = false;
 
 	reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
 	if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER)
@@ -1237,6 +1248,25 @@ static int flexcan_probe(struct platform_device *pdev)
 	dev->flags |= IFF_ECHO;
 
 	priv = netdev_priv(dev);
+
+	if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+		core_is_little = false;
+
+	if (of_property_read_bool(dev->dev.of_node, "little-endian"))
+		module_is_little = true;
+
+	if ((core_is_little && module_is_little) ||
+	    (!core_is_little && !module_is_little)) {
+		priv->read = flexcan_read_le;
+		priv->write = flexcan_write_le;
+	}
+
+	if ((!core_is_little && module_is_little) ||
+	    (core_is_little && !module_is_little)) {
+		priv->read = flexcan_read_be;
+		priv->write = flexcan_write_be;
+	}
+
 	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
  2015-05-14 11:33 ` Bhupesh Sharma
@ 2015-05-14 11:33   ` Bhupesh Sharma
  -1 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: arnd, linux-can, mkl
  Cc: bhupesh.sharma, bhupesh.linux, linux-arm-kernel, Sakar.Arora

This patch adds support for non RX-FIFO (legacy) mode in
the flexcan driver.

On certain SoCs, the RX-FIFO support might be broken, as
a result we need to fall-back on the legacy (non RX-FIFO)
mode to receive CAN frames.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
---
 drivers/net/can/flexcan.c |  188 +++++++++++++++++++++++++++++++++------------
 1 file changed, 138 insertions(+), 50 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index b0222ae..3240472 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -156,6 +156,9 @@
 #define FLEXCAN_IFLAG_DEFAULT \
 	(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \
 	 FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
+#define FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE \
+	 (FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
+#define FLEXCAN_IFLAG_RX_MB_RXMASK	((1 << FLEXCAN_TX_BUF_RESERVED) - 1)
 
 /* FLEXCAN message buffers */
 #define FLEXCAN_MB_CNT_CODE(x)		(((x) & 0xf) << 24)
@@ -198,6 +201,8 @@
 #define FLEXCAN_HAS_V10_FEATURES	BIT(1) /* For core version >= 10 */
 #define FLEXCAN_HAS_BROKEN_ERR_STATE	BIT(2) /* [TR]WRN_INT not connected */
 #define FLEXCAN_HAS_MECR_FEATURES	BIT(3) /* Memory error detection */
+#define FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT \
+	 BIT(4) /* No RX FIFO mode support */
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -252,6 +257,7 @@ struct flexcan_priv {
 	void __iomem *base;
 	u32 reg_esr;
 	u32 reg_ctrl_default;
+	u32 rx_msg_buf;
 
 	struct clk *clk_ipg;
 	struct clk *clk_per;
@@ -303,8 +309,7 @@ static const struct can_bittiming_const flexcan_bittiming_const = {
 	.brp_inc = 1,
 };
 
-/*
- * FlexCAN module is essentially modelled as a little-endian IP in most
+/* FlexCAN module is essentially modelled as a little-endian IP in most
  * SoCs, i.e the registers as well as the message buffer areas are
  * implemented in a little-endian fashion.
  *
@@ -646,12 +651,10 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
 	return 1;
 }
 
-static void flexcan_read_fifo(const struct net_device *dev,
-			      struct can_frame *cf)
+static void flexcan_read_can_frame(const struct flexcan_priv *priv,
+				   struct flexcan_mb __iomem *mb,
+				   struct can_frame *cf)
 {
-	const struct flexcan_priv *priv = netdev_priv(dev);
-	struct flexcan_regs __iomem *regs = priv->base;
-	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
 	u32 reg_ctrl, reg_id;
 
 	reg_ctrl = priv->read(&mb->can_ctrl);
@@ -667,14 +670,39 @@ static void flexcan_read_fifo(const struct net_device *dev,
 
 	*(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0]));
 	*(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1]));
+}
+
+static void flexcan_read_fifo(const struct net_device *dev,
+			      struct can_frame *cf)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
+
+	flexcan_read_can_frame(priv, mb, cf);
 
 	/* mark as read */
 	priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
 	priv->read(&regs->timer);
 }
 
+static void flexcan_read_msg_buf(const struct net_device *dev,
+				 struct can_frame *cf, u32 msg_buf)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	struct flexcan_mb __iomem *mb = &regs->cantxfg[msg_buf];
+
+	flexcan_read_can_frame(priv, mb, cf);
+
+	/* mark as read */
+	priv->write(BIT(msg_buf), &regs->iflag1);
+	priv->read(&regs->timer);
+}
+
 static int flexcan_read_frame(struct net_device *dev)
 {
+	const struct flexcan_priv *priv = netdev_priv(dev);
 	struct net_device_stats *stats = &dev->stats;
 	struct can_frame *cf;
 	struct sk_buff *skb;
@@ -685,7 +713,11 @@ static int flexcan_read_frame(struct net_device *dev)
 		return 0;
 	}
 
-	flexcan_read_fifo(dev, cf);
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+		flexcan_read_msg_buf(dev, cf, priv->rx_msg_buf);
+	else
+		flexcan_read_fifo(dev, cf);
+
 	netif_receive_skb(skb);
 
 	stats->rx_packets++;
@@ -699,9 +731,10 @@ static int flexcan_read_frame(struct net_device *dev)
 static int flexcan_poll(struct napi_struct *napi, int quota)
 {
 	struct net_device *dev = napi->dev;
-	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
+	unsigned long iflag1;
 	int work_done = 0;
 
 	/*
@@ -713,12 +746,25 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	/* handle state changes */
 	work_done += flexcan_poll_state(dev, reg_esr);
 
-	/* handle RX-FIFO */
 	reg_iflag1 = priv->read(&regs->iflag1);
-	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
-	       work_done < quota) {
-		work_done += flexcan_read_frame(dev);
-		reg_iflag1 = priv->read(&regs->iflag1);
+
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) {
+		/* handle legacy RX mode */
+		iflag1 = reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK;
+		while ((reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK) &&
+		       work_done < quota) {
+			priv->rx_msg_buf = find_first_bit(&iflag1,
+						(FLEXCAN_TX_BUF_ID - 1));
+			work_done += flexcan_read_frame(dev);
+			reg_iflag1 = priv->read(&regs->iflag1);
+		}
+	} else {
+		/* handle RX-FIFO */
+		while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
+		       work_done < quota) {
+			work_done += flexcan_read_frame(dev);
+			reg_iflag1 = priv->read(&regs->iflag1);
+		}
 	}
 
 	/* report bus errors */
@@ -728,7 +774,13 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	if (work_done < quota) {
 		napi_complete(napi);
 		/* enable IRQs */
-		priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+		if (priv->devtype_data->features &
+		    FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+			priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE |
+				    FLEXCAN_IFLAG_RX_MB_RXMASK, &regs->imask1);
+		else
+			priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+
 		priv->write(priv->reg_ctrl_default, &regs->ctrl);
 	}
 
@@ -755,26 +807,45 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	 * - state change IRQ
 	 * - bus error IRQ and bus error reporting is activated
 	 */
-	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
-	    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
-	    flexcan_has_and_handle_berr(priv, reg_esr)) {
-		/*
-		 * The error bits are cleared on read,
-		 * save them for later use.
-		 */
-		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
-		priv->write(FLEXCAN_IFLAG_DEFAULT &
-			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
-		priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
-		       &regs->ctrl);
-		napi_schedule(&priv->napi);
-	}
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) {
+		if ((reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK) ||
+		    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
+		    flexcan_has_and_handle_berr(priv, reg_esr)) {
+			/* The error bits are cleared on read,
+			 * save them for later use.
+			 */
+			priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
+			priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE &
+				    ~FLEXCAN_IFLAG_RX_MB_RXMASK, &regs->imask1);
+			priv->write(priv->reg_ctrl_default &
+				    ~FLEXCAN_CTRL_ERR_ALL, &regs->ctrl);
+
+			napi_schedule(&priv->napi);
+		}
+	} else {
+		if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
+		    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
+		    flexcan_has_and_handle_berr(priv, reg_esr)) {
+			/*
+			 * The error bits are cleared on read,
+			 * save them for later use.
+			 */
+			priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
+			priv->write(FLEXCAN_IFLAG_DEFAULT &
+				    ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE,
+				    &regs->imask1);
+			priv->write(priv->reg_ctrl_default &
+				    ~FLEXCAN_CTRL_ERR_ALL, &regs->ctrl);
+			napi_schedule(&priv->napi);
+		}
 
-	/* FIFO overflow */
-	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
-		priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
-		dev->stats.rx_over_errors++;
-		dev->stats.rx_errors++;
+		/* FIFO overflow */
+		if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
+			priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW,
+				    &regs->iflag1);
+			dev->stats.rx_over_errors++;
+			dev->stats.rx_errors++;
+		}
 	}
 
 	/* transmission complete interrupt */
@@ -869,7 +940,12 @@ static int flexcan_chip_start(struct net_device *dev)
 	 */
 	reg_mcr = priv->read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
-	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+		reg_mcr &= ~FLEXCAN_MCR_FEN;
+	else
+		reg_mcr |= FLEXCAN_MCR_FEN;
+
+	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
 		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
 		FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
@@ -966,8 +1042,13 @@ static int flexcan_chip_start(struct net_device *dev)
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
-	/* enable FIFO interrupts */
-	priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+		/* enable mb interrupts */
+		priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE |
+			    FLEXCAN_IFLAG_RX_MB_RXMASK, &regs->imask1);
+	else
+		/* enable FIFO interrupts */
+		priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
 
 	/* print chip status */
 	netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
@@ -1125,22 +1206,29 @@ static int register_flexcandev(struct net_device *dev)
 	if (err)
 		goto out_chip_disable;
 
-	/* set freeze, halt and activate FIFO, restrict register access */
+	/* set freeze, halt and activate FIFO/legacy mode, restrict
+	 * register access
+	 */
 	reg = priv->read(&regs->mcr);
-	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
-		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+		reg &= ~FLEXCAN_MCR_FEN;
+	else
+		reg |= FLEXCAN_MCR_FEN;
+
+	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV;
 	priv->write(reg, &regs->mcr);
 
-	/*
-	 * Currently we only support newer versions of this core
-	 * featuring a RX FIFO. Older cores found on some Coldfire
-	 * derivates are not yet supported.
-	 */
-	reg = priv->read(&regs->mcr);
-	if (!(reg & FLEXCAN_MCR_FEN)) {
-		netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
-		err = -ENODEV;
-		goto out_chip_disable;
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) {
+		/* Legacy RX mode*/
+		netdev_info(dev, "Legacy mode (non RX-FIFO) enabled\n");
+	} else {
+		/* RX FIFO mode */
+		reg = priv->read(&regs->mcr);
+		if (!(reg & FLEXCAN_MCR_FEN)) {
+			netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
+			err = -ENODEV;
+			goto out_chip_disable;
+		}
 	}
 
 	err = register_candev(dev);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
@ 2015-05-14 11:33   ` Bhupesh Sharma
  0 siblings, 0 replies; 41+ messages in thread
From: Bhupesh Sharma @ 2015-05-14 11:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds support for non RX-FIFO (legacy) mode in
the flexcan driver.

On certain SoCs, the RX-FIFO support might be broken, as
a result we need to fall-back on the legacy (non RX-FIFO)
mode to receive CAN frames.

Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
---
 drivers/net/can/flexcan.c |  188 +++++++++++++++++++++++++++++++++------------
 1 file changed, 138 insertions(+), 50 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index b0222ae..3240472 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -156,6 +156,9 @@
 #define FLEXCAN_IFLAG_DEFAULT \
 	(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \
 	 FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
+#define FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE \
+	 (FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
+#define FLEXCAN_IFLAG_RX_MB_RXMASK	((1 << FLEXCAN_TX_BUF_RESERVED) - 1)
 
 /* FLEXCAN message buffers */
 #define FLEXCAN_MB_CNT_CODE(x)		(((x) & 0xf) << 24)
@@ -198,6 +201,8 @@
 #define FLEXCAN_HAS_V10_FEATURES	BIT(1) /* For core version >= 10 */
 #define FLEXCAN_HAS_BROKEN_ERR_STATE	BIT(2) /* [TR]WRN_INT not connected */
 #define FLEXCAN_HAS_MECR_FEATURES	BIT(3) /* Memory error detection */
+#define FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT \
+	 BIT(4) /* No RX FIFO mode support */
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -252,6 +257,7 @@ struct flexcan_priv {
 	void __iomem *base;
 	u32 reg_esr;
 	u32 reg_ctrl_default;
+	u32 rx_msg_buf;
 
 	struct clk *clk_ipg;
 	struct clk *clk_per;
@@ -303,8 +309,7 @@ static const struct can_bittiming_const flexcan_bittiming_const = {
 	.brp_inc = 1,
 };
 
-/*
- * FlexCAN module is essentially modelled as a little-endian IP in most
+/* FlexCAN module is essentially modelled as a little-endian IP in most
  * SoCs, i.e the registers as well as the message buffer areas are
  * implemented in a little-endian fashion.
  *
@@ -646,12 +651,10 @@ static int flexcan_poll_state(struct net_device *dev, u32 reg_esr)
 	return 1;
 }
 
-static void flexcan_read_fifo(const struct net_device *dev,
-			      struct can_frame *cf)
+static void flexcan_read_can_frame(const struct flexcan_priv *priv,
+				   struct flexcan_mb __iomem *mb,
+				   struct can_frame *cf)
 {
-	const struct flexcan_priv *priv = netdev_priv(dev);
-	struct flexcan_regs __iomem *regs = priv->base;
-	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
 	u32 reg_ctrl, reg_id;
 
 	reg_ctrl = priv->read(&mb->can_ctrl);
@@ -667,14 +670,39 @@ static void flexcan_read_fifo(const struct net_device *dev,
 
 	*(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0]));
 	*(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1]));
+}
+
+static void flexcan_read_fifo(const struct net_device *dev,
+			      struct can_frame *cf)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
+
+	flexcan_read_can_frame(priv, mb, cf);
 
 	/* mark as read */
 	priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
 	priv->read(&regs->timer);
 }
 
+static void flexcan_read_msg_buf(const struct net_device *dev,
+				 struct can_frame *cf, u32 msg_buf)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	struct flexcan_mb __iomem *mb = &regs->cantxfg[msg_buf];
+
+	flexcan_read_can_frame(priv, mb, cf);
+
+	/* mark as read */
+	priv->write(BIT(msg_buf), &regs->iflag1);
+	priv->read(&regs->timer);
+}
+
 static int flexcan_read_frame(struct net_device *dev)
 {
+	const struct flexcan_priv *priv = netdev_priv(dev);
 	struct net_device_stats *stats = &dev->stats;
 	struct can_frame *cf;
 	struct sk_buff *skb;
@@ -685,7 +713,11 @@ static int flexcan_read_frame(struct net_device *dev)
 		return 0;
 	}
 
-	flexcan_read_fifo(dev, cf);
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+		flexcan_read_msg_buf(dev, cf, priv->rx_msg_buf);
+	else
+		flexcan_read_fifo(dev, cf);
+
 	netif_receive_skb(skb);
 
 	stats->rx_packets++;
@@ -699,9 +731,10 @@ static int flexcan_read_frame(struct net_device *dev)
 static int flexcan_poll(struct napi_struct *napi, int quota)
 {
 	struct net_device *dev = napi->dev;
-	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
+	unsigned long iflag1;
 	int work_done = 0;
 
 	/*
@@ -713,12 +746,25 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	/* handle state changes */
 	work_done += flexcan_poll_state(dev, reg_esr);
 
-	/* handle RX-FIFO */
 	reg_iflag1 = priv->read(&regs->iflag1);
-	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
-	       work_done < quota) {
-		work_done += flexcan_read_frame(dev);
-		reg_iflag1 = priv->read(&regs->iflag1);
+
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) {
+		/* handle legacy RX mode */
+		iflag1 = reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK;
+		while ((reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK) &&
+		       work_done < quota) {
+			priv->rx_msg_buf = find_first_bit(&iflag1,
+						(FLEXCAN_TX_BUF_ID - 1));
+			work_done += flexcan_read_frame(dev);
+			reg_iflag1 = priv->read(&regs->iflag1);
+		}
+	} else {
+		/* handle RX-FIFO */
+		while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
+		       work_done < quota) {
+			work_done += flexcan_read_frame(dev);
+			reg_iflag1 = priv->read(&regs->iflag1);
+		}
 	}
 
 	/* report bus errors */
@@ -728,7 +774,13 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	if (work_done < quota) {
 		napi_complete(napi);
 		/* enable IRQs */
-		priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+		if (priv->devtype_data->features &
+		    FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+			priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE |
+				    FLEXCAN_IFLAG_RX_MB_RXMASK, &regs->imask1);
+		else
+			priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+
 		priv->write(priv->reg_ctrl_default, &regs->ctrl);
 	}
 
@@ -755,26 +807,45 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	 * - state change IRQ
 	 * - bus error IRQ and bus error reporting is activated
 	 */
-	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
-	    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
-	    flexcan_has_and_handle_berr(priv, reg_esr)) {
-		/*
-		 * The error bits are cleared on read,
-		 * save them for later use.
-		 */
-		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
-		priv->write(FLEXCAN_IFLAG_DEFAULT &
-			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
-		priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
-		       &regs->ctrl);
-		napi_schedule(&priv->napi);
-	}
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) {
+		if ((reg_iflag1 & FLEXCAN_IFLAG_RX_MB_RXMASK) ||
+		    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
+		    flexcan_has_and_handle_berr(priv, reg_esr)) {
+			/* The error bits are cleared on read,
+			 * save them for later use.
+			 */
+			priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
+			priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE &
+				    ~FLEXCAN_IFLAG_RX_MB_RXMASK, &regs->imask1);
+			priv->write(priv->reg_ctrl_default &
+				    ~FLEXCAN_CTRL_ERR_ALL, &regs->ctrl);
+
+			napi_schedule(&priv->napi);
+		}
+	} else {
+		if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
+		    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
+		    flexcan_has_and_handle_berr(priv, reg_esr)) {
+			/*
+			 * The error bits are cleared on read,
+			 * save them for later use.
+			 */
+			priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
+			priv->write(FLEXCAN_IFLAG_DEFAULT &
+				    ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE,
+				    &regs->imask1);
+			priv->write(priv->reg_ctrl_default &
+				    ~FLEXCAN_CTRL_ERR_ALL, &regs->ctrl);
+			napi_schedule(&priv->napi);
+		}
 
-	/* FIFO overflow */
-	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
-		priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
-		dev->stats.rx_over_errors++;
-		dev->stats.rx_errors++;
+		/* FIFO overflow */
+		if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
+			priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW,
+				    &regs->iflag1);
+			dev->stats.rx_over_errors++;
+			dev->stats.rx_errors++;
+		}
 	}
 
 	/* transmission complete interrupt */
@@ -869,7 +940,12 @@ static int flexcan_chip_start(struct net_device *dev)
 	 */
 	reg_mcr = priv->read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
-	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+		reg_mcr &= ~FLEXCAN_MCR_FEN;
+	else
+		reg_mcr |= FLEXCAN_MCR_FEN;
+
+	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
 		FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
 		FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
@@ -966,8 +1042,13 @@ static int flexcan_chip_start(struct net_device *dev)
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
-	/* enable FIFO interrupts */
-	priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+		/* enable mb interrupts */
+		priv->write(FLEXCAN_IFLAG_DEFAULT_RX_MB_MODE |
+			    FLEXCAN_IFLAG_RX_MB_RXMASK, &regs->imask1);
+	else
+		/* enable FIFO interrupts */
+		priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
 
 	/* print chip status */
 	netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
@@ -1125,22 +1206,29 @@ static int register_flexcandev(struct net_device *dev)
 	if (err)
 		goto out_chip_disable;
 
-	/* set freeze, halt and activate FIFO, restrict register access */
+	/* set freeze, halt and activate FIFO/legacy mode, restrict
+	 * register access
+	 */
 	reg = priv->read(&regs->mcr);
-	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
-		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT)
+		reg &= ~FLEXCAN_MCR_FEN;
+	else
+		reg |= FLEXCAN_MCR_FEN;
+
+	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_SUPV;
 	priv->write(reg, &regs->mcr);
 
-	/*
-	 * Currently we only support newer versions of this core
-	 * featuring a RX FIFO. Older cores found on some Coldfire
-	 * derivates are not yet supported.
-	 */
-	reg = priv->read(&regs->mcr);
-	if (!(reg & FLEXCAN_MCR_FEN)) {
-		netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
-		err = -ENODEV;
-		goto out_chip_disable;
+	if (priv->devtype_data->features & FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT) {
+		/* Legacy RX mode*/
+		netdev_info(dev, "Legacy mode (non RX-FIFO) enabled\n");
+	} else {
+		/* RX FIFO mode */
+		reg = priv->read(&regs->mcr);
+		if (!(reg & FLEXCAN_MCR_FEN)) {
+			netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
+			err = -ENODEV;
+			goto out_chip_disable;
+		}
 	}
 
 	err = register_candev(dev);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry
  2015-05-14 11:33   ` Bhupesh Sharma
@ 2015-05-14 15:38     ` Marc Kleine-Budde
  -1 siblings, 0 replies; 41+ messages in thread
From: Marc Kleine-Budde @ 2015-05-14 15:38 UTC (permalink / raw)
  To: Bhupesh Sharma, arnd, linux-can
  Cc: bhupesh.linux, Sakar.Arora, linux-arm-kernel

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

On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> This patch adds ls1021a flexcan device entry to the flexcan driver code.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> ---
>  drivers/net/can/flexcan.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index ad0a7e8..deaa24e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -267,10 +267,26 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data;
>  static struct flexcan_devtype_data fsl_imx6q_devtype_data = {
>  	.features = FLEXCAN_HAS_V10_FEATURES,
>  };
> +
>  static struct flexcan_devtype_data fsl_vf610_devtype_data = {
>  	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES,
>  };
>  
> +/* LS1021A-Rev1 has a broken RX-FIFO support. So only legacy RX message-buffers
> + * work here.
> + */
> +static struct flexcan_devtype_data fsl_ls1021a_devtype_data = {
> +	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES |
> +		    FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT,
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This will probably not compile.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry
@ 2015-05-14 15:38     ` Marc Kleine-Budde
  0 siblings, 0 replies; 41+ messages in thread
From: Marc Kleine-Budde @ 2015-05-14 15:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> This patch adds ls1021a flexcan device entry to the flexcan driver code.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> ---
>  drivers/net/can/flexcan.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index ad0a7e8..deaa24e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -267,10 +267,26 @@ static struct flexcan_devtype_data fsl_imx28_devtype_data;
>  static struct flexcan_devtype_data fsl_imx6q_devtype_data = {
>  	.features = FLEXCAN_HAS_V10_FEATURES,
>  };
> +
>  static struct flexcan_devtype_data fsl_vf610_devtype_data = {
>  	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES,
>  };
>  
> +/* LS1021A-Rev1 has a broken RX-FIFO support. So only legacy RX message-buffers
> + * work here.
> + */
> +static struct flexcan_devtype_data fsl_ls1021a_devtype_data = {
> +	.features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_MECR_FEATURES |
> +		    FLEXCAN_HAS_ONLY_LEGACY_RX_SUPPORT,
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This will probably not compile.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150514/f82e8ded/attachment.sig>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
  2015-05-14 11:33   ` Bhupesh Sharma
@ 2015-05-14 15:41     ` Marc Kleine-Budde
  -1 siblings, 0 replies; 41+ messages in thread
From: Marc Kleine-Budde @ 2015-05-14 15:41 UTC (permalink / raw)
  To: Bhupesh Sharma, arnd, linux-can
  Cc: bhupesh.linux, Sakar.Arora, linux-arm-kernel

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

On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> This patch adds support for non RX-FIFO (legacy) mode in
> the flexcan driver.
> 
> On certain SoCs, the RX-FIFO support might be broken, as
> a result we need to fall-back on the legacy (non RX-FIFO)
> mode to receive CAN frames.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>

The non FIFO mode doesn't guarantee the order of the incoming frames,
not does not even try to...this is not acceptable. I'm currently working
on a patch by David Jander that brings in non FIFO mode, but tries to
keep the order of the frames.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
@ 2015-05-14 15:41     ` Marc Kleine-Budde
  0 siblings, 0 replies; 41+ messages in thread
From: Marc Kleine-Budde @ 2015-05-14 15:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> This patch adds support for non RX-FIFO (legacy) mode in
> the flexcan driver.
> 
> On certain SoCs, the RX-FIFO support might be broken, as
> a result we need to fall-back on the legacy (non RX-FIFO)
> mode to receive CAN frames.
> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>

The non FIFO mode doesn't guarantee the order of the incoming frames,
not does not even try to...this is not acceptable. I'm currently working
on a patch by David Jander that brings in non FIFO mode, but tries to
keep the order of the frames.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150514/f269ccab/attachment.sig>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
  2015-05-14 15:41     ` Marc Kleine-Budde
@ 2015-05-14 15:44       ` Sharma Bhupesh
  -1 siblings, 0 replies; 41+ messages in thread
From: Sharma Bhupesh @ 2015-05-14 15:44 UTC (permalink / raw)
  To: Marc Kleine-Budde, arnd, linux-can
  Cc: bhupesh.linux, Arora Sakar, linux-arm-kernel

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: Thursday, May 14, 2015 9:12 PM
> To: Sharma Bhupesh-B45370; arnd@arndb.de; linux-can@vger.kernel.org
> Cc: bhupesh.linux@gmail.com; Arora Sakar-B45205; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
> mode
> 
> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> > This patch adds support for non RX-FIFO (legacy) mode in the flexcan
> > driver.
> >
> > On certain SoCs, the RX-FIFO support might be broken, as a result we
> > need to fall-back on the legacy (non RX-FIFO) mode to receive CAN
> > frames.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
> 
> The non FIFO mode doesn't guarantee the order of the incoming frames, not
> does not even try to...this is not acceptable. I'm currently working on a
> patch by David Jander that brings in non FIFO mode, but tries to keep the
> order of the frames.

That is already WIP at our end. V3 will contain the same change.
If you are already working on it, I don't know how to proceed further
as we had already v1 of this patchset with the non FIFO mode out
since a month or so.

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
@ 2015-05-14 15:44       ` Sharma Bhupesh
  0 siblings, 0 replies; 41+ messages in thread
From: Sharma Bhupesh @ 2015-05-14 15:44 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl at pengutronix.de]
> Sent: Thursday, May 14, 2015 9:12 PM
> To: Sharma Bhupesh-B45370; arnd at arndb.de; linux-can at vger.kernel.org
> Cc: bhupesh.linux at gmail.com; Arora Sakar-B45205; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
> mode
> 
> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> > This patch adds support for non RX-FIFO (legacy) mode in the flexcan
> > driver.
> >
> > On certain SoCs, the RX-FIFO support might be broken, as a result we
> > need to fall-back on the legacy (non RX-FIFO) mode to receive CAN
> > frames.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
> 
> The non FIFO mode doesn't guarantee the order of the incoming frames, not
> does not even try to...this is not acceptable. I'm currently working on a
> patch by David Jander that brings in non FIFO mode, but tries to keep the
> order of the frames.

That is already WIP at our end. V3 will contain the same change.
If you are already working on it, I don't know how to proceed further
as we had already v1 of this patchset with the non FIFO mode out
since a month or so.

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
  2015-05-14 15:41     ` Marc Kleine-Budde
@ 2015-05-15  0:09       ` Tom Evans
  -1 siblings, 0 replies; 41+ messages in thread
From: Tom Evans @ 2015-05-15  0:09 UTC (permalink / raw)
  To: Marc Kleine-Budde, Bhupesh Sharma, arnd, linux-can
  Cc: bhupesh.linux, Sakar.Arora, linux-arm-kernel

On 15/05/15 01:41, Marc Kleine-Budde wrote:
> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
>> This patch adds support for non RX-FIFO (legacy) mode in
>> the flexcan driver.
>>
>> On certain SoCs, the RX-FIFO support might be broken, as
>> a result we need to fall-back on the legacy (non RX-FIFO)
>> mode to receive CAN frames.
>>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
>
> The non FIFO mode doesn't guarantee the order of the incoming frames,
> not does not even try to...this is not acceptable. I'm currently working
> on a patch by David Jander that brings in non FIFO mode, but tries to
> keep the order of the frames.
>
> Marc

In case it is of any help as a reference, here's a thread on the problems of 
receiving messages in order on another non-FIFO chip. This applies to the 
normal Microchip CAN controllers, in this case the very-hard-to-program 
on-the-end-of-an-SPI-bus MCP2515:

http://www.microchip.com/forums/tm.aspx?m=620741

It has two receive buffers, and a "rollover" setting. This does not make the 
registers operate as a FIFO and it needs a state machine to keep track of 
which buffer holds which sequence message.

Tom



^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
@ 2015-05-15  0:09       ` Tom Evans
  0 siblings, 0 replies; 41+ messages in thread
From: Tom Evans @ 2015-05-15  0:09 UTC (permalink / raw)
  To: linux-arm-kernel

On 15/05/15 01:41, Marc Kleine-Budde wrote:
> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
>> This patch adds support for non RX-FIFO (legacy) mode in
>> the flexcan driver.
>>
>> On certain SoCs, the RX-FIFO support might be broken, as
>> a result we need to fall-back on the legacy (non RX-FIFO)
>> mode to receive CAN frames.
>>
>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
>
> The non FIFO mode doesn't guarantee the order of the incoming frames,
> not does not even try to...this is not acceptable. I'm currently working
> on a patch by David Jander that brings in non FIFO mode, but tries to
> keep the order of the frames.
>
> Marc

In case it is of any help as a reference, here's a thread on the problems of 
receiving messages in order on another non-FIFO chip. This applies to the 
normal Microchip CAN controllers, in this case the very-hard-to-program 
on-the-end-of-an-SPI-bus MCP2515:

http://www.microchip.com/forums/tm.aspx?m=620741

It has two receive buffers, and a "rollover" setting. This does not make the 
registers operate as a FIFO and it needs a state machine to keep track of 
which buffer holds which sequence message.

Tom

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances
  2015-05-14 11:33   ` Bhupesh Sharma
@ 2015-05-18 16:17     ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 41+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2015-05-18 16:17 UTC (permalink / raw)
  To: Bhupesh Sharma, arnd, linux-can, mkl
  Cc: bhupesh.linux, linux-arm-kernel, Sakar.Arora

Am 14.05.2015 um 13:33 schrieb Bhupesh Sharma:

> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index deaa24e..b0222ae 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -258,6 +258,10 @@ struct flexcan_priv {
>       struct flexcan_platform_data *pdata;
>       const struct flexcan_devtype_data *devtype_data;
>       struct regulator *reg_xceiver;
> +
> +     /* Read and Write APIs */
> +     u32 (*read)(void __iomem *addr);
> +     void (*write)(u32 val, void __iomem *addr);

Does it *really* need to be a far call ?
Why not just give the existing flexcan_read()/flexcan_write() inline's
an additional bit to decide whether to swab or not ?

> -#if defined(CONFIG_PPC)
> -static inline u32 flexcan_read(void __iomem *addr)
> +static inline u32 flexcan_read_le(void __iomem *addr)

_static inline_ as a callback ? seriously ?

>   static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
>   {
> @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct flexcan_priv *priv)
>       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg &= ~FLEXCAN_MCR_MDIS;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
> -     while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> +     while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
>               udelay(10);
>
> -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
> +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
>               return -ETIMEDOUT;
>
>       return 0;
> @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct flexcan_priv *priv)
>       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg |= FLEXCAN_MCR_MDIS;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
> -     while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> +     while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
>               udelay(10);
>
> -     if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> +     if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
>               return -ETIMEDOUT;
>
>       return 0;
> @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv)
>       unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg |= FLEXCAN_MCR_HALT;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
> -     while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +     while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
>               udelay(100);
>
> -     if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +     if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
>               return -ETIMEDOUT;
>
>       return 0;
> @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv)
>       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg &= ~FLEXCAN_MCR_HALT;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
> -     while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +     while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
>               udelay(10);
>
> -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
> +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
>               return -ETIMEDOUT;
>
>       return 0;
> @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv)
>       struct flexcan_regs __iomem *regs = priv->base;
>       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
>
> -     flexcan_write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
> -     while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST))
> +     priv->write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
> +     while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST))
>               udelay(10);
>
> -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
> +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
>               return -ETIMEDOUT;
>
>       return 0;
>   }
>
> -
>   static int __flexcan_get_berr_counter(const struct net_device *dev,
>                                     struct can_berr_counter *bec)
>   {
>       const struct flexcan_priv *priv = netdev_priv(dev);
>       struct flexcan_regs __iomem *regs = priv->base;
> -     u32 reg = flexcan_read(&regs->ecr);
> +     u32 reg = priv->read(&regs->ecr);
>
>       bec->txerr = (reg >> 0) & 0xff;
>       bec->rxerr = (reg >> 8) & 0xff;
> @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>
>       if (cf->can_dlc > 0) {
>               u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
> -             flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
> +             priv->write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
>       }
>       if (cf->can_dlc > 3) {
>               u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
> -             flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
> +             priv->write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
>       }
>
>       can_put_echo_skb(skb, dev, 0);
>
> -     flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> -     flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> +     priv->write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> +     priv->write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>
>       /* Errata ERR005829 step8:
>        * Write twice INACTIVE(0x8) code to first MB.
>        */
> -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
> -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
>
>       return NETDEV_TX_OK;
> @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct net_device *dev,
>       struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
>       u32 reg_ctrl, reg_id;
>
> -     reg_ctrl = flexcan_read(&mb->can_ctrl);
> -     reg_id = flexcan_read(&mb->can_id);
> +     reg_ctrl = priv->read(&mb->can_ctrl);
> +     reg_id = priv->read(&mb->can_id);
>       if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
>               cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
>       else
> @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct net_device *dev,
>               cf->can_id |= CAN_RTR_FLAG;
>       cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
>
> -     *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
> -     *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
> +     *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0]));
> +     *(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1]));
>
>       /* mark as read */
> -     flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> -     flexcan_read(&regs->timer);
> +     priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> +     priv->read(&regs->timer);
>   }
>
>   static int flexcan_read_frame(struct net_device *dev)
> @@ -699,17 +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
>        * The error bits are cleared on read,
>        * use saved value from irq handler.
>        */
> -     reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
> +     reg_esr = priv->read(&regs->esr) | priv->reg_esr;
>
>       /* handle state changes */
>       work_done += flexcan_poll_state(dev, reg_esr);
>
>       /* handle RX-FIFO */
> -     reg_iflag1 = flexcan_read(&regs->iflag1);
> +     reg_iflag1 = priv->read(&regs->iflag1);
>       while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
>              work_done < quota) {
>               work_done += flexcan_read_frame(dev);
> -             reg_iflag1 = flexcan_read(&regs->iflag1);
> +             reg_iflag1 = priv->read(&regs->iflag1);
>       }
>
>       /* report bus errors */
> @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
>       if (work_done < quota) {
>               napi_complete(napi);
>               /* enable IRQs */
> -             flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> -             flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
> +             priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> +             priv->write(priv->reg_ctrl_default, &regs->ctrl);
>       }
>
>       return work_done;
> @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>       struct flexcan_regs __iomem *regs = priv->base;
>       u32 reg_iflag1, reg_esr;
>
> -     reg_iflag1 = flexcan_read(&regs->iflag1);
> -     reg_esr = flexcan_read(&regs->esr);
> +     reg_iflag1 = priv->read(&regs->iflag1);
> +     reg_esr = priv->read(&regs->esr);
>       /* ACK all bus error and state change IRQ sources */
>       if (reg_esr & FLEXCAN_ESR_ALL_INT)
> -             flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
> +             priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
>
>       /*
>        * schedule NAPI in case of:
> @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>                * save them for later use.
>                */
>               priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
> -             flexcan_write(FLEXCAN_IFLAG_DEFAULT &
> +             priv->write(FLEXCAN_IFLAG_DEFAULT &
>                       ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
> -             flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> +             priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
>                      &regs->ctrl);
>               napi_schedule(&priv->napi);
>       }
>
>       /* FIFO overflow */
>       if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
> -             flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
> +             priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
>               dev->stats.rx_over_errors++;
>               dev->stats.rx_errors++;
>       }
> @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>               stats->tx_packets++;
>               can_led_event(dev, CAN_LED_EVENT_TX);
>               /* after sending a RTR frame mailbox is in RX mode */
> -             flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +             priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                             &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> -             flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
> +             priv->write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
>               netif_wake_queue(dev);
>       }
>
> @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device *dev)
>       struct flexcan_regs __iomem *regs = priv->base;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->ctrl);
> +     reg = priv->read(&regs->ctrl);
>       reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
>                FLEXCAN_CTRL_RJW(0x3) |
>                FLEXCAN_CTRL_PSEG1(0x7) |
> @@ -814,11 +823,11 @@ static void flexcan_set_bittiming(struct net_device *dev)
>               reg |= FLEXCAN_CTRL_SMP;
>
>       netdev_info(dev, "writing ctrl=0x%08x\n", reg);
> -     flexcan_write(reg, &regs->ctrl);
> +     priv->write(reg, &regs->ctrl);
>
>       /* print chip status */
>       netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
> -                flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
> +                priv->read(&regs->mcr), priv->read(&regs->ctrl));
>   }
>
>   /*
> @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device *dev)
>        * disable local echo
>        *
>        */
> -     reg_mcr = flexcan_read(&regs->mcr);
> +     reg_mcr = priv->read(&regs->mcr);
>       reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
>       reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
>               FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
>               FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
>               FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
>       netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
> -     flexcan_write(reg_mcr, &regs->mcr);
> +     priv->write(reg_mcr, &regs->mcr);
>
>       /*
>        * CTRL
> @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device *dev)
>        * enable bus off interrupt
>        * (== FLEXCAN_CTRL_ERR_STATE)
>        */
> -     reg_ctrl = flexcan_read(&regs->ctrl);
> +     reg_ctrl = priv->read(&regs->ctrl);
>       reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
>       reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
>               FLEXCAN_CTRL_ERR_STATE;
> @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device *dev)
>       /* save for later use */
>       priv->reg_ctrl_default = reg_ctrl;
>       netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
> -     flexcan_write(reg_ctrl, &regs->ctrl);
> +     priv->write(reg_ctrl, &regs->ctrl);
>
>       /* clear and invalidate all mailboxes first */
>       for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) {
> -             flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
> +             priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
>                             &regs->cantxfg[i].can_ctrl);
>       }
>
>       /* Errata ERR005829: mark first TX mailbox as INACTIVE */
> -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
>
>       /* mark TX mailbox as INACTIVE */
> -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                     &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>
>       /* acceptance mask/acceptance code (accept everything) */
> -     flexcan_write(0x0, &regs->rxgmask);
> -     flexcan_write(0x0, &regs->rx14mask);
> -     flexcan_write(0x0, &regs->rx15mask);
> +     priv->write(0x0, &regs->rxgmask);
> +     priv->write(0x0, &regs->rx14mask);
> +     priv->write(0x0, &regs->rx15mask);
>
>       if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
> -             flexcan_write(0x0, &regs->rxfgmask);
> +             priv->write(0x0, &regs->rxfgmask);
>
>       /*
>        * On Vybrid, disable memory error detection interrupts
> @@ -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device *dev)
>                * and Correction of Memory Errors" to write to
>                * MECR register
>                */
> -             reg_crl2 = flexcan_read(&regs->crl2);
> +             reg_crl2 = priv->read(&regs->crl2);
>               reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
> -             flexcan_write(reg_crl2, &regs->crl2);
> +             priv->write(reg_crl2, &regs->crl2);
>
> -             reg_mecr = flexcan_read(&regs->mecr);
> +             reg_mecr = priv->read(&regs->mecr);
>               reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
> -             flexcan_write(reg_mecr, &regs->mecr);
> +             priv->write(reg_mecr, &regs->mecr);
>               reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK |
>                               FLEXCAN_MECR_FANCEI_MSK);
> -             flexcan_write(reg_mecr, &regs->mecr);
> +             priv->write(reg_mecr, &regs->mecr);
>       }
>
>       err = flexcan_transceiver_enable(priv);
> @@ -958,11 +967,11 @@ static int flexcan_chip_start(struct net_device *dev)
>       priv->can.state = CAN_STATE_ERROR_ACTIVE;
>
>       /* enable FIFO interrupts */
> -     flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> +     priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
>
>       /* print chip status */
>       netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
> -                flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
> +                priv->read(&regs->mcr), priv->read(&regs->ctrl));
>
>       return 0;
>
> @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device *dev)
>       flexcan_chip_disable(priv);
>
>       /* Disable all interrupts */
> -     flexcan_write(0, &regs->imask1);
> -     flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> +     priv->write(0, &regs->imask1);
> +     priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
>                     &regs->ctrl);
>
>       flexcan_transceiver_disable(priv);
> @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct net_device *dev)
>       err = flexcan_chip_disable(priv);
>       if (err)
>               goto out_disable_per;
> -     reg = flexcan_read(&regs->ctrl);
> +     reg = priv->read(&regs->ctrl);
>       reg |= FLEXCAN_CTRL_CLK_SRC;
> -     flexcan_write(reg, &regs->ctrl);
> +     priv->write(reg, &regs->ctrl);
>
>       err = flexcan_chip_enable(priv);
>       if (err)
>               goto out_chip_disable;
>
>       /* set freeze, halt and activate FIFO, restrict register access */
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
>               FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
>       /*
>        * Currently we only support newer versions of this core
>        * featuring a RX FIFO. Older cores found on some Coldfire
>        * derivates are not yet supported.
>        */
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       if (!(reg & FLEXCAN_MCR_FEN)) {
>               netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
>               err = -ENODEV;
> @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device *pdev)
>       void __iomem *base;
>       int err, irq;
>       u32 clock_freq = 0;
> +     /* Default case for most ARM based FSL SoC having BE FlexCAN IP */
> +     bool core_is_little = true, module_is_little = false;
>
>       reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
>       if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER)
> @@ -1237,6 +1248,25 @@ static int flexcan_probe(struct platform_device *pdev)
>       dev->flags |= IFF_ECHO;
>
>       priv = netdev_priv(dev);
> +
> +     if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
> +             core_is_little = false;

Why not just using IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) directly ?
(anyways, cpu_is_le seems a more appropriate name)

> +     if (of_property_read_bool(dev->dev.of_node, "little-endian"))
> +             module_is_little = true;

Why that misleading name, instead of, eg. "chip_is_le" ?

Oh, and you'll expect explicitly set a new DTB attribute, if the chip
is in LE - the default case.

Didn't you just say, you dont wanna break anything ?

Can you imagine that some people use DTB as a _board_ config
(eg. in bootloader) instead of externalized kernel config ? ;-o

> +     if ((core_is_little && module_is_little) ||
> +         (!core_is_little && !module_is_little)) {
> +             priv->read = flexcan_read_le;
> +             priv->write = flexcan_write_le;
> +     }
> +
> +     if ((!core_is_little && module_is_little) ||
> +         (core_is_little && !module_is_little)) {
> +             priv->read = flexcan_read_be;
> +             priv->write = flexcan_write_be;
> +     }

At that point, the naming gets completely wrong - instead should
be something like flexcan_read_swab / flexcan_write_swab.
And the decision can easily be made on:

    IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) != module_is_little


Putting these together, I'd rather:

#1: add a new feature flag:
     #define FLEXCAN_HAS_BIG_ENDIAN BIT(4)

#2: add a new dts device type for the new chip variant
     (eg. fsl,flexcan-qorliq)

#2: pass the priv pointer to the *_read()/*_write() functions,
     so they can pick up the BE flag and decide whether to
     swab or not (based on cpu endianess)

At that point, maybe we could even replace the struct flexcan_regs
by a list of #define's.



cu

By the way: who had the genious idea of adding _hardware specific_
hacks into a widget library (qt) ? Perhaps the same guy, who decided
that _unprivileged userland_ should tell ipu/vpu/gpu which _hardware_
addresses to use ? ;-o

--
Enrico Weigelt, metux IT consult
+49-151-27565287
MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B

Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur für einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschließlich für denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, dürfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrtümlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte löschen Sie in diesem Fall diese Nachricht und alle Anhänge, ohne eine Kopie zu behalten.
Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances
@ 2015-05-18 16:17     ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 41+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2015-05-18 16:17 UTC (permalink / raw)
  To: linux-arm-kernel

Am 14.05.2015 um 13:33 schrieb Bhupesh Sharma:

> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index deaa24e..b0222ae 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -258,6 +258,10 @@ struct flexcan_priv {
>       struct flexcan_platform_data *pdata;
>       const struct flexcan_devtype_data *devtype_data;
>       struct regulator *reg_xceiver;
> +
> +     /* Read and Write APIs */
> +     u32 (*read)(void __iomem *addr);
> +     void (*write)(u32 val, void __iomem *addr);

Does it *really* need to be a far call ?
Why not just give the existing flexcan_read()/flexcan_write() inline's
an additional bit to decide whether to swab or not ?

> -#if defined(CONFIG_PPC)
> -static inline u32 flexcan_read(void __iomem *addr)
> +static inline u32 flexcan_read_le(void __iomem *addr)

_static inline_ as a callback ? seriously ?

>   static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
>   {
> @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct flexcan_priv *priv)
>       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg &= ~FLEXCAN_MCR_MDIS;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
> -     while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> +     while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
>               udelay(10);
>
> -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
> +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
>               return -ETIMEDOUT;
>
>       return 0;
> @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct flexcan_priv *priv)
>       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg |= FLEXCAN_MCR_MDIS;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
> -     while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> +     while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
>               udelay(10);
>
> -     if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> +     if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
>               return -ETIMEDOUT;
>
>       return 0;
> @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct flexcan_priv *priv)
>       unsigned int timeout = 1000 * 1000 * 10 / priv->can.bittiming.bitrate;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg |= FLEXCAN_MCR_HALT;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
> -     while (timeout-- && !(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +     while (timeout-- && !(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
>               udelay(100);
>
> -     if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +     if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
>               return -ETIMEDOUT;
>
>       return 0;
> @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct flexcan_priv *priv)
>       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg &= ~FLEXCAN_MCR_HALT;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
> -     while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> +     while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
>               udelay(10);
>
> -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
> +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
>               return -ETIMEDOUT;
>
>       return 0;
> @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv)
>       struct flexcan_regs __iomem *regs = priv->base;
>       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
>
> -     flexcan_write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
> -     while (timeout-- && (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST))
> +     priv->write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
> +     while (timeout-- && (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST))
>               udelay(10);
>
> -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
> +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
>               return -ETIMEDOUT;
>
>       return 0;
>   }
>
> -
>   static int __flexcan_get_berr_counter(const struct net_device *dev,
>                                     struct can_berr_counter *bec)
>   {
>       const struct flexcan_priv *priv = netdev_priv(dev);
>       struct flexcan_regs __iomem *regs = priv->base;
> -     u32 reg = flexcan_read(&regs->ecr);
> +     u32 reg = priv->read(&regs->ecr);
>
>       bec->txerr = (reg >> 0) & 0xff;
>       bec->rxerr = (reg >> 8) & 0xff;
> @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>
>       if (cf->can_dlc > 0) {
>               u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
> -             flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
> +             priv->write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
>       }
>       if (cf->can_dlc > 3) {
>               u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
> -             flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
> +             priv->write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
>       }
>
>       can_put_echo_skb(skb, dev, 0);
>
> -     flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> -     flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> +     priv->write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> +     priv->write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>
>       /* Errata ERR005829 step8:
>        * Write twice INACTIVE(0x8) code to first MB.
>        */
> -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
> -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
>
>       return NETDEV_TX_OK;
> @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct net_device *dev,
>       struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
>       u32 reg_ctrl, reg_id;
>
> -     reg_ctrl = flexcan_read(&mb->can_ctrl);
> -     reg_id = flexcan_read(&mb->can_id);
> +     reg_ctrl = priv->read(&mb->can_ctrl);
> +     reg_id = priv->read(&mb->can_id);
>       if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
>               cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
>       else
> @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct net_device *dev,
>               cf->can_id |= CAN_RTR_FLAG;
>       cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
>
> -     *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
> -     *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
> +     *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0]));
> +     *(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1]));
>
>       /* mark as read */
> -     flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> -     flexcan_read(&regs->timer);
> +     priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> +     priv->read(&regs->timer);
>   }
>
>   static int flexcan_read_frame(struct net_device *dev)
> @@ -699,17 +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
>        * The error bits are cleared on read,
>        * use saved value from irq handler.
>        */
> -     reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
> +     reg_esr = priv->read(&regs->esr) | priv->reg_esr;
>
>       /* handle state changes */
>       work_done += flexcan_poll_state(dev, reg_esr);
>
>       /* handle RX-FIFO */
> -     reg_iflag1 = flexcan_read(&regs->iflag1);
> +     reg_iflag1 = priv->read(&regs->iflag1);
>       while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
>              work_done < quota) {
>               work_done += flexcan_read_frame(dev);
> -             reg_iflag1 = flexcan_read(&regs->iflag1);
> +             reg_iflag1 = priv->read(&regs->iflag1);
>       }
>
>       /* report bus errors */
> @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
>       if (work_done < quota) {
>               napi_complete(napi);
>               /* enable IRQs */
> -             flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> -             flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
> +             priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> +             priv->write(priv->reg_ctrl_default, &regs->ctrl);
>       }
>
>       return work_done;
> @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>       struct flexcan_regs __iomem *regs = priv->base;
>       u32 reg_iflag1, reg_esr;
>
> -     reg_iflag1 = flexcan_read(&regs->iflag1);
> -     reg_esr = flexcan_read(&regs->esr);
> +     reg_iflag1 = priv->read(&regs->iflag1);
> +     reg_esr = priv->read(&regs->esr);
>       /* ACK all bus error and state change IRQ sources */
>       if (reg_esr & FLEXCAN_ESR_ALL_INT)
> -             flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
> +             priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
>
>       /*
>        * schedule NAPI in case of:
> @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>                * save them for later use.
>                */
>               priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
> -             flexcan_write(FLEXCAN_IFLAG_DEFAULT &
> +             priv->write(FLEXCAN_IFLAG_DEFAULT &
>                       ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
> -             flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> +             priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
>                      &regs->ctrl);
>               napi_schedule(&priv->napi);
>       }
>
>       /* FIFO overflow */
>       if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
> -             flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
> +             priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
>               dev->stats.rx_over_errors++;
>               dev->stats.rx_errors++;
>       }
> @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>               stats->tx_packets++;
>               can_led_event(dev, CAN_LED_EVENT_TX);
>               /* after sending a RTR frame mailbox is in RX mode */
> -             flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +             priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                             &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> -             flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
> +             priv->write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
>               netif_wake_queue(dev);
>       }
>
> @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device *dev)
>       struct flexcan_regs __iomem *regs = priv->base;
>       u32 reg;
>
> -     reg = flexcan_read(&regs->ctrl);
> +     reg = priv->read(&regs->ctrl);
>       reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
>                FLEXCAN_CTRL_RJW(0x3) |
>                FLEXCAN_CTRL_PSEG1(0x7) |
> @@ -814,11 +823,11 @@ static void flexcan_set_bittiming(struct net_device *dev)
>               reg |= FLEXCAN_CTRL_SMP;
>
>       netdev_info(dev, "writing ctrl=0x%08x\n", reg);
> -     flexcan_write(reg, &regs->ctrl);
> +     priv->write(reg, &regs->ctrl);
>
>       /* print chip status */
>       netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
> -                flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
> +                priv->read(&regs->mcr), priv->read(&regs->ctrl));
>   }
>
>   /*
> @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device *dev)
>        * disable local echo
>        *
>        */
> -     reg_mcr = flexcan_read(&regs->mcr);
> +     reg_mcr = priv->read(&regs->mcr);
>       reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
>       reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
>               FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
>               FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
>               FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
>       netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
> -     flexcan_write(reg_mcr, &regs->mcr);
> +     priv->write(reg_mcr, &regs->mcr);
>
>       /*
>        * CTRL
> @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device *dev)
>        * enable bus off interrupt
>        * (== FLEXCAN_CTRL_ERR_STATE)
>        */
> -     reg_ctrl = flexcan_read(&regs->ctrl);
> +     reg_ctrl = priv->read(&regs->ctrl);
>       reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
>       reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
>               FLEXCAN_CTRL_ERR_STATE;
> @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device *dev)
>       /* save for later use */
>       priv->reg_ctrl_default = reg_ctrl;
>       netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
> -     flexcan_write(reg_ctrl, &regs->ctrl);
> +     priv->write(reg_ctrl, &regs->ctrl);
>
>       /* clear and invalidate all mailboxes first */
>       for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) {
> -             flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
> +             priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
>                             &regs->cantxfg[i].can_ctrl);
>       }
>
>       /* Errata ERR005829: mark first TX mailbox as INACTIVE */
> -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
>
>       /* mark TX mailbox as INACTIVE */
> -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>                     &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>
>       /* acceptance mask/acceptance code (accept everything) */
> -     flexcan_write(0x0, &regs->rxgmask);
> -     flexcan_write(0x0, &regs->rx14mask);
> -     flexcan_write(0x0, &regs->rx15mask);
> +     priv->write(0x0, &regs->rxgmask);
> +     priv->write(0x0, &regs->rx14mask);
> +     priv->write(0x0, &regs->rx15mask);
>
>       if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
> -             flexcan_write(0x0, &regs->rxfgmask);
> +             priv->write(0x0, &regs->rxfgmask);
>
>       /*
>        * On Vybrid, disable memory error detection interrupts
> @@ -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device *dev)
>                * and Correction of Memory Errors" to write to
>                * MECR register
>                */
> -             reg_crl2 = flexcan_read(&regs->crl2);
> +             reg_crl2 = priv->read(&regs->crl2);
>               reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
> -             flexcan_write(reg_crl2, &regs->crl2);
> +             priv->write(reg_crl2, &regs->crl2);
>
> -             reg_mecr = flexcan_read(&regs->mecr);
> +             reg_mecr = priv->read(&regs->mecr);
>               reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
> -             flexcan_write(reg_mecr, &regs->mecr);
> +             priv->write(reg_mecr, &regs->mecr);
>               reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK |
>                               FLEXCAN_MECR_FANCEI_MSK);
> -             flexcan_write(reg_mecr, &regs->mecr);
> +             priv->write(reg_mecr, &regs->mecr);
>       }
>
>       err = flexcan_transceiver_enable(priv);
> @@ -958,11 +967,11 @@ static int flexcan_chip_start(struct net_device *dev)
>       priv->can.state = CAN_STATE_ERROR_ACTIVE;
>
>       /* enable FIFO interrupts */
> -     flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> +     priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
>
>       /* print chip status */
>       netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
> -                flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
> +                priv->read(&regs->mcr), priv->read(&regs->ctrl));
>
>       return 0;
>
> @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device *dev)
>       flexcan_chip_disable(priv);
>
>       /* Disable all interrupts */
> -     flexcan_write(0, &regs->imask1);
> -     flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> +     priv->write(0, &regs->imask1);
> +     priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
>                     &regs->ctrl);
>
>       flexcan_transceiver_disable(priv);
> @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct net_device *dev)
>       err = flexcan_chip_disable(priv);
>       if (err)
>               goto out_disable_per;
> -     reg = flexcan_read(&regs->ctrl);
> +     reg = priv->read(&regs->ctrl);
>       reg |= FLEXCAN_CTRL_CLK_SRC;
> -     flexcan_write(reg, &regs->ctrl);
> +     priv->write(reg, &regs->ctrl);
>
>       err = flexcan_chip_enable(priv);
>       if (err)
>               goto out_chip_disable;
>
>       /* set freeze, halt and activate FIFO, restrict register access */
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
>               FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
> -     flexcan_write(reg, &regs->mcr);
> +     priv->write(reg, &regs->mcr);
>
>       /*
>        * Currently we only support newer versions of this core
>        * featuring a RX FIFO. Older cores found on some Coldfire
>        * derivates are not yet supported.
>        */
> -     reg = flexcan_read(&regs->mcr);
> +     reg = priv->read(&regs->mcr);
>       if (!(reg & FLEXCAN_MCR_FEN)) {
>               netdev_err(dev, "Could not enable RX FIFO, unsupported core\n");
>               err = -ENODEV;
> @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device *pdev)
>       void __iomem *base;
>       int err, irq;
>       u32 clock_freq = 0;
> +     /* Default case for most ARM based FSL SoC having BE FlexCAN IP */
> +     bool core_is_little = true, module_is_little = false;
>
>       reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
>       if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER)
> @@ -1237,6 +1248,25 @@ static int flexcan_probe(struct platform_device *pdev)
>       dev->flags |= IFF_ECHO;
>
>       priv = netdev_priv(dev);
> +
> +     if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
> +             core_is_little = false;

Why not just using IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) directly ?
(anyways, cpu_is_le seems a more appropriate name)

> +     if (of_property_read_bool(dev->dev.of_node, "little-endian"))
> +             module_is_little = true;

Why that misleading name, instead of, eg. "chip_is_le" ?

Oh, and you'll expect explicitly set a new DTB attribute, if the chip
is in LE - the default case.

Didn't you just say, you dont wanna break anything ?

Can you imagine that some people use DTB as a _board_ config
(eg. in bootloader) instead of externalized kernel config ? ;-o

> +     if ((core_is_little && module_is_little) ||
> +         (!core_is_little && !module_is_little)) {
> +             priv->read = flexcan_read_le;
> +             priv->write = flexcan_write_le;
> +     }
> +
> +     if ((!core_is_little && module_is_little) ||
> +         (core_is_little && !module_is_little)) {
> +             priv->read = flexcan_read_be;
> +             priv->write = flexcan_write_be;
> +     }

At that point, the naming gets completely wrong - instead should
be something like flexcan_read_swab / flexcan_write_swab.
And the decision can easily be made on:

    IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) != module_is_little


Putting these together, I'd rather:

#1: add a new feature flag:
     #define FLEXCAN_HAS_BIG_ENDIAN BIT(4)

#2: add a new dts device type for the new chip variant
     (eg. fsl,flexcan-qorliq)

#2: pass the priv pointer to the *_read()/*_write() functions,
     so they can pick up the BE flag and decide whether to
     swab or not (based on cpu endianess)

At that point, maybe we could even replace the struct flexcan_regs
by a list of #define's.



cu

By the way: who had the genious idea of adding _hardware specific_
hacks into a widget library (qt) ? Perhaps the same guy, who decided
that _unprivileged userland_ should tell ipu/vpu/gpu which _hardware_
addresses to use ? ;-o

--
Enrico Weigelt, metux IT consult
+49-151-27565287
MELAG Medizintechnik oHG Sitz Berlin Registergericht AG Charlottenburg HRA 21333 B

Wichtiger Hinweis: Diese Nachricht kann vertrauliche oder nur f?r einen begrenzten Personenkreis bestimmte Informationen enthalten. Sie ist ausschlie?lich f?r denjenigen bestimmt, an den sie gerichtet worden ist. Wenn Sie nicht der Adressat dieser E-Mail sind, d?rfen Sie diese nicht kopieren, weiterleiten, weitergeben oder sie ganz oder teilweise in irgendeiner Weise nutzen. Sollten Sie diese E-Mail irrt?mlich erhalten haben, so benachrichtigen Sie bitte den Absender, indem Sie auf diese Nachricht antworten. Bitte l?schen Sie in diesem Fall diese Nachricht und alle Anh?nge, ohne eine Kopie zu behalten.
Important Notice: This message may contain confidential or privileged information. It is intended only for the person it was addressed to. If you are not the intended recipient of this email you may not copy, forward, disclose or otherwise use it or any part of it in any form whatsoever. If you received this email in error please notify the sender by replying and delete this message and any attachments without retaining a copy.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* RE: [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances
  2015-05-18 16:17     ` Enrico Weigelt, metux IT consult
@ 2015-05-18 16:37       ` Sharma Bhupesh
  -1 siblings, 0 replies; 41+ messages in thread
From: Sharma Bhupesh @ 2015-05-18 16:37 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, arnd, linux-can, mkl
  Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar

> -----Original Message-----
> From: Enrico Weigelt, metux IT consult [mailto:weigelt@melag.de]
> Am 14.05.2015 um 13:33 schrieb Bhupesh Sharma:
> 
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index deaa24e..b0222ae 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -258,6 +258,10 @@ struct flexcan_priv {
> >       struct flexcan_platform_data *pdata;
> >       const struct flexcan_devtype_data *devtype_data;
> >       struct regulator *reg_xceiver;
> > +
> > +     /* Read and Write APIs */
> > +     u32 (*read)(void __iomem *addr);
> > +     void (*write)(u32 val, void __iomem *addr);
> 
> Does it *really* need to be a far call ?
> Why not just give the existing flexcan_read()/flexcan_write() inline's an
> additional bit to decide whether to swab or not ?
> 
> > -#if defined(CONFIG_PPC)
> > -static inline u32 flexcan_read(void __iomem *addr)
> > +static inline u32 flexcan_read_le(void __iomem *addr)
> 
> _static inline_ as a callback ? seriously ?
> 
> >   static inline int flexcan_transceiver_enable(const struct
> flexcan_priv *priv)
> >   {
> > @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct
> flexcan_priv *priv)
> >       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg &= ~FLEXCAN_MCR_MDIS;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> > -     while (timeout-- && (flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_LPM_ACK))
> > +     while (timeout-- && (priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_LPM_ACK))
> >               udelay(10);
> >
> > -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
> > +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
> >               return -ETIMEDOUT;
> >
> >       return 0;
> > @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct
> flexcan_priv *priv)
> >       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg |= FLEXCAN_MCR_MDIS;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> > -     while (timeout-- && !(flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_LPM_ACK))
> > +     while (timeout-- && !(priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_LPM_ACK))
> >               udelay(10);
> >
> > -     if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> > +     if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> >               return -ETIMEDOUT;
> >
> >       return 0;
> > @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct
> flexcan_priv *priv)
> >       unsigned int timeout = 1000 * 1000 * 10 / priv-
> >can.bittiming.bitrate;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg |= FLEXCAN_MCR_HALT;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> > -     while (timeout-- && !(flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_FRZ_ACK))
> > +     while (timeout-- && !(priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_FRZ_ACK))
> >               udelay(100);
> >
> > -     if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> > +     if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> >               return -ETIMEDOUT;
> >
> >       return 0;
> > @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct
> flexcan_priv *priv)
> >       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg &= ~FLEXCAN_MCR_HALT;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> > -     while (timeout-- && (flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_FRZ_ACK))
> > +     while (timeout-- && (priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_FRZ_ACK))
> >               udelay(10);
> >
> > -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
> > +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
> >               return -ETIMEDOUT;
> >
> >       return 0;
> > @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct
> flexcan_priv *priv)
> >       struct flexcan_regs __iomem *regs = priv->base;
> >       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> >
> > -     flexcan_write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
> > -     while (timeout-- && (flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_SOFTRST))
> > +     priv->write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
> > +     while (timeout-- && (priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_SOFTRST))
> >               udelay(10);
> >
> > -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
> > +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
> >               return -ETIMEDOUT;
> >
> >       return 0;
> >   }
> >
> > -
> >   static int __flexcan_get_berr_counter(const struct net_device *dev,
> >                                     struct can_berr_counter *bec)
> >   {
> >       const struct flexcan_priv *priv = netdev_priv(dev);
> >       struct flexcan_regs __iomem *regs = priv->base;
> > -     u32 reg = flexcan_read(&regs->ecr);
> > +     u32 reg = priv->read(&regs->ecr);
> >
> >       bec->txerr = (reg >> 0) & 0xff;
> >       bec->rxerr = (reg >> 8) & 0xff;
> > @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff
> > *skb, struct net_device *dev)
> >
> >       if (cf->can_dlc > 0) {
> >               u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
> > -             flexcan_write(data, &regs-
> >cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
> > +             priv->write(data,
> > + &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
> >       }
> >       if (cf->can_dlc > 3) {
> >               u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
> > -             flexcan_write(data, &regs-
> >cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
> > +             priv->write(data,
> > + &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
> >       }
> >
> >       can_put_echo_skb(skb, dev, 0);
> >
> > -     flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> > -     flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> > +     priv->write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> > +     priv->write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> >
> >       /* Errata ERR005829 step8:
> >        * Write twice INACTIVE(0x8) code to first MB.
> >        */
> > -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
> > -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
> >
> >       return NETDEV_TX_OK;
> > @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct
> net_device *dev,
> >       struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
> >       u32 reg_ctrl, reg_id;
> >
> > -     reg_ctrl = flexcan_read(&mb->can_ctrl);
> > -     reg_id = flexcan_read(&mb->can_id);
> > +     reg_ctrl = priv->read(&mb->can_ctrl);
> > +     reg_id = priv->read(&mb->can_id);
> >       if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
> >               cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) |
> CAN_EFF_FLAG;
> >       else
> > @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct
> net_device *dev,
> >               cf->can_id |= CAN_RTR_FLAG;
> >       cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
> >
> > -     *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb-
> >data[0]));
> > -     *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb-
> >data[1]));
> > +     *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb-
> >data[0]));
> > +     *(__be32 *)(cf->data + 4) =
> > + cpu_to_be32(priv->read(&mb->data[1]));
> >
> >       /* mark as read */
> > -     flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> > -     flexcan_read(&regs->timer);
> > +     priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> > +     priv->read(&regs->timer);
> >   }
> >
> >   static int flexcan_read_frame(struct net_device *dev) @@ -699,17
> > +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
> >        * The error bits are cleared on read,
> >        * use saved value from irq handler.
> >        */
> > -     reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
> > +     reg_esr = priv->read(&regs->esr) | priv->reg_esr;
> >
> >       /* handle state changes */
> >       work_done += flexcan_poll_state(dev, reg_esr);
> >
> >       /* handle RX-FIFO */
> > -     reg_iflag1 = flexcan_read(&regs->iflag1);
> > +     reg_iflag1 = priv->read(&regs->iflag1);
> >       while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
> >              work_done < quota) {
> >               work_done += flexcan_read_frame(dev);
> > -             reg_iflag1 = flexcan_read(&regs->iflag1);
> > +             reg_iflag1 = priv->read(&regs->iflag1);
> >       }
> >
> >       /* report bus errors */
> > @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi,
> int quota)
> >       if (work_done < quota) {
> >               napi_complete(napi);
> >               /* enable IRQs */
> > -             flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> > -             flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
> > +             priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> > +             priv->write(priv->reg_ctrl_default, &regs->ctrl);
> >       }
> >
> >       return work_done;
> > @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void
> *dev_id)
> >       struct flexcan_regs __iomem *regs = priv->base;
> >       u32 reg_iflag1, reg_esr;
> >
> > -     reg_iflag1 = flexcan_read(&regs->iflag1);
> > -     reg_esr = flexcan_read(&regs->esr);
> > +     reg_iflag1 = priv->read(&regs->iflag1);
> > +     reg_esr = priv->read(&regs->esr);
> >       /* ACK all bus error and state change IRQ sources */
> >       if (reg_esr & FLEXCAN_ESR_ALL_INT)
> > -             flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
> > +             priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
> >
> >       /*
> >        * schedule NAPI in case of:
> > @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void
> *dev_id)
> >                * save them for later use.
> >                */
> >               priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
> > -             flexcan_write(FLEXCAN_IFLAG_DEFAULT &
> > +             priv->write(FLEXCAN_IFLAG_DEFAULT &
> >                       ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
> > -             flexcan_write(priv->reg_ctrl_default &
> ~FLEXCAN_CTRL_ERR_ALL,
> > +             priv->write(priv->reg_ctrl_default &
> > + ~FLEXCAN_CTRL_ERR_ALL,
> >                      &regs->ctrl);
> >               napi_schedule(&priv->napi);
> >       }
> >
> >       /* FIFO overflow */
> >       if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
> > -             flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs-
> >iflag1);
> > +             priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW,
> > + &regs->iflag1);
> >               dev->stats.rx_over_errors++;
> >               dev->stats.rx_errors++;
> >       }
> > @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void
> *dev_id)
> >               stats->tx_packets++;
> >               can_led_event(dev, CAN_LED_EVENT_TX);
> >               /* after sending a RTR frame mailbox is in RX mode */
> > -             flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +             priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                             &regs-
> >cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> > -             flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
> > +             priv->write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
> >               netif_wake_queue(dev);
> >       }
> >
> > @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device
> *dev)
> >       struct flexcan_regs __iomem *regs = priv->base;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->ctrl);
> > +     reg = priv->read(&regs->ctrl);
> >       reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
> >                FLEXCAN_CTRL_RJW(0x3) |
> >                FLEXCAN_CTRL_PSEG1(0x7) | @@ -814,11 +823,11 @@ static
> > void flexcan_set_bittiming(struct net_device *dev)
> >               reg |= FLEXCAN_CTRL_SMP;
> >
> >       netdev_info(dev, "writing ctrl=0x%08x\n", reg);
> > -     flexcan_write(reg, &regs->ctrl);
> > +     priv->write(reg, &regs->ctrl);
> >
> >       /* print chip status */
> >       netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
> > -                flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
> > +                priv->read(&regs->mcr), priv->read(&regs->ctrl));
> >   }
> >
> >   /*
> > @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device
> *dev)
> >        * disable local echo
> >        *
> >        */
> > -     reg_mcr = flexcan_read(&regs->mcr);
> > +     reg_mcr = priv->read(&regs->mcr);
> >       reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
> >       reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
> >               FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
> >               FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
> >               FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
> >       netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
> > -     flexcan_write(reg_mcr, &regs->mcr);
> > +     priv->write(reg_mcr, &regs->mcr);
> >
> >       /*
> >        * CTRL
> > @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device
> *dev)
> >        * enable bus off interrupt
> >        * (== FLEXCAN_CTRL_ERR_STATE)
> >        */
> > -     reg_ctrl = flexcan_read(&regs->ctrl);
> > +     reg_ctrl = priv->read(&regs->ctrl);
> >       reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
> >       reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
> >               FLEXCAN_CTRL_ERR_STATE;
> > @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device
> *dev)
> >       /* save for later use */
> >       priv->reg_ctrl_default = reg_ctrl;
> >       netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
> > -     flexcan_write(reg_ctrl, &regs->ctrl);
> > +     priv->write(reg_ctrl, &regs->ctrl);
> >
> >       /* clear and invalidate all mailboxes first */
> >       for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) {
> > -             flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
> > +             priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
> >                             &regs->cantxfg[i].can_ctrl);
> >       }
> >
> >       /* Errata ERR005829: mark first TX mailbox as INACTIVE */
> > -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
> >
> >       /* mark TX mailbox as INACTIVE */
> > -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                     &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> >
> >       /* acceptance mask/acceptance code (accept everything) */
> > -     flexcan_write(0x0, &regs->rxgmask);
> > -     flexcan_write(0x0, &regs->rx14mask);
> > -     flexcan_write(0x0, &regs->rx15mask);
> > +     priv->write(0x0, &regs->rxgmask);
> > +     priv->write(0x0, &regs->rx14mask);
> > +     priv->write(0x0, &regs->rx15mask);
> >
> >       if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
> > -             flexcan_write(0x0, &regs->rxfgmask);
> > +             priv->write(0x0, &regs->rxfgmask);
> >
> >       /*
> >        * On Vybrid, disable memory error detection interrupts @@
> > -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device
> *dev)
> >                * and Correction of Memory Errors" to write to
> >                * MECR register
> >                */
> > -             reg_crl2 = flexcan_read(&regs->crl2);
> > +             reg_crl2 = priv->read(&regs->crl2);
> >               reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
> > -             flexcan_write(reg_crl2, &regs->crl2);
> > +             priv->write(reg_crl2, &regs->crl2);
> >
> > -             reg_mecr = flexcan_read(&regs->mecr);
> > +             reg_mecr = priv->read(&regs->mecr);
> >               reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
> > -             flexcan_write(reg_mecr, &regs->mecr);
> > +             priv->write(reg_mecr, &regs->mecr);
> >               reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ |
> FLEXCAN_MECR_HANCEI_MSK |
> >                               FLEXCAN_MECR_FANCEI_MSK);
> > -             flexcan_write(reg_mecr, &regs->mecr);
> > +             priv->write(reg_mecr, &regs->mecr);
> >       }
> >
> >       err = flexcan_transceiver_enable(priv); @@ -958,11 +967,11 @@
> > static int flexcan_chip_start(struct net_device *dev)
> >       priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >
> >       /* enable FIFO interrupts */
> > -     flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> > +     priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> >
> >       /* print chip status */
> >       netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
> > -                flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
> > +                priv->read(&regs->mcr), priv->read(&regs->ctrl));
> >
> >       return 0;
> >
> > @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device
> *dev)
> >       flexcan_chip_disable(priv);
> >
> >       /* Disable all interrupts */
> > -     flexcan_write(0, &regs->imask1);
> > -     flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> > +     priv->write(0, &regs->imask1);
> > +     priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> >                     &regs->ctrl);
> >
> >       flexcan_transceiver_disable(priv);
> > @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct
> net_device *dev)
> >       err = flexcan_chip_disable(priv);
> >       if (err)
> >               goto out_disable_per;
> > -     reg = flexcan_read(&regs->ctrl);
> > +     reg = priv->read(&regs->ctrl);
> >       reg |= FLEXCAN_CTRL_CLK_SRC;
> > -     flexcan_write(reg, &regs->ctrl);
> > +     priv->write(reg, &regs->ctrl);
> >
> >       err = flexcan_chip_enable(priv);
> >       if (err)
> >               goto out_chip_disable;
> >
> >       /* set freeze, halt and activate FIFO, restrict register access
> */
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
> >               FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> >       /*
> >        * Currently we only support newer versions of this core
> >        * featuring a RX FIFO. Older cores found on some Coldfire
> >        * derivates are not yet supported.
> >        */
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       if (!(reg & FLEXCAN_MCR_FEN)) {
> >               netdev_err(dev, "Could not enable RX FIFO, unsupported
> core\n");
> >               err = -ENODEV;
> > @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device
> *pdev)
> >       void __iomem *base;
> >       int err, irq;
> >       u32 clock_freq = 0;
> > +     /* Default case for most ARM based FSL SoC having BE FlexCAN IP
> */
> > +     bool core_is_little = true, module_is_little = false;
> >
> >       reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
> >       if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) @@ -1237,6 +1248,25
> > @@ static int flexcan_probe(struct platform_device *pdev)
> >       dev->flags |= IFF_ECHO;
> >
> >       priv = netdev_priv(dev);
> > +
> > +     if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
> > +             core_is_little = false;
> 
> Why not just using IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) directly ?
> (anyways, cpu_is_le seems a more appropriate name)
> 
> > +     if (of_property_read_bool(dev->dev.of_node, "little-endian"))
> > +             module_is_little = true;
> 
> Why that misleading name, instead of, eg. "chip_is_le" ?
> 
> Oh, and you'll expect explicitly set a new DTB attribute, if the chip is
> in LE - the default case.
> 
> Didn't you just say, you dont wanna break anything ?
> 
> Can you imagine that some people use DTB as a _board_ config (eg. in
> bootloader) instead of externalized kernel config ? ;-o
> 
> > +     if ((core_is_little && module_is_little) ||
> > +         (!core_is_little && !module_is_little)) {
> > +             priv->read = flexcan_read_le;
> > +             priv->write = flexcan_write_le;
> > +     }
> > +
> > +     if ((!core_is_little && module_is_little) ||
> > +         (core_is_little && !module_is_little)) {
> > +             priv->read = flexcan_read_be;
> > +             priv->write = flexcan_write_be;
> > +     }
> 
> At that point, the naming gets completely wrong - instead should be
> something like flexcan_read_swab / flexcan_write_swab.
> And the decision can easily be made on:
> 
>     IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) != module_is_little
> 
> 
> Putting these together, I'd rather:
> 
> #1: add a new feature flag:
>      #define FLEXCAN_HAS_BIG_ENDIAN BIT(4)
> 
> #2: add a new dts device type for the new chip variant
>      (eg. fsl,flexcan-qorliq)
> 
> #2: pass the priv pointer to the *_read()/*_write() functions,
>      so they can pick up the BE flag and decide whether to
>      swab or not (based on cpu endianess)
> 

You seem to have missed the endian property discussions that happened regarding DTS
Several months back. Have a look at them: [1] & [2]. This patch is compliant to various DTS that
already have such a property.

And BTW, I have already mentioned this during the v1 review of this patchset:

We are dealing with 4 possible cases here:
1. Core is LE (ARM) <-> FlexCAN IP is BE = FSL LS1021A SoC
2. Core is BE (PPC) <-> FlexCAN IP is BE = FSL P1010 SoC
3. Core is BE (PPC) <-> FlexCAN IP is LE = No SoC with this configuration yet
4. Core is LE (PPC) <-> FlexCAN IP is LE = FSL I.MX series

So, no this _simply_ won't work the way you described for the above cases.

I remember testing this patchset also on a P1010, so I don't find any legacy stuff breaking.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/254533.html
[2] http://lxr.free-electrons.com/source/arch/arm/boot/dts/ls1021a.dtsi#L127

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances
@ 2015-05-18 16:37       ` Sharma Bhupesh
  0 siblings, 0 replies; 41+ messages in thread
From: Sharma Bhupesh @ 2015-05-18 16:37 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Enrico Weigelt, metux IT consult [mailto:weigelt at melag.de]
> Am 14.05.2015 um 13:33 schrieb Bhupesh Sharma:
> 
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index deaa24e..b0222ae 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -258,6 +258,10 @@ struct flexcan_priv {
> >       struct flexcan_platform_data *pdata;
> >       const struct flexcan_devtype_data *devtype_data;
> >       struct regulator *reg_xceiver;
> > +
> > +     /* Read and Write APIs */
> > +     u32 (*read)(void __iomem *addr);
> > +     void (*write)(u32 val, void __iomem *addr);
> 
> Does it *really* need to be a far call ?
> Why not just give the existing flexcan_read()/flexcan_write() inline's an
> additional bit to decide whether to swab or not ?
> 
> > -#if defined(CONFIG_PPC)
> > -static inline u32 flexcan_read(void __iomem *addr)
> > +static inline u32 flexcan_read_le(void __iomem *addr)
> 
> _static inline_ as a callback ? seriously ?
> 
> >   static inline int flexcan_transceiver_enable(const struct
> flexcan_priv *priv)
> >   {
> > @@ -356,14 +366,14 @@ static int flexcan_chip_enable(struct
> flexcan_priv *priv)
> >       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg &= ~FLEXCAN_MCR_MDIS;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> > -     while (timeout-- && (flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_LPM_ACK))
> > +     while (timeout-- && (priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_LPM_ACK))
> >               udelay(10);
> >
> > -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
> > +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK)
> >               return -ETIMEDOUT;
> >
> >       return 0;
> > @@ -375,14 +385,14 @@ static int flexcan_chip_disable(struct
> flexcan_priv *priv)
> >       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg |= FLEXCAN_MCR_MDIS;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> > -     while (timeout-- && !(flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_LPM_ACK))
> > +     while (timeout-- && !(priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_LPM_ACK))
> >               udelay(10);
> >
> > -     if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> > +     if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_LPM_ACK))
> >               return -ETIMEDOUT;
> >
> >       return 0;
> > @@ -394,14 +404,14 @@ static int flexcan_chip_freeze(struct
> flexcan_priv *priv)
> >       unsigned int timeout = 1000 * 1000 * 10 / priv-
> >can.bittiming.bitrate;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg |= FLEXCAN_MCR_HALT;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> > -     while (timeout-- && !(flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_FRZ_ACK))
> > +     while (timeout-- && !(priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_FRZ_ACK))
> >               udelay(100);
> >
> > -     if (!(flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> > +     if (!(priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK))
> >               return -ETIMEDOUT;
> >
> >       return 0;
> > @@ -413,14 +423,14 @@ static int flexcan_chip_unfreeze(struct
> flexcan_priv *priv)
> >       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg &= ~FLEXCAN_MCR_HALT;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> > -     while (timeout-- && (flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_FRZ_ACK))
> > +     while (timeout-- && (priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_FRZ_ACK))
> >               udelay(10);
> >
> > -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
> > +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_FRZ_ACK)
> >               return -ETIMEDOUT;
> >
> >       return 0;
> > @@ -431,23 +441,22 @@ static int flexcan_chip_softreset(struct
> flexcan_priv *priv)
> >       struct flexcan_regs __iomem *regs = priv->base;
> >       unsigned int timeout = FLEXCAN_TIMEOUT_US / 10;
> >
> > -     flexcan_write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
> > -     while (timeout-- && (flexcan_read(&regs->mcr) &
> FLEXCAN_MCR_SOFTRST))
> > +     priv->write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
> > +     while (timeout-- && (priv->read(&regs->mcr) &
> > + FLEXCAN_MCR_SOFTRST))
> >               udelay(10);
> >
> > -     if (flexcan_read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
> > +     if (priv->read(&regs->mcr) & FLEXCAN_MCR_SOFTRST)
> >               return -ETIMEDOUT;
> >
> >       return 0;
> >   }
> >
> > -
> >   static int __flexcan_get_berr_counter(const struct net_device *dev,
> >                                     struct can_berr_counter *bec)
> >   {
> >       const struct flexcan_priv *priv = netdev_priv(dev);
> >       struct flexcan_regs __iomem *regs = priv->base;
> > -     u32 reg = flexcan_read(&regs->ecr);
> > +     u32 reg = priv->read(&regs->ecr);
> >
> >       bec->txerr = (reg >> 0) & 0xff;
> >       bec->rxerr = (reg >> 8) & 0xff;
> > @@ -503,24 +512,24 @@ static int flexcan_start_xmit(struct sk_buff
> > *skb, struct net_device *dev)
> >
> >       if (cf->can_dlc > 0) {
> >               u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
> > -             flexcan_write(data, &regs-
> >cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
> > +             priv->write(data,
> > + &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
> >       }
> >       if (cf->can_dlc > 3) {
> >               u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
> > -             flexcan_write(data, &regs-
> >cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
> > +             priv->write(data,
> > + &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
> >       }
> >
> >       can_put_echo_skb(skb, dev, 0);
> >
> > -     flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> > -     flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> > +     priv->write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
> > +     priv->write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> >
> >       /* Errata ERR005829 step8:
> >        * Write twice INACTIVE(0x8) code to first MB.
> >        */
> > -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
> > -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
> >
> >       return NETDEV_TX_OK;
> > @@ -645,8 +654,8 @@ static void flexcan_read_fifo(const struct
> net_device *dev,
> >       struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
> >       u32 reg_ctrl, reg_id;
> >
> > -     reg_ctrl = flexcan_read(&mb->can_ctrl);
> > -     reg_id = flexcan_read(&mb->can_id);
> > +     reg_ctrl = priv->read(&mb->can_ctrl);
> > +     reg_id = priv->read(&mb->can_id);
> >       if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
> >               cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) |
> CAN_EFF_FLAG;
> >       else
> > @@ -656,12 +665,12 @@ static void flexcan_read_fifo(const struct
> net_device *dev,
> >               cf->can_id |= CAN_RTR_FLAG;
> >       cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
> >
> > -     *(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb-
> >data[0]));
> > -     *(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb-
> >data[1]));
> > +     *(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb-
> >data[0]));
> > +     *(__be32 *)(cf->data + 4) =
> > + cpu_to_be32(priv->read(&mb->data[1]));
> >
> >       /* mark as read */
> > -     flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> > -     flexcan_read(&regs->timer);
> > +     priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> > +     priv->read(&regs->timer);
> >   }
> >
> >   static int flexcan_read_frame(struct net_device *dev) @@ -699,17
> > +708,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
> >        * The error bits are cleared on read,
> >        * use saved value from irq handler.
> >        */
> > -     reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
> > +     reg_esr = priv->read(&regs->esr) | priv->reg_esr;
> >
> >       /* handle state changes */
> >       work_done += flexcan_poll_state(dev, reg_esr);
> >
> >       /* handle RX-FIFO */
> > -     reg_iflag1 = flexcan_read(&regs->iflag1);
> > +     reg_iflag1 = priv->read(&regs->iflag1);
> >       while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
> >              work_done < quota) {
> >               work_done += flexcan_read_frame(dev);
> > -             reg_iflag1 = flexcan_read(&regs->iflag1);
> > +             reg_iflag1 = priv->read(&regs->iflag1);
> >       }
> >
> >       /* report bus errors */
> > @@ -719,8 +728,8 @@ static int flexcan_poll(struct napi_struct *napi,
> int quota)
> >       if (work_done < quota) {
> >               napi_complete(napi);
> >               /* enable IRQs */
> > -             flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> > -             flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
> > +             priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> > +             priv->write(priv->reg_ctrl_default, &regs->ctrl);
> >       }
> >
> >       return work_done;
> > @@ -734,11 +743,11 @@ static irqreturn_t flexcan_irq(int irq, void
> *dev_id)
> >       struct flexcan_regs __iomem *regs = priv->base;
> >       u32 reg_iflag1, reg_esr;
> >
> > -     reg_iflag1 = flexcan_read(&regs->iflag1);
> > -     reg_esr = flexcan_read(&regs->esr);
> > +     reg_iflag1 = priv->read(&regs->iflag1);
> > +     reg_esr = priv->read(&regs->esr);
> >       /* ACK all bus error and state change IRQ sources */
> >       if (reg_esr & FLEXCAN_ESR_ALL_INT)
> > -             flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
> > +             priv->write(reg_esr & FLEXCAN_ESR_ALL_INT, &regs->esr);
> >
> >       /*
> >        * schedule NAPI in case of:
> > @@ -754,16 +763,16 @@ static irqreturn_t flexcan_irq(int irq, void
> *dev_id)
> >                * save them for later use.
> >                */
> >               priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
> > -             flexcan_write(FLEXCAN_IFLAG_DEFAULT &
> > +             priv->write(FLEXCAN_IFLAG_DEFAULT &
> >                       ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
> > -             flexcan_write(priv->reg_ctrl_default &
> ~FLEXCAN_CTRL_ERR_ALL,
> > +             priv->write(priv->reg_ctrl_default &
> > + ~FLEXCAN_CTRL_ERR_ALL,
> >                      &regs->ctrl);
> >               napi_schedule(&priv->napi);
> >       }
> >
> >       /* FIFO overflow */
> >       if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
> > -             flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs-
> >iflag1);
> > +             priv->write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW,
> > + &regs->iflag1);
> >               dev->stats.rx_over_errors++;
> >               dev->stats.rx_errors++;
> >       }
> > @@ -774,9 +783,9 @@ static irqreturn_t flexcan_irq(int irq, void
> *dev_id)
> >               stats->tx_packets++;
> >               can_led_event(dev, CAN_LED_EVENT_TX);
> >               /* after sending a RTR frame mailbox is in RX mode */
> > -             flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +             priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                             &regs-
> >cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> > -             flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
> > +             priv->write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
> >               netif_wake_queue(dev);
> >       }
> >
> > @@ -790,7 +799,7 @@ static void flexcan_set_bittiming(struct net_device
> *dev)
> >       struct flexcan_regs __iomem *regs = priv->base;
> >       u32 reg;
> >
> > -     reg = flexcan_read(&regs->ctrl);
> > +     reg = priv->read(&regs->ctrl);
> >       reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
> >                FLEXCAN_CTRL_RJW(0x3) |
> >                FLEXCAN_CTRL_PSEG1(0x7) | @@ -814,11 +823,11 @@ static
> > void flexcan_set_bittiming(struct net_device *dev)
> >               reg |= FLEXCAN_CTRL_SMP;
> >
> >       netdev_info(dev, "writing ctrl=0x%08x\n", reg);
> > -     flexcan_write(reg, &regs->ctrl);
> > +     priv->write(reg, &regs->ctrl);
> >
> >       /* print chip status */
> >       netdev_dbg(dev, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
> > -                flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
> > +                priv->read(&regs->mcr), priv->read(&regs->ctrl));
> >   }
> >
> >   /*
> > @@ -858,14 +867,14 @@ static int flexcan_chip_start(struct net_device
> *dev)
> >        * disable local echo
> >        *
> >        */
> > -     reg_mcr = flexcan_read(&regs->mcr);
> > +     reg_mcr = priv->read(&regs->mcr);
> >       reg_mcr &= ~FLEXCAN_MCR_MAXMB(0xff);
> >       reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
> >               FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
> >               FLEXCAN_MCR_IDAM_C | FLEXCAN_MCR_SRX_DIS |
> >               FLEXCAN_MCR_MAXMB(FLEXCAN_TX_BUF_ID);
> >       netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
> > -     flexcan_write(reg_mcr, &regs->mcr);
> > +     priv->write(reg_mcr, &regs->mcr);
> >
> >       /*
> >        * CTRL
> > @@ -879,7 +888,7 @@ static int flexcan_chip_start(struct net_device
> *dev)
> >        * enable bus off interrupt
> >        * (== FLEXCAN_CTRL_ERR_STATE)
> >        */
> > -     reg_ctrl = flexcan_read(&regs->ctrl);
> > +     reg_ctrl = priv->read(&regs->ctrl);
> >       reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
> >       reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
> >               FLEXCAN_CTRL_ERR_STATE;
> > @@ -897,29 +906,29 @@ static int flexcan_chip_start(struct net_device
> *dev)
> >       /* save for later use */
> >       priv->reg_ctrl_default = reg_ctrl;
> >       netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
> > -     flexcan_write(reg_ctrl, &regs->ctrl);
> > +     priv->write(reg_ctrl, &regs->ctrl);
> >
> >       /* clear and invalidate all mailboxes first */
> >       for (i = FLEXCAN_TX_BUF_ID; i < ARRAY_SIZE(regs->cantxfg); i++) {
> > -             flexcan_write(FLEXCAN_MB_CODE_RX_INACTIVE,
> > +             priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
> >                             &regs->cantxfg[i].can_ctrl);
> >       }
> >
> >       /* Errata ERR005829: mark first TX mailbox as INACTIVE */
> > -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                     &regs->cantxfg[FLEXCAN_TX_BUF_RESERVED].can_ctrl);
> >
> >       /* mark TX mailbox as INACTIVE */
> > -     flexcan_write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > +     priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> >                     &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
> >
> >       /* acceptance mask/acceptance code (accept everything) */
> > -     flexcan_write(0x0, &regs->rxgmask);
> > -     flexcan_write(0x0, &regs->rx14mask);
> > -     flexcan_write(0x0, &regs->rx15mask);
> > +     priv->write(0x0, &regs->rxgmask);
> > +     priv->write(0x0, &regs->rx14mask);
> > +     priv->write(0x0, &regs->rx15mask);
> >
> >       if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
> > -             flexcan_write(0x0, &regs->rxfgmask);
> > +             priv->write(0x0, &regs->rxfgmask);
> >
> >       /*
> >        * On Vybrid, disable memory error detection interrupts @@
> > -934,16 +943,16 @@ static int flexcan_chip_start(struct net_device
> *dev)
> >                * and Correction of Memory Errors" to write to
> >                * MECR register
> >                */
> > -             reg_crl2 = flexcan_read(&regs->crl2);
> > +             reg_crl2 = priv->read(&regs->crl2);
> >               reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
> > -             flexcan_write(reg_crl2, &regs->crl2);
> > +             priv->write(reg_crl2, &regs->crl2);
> >
> > -             reg_mecr = flexcan_read(&regs->mecr);
> > +             reg_mecr = priv->read(&regs->mecr);
> >               reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
> > -             flexcan_write(reg_mecr, &regs->mecr);
> > +             priv->write(reg_mecr, &regs->mecr);
> >               reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ |
> FLEXCAN_MECR_HANCEI_MSK |
> >                               FLEXCAN_MECR_FANCEI_MSK);
> > -             flexcan_write(reg_mecr, &regs->mecr);
> > +             priv->write(reg_mecr, &regs->mecr);
> >       }
> >
> >       err = flexcan_transceiver_enable(priv); @@ -958,11 +967,11 @@
> > static int flexcan_chip_start(struct net_device *dev)
> >       priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >
> >       /* enable FIFO interrupts */
> > -     flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> > +     priv->write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
> >
> >       /* print chip status */
> >       netdev_dbg(dev, "%s: reading mcr=0x%08x ctrl=0x%08x\n", __func__,
> > -                flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
> > +                priv->read(&regs->mcr), priv->read(&regs->ctrl));
> >
> >       return 0;
> >
> > @@ -989,8 +998,8 @@ static void flexcan_chip_stop(struct net_device
> *dev)
> >       flexcan_chip_disable(priv);
> >
> >       /* Disable all interrupts */
> > -     flexcan_write(0, &regs->imask1);
> > -     flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> > +     priv->write(0, &regs->imask1);
> > +     priv->write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> >                     &regs->ctrl);
> >
> >       flexcan_transceiver_disable(priv);
> > @@ -1108,26 +1117,26 @@ static int register_flexcandev(struct
> net_device *dev)
> >       err = flexcan_chip_disable(priv);
> >       if (err)
> >               goto out_disable_per;
> > -     reg = flexcan_read(&regs->ctrl);
> > +     reg = priv->read(&regs->ctrl);
> >       reg |= FLEXCAN_CTRL_CLK_SRC;
> > -     flexcan_write(reg, &regs->ctrl);
> > +     priv->write(reg, &regs->ctrl);
> >
> >       err = flexcan_chip_enable(priv);
> >       if (err)
> >               goto out_chip_disable;
> >
> >       /* set freeze, halt and activate FIFO, restrict register access
> */
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
> >               FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
> > -     flexcan_write(reg, &regs->mcr);
> > +     priv->write(reg, &regs->mcr);
> >
> >       /*
> >        * Currently we only support newer versions of this core
> >        * featuring a RX FIFO. Older cores found on some Coldfire
> >        * derivates are not yet supported.
> >        */
> > -     reg = flexcan_read(&regs->mcr);
> > +     reg = priv->read(&regs->mcr);
> >       if (!(reg & FLEXCAN_MCR_FEN)) {
> >               netdev_err(dev, "Could not enable RX FIFO, unsupported
> core\n");
> >               err = -ENODEV;
> > @@ -1183,6 +1192,8 @@ static int flexcan_probe(struct platform_device
> *pdev)
> >       void __iomem *base;
> >       int err, irq;
> >       u32 clock_freq = 0;
> > +     /* Default case for most ARM based FSL SoC having BE FlexCAN IP
> */
> > +     bool core_is_little = true, module_is_little = false;
> >
> >       reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
> >       if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) @@ -1237,6 +1248,25
> > @@ static int flexcan_probe(struct platform_device *pdev)
> >       dev->flags |= IFF_ECHO;
> >
> >       priv = netdev_priv(dev);
> > +
> > +     if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
> > +             core_is_little = false;
> 
> Why not just using IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) directly ?
> (anyways, cpu_is_le seems a more appropriate name)
> 
> > +     if (of_property_read_bool(dev->dev.of_node, "little-endian"))
> > +             module_is_little = true;
> 
> Why that misleading name, instead of, eg. "chip_is_le" ?
> 
> Oh, and you'll expect explicitly set a new DTB attribute, if the chip is
> in LE - the default case.
> 
> Didn't you just say, you dont wanna break anything ?
> 
> Can you imagine that some people use DTB as a _board_ config (eg. in
> bootloader) instead of externalized kernel config ? ;-o
> 
> > +     if ((core_is_little && module_is_little) ||
> > +         (!core_is_little && !module_is_little)) {
> > +             priv->read = flexcan_read_le;
> > +             priv->write = flexcan_write_le;
> > +     }
> > +
> > +     if ((!core_is_little && module_is_little) ||
> > +         (core_is_little && !module_is_little)) {
> > +             priv->read = flexcan_read_be;
> > +             priv->write = flexcan_write_be;
> > +     }
> 
> At that point, the naming gets completely wrong - instead should be
> something like flexcan_read_swab / flexcan_write_swab.
> And the decision can easily be made on:
> 
>     IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) != module_is_little
> 
> 
> Putting these together, I'd rather:
> 
> #1: add a new feature flag:
>      #define FLEXCAN_HAS_BIG_ENDIAN BIT(4)
> 
> #2: add a new dts device type for the new chip variant
>      (eg. fsl,flexcan-qorliq)
> 
> #2: pass the priv pointer to the *_read()/*_write() functions,
>      so they can pick up the BE flag and decide whether to
>      swab or not (based on cpu endianess)
> 

You seem to have missed the endian property discussions that happened regarding DTS
Several months back. Have a look at them: [1] & [2]. This patch is compliant to various DTS that
already have such a property.

And BTW, I have already mentioned this during the v1 review of this patchset:

We are dealing with 4 possible cases here:
1. Core is LE (ARM) <-> FlexCAN IP is BE = FSL LS1021A SoC
2. Core is BE (PPC) <-> FlexCAN IP is BE = FSL P1010 SoC
3. Core is BE (PPC) <-> FlexCAN IP is LE = No SoC with this configuration yet
4. Core is LE (PPC) <-> FlexCAN IP is LE = FSL I.MX series

So, no this _simply_ won't work the way you described for the above cases.

I remember testing this patchset also on a P1010, so I don't find any legacy stuff breaking.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/254533.html
[2] http://lxr.free-electrons.com/source/arch/arm/boot/dts/ls1021a.dtsi#L127

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 41+ messages in thread

* RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
  2015-05-14 15:44       ` Sharma Bhupesh
@ 2015-12-10 11:05         ` Sharma Bhupesh
  -1 siblings, 0 replies; 41+ messages in thread
From: Sharma Bhupesh @ 2015-12-10 11:05 UTC (permalink / raw)
  To: Marc Kleine-Budde, arnd, linux-can
  Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar

Hi Mark,

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-
> bounces@lists.infradead.org] On Behalf Of Sharma Bhupesh
> Sent: Thursday, May 14, 2015 9:14 PM
> To: Marc Kleine-Budde; arnd@arndb.de; linux-can@vger.kernel.org
> Cc: bhupesh.linux@gmail.com; linux-arm-kernel@lists.infradead.org; Arora
> Sakar-B45205
> Subject: RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
> mode
> 
> > -----Original Message-----
> > From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> > Sent: Thursday, May 14, 2015 9:12 PM
> > To: Sharma Bhupesh-B45370; arnd@arndb.de; linux-can@vger.kernel.org
> > Cc: bhupesh.linux@gmail.com; Arora Sakar-B45205; linux-arm-
> > kernel@lists.infradead.org
> > Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
> > mode
> >
> > On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> > > This patch adds support for non RX-FIFO (legacy) mode in the flexcan
> > > driver.
> > >
> > > On certain SoCs, the RX-FIFO support might be broken, as a result we
> > > need to fall-back on the legacy (non RX-FIFO) mode to receive CAN
> > > frames.
> > >
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
> >
> > The non FIFO mode doesn't guarantee the order of the incoming frames,
> > not does not even try to...this is not acceptable. I'm currently
> > working on a patch by David Jander that brings in non FIFO mode, but
> > tries to keep the order of the frames.
> 
> That is already WIP at our end. V3 will contain the same change.
> If you are already working on it, I don't know how to proceed further as
> we had already v1 of this patchset with the non FIFO mode out since a
> month or so.

I don't remember seeing a patch from you which supports non-FIFO mode for flexcan IP.
Since we now have Freescale LS1021A chip out in the field on which the FIFO mode broken,
we cannot use the upstream flexcan driver for enabling flexcan IP on these chips.

Do you still have plans to work on this? Or should I submit my v3 with in-order reception
supported for rx frames.

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
@ 2015-12-10 11:05         ` Sharma Bhupesh
  0 siblings, 0 replies; 41+ messages in thread
From: Sharma Bhupesh @ 2015-12-10 11:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mark,

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-
> bounces at lists.infradead.org] On Behalf Of Sharma Bhupesh
> Sent: Thursday, May 14, 2015 9:14 PM
> To: Marc Kleine-Budde; arnd at arndb.de; linux-can at vger.kernel.org
> Cc: bhupesh.linux at gmail.com; linux-arm-kernel at lists.infradead.org; Arora
> Sakar-B45205
> Subject: RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
> mode
> 
> > -----Original Message-----
> > From: Marc Kleine-Budde [mailto:mkl at pengutronix.de]
> > Sent: Thursday, May 14, 2015 9:12 PM
> > To: Sharma Bhupesh-B45370; arnd at arndb.de; linux-can at vger.kernel.org
> > Cc: bhupesh.linux at gmail.com; Arora Sakar-B45205; linux-arm-
> > kernel at lists.infradead.org
> > Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
> > mode
> >
> > On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> > > This patch adds support for non RX-FIFO (legacy) mode in the flexcan
> > > driver.
> > >
> > > On certain SoCs, the RX-FIFO support might be broken, as a result we
> > > need to fall-back on the legacy (non RX-FIFO) mode to receive CAN
> > > frames.
> > >
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
> >
> > The non FIFO mode doesn't guarantee the order of the incoming frames,
> > not does not even try to...this is not acceptable. I'm currently
> > working on a patch by David Jander that brings in non FIFO mode, but
> > tries to keep the order of the frames.
> 
> That is already WIP at our end. V3 will contain the same change.
> If you are already working on it, I don't know how to proceed further as
> we had already v1 of this patchset with the non FIFO mode out since a
> month or so.

I don't remember seeing a patch from you which supports non-FIFO mode for flexcan IP.
Since we now have Freescale LS1021A chip out in the field on which the FIFO mode broken,
we cannot use the upstream flexcan driver for enabling flexcan IP on these chips.

Do you still have plans to work on this? Or should I submit my v3 with in-order reception
supported for rx frames.

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 41+ messages in thread

* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
  2015-12-10 11:05         ` Sharma Bhupesh
  (?)
@ 2015-12-10 11:44         ` Tom Evans
  2015-12-10 12:44             ` Marc Kleine-Budde
  -1 siblings, 1 reply; 41+ messages in thread
From: Tom Evans @ 2015-12-10 11:44 UTC (permalink / raw)
  To: Sharma Bhupesh, Marc Kleine-Budde, arnd, linux-can
  Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar

On 10/12/2015 10:05 PM, Sharma Bhupesh wrote:
> Hi Mark,
>>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
>>> mode

I've just had to delve back into the deep murky past and try to get 
Freescale's Kernel 2.6.35 FlexCAN driver working. That's because the 
only code base that supports Freescale's Video Hardware is that one and 
we need working video.

I'm suggesting this code as an "historical reference" that might be 
useful for anyone looking for simple/simplistic ways to handle a FlexCAN 
port. It isn't like the driver actually works fully. It can't have ever 
been tested very much. Which is why I've had to fix it.

It has the Driver, Register handling and Message Buffer handling split 
into three separate source files, which is a useful simplification.

It can't be configured with "canconfig", but exposes a huge number of 
variables in the sysfs which is actually quite powerful and extendable.

Freescale's "2.6.35_maintain" kernel consists of 2.6.35 plus about 1200 
Freescale patches which basically replace all the hardware drivers.

The source for this driver in this tree (until someone renames it to NXP):

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/log/?h=imx_2.6.35_maintain

The Driver is in the expected place:

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/net/can/flexcan?h=imx_2.6.35_maintain

1 - It defaults to 32 receive and 32 transmit MBs, but that can be changed.

2 - It can't receive in order, but you'd only need to sort on the 
timestamps to fix that (or use the FIFO).

3 - It can't transmit in order either (unless you drop it to one 
transmit MB), but a simple change to the Transmit MB assignment would 
fix that.

4 - It supports FIFO mode, but it doesn't work at all. The FIFO hardware 
gets so badly screwed by the driver that it has to be power-cycled to 
get it working again.

5 - Transmit doesn't work either. If you push it it'll start sending ONE 
Message per SECOND. That's an easy fix by changing the code to actually 
unblock the netif queue instead of what it does.

6 - The sysfs variable that reserves receive message buffers is named to 
reserve transmit buffers, it messes up if the DLC is over 8 (because it 
doesn't call get_can_dlc()), the Dump routines don't work properly, 
off-by-one bug, comparison-reversal bug and so on.

7 - The only good thing about it is that it doesn't use NAPI, so when 
fixed it doesn't drop messages when you try to use MMC/eMMC/ESDHC.

FIFO bug documented and fixed here:

https://community.freescale.com/thread/381075

Transmit bug documented and fixed here (and the other ones listed), also 
MMC/eMMC problem:

https://community.freescale.com/message/595897

Patches to fix all of it on the end of this thread:

https://community.freescale.com/thread/303271

Tom


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
  2015-12-10 11:05         ` Sharma Bhupesh
@ 2015-12-10 12:19           ` Marc Kleine-Budde
  -1 siblings, 0 replies; 41+ messages in thread
From: Marc Kleine-Budde @ 2015-12-10 12:19 UTC (permalink / raw)
  To: Sharma Bhupesh, arnd, linux-can
  Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar

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

On 12/10/2015 12:05 PM, Sharma Bhupesh wrote:
>>> -----Original Message-----
>>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
>>> Sent: Thursday, May 14, 2015 9:12 PM
>>> To: Sharma Bhupesh-B45370; arnd@arndb.de; linux-can@vger.kernel.org
>>> Cc: bhupesh.linux@gmail.com; Arora Sakar-B45205; linux-arm-
>>> kernel@lists.infradead.org
>>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
>>> mode
>>>
>>> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
>>>> This patch adds support for non RX-FIFO (legacy) mode in the flexcan
>>>> driver.
>>>>
>>>> On certain SoCs, the RX-FIFO support might be broken, as a result we
>>>> need to fall-back on the legacy (non RX-FIFO) mode to receive CAN
>>>> frames.
>>>>
>>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
>>>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
>>>
>>> The non FIFO mode doesn't guarantee the order of the incoming frames,
>>> not does not even try to...this is not acceptable. I'm currently
>>> working on a patch by David Jander that brings in non FIFO mode, but
>>> tries to keep the order of the frames.
>>
>> That is already WIP at our end. V3 will contain the same change.
>> If you are already working on it, I don't know how to proceed further as
>> we had already v1 of this patchset with the non FIFO mode out since a
>> month or so.
> 
> I don't remember seeing a patch from you which supports non-FIFO mode for flexcan IP.
> Since we now have Freescale LS1021A chip out in the field on which the FIFO mode broken,
> we cannot use the upstream flexcan driver for enabling flexcan IP on these chips.
> 
> Do you still have plans to work on this? Or should I submit my v3 with in-order reception
> supported for rx frames.

The problem is, that the current code triggers a ordering issue (at
least on the imx6) in non FIFO mode. The frames are not received as the
documentation states. If you have access, take a look at "SR#
1-4074792564 : CAN Ordering Issues". It seems the freescale support
doesn't have much interest in confirming the issue, nor bringing us in
touch with the people who have access to the IP core source to see
what's going on.

If we have to rely on the timestamps the next logical step is to add
sorting by timestamp to the rx-fifo implementation. I'll send a patch
series of the current state.

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
@ 2015-12-10 12:19           ` Marc Kleine-Budde
  0 siblings, 0 replies; 41+ messages in thread
From: Marc Kleine-Budde @ 2015-12-10 12:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/10/2015 12:05 PM, Sharma Bhupesh wrote:
>>> -----Original Message-----
>>> From: Marc Kleine-Budde [mailto:mkl at pengutronix.de]
>>> Sent: Thursday, May 14, 2015 9:12 PM
>>> To: Sharma Bhupesh-B45370; arnd at arndb.de; linux-can at vger.kernel.org
>>> Cc: bhupesh.linux at gmail.com; Arora Sakar-B45205; linux-arm-
>>> kernel at lists.infradead.org
>>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO
>>> mode
>>>
>>> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
>>>> This patch adds support for non RX-FIFO (legacy) mode in the flexcan
>>>> driver.
>>>>
>>>> On certain SoCs, the RX-FIFO support might be broken, as a result we
>>>> need to fall-back on the legacy (non RX-FIFO) mode to receive CAN
>>>> frames.
>>>>
>>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
>>>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
>>>
>>> The non FIFO mode doesn't guarantee the order of the incoming frames,
>>> not does not even try to...this is not acceptable. I'm currently
>>> working on a patch by David Jander that brings in non FIFO mode, but
>>> tries to keep the order of the frames.
>>
>> That is already WIP at our end. V3 will contain the same change.
>> If you are already working on it, I don't know how to proceed further as
>> we had already v1 of this patchset with the non FIFO mode out since a
>> month or so.
> 
> I don't remember seeing a patch from you which supports non-FIFO mode for flexcan IP.
> Since we now have Freescale LS1021A chip out in the field on which the FIFO mode broken,
> we cannot use the upstream flexcan driver for enabling flexcan IP on these chips.
> 
> Do you still have plans to work on this? Or should I submit my v3 with in-order reception
> supported for rx frames.

The problem is, that the current code triggers a ordering issue (at
least on the imx6) in non FIFO mode. The frames are not received as the
documentation states. If you have access, take a look at "SR#
1-4074792564 : CAN Ordering Issues". It seems the freescale support
doesn't have much interest in confirming the issue, nor bringing us in
touch with the people who have access to the IP core source to see
what's going on.

If we have to rely on the timestamps the next logical step is to add
sorting by timestamp to the rx-fifo implementation. I'll send a patch
series of the current state.

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151210/496f3ff5/attachment.sig>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* RE: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
  2015-12-10 12:19           ` Marc Kleine-Budde
@ 2015-12-10 12:22             ` Sharma Bhupesh
  -1 siblings, 0 replies; 41+ messages in thread
From: Sharma Bhupesh @ 2015-12-10 12:22 UTC (permalink / raw)
  To: Marc Kleine-Budde, arnd, linux-can
  Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: Thursday, December 10, 2015 5:49 PM
> 
> On 12/10/2015 12:05 PM, Sharma Bhupesh wrote:
> >>> -----Original Message-----
> >>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> >>> Sent: Thursday, May 14, 2015 9:12 PM
> >>> To: Sharma Bhupesh-B45370; arnd@arndb.de; linux-can@vger.kernel.org
> >>> Cc: bhupesh.linux@gmail.com; Arora Sakar-B45205; linux-arm-
> >>> kernel@lists.infradead.org
> >>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non
> >>> RX-FIFO mode
> >>>
> >>> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> >>>> This patch adds support for non RX-FIFO (legacy) mode in the
> >>>> flexcan driver.
> >>>>
> >>>> On certain SoCs, the RX-FIFO support might be broken, as a result
> >>>> we need to fall-back on the legacy (non RX-FIFO) mode to receive
> >>>> CAN frames.
> >>>>
> >>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> >>>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
> >>>
> >>> The non FIFO mode doesn't guarantee the order of the incoming
> >>> frames, not does not even try to...this is not acceptable. I'm
> >>> currently working on a patch by David Jander that brings in non FIFO
> >>> mode, but tries to keep the order of the frames.
> >>
> >> That is already WIP at our end. V3 will contain the same change.
> >> If you are already working on it, I don't know how to proceed further
> >> as we had already v1 of this patchset with the non FIFO mode out
> >> since a month or so.
> >
> > I don't remember seeing a patch from you which supports non-FIFO mode
> for flexcan IP.
> > Since we now have Freescale LS1021A chip out in the field on which the
> > FIFO mode broken, we cannot use the upstream flexcan driver for
> enabling flexcan IP on these chips.
> >
> > Do you still have plans to work on this? Or should I submit my v3 with
> > in-order reception supported for rx frames.
> 
> The problem is, that the current code triggers a ordering issue (at least
> on the imx6) in non FIFO mode. The frames are not received as the
> documentation states. If you have access, take a look at "SR#
> 1-4074792564 : CAN Ordering Issues". It seems the freescale support
> doesn't have much interest in confirming the issue, nor bringing us in
> touch with the people who have access to the IP core source to see what's
> going on.
> 
> If we have to rely on the timestamps the next logical step is to add
> sorting by timestamp to the rx-fifo implementation. I'll send a patch
> series of the current state.

With my current v3, I see no rx-frame ordering issues atleast on LS1021A.

I can ask internally about more details on the "SR# 1-4074792564 : CAN Ordering Issues".

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
@ 2015-12-10 12:22             ` Sharma Bhupesh
  0 siblings, 0 replies; 41+ messages in thread
From: Sharma Bhupesh @ 2015-12-10 12:22 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl at pengutronix.de]
> Sent: Thursday, December 10, 2015 5:49 PM
> 
> On 12/10/2015 12:05 PM, Sharma Bhupesh wrote:
> >>> -----Original Message-----
> >>> From: Marc Kleine-Budde [mailto:mkl at pengutronix.de]
> >>> Sent: Thursday, May 14, 2015 9:12 PM
> >>> To: Sharma Bhupesh-B45370; arnd at arndb.de; linux-can at vger.kernel.org
> >>> Cc: bhupesh.linux at gmail.com; Arora Sakar-B45205; linux-arm-
> >>> kernel at lists.infradead.org
> >>> Subject: Re: [PATCH v2 5/5] can: flexcan: Add support for non
> >>> RX-FIFO mode
> >>>
> >>> On 05/14/2015 01:33 PM, Bhupesh Sharma wrote:
> >>>> This patch adds support for non RX-FIFO (legacy) mode in the
> >>>> flexcan driver.
> >>>>
> >>>> On certain SoCs, the RX-FIFO support might be broken, as a result
> >>>> we need to fall-back on the legacy (non RX-FIFO) mode to receive
> >>>> CAN frames.
> >>>>
> >>>> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> >>>> Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
> >>>
> >>> The non FIFO mode doesn't guarantee the order of the incoming
> >>> frames, not does not even try to...this is not acceptable. I'm
> >>> currently working on a patch by David Jander that brings in non FIFO
> >>> mode, but tries to keep the order of the frames.
> >>
> >> That is already WIP at our end. V3 will contain the same change.
> >> If you are already working on it, I don't know how to proceed further
> >> as we had already v1 of this patchset with the non FIFO mode out
> >> since a month or so.
> >
> > I don't remember seeing a patch from you which supports non-FIFO mode
> for flexcan IP.
> > Since we now have Freescale LS1021A chip out in the field on which the
> > FIFO mode broken, we cannot use the upstream flexcan driver for
> enabling flexcan IP on these chips.
> >
> > Do you still have plans to work on this? Or should I submit my v3 with
> > in-order reception supported for rx frames.
> 
> The problem is, that the current code triggers a ordering issue (at least
> on the imx6) in non FIFO mode. The frames are not received as the
> documentation states. If you have access, take a look at "SR#
> 1-4074792564 : CAN Ordering Issues". It seems the freescale support
> doesn't have much interest in confirming the issue, nor bringing us in
> touch with the people who have access to the IP core source to see what's
> going on.
> 
> If we have to rely on the timestamps the next logical step is to add
> sorting by timestamp to the rx-fifo implementation. I'll send a patch
> series of the current state.

With my current v3, I see no rx-frame ordering issues atleast on LS1021A.

I can ask internally about more details on the "SR# 1-4074792564 : CAN Ordering Issues".

Regards,
Bhupesh

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
  2015-12-10 11:44         ` can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works Tom Evans
@ 2015-12-10 12:44             ` Marc Kleine-Budde
  0 siblings, 0 replies; 41+ messages in thread
From: Marc Kleine-Budde @ 2015-12-10 12:44 UTC (permalink / raw)
  To: Tom Evans, Sharma Bhupesh, arnd, linux-can
  Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar

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

On 12/10/2015 12:44 PM, Tom Evans wrote:
> I've just had to delve back into the deep murky past and try to get 
> Freescale's Kernel 2.6.35 FlexCAN driver working. That's because the 
> only code base that supports Freescale's Video Hardware is that one and 
> we need working video.

2.6.x I feel your pain. Another option would be to port the flexcan
driver to that old kernel. But I'm getting off topic here :)

> I'm suggesting this code as an "historical reference" that might be 
> useful for anyone looking for simple/simplistic ways to handle a FlexCAN 
> port. It isn't like the driver actually works fully. It can't have ever 
> been tested very much. Which is why I've had to fix it.
> 
> It has the Driver, Register handling and Message Buffer handling split 
> into three separate source files, which is a useful simplification.

YMMV :)

> It can't be configured with "canconfig", but exposes a huge number of 
> variables in the sysfs which is actually quite powerful and extendable.
> 
> Freescale's "2.6.35_maintain" kernel consists of 2.6.35 plus about 1200 
> Freescale patches which basically replace all the hardware drivers.
> 
> The source for this driver in this tree (until someone renames it to NXP):
> 
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/log/?h=imx_2.6.35_maintain
> 
> The Driver is in the expected place:
> 
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/net/can/flexcan?h=imx_2.6.35_maintain
> 
> 1 - It defaults to 32 receive and 32 transmit MBs, but that can be changed.
> 
> 2 - It can't receive in order, but you'd only need to sort on the 
> timestamps to fix that (or use the FIFO).

In order reception is crucial.

> 3 - It can't transmit in order either (unless you drop it to one 
> transmit MB), but a simple change to the Transmit MB assignment would 
> fix that.

Same here.

> 4 - It supports FIFO mode, but it doesn't work at all. The FIFO hardware 
> gets so badly screwed by the driver that it has to be power-cycled to 
> get it working again.
> 
> 5 - Transmit doesn't work either. If you push it it'll start sending ONE 
> Message per SECOND. That's an easy fix by changing the code to actually 
> unblock the netif queue instead of what it does.
> 
> 6 - The sysfs variable that reserves receive message buffers is named to 
> reserve transmit buffers, it messes up if the DLC is over 8 (because it 
> doesn't call get_can_dlc()), the Dump routines don't work properly, 
> off-by-one bug, comparison-reversal bug and so on.
> 
> 7 - The only good thing about it is that it doesn't use NAPI, so when 
> fixed it doesn't drop messages when you try to use MMC/eMMC/ESDHC.
> 
> FIFO bug documented and fixed here:
> 
> https://community.freescale.com/thread/381075
> 
> Transmit bug documented and fixed here (and the other ones listed), also 
> MMC/eMMC problem:
> 
> https://community.freescale.com/message/595897
> 
> Patches to fix all of it on the end of this thread:
> 
> https://community.freescale.com/thread/303271

If there's interest in working with this prehistoric kernel, why isn't
there anyone that picks up all the patches and add them to a git repo.
fsl doesn't seem to care either, maybe unless you're using their newest
product :(

just my 2¢,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

^ permalink raw reply	[flat|nested] 41+ messages in thread

* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
@ 2015-12-10 12:44             ` Marc Kleine-Budde
  0 siblings, 0 replies; 41+ messages in thread
From: Marc Kleine-Budde @ 2015-12-10 12:44 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/10/2015 12:44 PM, Tom Evans wrote:
> I've just had to delve back into the deep murky past and try to get 
> Freescale's Kernel 2.6.35 FlexCAN driver working. That's because the 
> only code base that supports Freescale's Video Hardware is that one and 
> we need working video.

2.6.x I feel your pain. Another option would be to port the flexcan
driver to that old kernel. But I'm getting off topic here :)

> I'm suggesting this code as an "historical reference" that might be 
> useful for anyone looking for simple/simplistic ways to handle a FlexCAN 
> port. It isn't like the driver actually works fully. It can't have ever 
> been tested very much. Which is why I've had to fix it.
> 
> It has the Driver, Register handling and Message Buffer handling split 
> into three separate source files, which is a useful simplification.

YMMV :)

> It can't be configured with "canconfig", but exposes a huge number of 
> variables in the sysfs which is actually quite powerful and extendable.
> 
> Freescale's "2.6.35_maintain" kernel consists of 2.6.35 plus about 1200 
> Freescale patches which basically replace all the hardware drivers.
> 
> The source for this driver in this tree (until someone renames it to NXP):
> 
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/log/?h=imx_2.6.35_maintain
> 
> The Driver is in the expected place:
> 
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/net/can/flexcan?h=imx_2.6.35_maintain
> 
> 1 - It defaults to 32 receive and 32 transmit MBs, but that can be changed.
> 
> 2 - It can't receive in order, but you'd only need to sort on the 
> timestamps to fix that (or use the FIFO).

In order reception is crucial.

> 3 - It can't transmit in order either (unless you drop it to one 
> transmit MB), but a simple change to the Transmit MB assignment would 
> fix that.

Same here.

> 4 - It supports FIFO mode, but it doesn't work at all. The FIFO hardware 
> gets so badly screwed by the driver that it has to be power-cycled to 
> get it working again.
> 
> 5 - Transmit doesn't work either. If you push it it'll start sending ONE 
> Message per SECOND. That's an easy fix by changing the code to actually 
> unblock the netif queue instead of what it does.
> 
> 6 - The sysfs variable that reserves receive message buffers is named to 
> reserve transmit buffers, it messes up if the DLC is over 8 (because it 
> doesn't call get_can_dlc()), the Dump routines don't work properly, 
> off-by-one bug, comparison-reversal bug and so on.
> 
> 7 - The only good thing about it is that it doesn't use NAPI, so when 
> fixed it doesn't drop messages when you try to use MMC/eMMC/ESDHC.
> 
> FIFO bug documented and fixed here:
> 
> https://community.freescale.com/thread/381075
> 
> Transmit bug documented and fixed here (and the other ones listed), also 
> MMC/eMMC problem:
> 
> https://community.freescale.com/message/595897
> 
> Patches to fix all of it on the end of this thread:
> 
> https://community.freescale.com/thread/303271

If there's interest in working with this prehistoric kernel, why isn't
there anyone that picks up all the patches and add them to a git repo.
fsl doesn't seem to care either, maybe unless you're using their newest
product :(

just my 2?,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151210/e8f00de9/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
  2015-12-10 12:44             ` Marc Kleine-Budde
@ 2015-12-10 22:53               ` Tom Evans
  -1 siblings, 0 replies; 41+ messages in thread
From: Tom Evans @ 2015-12-10 22:53 UTC (permalink / raw)
  To: Marc Kleine-Budde, Sharma Bhupesh, arnd, linux-can
  Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar

On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote:
> On 12/10/2015 12:44 PM, Tom Evans wrote:
>> I've just had to delve back into the deep murky past and try to get
>> Freescale's Kernel 2.6.35 FlexCAN driver working...

On an i.MX53. That's the last supported kernel Freescale ever released 
for that series of parts.

> 2.6.x I feel your pain. Another option would be to port the flexcan
> driver to that old kernel. But I'm getting off topic here :)

Been there. Options are to port 2.6.38 (Freescale's Android release) 
which has the 2.6.38 mainstream driver, but looks to have a full 
collection of all the bugs fixed since then. Some kind soul has already 
done that and released a patch (but I don't like the bugs). Or the 3.4 
version we're running with our 3.4 kernel or something more modern. I 
don't trust later versions to even compile - the headers and interfaces 
change. Then I can't use modern ones anyway as they use NAPI and that 
simply doesn't work with Linux and CAN. So I'd have to apply all my 
de-NAPI patches. The simplest path was to fix the simple and stupidly 
obvious bugs in the existing driver rather than add new complicated bugs.

>> 2 - It can't receive in order, but you'd only need to sort on the
>> 3 - It can't transmit in order either (unless you drop it to one
>
> In order reception is crucial.

So what mind-set at Freescale doesn't see this as a problem? I suspect 
their CAN knowledge stopped at the HCS08 stage, making tiny devices that 
control just one thing, like a door lock. So just receive and transmit 
one or two messages where "priority order" is more important than 
running any debug or diagnostic protocols.

>> Patches to fix all of it on the end of this thread:
>>
>> https://community.freescale.com/thread/303271
>
> If there's interest in working with this prehistoric kernel, why isn't
> there anyone that picks up all the patches and add them to a git repo.

There was. That's what the above post is meant to be for. But the last 
actual activity was in January 2013. It details how to submit patches, 
but it is too onerous for me. It has to use their exact complete 
distribution (of everything, not just the kernel) running on their exact 
development board. It also needs your .gitconfig set up "just so" 
matching your Freescale registration details. I'm using the thread as a 
dumping ground for "useful patches for customers", and hope google or 
Freescale's search can find them.

> fsl doesn't seem to care either, maybe unless you're using
 > their newest product :(

The team seems to disappear almost before the product is released. They 
all moved to i.MX6 years ago.

Tom



^ permalink raw reply	[flat|nested] 41+ messages in thread

* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
@ 2015-12-10 22:53               ` Tom Evans
  0 siblings, 0 replies; 41+ messages in thread
From: Tom Evans @ 2015-12-10 22:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote:
> On 12/10/2015 12:44 PM, Tom Evans wrote:
>> I've just had to delve back into the deep murky past and try to get
>> Freescale's Kernel 2.6.35 FlexCAN driver working...

On an i.MX53. That's the last supported kernel Freescale ever released 
for that series of parts.

> 2.6.x I feel your pain. Another option would be to port the flexcan
> driver to that old kernel. But I'm getting off topic here :)

Been there. Options are to port 2.6.38 (Freescale's Android release) 
which has the 2.6.38 mainstream driver, but looks to have a full 
collection of all the bugs fixed since then. Some kind soul has already 
done that and released a patch (but I don't like the bugs). Or the 3.4 
version we're running with our 3.4 kernel or something more modern. I 
don't trust later versions to even compile - the headers and interfaces 
change. Then I can't use modern ones anyway as they use NAPI and that 
simply doesn't work with Linux and CAN. So I'd have to apply all my 
de-NAPI patches. The simplest path was to fix the simple and stupidly 
obvious bugs in the existing driver rather than add new complicated bugs.

>> 2 - It can't receive in order, but you'd only need to sort on the
>> 3 - It can't transmit in order either (unless you drop it to one
>
> In order reception is crucial.

So what mind-set at Freescale doesn't see this as a problem? I suspect 
their CAN knowledge stopped at the HCS08 stage, making tiny devices that 
control just one thing, like a door lock. So just receive and transmit 
one or two messages where "priority order" is more important than 
running any debug or diagnostic protocols.

>> Patches to fix all of it on the end of this thread:
>>
>> https://community.freescale.com/thread/303271
>
> If there's interest in working with this prehistoric kernel, why isn't
> there anyone that picks up all the patches and add them to a git repo.

There was. That's what the above post is meant to be for. But the last 
actual activity was in January 2013. It details how to submit patches, 
but it is too onerous for me. It has to use their exact complete 
distribution (of everything, not just the kernel) running on their exact 
development board. It also needs your .gitconfig set up "just so" 
matching your Freescale registration details. I'm using the thread as a 
dumping ground for "useful patches for customers", and hope google or 
Freescale's search can find them.

> fsl doesn't seem to care either, maybe unless you're using
 > their newest product :(

The team seems to disappear almost before the product is released. They 
all moved to i.MX6 years ago.

Tom

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
  2015-12-10 12:22             ` Sharma Bhupesh
@ 2015-12-11 16:01               ` Robert Schwebel
  -1 siblings, 0 replies; 41+ messages in thread
From: Robert Schwebel @ 2015-12-11 16:01 UTC (permalink / raw)
  To: Sharma Bhupesh
  Cc: Marc Kleine-Budde, arnd, linux-can, bhupesh.linux,
	linux-arm-kernel, Arora Sakar

On Thu, Dec 10, 2015 at 12:22:55PM +0000, Sharma Bhupesh wrote:
> > If we have to rely on the timestamps the next logical step is to add
> > sorting by timestamp to the rx-fifo implementation. I'll send a
> > patch series of the current state.
> 
> With my current v3, I see no rx-frame ordering issues atleast on
> LS1021A.

Marc can post our testcases here on the list (maybe with the description
from the service request); perhaps this can clarify the issues.

> I can ask internally about more details on the "SR# 1-4074792564 : CAN
> Ordering Issues".

That would be quite helpful, thanks! Understanding the root cause of the
issue is often a good start, maybe there is an option for a software
workaround.

rsc
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode
@ 2015-12-11 16:01               ` Robert Schwebel
  0 siblings, 0 replies; 41+ messages in thread
From: Robert Schwebel @ 2015-12-11 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 10, 2015 at 12:22:55PM +0000, Sharma Bhupesh wrote:
> > If we have to rely on the timestamps the next logical step is to add
> > sorting by timestamp to the rx-fifo implementation. I'll send a
> > patch series of the current state.
> 
> With my current v3, I see no rx-frame ordering issues atleast on
> LS1021A.

Marc can post our testcases here on the list (maybe with the description
from the service request); perhaps this can clarify the issues.

> I can ask internally about more details on the "SR# 1-4074792564 : CAN
> Ordering Issues".

That would be quite helpful, thanks! Understanding the root cause of the
issue is often a good start, maybe there is an option for a software
workaround.

rsc
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
  2015-12-10 22:53               ` Tom Evans
@ 2015-12-17  4:22                 ` Tom Evans
  -1 siblings, 0 replies; 41+ messages in thread
From: Tom Evans @ 2015-12-17  4:22 UTC (permalink / raw)
  To: Marc Kleine-Budde, Sharma Bhupesh, arnd, linux-can
  Cc: bhupesh.linux, linux-arm-kernel, Arora Sakar

On 11/12/15 09:53, Tom Evans wrote:
> On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote:
>> On 12/10/2015 12:44 PM, Tom Evans wrote:
>>> I've just had to delve back into the deep murky past and try to get
>>> Freescale's Kernel 2.6.35 FlexCAN driver working...
>
> On an i.MX53. That's the last supported kernel Freescale ever released for
> that series of parts.

> The simplest path was to fix the
> simple and stupidly obvious bugs in the existing driver rather
 > than add new complicated bugs.

There seems to be no end of bugs in this driver. It is a lesson in something.

What's the dumbest driver bug ever? Interrupt hazard. Yes, this one has that too.

The mainline sends a Message Buffer (or tries to) and DISABLES the netif queue 
if it can't send [1]. The transmit interrupt service routine ENABLES the netif 
queue. And when the interrupt routine happens in the middle of the mainline 
transmit routine? It locks forever. You have to power-cycle to get it back. 
Normally taking the interface "down" and "up" should fix it, as the "start" 
and "stop" functions normally stop and start the netif queue. Not these ones. 
They do now.

Note 1: This is in the case where the transmit queues and MBs are full. The 
transmit function is always called twice. The first time it is called it sends 
the MB, leaves the queue running and return "OK". The second time it finds out 
it is full, stops the queue and returns "BUSY". Even better, with the default 
32 transmit MBs it has to scan through all 32 MBs to find out it is full, and 
it may have also had to scan the same 32 to find an empty one. The FlexCAN 
registers are on a slow bus and take 180ns to read or write. So it can take 
the driver up to 12us to send one buffer. Or proportionally longer if you have 
more Transmit MBs (you might have 56).

If you never send enough data to flow-control the driver this won't happen. If 
you have more than one Transmit MB (and can handle out of order transmissions) 
then the subsequent transmit interrupts will repair the damage and you won't 
see it either. That's probably why it passed whatever testing it had.

Bug detailed here:

https://community.freescale.com/message/597952

Patch to fix here:

https://community.freescale.com/message/597951#597951
Tom


^ permalink raw reply	[flat|nested] 41+ messages in thread

* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
@ 2015-12-17  4:22                 ` Tom Evans
  0 siblings, 0 replies; 41+ messages in thread
From: Tom Evans @ 2015-12-17  4:22 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/12/15 09:53, Tom Evans wrote:
> On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote:
>> On 12/10/2015 12:44 PM, Tom Evans wrote:
>>> I've just had to delve back into the deep murky past and try to get
>>> Freescale's Kernel 2.6.35 FlexCAN driver working...
>
> On an i.MX53. That's the last supported kernel Freescale ever released for
> that series of parts.

> The simplest path was to fix the
> simple and stupidly obvious bugs in the existing driver rather
 > than add new complicated bugs.

There seems to be no end of bugs in this driver. It is a lesson in something.

What's the dumbest driver bug ever? Interrupt hazard. Yes, this one has that too.

The mainline sends a Message Buffer (or tries to) and DISABLES the netif queue 
if it can't send [1]. The transmit interrupt service routine ENABLES the netif 
queue. And when the interrupt routine happens in the middle of the mainline 
transmit routine? It locks forever. You have to power-cycle to get it back. 
Normally taking the interface "down" and "up" should fix it, as the "start" 
and "stop" functions normally stop and start the netif queue. Not these ones. 
They do now.

Note 1: This is in the case where the transmit queues and MBs are full. The 
transmit function is always called twice. The first time it is called it sends 
the MB, leaves the queue running and return "OK". The second time it finds out 
it is full, stops the queue and returns "BUSY". Even better, with the default 
32 transmit MBs it has to scan through all 32 MBs to find out it is full, and 
it may have also had to scan the same 32 to find an empty one. The FlexCAN 
registers are on a slow bus and take 180ns to read or write. So it can take 
the driver up to 12us to send one buffer. Or proportionally longer if you have 
more Transmit MBs (you might have 56).

If you never send enough data to flow-control the driver this won't happen. If 
you have more than one Transmit MB (and can handle out of order transmissions) 
then the subsequent transmit interrupts will repair the damage and you won't 
see it either. That's probably why it passed whatever testing it had.

Bug detailed here:

https://community.freescale.com/message/597952

Patch to fix here:

https://community.freescale.com/message/597951#597951
Tom

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
  2015-12-17  4:22                 ` Tom Evans
@ 2015-12-23  0:53                   ` Tom Evans
  -1 siblings, 0 replies; 41+ messages in thread
From: Tom Evans @ 2015-12-23  0:53 UTC (permalink / raw)
  To: arnd, linux-can; +Cc: linux-arm-kernel

On 17/12/15 15:22, Tom Evans wrote:
> On 11/12/15 09:53, Tom Evans wrote:
>> On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote:
 >>
>> The simplest path was to fix the simple and stupidly obvious
 >> bugs in the existing driver rather than add new complicated bugs.
>
> There seems to be no end of bugs in this driver. It is a lesson
 > in something.

The Bus Off Recovery doesn't work either. A Bus Off condition results in the 
module being soft-reset and then never woken up again, mainly because the 
calls to "mod_timer()" forget to add "jiffies", so the timer doesn't trigger. 
Then the Auto/Manual mode recovery test is probably backwards. Then it tests 
MDIS instead of HALT, so it never resets the module after the soft reset. If 
you program it in "Auto Recovery" mode it resets it anyway.

Bugs detailed here:

https://community.freescale.com/message/599099#599099

There'll probably be a link on that page to the patches to fix it after I've 
tested it some more.

Tom


^ permalink raw reply	[flat|nested] 41+ messages in thread

* can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works.
@ 2015-12-23  0:53                   ` Tom Evans
  0 siblings, 0 replies; 41+ messages in thread
From: Tom Evans @ 2015-12-23  0:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 17/12/15 15:22, Tom Evans wrote:
> On 11/12/15 09:53, Tom Evans wrote:
>> On 10/12/2015 11:44 PM, Marc Kleine-Budde wrote:
 >>
>> The simplest path was to fix the simple and stupidly obvious
 >> bugs in the existing driver rather than add new complicated bugs.
>
> There seems to be no end of bugs in this driver. It is a lesson
 > in something.

The Bus Off Recovery doesn't work either. A Bus Off condition results in the 
module being soft-reset and then never woken up again, mainly because the 
calls to "mod_timer()" forget to add "jiffies", so the timer doesn't trigger. 
Then the Auto/Manual mode recovery test is probably backwards. Then it tests 
MDIS instead of HALT, so it never resets the module after the soft reset. If 
you program it in "Auto Recovery" mode it resets it anyway.

Bugs detailed here:

https://community.freescale.com/message/599099#599099

There'll probably be a link on that page to the patches to fix it after I've 
tested it some more.

Tom

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2015-12-23  0:53 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-14 11:33 [PATCH v2 0/5] Add flexcan support for LS1021A SoCs Bhupesh Sharma
2015-05-14 11:33 ` Bhupesh Sharma
2015-05-14 11:33 ` [PATCH v2 1/5] doc/bindings: Add 'endianess' optional-property for FlexCAN controller Bhupesh Sharma
2015-05-14 11:33   ` Bhupesh Sharma
2015-05-14 11:33 ` [PATCH v2 2/5] arm/dts: Add nodes for flexcan devices present on LS1021A SoC Bhupesh Sharma
2015-05-14 11:33   ` Bhupesh Sharma
2015-05-14 11:33 ` [PATCH v2 3/5] can: flexcan: Add ls1021a flexcan device entry Bhupesh Sharma
2015-05-14 11:33   ` Bhupesh Sharma
2015-05-14 15:38   ` Marc Kleine-Budde
2015-05-14 15:38     ` Marc Kleine-Budde
2015-05-14 11:33 ` [PATCH v2 4/5] can: flexcan: Remodel FlexCAN register r/w APIs for BE instances Bhupesh Sharma
2015-05-14 11:33   ` Bhupesh Sharma
2015-05-18 16:17   ` Enrico Weigelt, metux IT consult
2015-05-18 16:17     ` Enrico Weigelt, metux IT consult
2015-05-18 16:37     ` Sharma Bhupesh
2015-05-18 16:37       ` Sharma Bhupesh
2015-05-14 11:33 ` [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode Bhupesh Sharma
2015-05-14 11:33   ` Bhupesh Sharma
2015-05-14 15:41   ` Marc Kleine-Budde
2015-05-14 15:41     ` Marc Kleine-Budde
2015-05-14 15:44     ` Sharma Bhupesh
2015-05-14 15:44       ` Sharma Bhupesh
2015-12-10 11:05       ` Sharma Bhupesh
2015-12-10 11:05         ` Sharma Bhupesh
2015-12-10 11:44         ` can: flexcan: Ancient Freescale Reference FlexCAN Driver and bug fixes so it works Tom Evans
2015-12-10 12:44           ` Marc Kleine-Budde
2015-12-10 12:44             ` Marc Kleine-Budde
2015-12-10 22:53             ` Tom Evans
2015-12-10 22:53               ` Tom Evans
2015-12-17  4:22               ` Tom Evans
2015-12-17  4:22                 ` Tom Evans
2015-12-23  0:53                 ` Tom Evans
2015-12-23  0:53                   ` Tom Evans
2015-12-10 12:19         ` [PATCH v2 5/5] can: flexcan: Add support for non RX-FIFO mode Marc Kleine-Budde
2015-12-10 12:19           ` Marc Kleine-Budde
2015-12-10 12:22           ` Sharma Bhupesh
2015-12-10 12:22             ` Sharma Bhupesh
2015-12-11 16:01             ` Robert Schwebel
2015-12-11 16:01               ` Robert Schwebel
2015-05-15  0:09     ` Tom Evans
2015-05-15  0:09       ` Tom Evans

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.