[PATCH v2 0/4] ARM: vf610: add FlexCAN support

[PATCH v2 0/4] ARM: vf610: add FlexCAN support

[Stefan Agner]

This series adds support for Vybrid (aka vf610) to the FlexCAN
controller and extends the device tree accordingly.

Changes in v2:
- Split out patch (seperate dt, clocks and driver changes)
- Use spaces for hardware feature flags table
- Remove drive-by change of esdhc register length
- Added related fix which enables clock in berr_counter

Stefan Agner (4):
  ARM: dts: vf610: add FlexCAN node
  ARM: imx: clk-vf610: fix FlexCAN clock gating
  can: flexcan: switch on clocks before accessing ecr register
  can: flexcan: add vf610 support for FlexCAN

 arch/arm/boot/dts/vf610.dtsi  | 23 +++++++++++
 arch/arm/mach-imx/clk-vf610.c |  6 ++-
 drivers/net/can/flexcan.c     | 89 ++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 106 insertions(+), 12 deletions(-)

-- 
2.0.1

[PATCH v2 1/4] ARM: dts: vf610: add FlexCAN node

[Stefan Agner]

Add FlexCAN node for the two FlexCAN IP instances in Vybrid.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/vf610.dtsi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index 3fb127a..9404853 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -14,6 +14,8 @@
 
 / {
 	aliases {
+		can0 = &can0;
+		can1 = &can1;
 		serial0 = &uart0;
 		serial1 = &uart1;
 		serial2 = &uart2;
@@ -103,6 +105,16 @@
 					<&clks VF610_CLK_DMAMUX1>;
 			};
 
+			can0: flexcan at 40020000 {
+				compatible = "fsl,vf610-flexcan";
+				reg = <0x40020000 0x4000>;
+				interrupts = <0 58 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks VF610_CLK_FLEXCAN0>,
+					 <&clks VF610_CLK_FLEXCAN0>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
 			uart0: serial at 40027000 {
 				compatible = "fsl,vf610-lpuart";
 				reg = <0x40027000 0x1000>;
@@ -414,6 +426,17 @@
 				clock-names = "nfc";
 				status = "disabled";
 			};
+
+			can1: flexcan at 400d4000 {
+				compatible = "fsl,vf610-flexcan";
+				reg = <0x400d4000 0x4000>;
+				interrupts = <0 59 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks VF610_CLK_FLEXCAN1>,
+					 <&clks VF610_CLK_FLEXCAN1>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
 		};
 	};
 };
-- 
2.0.1

[PATCH v2 2/4] ARM: imx: clk-vf610: fix FlexCAN clock gating

[Stefan Agner]

Extend the clock control for FlexCAN with the second gate which
enable the clocks in the Clock Divider (CCM_CSCDR2) register too.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/mach-imx/clk-vf610.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/clk-vf610.c b/arch/arm/mach-imx/clk-vf610.c
index 22dc3ee..b12b888 100644
--- a/arch/arm/mach-imx/clk-vf610.c
+++ b/arch/arm/mach-imx/clk-vf610.c
@@ -295,8 +295,10 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
 
 	clk[VF610_CLK_ASRC] = imx_clk_gate2("asrc", "ipg_bus", CCM_CCGR4, CCM_CCGRx_CGn(1));
 
-	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate2("flexcan0", "ipg_bus", CCM_CCGR0, CCM_CCGRx_CGn(0));
-	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate2("flexcan1", "ipg_bus", CCM_CCGR9, CCM_CCGRx_CGn(4));
+	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate("flexcan0_en", "ipg_bus", CCM_CSCDR2, 11);
+	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate2("flexcan0", "flexcan0_en", CCM_CCGR0, CCM_CCGRx_CGn(0));
+	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate("flexcan1_en", "ipg_bus", CCM_CSCDR2, 12);
+	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate2("flexcan1", "flexcan1_en", CCM_CCGR9, CCM_CCGRx_CGn(4));
 
 	clk[VF610_CLK_DMAMUX0] = imx_clk_gate2("dmamux0", "platform_bus", CCM_CCGR0, CCM_CCGRx_CGn(4));
 	clk[VF610_CLK_DMAMUX1] = imx_clk_gate2("dmamux1", "platform_bus", CCM_CCGR0, CCM_CCGRx_CGn(5));
-- 
2.0.1

[PATCH v2 2/4] ARM: imx: clk-vf610: fix FlexCAN clock gating

[Shawn Guo]

Copy Jingchang ...

On Mon, Jul 14, 2014 at 09:48:29AM +0200, Stefan Agner wrote:
> Extend the clock control for FlexCAN with the second gate which
> enable the clocks in the Clock Divider (CCM_CSCDR2) register too.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  arch/arm/mach-imx/clk-vf610.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-vf610.c b/arch/arm/mach-imx/clk-vf610.c
> index 22dc3ee..b12b888 100644
> --- a/arch/arm/mach-imx/clk-vf610.c
> +++ b/arch/arm/mach-imx/clk-vf610.c
> @@ -295,8 +295,10 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
>  
>  	clk[VF610_CLK_ASRC] = imx_clk_gate2("asrc", "ipg_bus", CCM_CCGR4, CCM_CCGRx_CGn(1));
>  
> -	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate2("flexcan0", "ipg_bus", CCM_CCGR0, CCM_CCGRx_CGn(0));
> -	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate2("flexcan1", "ipg_bus", CCM_CCGR9, CCM_CCGRx_CGn(4));
> +	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate("flexcan0_en", "ipg_bus", CCM_CSCDR2, 11);
> +	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate2("flexcan0", "flexcan0_en", CCM_CCGR0, CCM_CCGRx_CGn(0));

I do not quite understand what "flexcan0_en" clock is and the
relationship between it and clock "flexcan0".  I do not think it's a
parent-child clock relationship.  Jingchang, do you have more info on
this?

Also when you add a new clock, you should have a new clock ID, something
like VF610_CLK_FLEXCAN0_EN.

Shawn

> +	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate("flexcan1_en", "ipg_bus", CCM_CSCDR2, 12);
> +	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate2("flexcan1", "flexcan1_en", CCM_CCGR9, CCM_CCGRx_CGn(4));
>  
>  	clk[VF610_CLK_DMAMUX0] = imx_clk_gate2("dmamux0", "platform_bus", CCM_CCGR0, CCM_CCGRx_CGn(4));
>  	clk[VF610_CLK_DMAMUX1] = imx_clk_gate2("dmamux1", "platform_bus", CCM_CCGR0, CCM_CCGRx_CGn(5));
> -- 
> 2.0.1
> 

[PATCH v2 2/4] ARM: imx: clk-vf610: fix FlexCAN clock gating

[Stefan Agner]

Am 2014-07-14 15:39, schrieb Shawn Guo:
> Copy Jingchang ...
> 
> On Mon, Jul 14, 2014 at 09:48:29AM +0200, Stefan Agner wrote:
>> Extend the clock control for FlexCAN with the second gate which
>> enable the clocks in the Clock Divider (CCM_CSCDR2) register too.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  arch/arm/mach-imx/clk-vf610.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/clk-vf610.c b/arch/arm/mach-imx/clk-vf610.c
>> index 22dc3ee..b12b888 100644
>> --- a/arch/arm/mach-imx/clk-vf610.c
>> +++ b/arch/arm/mach-imx/clk-vf610.c
>> @@ -295,8 +295,10 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
>>
>>  	clk[VF610_CLK_ASRC] = imx_clk_gate2("asrc", "ipg_bus", CCM_CCGR4, CCM_CCGRx_CGn(1));
>>
>> -	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate2("flexcan0", "ipg_bus", CCM_CCGR0, CCM_CCGRx_CGn(0));
>> -	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate2("flexcan1", "ipg_bus", CCM_CCGR9, CCM_CCGRx_CGn(4));
>> +	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate("flexcan0_en", "ipg_bus", CCM_CSCDR2, 11);
>> +	clk[VF610_CLK_FLEXCAN0] = imx_clk_gate2("flexcan0", "flexcan0_en", CCM_CCGR0, CCM_CCGRx_CGn(0));
> 
> I do not quite understand what "flexcan0_en" clock is and the
> relationship between it and clock "flexcan0".  I do not think it's a
> parent-child clock relationship.  Jingchang, do you have more info on
> this?
> 
> Also when you add a new clock, you should have a new clock ID, something
> like VF610_CLK_FLEXCAN0_EN.

There are two enable (gates) bits to enable the FlexCAN clocks: the
first is in the divider register, the second in the clock gate register.
For most clocks there is a divider in between, then it looks like this:

clk[VF610_CLK_ESDHC0_SEL] = imx_clk_mux("esdhc0_sel", CCM_CSCMR1, 16, 2,
esdhc_sels, 4);
clk[VF610_CLK_ESDHC0_EN] = imx_clk_gate("esdhc0_en", "esdhc0_sel",
CCM_CSCDR2, 28);
clk[VF610_CLK_ESDHC0_DIV] = imx_clk_divider("esdhc0_div", "esdhc0_en",
CCM_CSCDR2, 16, 4);
clk[VF610_CLK_ESDHC0] = imx_clk_gate2("eshc0", "esdhc0_div", CCM_CCGR7,
CCM_CCGRx_CGn(1));

However, for FlexCAN no clock selection and no divider is available,
hence its just a chain of an enable and gate register...

But yes, there should be two clock entries for this (I just asking
myself how this code actually could work, afaik the boot loader don't
touches these clocks).

--
Stefan


> 
> Shawn
> 
>> +	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate("flexcan1_en", "ipg_bus", CCM_CSCDR2, 12);
>> +	clk[VF610_CLK_FLEXCAN1] = imx_clk_gate2("flexcan1", "flexcan1_en", CCM_CCGR9, CCM_CCGRx_CGn(4));
>>
>>  	clk[VF610_CLK_DMAMUX0] = imx_clk_gate2("dmamux0", "platform_bus", CCM_CCGR0, CCM_CCGRx_CGn(4));
>>  	clk[VF610_CLK_DMAMUX1] = imx_clk_gate2("dmamux1", "platform_bus", CCM_CCGR0, CCM_CCGRx_CGn(5));
>> --
>> 2.0.1
>>

[PATCH v2 2/4] ARM: imx: clk-vf610: fix FlexCAN clock gating

[Shawn Guo]

On Mon, Jul 14, 2014 at 03:55:29PM +0200, Stefan Agner wrote:
> There are two enable (gates) bits to enable the FlexCAN clocks: the
> first is in the divider register, the second in the clock gate register.
> For most clocks there is a divider in between, then it looks like this:
> 
> clk[VF610_CLK_ESDHC0_SEL] = imx_clk_mux("esdhc0_sel", CCM_CSCMR1, 16, 2,
> esdhc_sels, 4);
> clk[VF610_CLK_ESDHC0_EN] = imx_clk_gate("esdhc0_en", "esdhc0_sel",
> CCM_CSCDR2, 28);
> clk[VF610_CLK_ESDHC0_DIV] = imx_clk_divider("esdhc0_div", "esdhc0_en",
> CCM_CSCDR2, 16, 4);
> clk[VF610_CLK_ESDHC0] = imx_clk_gate2("eshc0", "esdhc0_div", CCM_CCGR7,
> CCM_CCGRx_CGn(1));
> 
> However, for FlexCAN no clock selection and no divider is available,
> hence its just a chain of an enable and gate register...

Ah, okay.  Thanks for the explanation.

Shawn

[PATCH v2 3/4] can: flexcan: switch on clocks before accessing ecr register

[Stefan Agner]

Reported-by: Ashutosh Singh <ashuleapyear@gmail.com>
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
[stefan at agner.ch: added return check for clk_enable_prepare]

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/net/can/flexcan.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f425ec2..4c598c9 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -383,11 +383,25 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
-	u32 reg = flexcan_read(&regs->ecr);
+	u32 reg, err;
+
+	err = clk_prepare_enable(priv->clk_ipg);
+	if (err)
+		return err;
+
+	err = clk_prepare_enable(priv->clk_per);
+	if (err)
+		goto out_disable_ipg;
+
+	reg = flexcan_read(&regs->ecr);
 
 	bec->txerr = (reg >> 0) & 0xff;
 	bec->rxerr = (reg >> 8) & 0xff;
 
+	clk_disable_unprepare(priv->clk_per);
+ out_disable_ipg:
+	clk_disable_unprepare(priv->clk_ipg);
+
 	return 0;
 }
 
-- 
2.0.1

[PATCH v2 3/4] can: flexcan: switch on clocks before accessing ecr register

[Marc Kleine-Budde]

On 07/14/2014 09:48 AM, Stefan Agner wrote:
> Reported-by: Ashutosh Singh <ashuleapyear@gmail.com>
> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> [stefan at agner.ch: added return check for clk_enable_prepare]
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/net/can/flexcan.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f425ec2..4c598c9 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -383,11 +383,25 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
>  {
>  	const struct flexcan_priv *priv = netdev_priv(dev);
>  	struct flexcan_regs __iomem *regs = priv->base;
> -	u32 reg = flexcan_read(&regs->ecr);
> +	u32 reg, err;
> +
> +	err = clk_prepare_enable(priv->clk_ipg);
> +	if (err)
> +		return err;
> +
> +	err = clk_prepare_enable(priv->clk_per);
> +	if (err)
> +		goto out_disable_ipg;
> +
> +	reg = flexcan_read(&regs->ecr);
>  
>  	bec->txerr = (reg >> 0) & 0xff;
>  	bec->rxerr = (reg >> 8) & 0xff;
>  
> +	clk_disable_unprepare(priv->clk_per);
> + out_disable_ipg:
> +	clk_disable_unprepare(priv->clk_ipg);
> +
>  	return 0;

You should return an error in case of an error.

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: 242 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/bdeb5094/attachment-0001.sig>

[PATCH v2 4/4] can: flexcan: add vf610 support for FlexCAN

[Stefan Agner]

Extend FlexCAN driver to support Vybrid. Vybrids variant of the IP
has ECC support which is controlled through the memory error
control register (MECR). There is also an errata which leads to
false positive error detections (ID e5295). This patch disables
the memory error detection completely.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/net/can/flexcan.c | 73 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 4c598c9..a0fc53e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -92,6 +92,27 @@
 #define FLEXCAN_CTRL_ERR_ALL \
 	(FLEXCAN_CTRL_ERR_BUS | FLEXCAN_CTRL_ERR_STATE)
 
+/* FLEXCAN control register 2 (CTRL2) bits */
+#define FLEXCAN_CRL2_ECRWRE		BIT(29)
+#define FLEXCAN_CRL2_WRMFRZ		BIT(28)
+#define FLEXCAN_CRL2_RFFN(x)		(((x) & 0x0f) << 24)
+#define FLEXCAN_CRL2_TASD(x)		(((x) & 0x1f) << 19)
+#define FLEXCAN_CRL2_MRP		BIT(18)
+#define FLEXCAN_CRL2_RRS		BIT(17)
+#define FLEXCAN_CRL2_EACEN		BIT(16)
+
+/* FLEXCAN memory error control register (MECR) bits */
+#define FLEXCAN_MECR_ECRWRDIS		BIT(31)
+#define FLEXCAN_MECR_HANCEI_MSK		BIT(19)
+#define FLEXCAN_MECR_FANCEI_MSK		BIT(18)
+#define FLEXCAN_MECR_CEI_MSK		BIT(16)
+#define FLEXCAN_MECR_HAERRIE		BIT(15)
+#define FLEXCAN_MECR_FAERRIE		BIT(14)
+#define FLEXCAN_MECR_EXTERRIE		BIT(13)
+#define FLEXCAN_MECR_RERRDIS		BIT(9)
+#define FLEXCAN_MECR_ECCDIS		BIT(8)
+#define FLEXCAN_MECR_NCEFAFRZ		BIT(7)
+
 /* FLEXCAN error and status register (ESR) bits */
 #define FLEXCAN_ESR_TWRN_INT		BIT(17)
 #define FLEXCAN_ESR_RWRN_INT		BIT(16)
@@ -150,18 +171,20 @@
  * FLEXCAN hardware feature flags
  *
  * Below is some version info we got:
- *    SOC   Version   IP-Version  Glitch-  [TR]WRN_INT
- *                                Filter?   connected?
- *   MX25  FlexCAN2  03.00.00.00     no         no
- *   MX28  FlexCAN2  03.00.04.00    yes        yes
- *   MX35  FlexCAN2  03.00.00.00     no         no
- *   MX53  FlexCAN2  03.00.00.00    yes         no
- *   MX6s  FlexCAN3  10.00.12.00    yes        yes
+ *    SOC   Version   IP-Version  Glitch-  [TR]WRN_INT  Memory err
+ *                                Filter?   connected?  detection
+ *   MX25  FlexCAN2  03.00.00.00     no         no         no
+ *   MX28  FlexCAN2  03.00.04.00    yes        yes         no
+ *   MX35  FlexCAN2  03.00.00.00     no         no         no
+ *   MX53  FlexCAN2  03.00.00.00    yes         no         no
+ *   MX6s  FlexCAN3  10.00.12.00    yes        yes         no
+ *   VF610 FlexCAN3  ?               no         no        yes
  *
  * Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
  */
 #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 */
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -192,8 +215,11 @@ struct flexcan_regs {
 	u32 crcr;		/* 0x44 */
 	u32 rxfgmask;		/* 0x48 */
 	u32 rxfir;		/* 0x4c */
-	u32 _reserved3[12];
+	u32 _reserved3[12];	/* 0x50 */
 	struct flexcan_mb cantxfg[64];
+	u32 _reserved4[408];
+	u32 mecr;		/* 0xae0 */
+	u32 erriar;		/* 0xae4 */
 };
 
 struct flexcan_devtype_data {
@@ -223,6 +249,9 @@ 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,
+};
 
 static const struct can_bittiming_const flexcan_bittiming_const = {
 	.name = DRV_NAME,
@@ -807,7 +836,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
 	int err;
-	u32 reg_mcr, reg_ctrl;
+	u32 reg_mcr, reg_ctrl, reg_crl2, reg_mecr;
 
 	/* enable module */
 	err = flexcan_chip_enable(priv);
@@ -884,6 +913,31 @@ static int flexcan_chip_start(struct net_device *dev)
 	if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
 		flexcan_write(0x0, &regs->rxfgmask);
 
+	/*
+	 * On Vybrid, disable memory error detection interrupts
+	 * and freeze mode.
+	 * This also works around errata e5295 which generates
+	 * false positive memory errors and put the device in
+	 * freeze mode.
+	 */
+	if (priv->devtype_data->features & FLEXCAN_HAS_MECR_FEATURES) {
+		/*
+		 * Follow the protocol as described in "Detection
+		 * and Correction of Memory Errors" to write to
+		 * MECR register
+		 */
+		reg_crl2 = flexcan_read(&regs->crl2);
+		reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
+		flexcan_write(reg_crl2, &regs->crl2);
+
+		reg_mecr = flexcan_read(&regs->mecr);
+		reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
+		flexcan_write(reg_mecr, &regs->mecr);
+		reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK |
+				FLEXCAN_MECR_FANCEI_MSK);
+		flexcan_write(reg_mecr, &regs->mecr);
+	}
+
 	err = flexcan_transceiver_enable(priv);
 	if (err)
 		goto out_chip_disable;
@@ -1094,6 +1148,7 @@ 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,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
+	{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, flexcan_of_match);
-- 
2.0.1

[PATCH v2 4/4] can: flexcan: add vf610 support for FlexCAN

[Marc Kleine-Budde]

On 07/14/2014 09:48 AM, Stefan Agner wrote:
> Extend FlexCAN driver to support Vybrid. Vybrids variant of the IP
> has ECC support which is controlled through the memory error
> control register (MECR). There is also an errata which leads to
> false positive error detections (ID e5295). This patch disables
> the memory error detection completely.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Looks good, small nitpick inline.

> ---
>  drivers/net/can/flexcan.c | 73 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 64 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 4c598c9..a0fc53e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -92,6 +92,27 @@
>  #define FLEXCAN_CTRL_ERR_ALL \
>  	(FLEXCAN_CTRL_ERR_BUS | FLEXCAN_CTRL_ERR_STATE)
>  
> +/* FLEXCAN control register 2 (CTRL2) bits */
> +#define FLEXCAN_CRL2_ECRWRE		BIT(29)
> +#define FLEXCAN_CRL2_WRMFRZ		BIT(28)
> +#define FLEXCAN_CRL2_RFFN(x)		(((x) & 0x0f) << 24)
> +#define FLEXCAN_CRL2_TASD(x)		(((x) & 0x1f) << 19)
> +#define FLEXCAN_CRL2_MRP		BIT(18)
> +#define FLEXCAN_CRL2_RRS		BIT(17)
> +#define FLEXCAN_CRL2_EACEN		BIT(16)
> +
> +/* FLEXCAN memory error control register (MECR) bits */
> +#define FLEXCAN_MECR_ECRWRDIS		BIT(31)
> +#define FLEXCAN_MECR_HANCEI_MSK		BIT(19)
> +#define FLEXCAN_MECR_FANCEI_MSK		BIT(18)
> +#define FLEXCAN_MECR_CEI_MSK		BIT(16)
> +#define FLEXCAN_MECR_HAERRIE		BIT(15)
> +#define FLEXCAN_MECR_FAERRIE		BIT(14)
> +#define FLEXCAN_MECR_EXTERRIE		BIT(13)
> +#define FLEXCAN_MECR_RERRDIS		BIT(9)
> +#define FLEXCAN_MECR_ECCDIS		BIT(8)
> +#define FLEXCAN_MECR_NCEFAFRZ		BIT(7)
> +
>  /* FLEXCAN error and status register (ESR) bits */
>  #define FLEXCAN_ESR_TWRN_INT		BIT(17)
>  #define FLEXCAN_ESR_RWRN_INT		BIT(16)
> @@ -150,18 +171,20 @@
>   * FLEXCAN hardware feature flags
>   *
>   * Below is some version info we got:
> - *    SOC   Version   IP-Version  Glitch-  [TR]WRN_INT
> - *                                Filter?   connected?
> - *   MX25  FlexCAN2  03.00.00.00     no         no
> - *   MX28  FlexCAN2  03.00.04.00    yes        yes
> - *   MX35  FlexCAN2  03.00.00.00     no         no
> - *   MX53  FlexCAN2  03.00.00.00    yes         no
> - *   MX6s  FlexCAN3  10.00.12.00    yes        yes
> + *    SOC   Version   IP-Version  Glitch-  [TR]WRN_INT  Memory err
> + *                                Filter?   connected?  detection
> + *   MX25  FlexCAN2  03.00.00.00     no         no         no
> + *   MX28  FlexCAN2  03.00.04.00    yes        yes         no
> + *   MX35  FlexCAN2  03.00.00.00     no         no         no
> + *   MX53  FlexCAN2  03.00.00.00    yes         no         no
> + *   MX6s  FlexCAN3  10.00.12.00    yes        yes         no
> + *   VF610 FlexCAN3  ?               no         no        yes
>   *
>   * Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
>   */
>  #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 */
>  
>  /* Structure of the message buffer */
>  struct flexcan_mb {
> @@ -192,8 +215,11 @@ struct flexcan_regs {
>  	u32 crcr;		/* 0x44 */
>  	u32 rxfgmask;		/* 0x48 */
>  	u32 rxfir;		/* 0x4c */
> -	u32 _reserved3[12];
> +	u32 _reserved3[12];	/* 0x50 */
>  	struct flexcan_mb cantxfg[64];
> +	u32 _reserved4[408];
> +	u32 mecr;		/* 0xae0 */
> +	u32 erriar;		/* 0xae4 */
>  };
>  
>  struct flexcan_devtype_data {
> @@ -223,6 +249,9 @@ 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,
> +};
>  
>  static const struct can_bittiming_const flexcan_bittiming_const = {
>  	.name = DRV_NAME,
> @@ -807,7 +836,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	int err;
> -	u32 reg_mcr, reg_ctrl;
> +	u32 reg_mcr, reg_ctrl, reg_crl2, reg_mecr;
>  
>  	/* enable module */
>  	err = flexcan_chip_enable(priv);
> @@ -884,6 +913,31 @@ static int flexcan_chip_start(struct net_device *dev)
>  	if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES)
>  		flexcan_write(0x0, &regs->rxfgmask);
>  
> +	/*
> +	 * On Vybrid, disable memory error detection interrupts
> +	 * and freeze mode.
> +	 * This also works around errata e5295 which generates
> +	 * false positive memory errors and put the device in
> +	 * freeze mode.
> +	 */
> +	if (priv->devtype_data->features & FLEXCAN_HAS_MECR_FEATURES) {
> +		/*
> +		 * Follow the protocol as described in "Detection
> +		 * and Correction of Memory Errors" to write to
> +		 * MECR register
> +		 */


One nitpick regarding the comment. In driver/net, the preferred
multi-line comment style is

/* comment starts here
 * comment continues
 */

> +		reg_crl2 = flexcan_read(&regs->crl2);
> +		reg_crl2 |= FLEXCAN_CRL2_ECRWRE;
> +		flexcan_write(reg_crl2, &regs->crl2);
> +
> +		reg_mecr = flexcan_read(&regs->mecr);
> +		reg_mecr &= ~FLEXCAN_MECR_ECRWRDIS;
> +		flexcan_write(reg_mecr, &regs->mecr);
> +		reg_mecr &= ~(FLEXCAN_MECR_NCEFAFRZ | FLEXCAN_MECR_HANCEI_MSK |
> +				FLEXCAN_MECR_FANCEI_MSK);
> +		flexcan_write(reg_mecr, &regs->mecr);
> +	}
> +
>  	err = flexcan_transceiver_enable(priv);
>  	if (err)
>  		goto out_chip_disable;
> @@ -1094,6 +1148,7 @@ 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,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
> +	{ .compatible = "fsl,vf610-flexcan", .data = &fsl_vf610_devtype_data, },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, flexcan_of_match);
> 

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: 242 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/c26714ac/attachment.sig>

[PATCH v2 4/4] can: flexcan: add vf610 support for FlexCAN

[Stefan Agner]

Am 2014-07-14 10:07, schrieb Marc Kleine-Budde:
<snip>
>> +	/*
>> +	 * On Vybrid, disable memory error detection interrupts
>> +	 * and freeze mode.
>> +	 * This also works around errata e5295 which generates
>> +	 * false positive memory errors and put the device in
>> +	 * freeze mode.
>> +	 */
>> +	if (priv->devtype_data->features & FLEXCAN_HAS_MECR_FEATURES) {
>> +		/*
>> +		 * Follow the protocol as described in "Detection
>> +		 * and Correction of Memory Errors" to write to
>> +		 * MECR register
>> +		 */
> 
> 
> One nitpick regarding the comment. In driver/net, the preferred
> multi-line comment style is
> 

Just a few lines above that one, another multi-line comment is written
with the empty line at start... With this changed to the net comment
style, we would end up in a mixed style... What do you think?

--
Stefan

[PATCH v2 4/4] can: flexcan: add vf610 support for FlexCAN

[Marc Kleine-Budde]

On 07/14/2014 03:37 PM, Stefan Agner wrote:
> Am 2014-07-14 10:07, schrieb Marc Kleine-Budde:
> <snip>
>>> +	/*
>>> +	 * On Vybrid, disable memory error detection interrupts
>>> +	 * and freeze mode.
>>> +	 * This also works around errata e5295 which generates
>>> +	 * false positive memory errors and put the device in
>>> +	 * freeze mode.
>>> +	 */
>>> +	if (priv->devtype_data->features & FLEXCAN_HAS_MECR_FEATURES) {
>>> +		/*
>>> +		 * Follow the protocol as described in "Detection
>>> +		 * and Correction of Memory Errors" to write to
>>> +		 * MECR register
>>> +		 */
>>
>>
>> One nitpick regarding the comment. In driver/net, the preferred
>> multi-line comment style is
>>
> 
> Just a few lines above that one, another multi-line comment is written
> with the empty line at start... With this changed to the net comment
> style, we would end up in a mixed style... What do you think?

Okay, keep the commenting style of the driver.

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: 242 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/a258af90/attachment.sig>

[PATCH v2 0/4] ARM: vf610: add FlexCAN support

[Marc Kleine-Budde]

On 07/14/2014 09:48 AM, Stefan Agner wrote:
> This series adds support for Vybrid (aka vf610) to the FlexCAN
> controller and extends the device tree accordingly.
> 
> Changes in v2:
> - Split out patch (seperate dt, clocks and driver changes)
> - Use spaces for hardware feature flags table
> - Remove drive-by change of esdhc register length
> - Added related fix which enables clock in berr_counter
> 
> Stefan Agner (4):
>   ARM: dts: vf610: add FlexCAN node
>   ARM: imx: clk-vf610: fix FlexCAN clock gating
>   can: flexcan: switch on clocks before accessing ecr register
>   can: flexcan: add vf610 support for FlexCAN
> 
>  arch/arm/boot/dts/vf610.dtsi  | 23 +++++++++++
>  arch/arm/mach-imx/clk-vf610.c |  6 ++-
>  drivers/net/can/flexcan.c     | 89 ++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 106 insertions(+), 12 deletions(-)

Shawn, what's the best way to upstream this? With your Ack, I can take
everything via linux-can-next.

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: 242 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/d458cb36/attachment.sig>

[PATCH v2 0/4] ARM: vf610: add FlexCAN support

[Shawn Guo]

On Mon, Jul 14, 2014 at 10:09:54AM +0200, Marc Kleine-Budde wrote:
> On 07/14/2014 09:48 AM, Stefan Agner wrote:
> > This series adds support for Vybrid (aka vf610) to the FlexCAN
> > controller and extends the device tree accordingly.
> > 
> > Changes in v2:
> > - Split out patch (seperate dt, clocks and driver changes)
> > - Use spaces for hardware feature flags table
> > - Remove drive-by change of esdhc register length
> > - Added related fix which enables clock in berr_counter
> > 
> > Stefan Agner (4):
> >   ARM: dts: vf610: add FlexCAN node
> >   ARM: imx: clk-vf610: fix FlexCAN clock gating
> >   can: flexcan: switch on clocks before accessing ecr register
> >   can: flexcan: add vf610 support for FlexCAN
> > 
> >  arch/arm/boot/dts/vf610.dtsi  | 23 +++++++++++
> >  arch/arm/mach-imx/clk-vf610.c |  6 ++-
> >  drivers/net/can/flexcan.c     | 89 ++++++++++++++++++++++++++++++++++++++-----
> >  3 files changed, 106 insertions(+), 12 deletions(-)
> 
> Shawn, what's the best way to upstream this? With your Ack, I can take
> everything via linux-can-next.

Since this is a new support and there is nothing to maintain git bisect,
I think the best way is to have the first two patches to go via my tree.

Shawn

[PATCH v2 0/4] ARM: vf610: add FlexCAN support

[Marc Kleine-Budde]

On 07/14/2014 04:04 PM, Shawn Guo wrote:
> On Mon, Jul 14, 2014 at 10:09:54AM +0200, Marc Kleine-Budde wrote:
>> On 07/14/2014 09:48 AM, Stefan Agner wrote:
>>> This series adds support for Vybrid (aka vf610) to the FlexCAN
>>> controller and extends the device tree accordingly.
>>>
>>> Changes in v2:
>>> - Split out patch (seperate dt, clocks and driver changes)
>>> - Use spaces for hardware feature flags table
>>> - Remove drive-by change of esdhc register length
>>> - Added related fix which enables clock in berr_counter
>>>
>>> Stefan Agner (4):
>>>   ARM: dts: vf610: add FlexCAN node
>>>   ARM: imx: clk-vf610: fix FlexCAN clock gating
>>>   can: flexcan: switch on clocks before accessing ecr register
>>>   can: flexcan: add vf610 support for FlexCAN
>>>
>>>  arch/arm/boot/dts/vf610.dtsi  | 23 +++++++++++
>>>  arch/arm/mach-imx/clk-vf610.c |  6 ++-
>>>  drivers/net/can/flexcan.c     | 89 ++++++++++++++++++++++++++++++++++++++-----
>>>  3 files changed, 106 insertions(+), 12 deletions(-)
>>
>> Shawn, what's the best way to upstream this? With your Ack, I can take
>> everything via linux-can-next.
> 
> Since this is a new support and there is nothing to maintain git bisect,
> I think the best way is to have the first two patches to go via my tree.

Fine, with me.

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: 242 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140714/1a244771/attachment.sig>