LinuxLists.cc - [PATCH v3 0/4] ARM: vf610: add FlexCAN support

[permalink] [raw]

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

Reported-by: Ashutosh Singh <[email protected]>
Suggested-by: Marc Kleine-Budde <[email protected]>
[[email protected]: added return check for clk_enable_prepare]

Signed-off-by: Stefan Agner <[email protected]>
---
drivers/net/can/flexcan.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f425ec2..89745aa 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -383,12 +383,26 @@ 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;

- return 0;
+ clk_disable_unprepare(priv->clk_per);
+ out_disable_ipg:
+ clk_disable_unprepare(priv->clk_ipg);
+
+ return err;
}

static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
--
2.0.1

2014-07-15 12:57:18

[permalink] [raw]

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

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 <[email protected]>
---
arch/arm/mach-imx/clk-vf610.c | 6 ++++--
include/dt-bindings/clock/vf610-clock.h | 4 +++-
2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-imx/clk-vf610.c b/arch/arm/mach-imx/clk-vf610.c
index 22dc3ee..f964079 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_EN] = 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_EN] = 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));
diff --git a/include/dt-bindings/clock/vf610-clock.h b/include/dt-bindings/clock/vf610-clock.h
index a916029..00953d9 100644
--- a/include/dt-bindings/clock/vf610-clock.h
+++ b/include/dt-bindings/clock/vf610-clock.h
@@ -164,6 +164,8 @@
#define VF610_CLK_DMAMUX1 151
#define VF610_CLK_DMAMUX2 152
#define VF610_CLK_DMAMUX3 153
-#define VF610_CLK_END 154
+#define VF610_CLK_FLEXCAN0_EN 154
+#define VF610_CLK_FLEXCAN1_EN 155
+#define VF610_CLK_END 156

#endif /* __DT_BINDINGS_CLOCK_VF610_H */
--
2.0.1

2014-07-15 12:56:40

[permalink] [raw]

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

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 <[email protected]>
---
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 89745aa..1c31a5d 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

2014-07-15 12:58:07

[permalink] [raw]

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

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

Signed-off-by: Stefan Agner <[email protected]>
---
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@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@40027000 {
compatible = "fsl,vf610-lpuart";
reg = <0x40027000 0x1000>;
@@ -414,6 +426,17 @@
clock-names = "nfc";
status = "disabled";
};
+
+ can1: flexcan@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

2014-07-15 13:55:13

[permalink] [raw]

Subject: Re: [PATCH v3 0/4] ARM: vf610: add FlexCAN support

On 07/15/2014 02:56 PM, Stefan Agner wrote:
> This series adds support for Vybrid (aka vf610) to the FlexCAN
> controller and extends the device tree accordingly.
>
> Changes in v3:
> - Return error number in flexcan_get_berr_counter if clock enabling fails
> - Add the FLEXCANX_EN clocks as new clocks
>
> 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 | 91 +++++++++++++++++++++++++++++----
> include/dt-bindings/clock/vf610-clock.h | 4 +-
> 4 files changed, 110 insertions(+), 14 deletions(-)

I'm taking patch 3+4 via the linux-can-next tree.

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

Attachments:

2014-07-15 13:55:10

by Lothar Waßmann

[permalink] [raw]

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

Hi,

Stefan Agner wrote:
> Reported-by: Ashutosh Singh <[email protected]>
> Suggested-by: Marc Kleine-Budde <[email protected]>
> [[email protected]: added return check for clk_enable_prepare]
>
> Signed-off-by: Stefan Agner <[email protected]>
> ---
> drivers/net/can/flexcan.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f425ec2..89745aa 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -383,12 +383,26 @@ 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);
>
flexcan_get_berr_counter() may be called from interrupt context and
thus must not call any functions that can sleep.
Compiling the driver with CONFIG_DEBUG_ATOMIC_SLEEP would catch this!

Lothar Waßmann
--
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

http://www.karo-electronics.de | [email protected]
___________________________________________________________

2014-07-15 13:58:00

[permalink] [raw]

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

On 07/15/2014 03:54 PM, Lothar Waßmann wrote:
> Hi,
>
> Stefan Agner wrote:
>> Reported-by: Ashutosh Singh <[email protected]>
>> Suggested-by: Marc Kleine-Budde <[email protected]>
>> [[email protected]: added return check for clk_enable_prepare]
>>
>> Signed-off-by: Stefan Agner <[email protected]>
>> ---
>> drivers/net/can/flexcan.c | 18 ++++++++++++++++--
>> 1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> index f425ec2..89745aa 100644
>> --- a/drivers/net/can/flexcan.c
>> +++ b/drivers/net/can/flexcan.c
>> @@ -383,12 +383,26 @@ 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);
>>
> flexcan_get_berr_counter() may be called from interrupt context and
> thus must not call any functions that can sleep.
> Compiling the driver with CONFIG_DEBUG_ATOMIC_SLEEP would catch this!

It's called from the NAPI softirq. I'll prepare a patch.

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 |

Attachments:

2014-07-15 14:24:31

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

On 07/15/2014 02:56 PM, 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 <[email protected]>
> ---
> 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 89745aa..1c31a5d 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
^^ ^^

Can you check the datasheet if the flexcan core has a "Glitch Filter
Width Register (FLEXCANx_GFWR)"

Can you check if the core generates a warning interrupt with the current
setup, if you don't switch on bus error reporting? This means internally
the [TR]WRN_INT is connected and works as specified.

> *
> * 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,
> +};

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 |

Attachments:

2014-07-16 06:11:35

by Shawn Guo

[permalink] [raw]

Subject: Re: [PATCH v3 0/4] ARM: vf610: add FlexCAN support

On Tue, Jul 15, 2014 at 02:56:17PM +0200, Stefan Agner wrote:
> Stefan Agner (4):
> ARM: dts: vf610: add FlexCAN node
> ARM: imx: clk-vf610: fix FlexCAN clock gating

Applied these two, thanks.

> can: flexcan: switch on clocks before accessing ecr register
> can: flexcan: add vf610 support for FlexCAN

2014-07-16 06:43:26

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

Am 2014-07-15 16:24, schrieb Marc Kleine-Budde:
<snip>
>> @@ -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
> ^^ ^^
>
> Can you check the datasheet if the flexcan core has a "Glitch Filter
> Width Register (FLEXCANx_GFWR)"

There is no such register called GFWR/Glitch Filter or similar.

> Can you check if the core generates a warning interrupt with the current
> setup, if you don't switch on bus error reporting? This means internally
> the [TR]WRN_INT is connected and works as specified.

Ok, so I disabled TWRNMSK (Bit 11) in the control register and printed
out the error and status register (ESR1), this is what I get:
[ 191.285295] flexcan_irq, esr=00040080
[ 191.288996] flexcan_irq, ctrl=17092051

Bit 17 (TWRNINT) is not set while TWRNMSK is disabled. Hence [TR]WRN_INT
is not connected?

--
Stefan

2014-07-25 10:50:39

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

Am 2014-07-16 08:43, schrieb Stefan Agner:
> Am 2014-07-15 16:24, schrieb Marc Kleine-Budde:
> <snip>
>>> @@ -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
>> ^^ ^^
>>
>> Can you check the datasheet if the flexcan core has a "Glitch Filter
>> Width Register (FLEXCANx_GFWR)"
>
>
> There is no such register called GFWR/Glitch Filter or similar.
>
>> Can you check if the core generates a warning interrupt with the current
>> setup, if you don't switch on bus error reporting? This means internally
>> the [TR]WRN_INT is connected and works as specified.
>
> Ok, so I disabled TWRNMSK (Bit 11) in the control register and printed
> out the error and status register (ESR1), this is what I get:
> [ 191.285295] flexcan_irq, esr=00040080
> [ 191.288996] flexcan_irq, ctrl=17092051
>
> Bit 17 (TWRNINT) is not set while TWRNMSK is disabled. Hence [TR]WRN_INT
> is not connected?

Ping. Anything open/to do from my side?

--
Stefan

2014-07-25 13:33:31

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

On 07/25/2014 12:50 PM, Stefan Agner wrote:
> Am 2014-07-16 08:43, schrieb Stefan Agner:
>> Am 2014-07-15 16:24, schrieb Marc Kleine-Budde:
>> <snip>
>>>> @@ -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
>>> ^^ ^^
>>>
>>> Can you check the datasheet if the flexcan core has a "Glitch Filter
>>> Width Register (FLEXCANx_GFWR)"
>>
>>
>> There is no such register called GFWR/Glitch Filter or similar.
>>
>>> Can you check if the core generates a warning interrupt with the current
>>> setup, if you don't switch on bus error reporting? This means internally
>>> the [TR]WRN_INT is connected and works as specified.
>>
>> Ok, so I disabled TWRNMSK (Bit 11) in the control register and printed
>> out the error and status register (ESR1), this is what I get:
>> [ 191.285295] flexcan_irq, esr=00040080
>> [ 191.288996] flexcan_irq, ctrl=17092051
>>
>> Bit 17 (TWRNINT) is not set while TWRNMSK is disabled. Hence [TR]WRN_INT
>> is not connected?
>
> Ping. Anything open/to do from my side?

Please keep the printing of esr and ctrl in the interrupt handler, add a
#define DEBUG in the driver, but do not change anything else. Then:

start the driver as usual:
- configure bitrate
- ifconfig can0 up
- _do_not_ enable bit error reporint
- start candump from gitorious with:
candump -e any,0:0,#FFFFFFFF
- short circuit CAN-High and CAN-Low
- send a CAN frame

Another test is to setup a proper CAN bus, but configure the a second
CAN node with a different bitrate. Start the can0 on vf610 as usual,
then candump -e any,0:0,#FFFFFFFF, but do not send any CAN frames on the
vf610. Then on the other system keep sending CAN frames, you might have
to restart the CAN on the second system....

Please send the outputs of candump and demsg to the list. It should
generate a warning, error passive and finally a busoff message.

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 |

Attachments:

2014-07-28 16:19:41

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

Am 2014-07-25 15:33, schrieb Marc Kleine-Budde:
> On 07/25/2014 12:50 PM, Stefan Agner wrote:
>> Am 2014-07-16 08:43, schrieb Stefan Agner:
>>> Am 2014-07-15 16:24, schrieb Marc Kleine-Budde:
>>> <snip>
>>>>> @@ -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
>>>> ^^ ^^
>>>>
>>>> Can you check the datasheet if the flexcan core has a "Glitch Filter
>>>> Width Register (FLEXCANx_GFWR)"
>>>
>>>
>>> There is no such register called GFWR/Glitch Filter or similar.
>>>
>>>> Can you check if the core generates a warning interrupt with the current
>>>> setup, if you don't switch on bus error reporting? This means internally
>>>> the [TR]WRN_INT is connected and works as specified.
>>>
>>> Ok, so I disabled TWRNMSK (Bit 11) in the control register and printed
>>> out the error and status register (ESR1), this is what I get:
>>> [ 191.285295] flexcan_irq, esr=00040080
>>> [ 191.288996] flexcan_irq, ctrl=17092051
>>>
>>> Bit 17 (TWRNINT) is not set while TWRNMSK is disabled. Hence [TR]WRN_INT
>>> is not connected?
>>
>> Ping. Anything open/to do from my side?
>
> Please keep the printing of esr and ctrl in the interrupt handler, add a
> #define DEBUG in the driver, but do not change anything else. Then:
>

Ok, my changes look like this:

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 1c31a5d..fe8b81c 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -19,6 +19,7 @@
*
*/

+#define DEBUG
#include <linux/netdevice.h>
#include <linux/can.h>
#include <linux/can/dev.h>
@@ -743,6 +744,9 @@ static irqreturn_t flexcan_irq(int irq, void
*dev_id)

reg_iflag1 = flexcan_read(&regs->iflag1);
reg_esr = flexcan_read(&regs->esr);
+
+ printk("flexcan_irq, esr=%08x\n", reg_esr);
+ printk("flexcan_irq, ctrl=%08x\n", flexcan_read(&regs->ctrl));
/* 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);
@@ -885,8 +889,8 @@ static int flexcan_chip_start(struct net_device
*dev)
*/
reg_ctrl = flexcan_read(&regs->ctrl);
reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
- reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
- FLEXCAN_CTRL_ERR_STATE;
+ reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF;// |
+ //FLEXCAN_CTRL_ERR_STATE;
/*
* enable the "error interrupt" (FLEXCAN_CTRL_ERR_MSK),
* on most Flexcan cores, too. Otherwise we don't get

I'm not sure whether you really want to keep the FLEXCAN_CTRL_ERR_STATE
commented out...

> start the driver as usual:
> - configure bitrate
> - ifconfig can0 up
> - _do_not_ enable bit error reporint
> - start candump from gitorious with:
> candump -e any,0:0,#FFFFFFFF
> - short circuit CAN-High and CAN-Low
> - send a CAN frame

root@colibri-vf:~# ip link set can0 type can bitrate 500000
[ 146.140862] flexcan 40020000.flexcan can0: bitrate error 0.7%
root@colibri-vf:~# ip link set can0 up
[ 146.661430] flexcan 40020000.flexcan can0: writing ctrl=0x17092001
[ 146.667902] flexcan 40020000.flexcan can0: flexcan_set_bittiming:
mcr=0x5980000f ctrl=0x17092001
[ 146.676790] flexcan 40020000.flexcan can0: flexcan_chip_start:
writing mcr=0x79a20208
[ 146.684709] flexcan 40020000.flexcan can0: flexcan_chip_start:
writing ctrl=0x17092051
[ 146.698228] flexcan 40020000.flexcan can0: flexcan_chip_start:
reading mcr=0x60a20208 ctrl=0x1709205

# cansend can0 1F334455#1122334455667788
interface = can0, family = 29, type = 3, proto =

Nothing happens on candump.

> Another test is to setup a proper CAN bus, but configure the a second
> CAN node with a different bitrate. Start the can0 on vf610 as usual,
> then candump -e any,0:0,#FFFFFFFF, but do not send any CAN frames on the
> vf610. Then on the other system keep sending CAN frames, you might have
> to restart the CAN on the second system....

I'm using a PCAN-USB from Peak System for this test. When running with
the same bitrates on both sides (500000, things work smoothly as
expected:

# ip link set can0 type can bitrate 500000
# ip link set can0 up
# cansend can0 1F334455#1122334455667788

root@colibri-vf:~# ./candump -e any,0:0,#FFFFFFFF
can0 1F334455 [8] 11 22 33 44 55 66 77 88

dmesg:
[ 541.772502] flexcan_irq, esr=00040180
[ 541.776207] flexcan_irq, ctrl=17092051

Then I reconfigure the system on my host:
# ip link set can0 down
# ip link set can0 type can bitrate 1000000
# ip link set can0 up
# cansend can0 1F334455#1122334455667788

Nothing happens on Vybrid side.

However, I got once this Kernel panic after I reconfigured to normal
mode. But I could not reproduce this.

[ 461.954394] flexcan_irq, esr=00059d82
[ 461.958093] flexcan_irq, ctrl=17092051
[ 461.961873] ------------[ cut here ]------------
[ 461.966536] WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:826
mutex_trylock+0x1b0/0x208()
[ 461.974982] DEBUG_LOCKS_WARN_ON(in_interrupt())
[ 461.979347] Modules linked in:
[ 461.982621] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
3.16.0-rc4-00017-ge9a974f-dirty #215
[ 461.990896] Backtrace:
[ 461.993412] [<80011e1c>] (dump_backtrace) from [<80011fb8>]
(show_stack+0x18/0x1c)
[ 462.000991] r6:806718ec r5:00000000 r4:00000000 r3:00000000
[ 462.006836] [<80011fa0>] (show_stack) from [<8066e160>]
(dump_stack+0x88/0xa4)
[ 462.014143] [<8066e0d8>] (dump_stack) from [<80028f48>]
(warn_slowpath_common+0x70/0x94)
[ 462.022280] r5:00000009 r4:808f9d50
[ 462.025920] [<80028ed8>] (warn_slowpath_common) from [<80029010>]
(warn_slowpath_fmt+0x38/0x40)
[ 462.034666] r8:808f9e14 r7:8110cf7c r6:804c2fa0 r5:8e9ee000
r4:8094cdcc
[ 462.041478] [<80028fdc>] (warn_slowpath_fmt) from [<806718ec>]
(mutex_trylock+0x1b0/0x208)
[ 462.049792] r3:807e3f6c r2:807e1b8c
[ 462.053466] [<8067173c>] (mutex_trylock) from [<804c2fa0>]
(clk_prepare_lock+0x14/0xec)
[ 462.061479] r7:90880000 r6:8e81f800 r5:8e9ee000 r4:8e81f800
[ 462.067280] [<804c2f8c>] (clk_prepare_lock) from [<804c50d0>]
(clk_prepare+0x14/0x2c)
[ 462.075158] r6:8e81f800 r5:8e9ee000 r4:8e81f800 r3:00000000
[ 462.080918] [<804c50bc>] (clk_prepare) from [<803e7a50>]
(flexcan_get_berr_counter+0x24/0xd0)
[ 462.089487] r4:00000001 r3:00000000
[ 462.093156] [<803e7a2c>] (flexcan_get_berr_counter) from [<803e895c>]
(flexcan_poll+0x40c/0x58c)
[ 462.101992] r8:0000000a r7:0000000a r6:8e372388 r5:8e172a80
r4:00000001 r3:00000000
[ 462.109843] [<803e8550>] (flexcan_poll) from [<80511f90>]
(net_rx_action+0xcc/0x1b4)
[ 462.117630] r10:8e9ee6c0 r9:00000003 r8:0000000a r7:8fdde188
r6:808fa0c0 r5:8fdde180
[ 462.125587] r4:0000012c
[ 462.128164] [<80511ec4>] (net_rx_action) from [<8002d290>]
(__do_softirq+0x120/0x26c)
[ 462.136038] r10:808fa080 r9:00000003 r8:00000100 r7:00000003
r6:808f8000 r5:808fa08c
[ 462.143994] r4:00000000
[ 462.146566] [<8002d170>] (__do_softirq) from [<8002d6d4>]
(irq_exit+0xb0/0x104)
[ 462.153923] r10:806788ac r9:808f8000 r8:00000000 r7:0000005a
r6:808f8000 r5:808f5e2c
[ 462.161877] r4:808f8000
[ 462.164459] [<8002d624>] (irq_exit) from [<8000f4bc>]
(handle_IRQ+0x58/0xb8)
[ 462.171520] r4:80900d2c r3:000000a0
[ 462.175200] [<8000f464>] (handle_IRQ) from [<800086c0>]
(gic_handle_irq+0x30/0x68)
[ 462.182818] r8:8095c587 r7:90802100 r6:808f9f28 r5:80900ea0
r4:9080210c r3:000000a0
[ 462.190671] [<80008690>] (gic_handle_irq) from [<80012ae4>]
(__irq_svc+0x44/0x5c)
[ 462.198203] Exception stack(0x808f9f28 to 0x808f9f70)
[ 462.203315] 9f20: 00000001 00000001 00000000
80904070 8090098c 80900938
[ 462.211517] 9f40: 8095c587 00000000 8095c587 808f8000 806788ac
808f9f7c 808f9f40 808f9f70
[ 462.219746] 9f60: 80066a98 8000f834 20000013 ffffffff
[ 462.224852] r7:808f9f5c r6:ffffffff r5:20000013 r4:8000f834
[ 462.230621] [<8000f80c>] (arch_cpu_idle) from [<8005fba4>]
(cpu_startup_entry+0x104/0x16c)
[ 462.238964] [<8005faa0>] (cpu_startup_entry) from [<80668cf8>]
(rest_init+0xb0/0xd8)
[ 462.246757] r7:8ffffcc0 r3:00000000
[ 462.250419] [<80668c48>] (rest_init) from [<808a5bf8>]
(start_kernel+0x340/0x3ac)
[ 462.257979] r5:8095c7c0 r4:80900a38
[ 462.261620] [<808a58b8>] (start_kernel) from [<80008074>]
(0x80008074)
[ 462.268204] ---[ end trace c9c9dee0ce2272a7 ]---
[ 462.272899] flexcan 40020000.flexcan can0: Error Warning IRQ

candump showed this:
can0 20000004 [8] 00 04 00 00 00 00 00 00 ERRORFRAME
controller-problem{rx-error-warning}

>
> Please send the outputs of candump and demsg to the list. It should
> generate a warning, error passive and finally a busoff message.
>

--
Stefan

2014-07-28 16:29:10

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

On 07/28/2014 06:20 PM, Stefan Agner wrote:
>>> Ping. Anything open/to do from my side?
>>
>> Please keep the printing of esr and ctrl in the interrupt handler, add a
>> #define DEBUG in the driver, but do not change anything else. Then:
^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 1c31a5d..fe8b81c 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -19,6 +19,7 @@
> *
> */
>
> +#define DEBUG

keep

> #include <linux/netdevice.h>
> #include <linux/can.h>
> #include <linux/can/dev.h>
> @@ -743,6 +744,9 @@ static irqreturn_t flexcan_irq(int irq, void
> *dev_id)
>
> reg_iflag1 = flexcan_read(&regs->iflag1);
> reg_esr = flexcan_read(&regs->esr);
> +
> + printk("flexcan_irq, esr=%08x\n", reg_esr);
> + printk("flexcan_irq, ctrl=%08x\n", flexcan_read(&regs->ctrl));

keep

> /* 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);
> @@ -885,8 +889,8 @@ static int flexcan_chip_start(struct net_device
> *dev)
> */
> reg_ctrl = flexcan_read(&regs->ctrl);
> reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
> - reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
> - FLEXCAN_CTRL_ERR_STATE;
> + reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF;// |
> + //FLEXCAN_CTRL_ERR_STATE;
> /*
> * enable the "error interrupt" (FLEXCAN_CTRL_ERR_MSK),
> * on most Flexcan cores, too. Otherwise we don't get
>
> I'm not sure whether you really want to keep the FLEXCAN_CTRL_ERR_STATE
> commented out...

No, please remove this change and redo the test.

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 |

Attachments:

2014-07-28 16:31:28

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

On 07/28/2014 06:20 PM, Stefan Agner wrote:
[...]

> However, I got once this Kernel panic after I reconfigured to normal
> mode. But I could not reproduce this.
>
> [ 461.954394] flexcan_irq, esr=00059d82
> [ 461.958093] flexcan_irq, ctrl=17092051
> [ 461.961873] ------------[ cut here ]------------
> [ 461.966536] WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:826
> mutex_trylock+0x1b0/0x208()
> [ 461.974982] DEBUG_LOCKS_WARN_ON(in_interrupt())

Please use this updated version of your patch:

Marc

---

From b6b8ae7cdb04f1a3b9d3a6dd83f8146bfaf48553 Mon Sep 17 00:00:00 2001
From: Stefan Agner <[email protected]>
Date: Tue, 15 Jul 2014 14:56:20 +0200
Subject: [PATCH] can: flexcan: flexcan_get_berr_counter(): switch on clocks
before accessing ecr register

The funcion flexcan_get_berr_counter() may be called from userspace even if the
interface is down, this the clocks are disabled. This patch switches on the
clocks before accessing the ecr register.

Reported-by: Ashutosh Singh <[email protected]>
Signed-off-by: Stefan Agner <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
---
drivers/net/can/flexcan.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f425ec2..6bfe24a 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -378,8 +378,9 @@ static int flexcan_chip_softreset(struct flexcan_priv *priv)
return 0;
}

-static int flexcan_get_berr_counter(const struct net_device *dev,
- struct can_berr_counter *bec)
+
+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;
@@ -391,6 +392,29 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
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);
+ int 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;
+
+ err = __flexcan_get_berr_counter(dev, bec);
+
+ clk_disable_unprepare(priv->clk_per);
+ out_disable_ipg:
+ clk_disable_unprepare(priv->clk_ipg);
+
+ return err;
+}
+
static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
const struct flexcan_priv *priv = netdev_priv(dev);
@@ -503,7 +527,7 @@ static void do_state(struct net_device *dev,
struct flexcan_priv *priv = netdev_priv(dev);
struct can_berr_counter bec;

- flexcan_get_berr_counter(dev, &bec);
+ __flexcan_get_berr_counter(dev, &bec);

switch (priv->can.state) {
case CAN_STATE_ERROR_ACTIVE:
--
2.0.1

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

Attachments:

2014-07-29 07:28:45

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

Am 2014-07-28 18:28, schrieb Marc Kleine-Budde:
> On 07/28/2014 06:20 PM, Stefan Agner wrote:
>> I'm not sure whether you really want to keep the FLEXCAN_CTRL_ERR_STATE
>> commented out...
>
> No, please remove this change and redo the test.
>

Ok, removed that change and did the tests again:

== Wrong Bitrate test

[ 146.485022] flexcan 40020000.flexcan can0: bitrate error 0.7%
[ 148.401793] flexcan 40020000.flexcan can0: writing ctrl=0x17092001
[ 148.408263] flexcan 40020000.flexcan can0: flexcan_set_bittiming:
mcr=0x5980000f ctrl=0x17092001
[ 148.408298] flexcan 40020000.flexcan can0: flexcan_chip_start:
writing mcr=0x79a20208
[ 148.408328] flexcan 40020000.flexcan can0: flexcan_chip_start:
writing ctrl=0x1709ac51
[ 148.414373] flexcan 40020000.flexcan can0: flexcan_chip_start:
reading mcr=0x60a20208 ctrl=0x1709ac51
^---- Initialization
[ 152.968685] flexcan_irq, esr=00040080
[ 152.972386] flexcan_irq, ctrl=1709ac51
[ 155.488623] flexcan_irq, esr=00040080
[ 155.492326] flexcan_irq, ctrl=1709ac51
^---- Two messages with right bitrate
[ 171.014124] flexcan_irq, esr=00058d0a
[ 171.017823] flexcan_irq, ctrl=1709ac51
^---- One message with wrong bitrate
[ 171.021631] flexcan 40020000.flexcan can0: Error Warning IRQ
[ 171.021660] flexcan 40020000.flexcan can0: Error Passive IRQ

# ./candump -e any,0:0,#FFFFFFFF
can0 1F334455 [8] 11 22 33 44 55 66 77 88
can0 1F334455 [8] 11 22 33 44 55 66 77 88
can0 20000004 [8] 00 10 00 00 00 00 00 00 ERRORFRAME
controller-problem{rx-error-passive}

== Loopback test

[ 464.003776] flexcan 40020000.flexcan can0: writing ctrl=0x17092051
[ 464.010005] flexcan 40020000.flexcan can0: flexcan_set_bittiming:
mcr=0x5980000f ctrl=0x17092051
[ 464.010035] flexcan 40020000.flexcan can0: flexcan_chip_start:
writing mcr=0x79a20208
[ 464.010064] flexcan 40020000.flexcan can0: flexcan_chip_start:
writing ctrl=0x1709ac51
[ 464.010199] flexcan 40020000.flexcan can0: flexcan_chip_start:
reading mcr=0x60a20208 ctrl=0x1709ac51
[ 464.011682] IPv6: ADDRCONF(NETDEV_CHANGE): can0: link becomes ready
[ 470.970133] flexcan_irq, esr=00064252
[ 470.974833] flexcan_irq, ctrl=1709ac51
[ 470.980171] flexcan 40020000.flexcan can0: Error Warning IRQ
[ 470.980203] flexcan 40020000.flexcan can0: Error Passive IRQ
[ 470.980328] flexcan_irq, esr=00000034
[ 470.984998] flexcan_irq, ctrl=1709ac51

# ./candump -e any,0:0,#FFFFFFFF
can0 20000044 [8] 00 10 00 00 00 00 00 00 ERRORFRAME
controller-problem{rx-error-passive}
bus-off

--
Stefan

2014-07-30 11:47:51

[permalink] [raw]

Subject: Re: [PATCH v3 4/4] can: flexcan: add vf610 support for FlexCAN

On 07/29/2014 09:29 AM, Stefan Agner wrote:
> Am 2014-07-28 18:28, schrieb Marc Kleine-Budde:
>> On 07/28/2014 06:20 PM, Stefan Agner wrote:
>>> I'm not sure whether you really want to keep the FLEXCAN_CTRL_ERR_STATE
>>> commented out...
>>
>> No, please remove this change and redo the test.
>>
>
> Ok, removed that change and did the tests again:
>
> == Wrong Bitrate test
>
> [ 146.485022] flexcan 40020000.flexcan can0: bitrate error 0.7%
> [ 148.401793] flexcan 40020000.flexcan can0: writing ctrl=0x17092001
> [ 148.408263] flexcan 40020000.flexcan can0: flexcan_set_bittiming:
> mcr=0x5980000f ctrl=0x17092001
> [ 148.408298] flexcan 40020000.flexcan can0: flexcan_chip_start:
> writing mcr=0x79a20208
> [ 148.408328] flexcan 40020000.flexcan can0: flexcan_chip_start:
> writing ctrl=0x1709ac51
> [ 148.414373] flexcan 40020000.flexcan can0: flexcan_chip_start:
> reading mcr=0x60a20208 ctrl=0x1709ac51
> ^---- Initialization
> [ 152.968685] flexcan_irq, esr=00040080
> [ 152.972386] flexcan_irq, ctrl=1709ac51
> [ 155.488623] flexcan_irq, esr=00040080
> [ 155.492326] flexcan_irq, ctrl=1709ac51
> ^---- Two messages with right bitrate
> [ 171.014124] flexcan_irq, esr=00058d0a
> [ 171.017823] flexcan_irq, ctrl=1709ac51
> ^---- One message with wrong bitrate
> [ 171.021631] flexcan 40020000.flexcan can0: Error Warning IRQ
> [ 171.021660] flexcan 40020000.flexcan can0: Error Passive IRQ

Thanks for the test, so far looks promising :) With this setup the other
CAN node repeats the CAN frame until it's ACKed. Because there is no
node with a compatible bitrate, there is no ACking CAN node.

Can you add a third CAN node to the network. The second and third node
should use the same bitrate, while your vf610 uses a different one. With
the new setup it should take more than one frame until the vf610 goes
into error warning and even more frames to error passive. This way we
can see it the error warning interrupt is connected or not. The error
counters should increase with each "wrong" bitrate frame it sees, you
can check with:

ip -details link show can0

The output looks like this:

> 4: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT qlen 10
> link/can
> can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
^^^^^^^^^^^^^^^^^^^^^^
> bitrate 1000000 sample-point 0.750
> tq 125 prop-seg 2 phase-seg1 3 phase-seg2 2 sjw 1
> sja1000: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
> clock 8000000

When one of the berr-counter crosses 96 (and stays below 128) a warning
interrupt should be generated.

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 |

Attachments: