| Message ID | 5065A35B.3020702@grandegger.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Wolfgang Grandegger wrote: > Due to a bug in most Flexcan cores, the bus error interrupt needs > to be enabled. Otherwise we don't get any error warning or passive > interrupts. This is _not_ necessay for the i.MX28 and this patch > s/necessay/necessary/ Other looks fine to me. Reviewed-by: Hui Wang <jason77.wang@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 28, 2012 at 03:17:15PM +0200, Wolfgang Grandegger wrote: > Due to a bug in most Flexcan cores, the bus error interrupt needs > to be enabled. Otherwise we don't get any error warning or passive > interrupts. This is _not_ necessay for the i.MX28 and this patch > disables bus error interrupts if "berr-reporting" is not requested. > This avoids bus error flooding, which might harm, especially on > low-end systems. > > To handle such quirks of the Flexcan cores, a hardware feature flag > has been introduced, also replacing the "hw_ver" variable. So far > nobody could tell what Flexcan core version is available on what > Freescale SOC, apart from the i.MX6Q and P1010, and which bugs or > features are present on the various "hw_rev". > > CC: Hui Wang <jason77.wang@gmail.com> > CC: Shawn Guo <shawn.guo@linaro.org> > Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> > --- > > Concerning the bug, I know that the i.MX35 does have it. Maybe other > Flexcan cores than on the i.MX28 does *not* have it either. If you > have a chance, please check on the P1010, i.MX6Q, i.MX51, i.MX53, > etc. From what I can tell, i.MX35, i.MX51 and i.MX53 use the same version, so they should all have the bug. And for i.MX6Q, since it uses a newer version even than i.MX28, I would believe it's affected by the bug. But I'm copying Dong who should have better knowledge about this to confirm. Shawn > > Wolfgang. > > > drivers/net/can/flexcan.c | 29 +++++++++++++++++++---------- > 1 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > index c5f1431..c78ecfc 100644 > --- a/drivers/net/can/flexcan.c > +++ b/drivers/net/can/flexcan.c > @@ -144,6 +144,10 @@ > > #define FLEXCAN_MB_CODE_MASK (0xf0ffffff) > > +/* FLEXCAN hardware feature flags */ > +#define FLEXCAN_HAS_V10_FEATURES BIT(1) /* For core version >= 10 */ > +#define FLEXCAN_HAS_BROKEN_ERR_STATE BIT(2) /* Broken error state handling */ > + > /* Structure of the message buffer */ > struct flexcan_mb { > u32 can_ctrl; > @@ -178,7 +182,7 @@ struct flexcan_regs { > }; > > struct flexcan_devtype_data { > - u32 hw_ver; /* hardware controller version */ > + u32 features; /* hardware controller features */ > }; > > struct flexcan_priv { > @@ -197,11 +201,11 @@ struct flexcan_priv { > }; > > static struct flexcan_devtype_data fsl_p1010_devtype_data = { > - .hw_ver = 3, > + .features = FLEXCAN_HAS_BROKEN_ERR_STATE, > }; > - > +static struct flexcan_devtype_data fsl_imx28_devtype_data; > static struct flexcan_devtype_data fsl_imx6q_devtype_data = { > - .hw_ver = 10, > + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_BROKEN_ERR_STATE, > }; > > static const struct can_bittiming_const flexcan_bittiming_const = { > @@ -741,15 +745,19 @@ static int flexcan_chip_start(struct net_device *dev) > * enable tx and rx warning interrupt > * enable bus off interrupt > * (== FLEXCAN_CTRL_ERR_STATE) > - * > - * _note_: we enable the "error interrupt" > - * (FLEXCAN_CTRL_ERR_MSK), too. Otherwise we don't get any > - * warning or bus passive interrupts. > */ > reg_ctrl = flexcan_read(®s->ctrl); > reg_ctrl &= ~FLEXCAN_CTRL_TSYN; > reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | > - FLEXCAN_CTRL_ERR_STATE | FLEXCAN_CTRL_ERR_MSK; > + FLEXCAN_CTRL_ERR_STATE; > + /* > + * enable the "error interrupt" (FLEXCAN_CTRL_ERR_MSK), > + * on most Flexcan cores, too. Otherwise we don't get > + * any error warning or passive interrupts. > + */ > + if (priv->devtype_data->features & FLEXCAN_HAS_BROKEN_ERR_STATE || > + priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) > + reg_ctrl |= FLEXCAN_CTRL_ERR_MSK; > > /* save for later use */ > priv->reg_ctrl_default = reg_ctrl; > @@ -772,7 +780,7 @@ static int flexcan_chip_start(struct net_device *dev) > flexcan_write(0x0, ®s->rx14mask); > flexcan_write(0x0, ®s->rx15mask); > > - if (priv->devtype_data->hw_ver >= 10) > + if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) > flexcan_write(0x0, ®s->rxfgmask); > > flexcan_transceiver_switch(priv, 1); > @@ -954,6 +962,7 @@ static void __devexit unregister_flexcandev(struct net_device *dev) > > static const struct of_device_id flexcan_of_match[] = { > { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, }, > + { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, }, > { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, }, > { /* sentinel */ }, > }; > -- > 1.7.7.6 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/07/2012 05:09 AM, Shawn Guo wrote: > On Fri, Sep 28, 2012 at 03:17:15PM +0200, Wolfgang Grandegger wrote: >> Due to a bug in most Flexcan cores, the bus error interrupt needs >> to be enabled. Otherwise we don't get any error warning or passive >> interrupts. This is _not_ necessay for the i.MX28 and this patch >> disables bus error interrupts if "berr-reporting" is not requested. >> This avoids bus error flooding, which might harm, especially on >> low-end systems. >> >> To handle such quirks of the Flexcan cores, a hardware feature flag >> has been introduced, also replacing the "hw_ver" variable. So far >> nobody could tell what Flexcan core version is available on what >> Freescale SOC, apart from the i.MX6Q and P1010, and which bugs or >> features are present on the various "hw_rev". >> >> CC: Hui Wang <jason77.wang@gmail.com> >> CC: Shawn Guo <shawn.guo@linaro.org> >> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> >> --- >> >> Concerning the bug, I know that the i.MX35 does have it. Maybe other >> Flexcan cores than on the i.MX28 does *not* have it either. If you >> have a chance, please check on the P1010, i.MX6Q, i.MX51, i.MX53, >> etc. > >>From what I can tell, i.MX35, i.MX51 and i.MX53 use the same version, > so they should all have the bug. And for i.MX6Q, since it uses a newer > version even than i.MX28, I would believe it's affected by the bug. > But I'm copying Dong who should have better knowledge about this to > confirm. Thank for clarification. I have a i.MX6Q board but without CAN adapter :(, unfortunately. Otherwise I would try it out myself. Wolfgang. >> drivers/net/can/flexcan.c | 29 +++++++++++++++++++---------- >> 1 files changed, 19 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c >> index c5f1431..c78ecfc 100644 >> --- a/drivers/net/can/flexcan.c >> +++ b/drivers/net/can/flexcan.c >> @@ -144,6 +144,10 @@ >> >> #define FLEXCAN_MB_CODE_MASK (0xf0ffffff) >> >> +/* FLEXCAN hardware feature flags */ >> +#define FLEXCAN_HAS_V10_FEATURES BIT(1) /* For core version >= 10 */ >> +#define FLEXCAN_HAS_BROKEN_ERR_STATE BIT(2) /* Broken error state handling */ >> + >> /* Structure of the message buffer */ >> struct flexcan_mb { >> u32 can_ctrl; >> @@ -178,7 +182,7 @@ struct flexcan_regs { >> }; >> >> struct flexcan_devtype_data { >> - u32 hw_ver; /* hardware controller version */ >> + u32 features; /* hardware controller features */ >> }; >> >> struct flexcan_priv { >> @@ -197,11 +201,11 @@ struct flexcan_priv { >> }; >> >> static struct flexcan_devtype_data fsl_p1010_devtype_data = { >> - .hw_ver = 3, >> + .features = FLEXCAN_HAS_BROKEN_ERR_STATE, >> }; >> - >> +static struct flexcan_devtype_data fsl_imx28_devtype_data; >> static struct flexcan_devtype_data fsl_imx6q_devtype_data = { >> - .hw_ver = 10, >> + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_BROKEN_ERR_STATE, >> }; >> >> static const struct can_bittiming_const flexcan_bittiming_const = { >> @@ -741,15 +745,19 @@ static int flexcan_chip_start(struct net_device *dev) >> * enable tx and rx warning interrupt >> * enable bus off interrupt >> * (== FLEXCAN_CTRL_ERR_STATE) >> - * >> - * _note_: we enable the "error interrupt" >> - * (FLEXCAN_CTRL_ERR_MSK), too. Otherwise we don't get any >> - * warning or bus passive interrupts. >> */ >> reg_ctrl = flexcan_read(®s->ctrl); >> reg_ctrl &= ~FLEXCAN_CTRL_TSYN; >> reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | >> - FLEXCAN_CTRL_ERR_STATE | FLEXCAN_CTRL_ERR_MSK; >> + FLEXCAN_CTRL_ERR_STATE; >> + /* >> + * enable the "error interrupt" (FLEXCAN_CTRL_ERR_MSK), >> + * on most Flexcan cores, too. Otherwise we don't get >> + * any error warning or passive interrupts. >> + */ >> + if (priv->devtype_data->features & FLEXCAN_HAS_BROKEN_ERR_STATE || >> + priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) >> + reg_ctrl |= FLEXCAN_CTRL_ERR_MSK; >> >> /* save for later use */ >> priv->reg_ctrl_default = reg_ctrl; >> @@ -772,7 +780,7 @@ static int flexcan_chip_start(struct net_device *dev) >> flexcan_write(0x0, ®s->rx14mask); >> flexcan_write(0x0, ®s->rx15mask); >> >> - if (priv->devtype_data->hw_ver >= 10) >> + if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) >> flexcan_write(0x0, ®s->rxfgmask); >> >> flexcan_transceiver_switch(priv, 1); >> @@ -954,6 +962,7 @@ static void __devexit unregister_flexcandev(struct net_device *dev) >> >> static const struct of_device_id flexcan_of_match[] = { >> { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, }, >> + { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, }, >> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, }, >> { /* sentinel */ }, >> }; >> -- >> 1.7.7.6 >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-can" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Wolfgang, >On 10/07/2012 05:09 AM, Shawn Guo wrote: >> On Fri, Sep 28, 2012 at 03:17:15PM +0200, Wolfgang Grandegger wrote: >>> Due to a bug in most Flexcan cores, the bus error interrupt needs to >>> be enabled. Otherwise we don't get any error warning or passive >>> interrupts. This is _not_ necessay for the i.MX28 and this patch >>> disables bus error interrupts if "berr-reporting" is not requested. >>> This avoids bus error flooding, which might harm, especially on >>> low-end systems. >>> >>> To handle such quirks of the Flexcan cores, a hardware feature flag >>> has been introduced, also replacing the "hw_ver" variable. So far >>> nobody could tell what Flexcan core version is available on what >>> Freescale SOC, apart from the i.MX6Q and P1010, and which bugs or >>> features are present on the various "hw_rev". >>> >>> CC: Hui Wang <jason77.wang@gmail.com> >>> CC: Shawn Guo <shawn.guo@linaro.org> >>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> >>> --- >>> >>> Concerning the bug, I know that the i.MX35 does have it. Maybe other >>> Flexcan cores than on the i.MX28 does *not* have it either. If you >>> have a chance, please check on the P1010, i.MX6Q, i.MX51, i.MX53, >>> etc. >> >>>From what I can tell, i.MX35, i.MX51 and i.MX53 use the same version, >> so they should all have the bug. And for i.MX6Q, since it uses a >> newer version even than i.MX28, I would believe it's affected by the bug. >> But I'm copying Dong who should have better knowledge about this to >> confirm. > >Thank for clarification. I have a i.MX6Q board but without CAN adapter :(, >unfortunately. Otherwise I would try it out myself. > How did you verify this issue? I just checked our ic guy of flexcan, it seems he also had no sense of this issue. Below is some version info what I got: Mx6s use FlexCAN3, with IP version 10.00.12.00 Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 Mx35 use FlexCAN2 (without glitch filter) , with IP version 03.00.00.00 Mx25 use FlexCAN2 (without glitch filter), with IP version 03.00.00.00 I'm not sure if mx6q has such issue. Regards Dong Aisheng >>> drivers/net/can/flexcan.c | 29 +++++++++++++++++++---------- >>> 1 files changed, 19 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c >>> index c5f1431..c78ecfc 100644 >>> --- a/drivers/net/can/flexcan.c >>> +++ b/drivers/net/can/flexcan.c >>> @@ -144,6 +144,10 @@ >>> >>> #define FLEXCAN_MB_CODE_MASK (0xf0ffffff) >>> >>> +/* FLEXCAN hardware feature flags */ >>> +#define FLEXCAN_HAS_V10_FEATURES BIT(1) /* For core version >= 10 >*/ >>> +#define FLEXCAN_HAS_BROKEN_ERR_STATE BIT(2) /* Broken error state >handling */ >>> + >>> /* Structure of the message buffer */ struct flexcan_mb { >>> u32 can_ctrl; >>> @@ -178,7 +182,7 @@ struct flexcan_regs { }; >>> >>> struct flexcan_devtype_data { >>> - u32 hw_ver; /* hardware controller version */ >>> + u32 features; /* hardware controller features */ >>> }; >>> >>> struct flexcan_priv { >>> @@ -197,11 +201,11 @@ struct flexcan_priv { }; >>> >>> static struct flexcan_devtype_data fsl_p1010_devtype_data = { >>> - .hw_ver = 3, >>> + .features = FLEXCAN_HAS_BROKEN_ERR_STATE, >>> }; >>> - >>> +static struct flexcan_devtype_data fsl_imx28_devtype_data; >>> static struct flexcan_devtype_data fsl_imx6q_devtype_data = { >>> - .hw_ver = 10, >>> + .features = FLEXCAN_HAS_V10_FEATURES | >>> +FLEXCAN_HAS_BROKEN_ERR_STATE, >>> }; >>> >>> static const struct can_bittiming_const flexcan_bittiming_const = { >>> @@ -741,15 +745,19 @@ static int flexcan_chip_start(struct net_device >*dev) >>> * enable tx and rx warning interrupt >>> * enable bus off interrupt >>> * (== FLEXCAN_CTRL_ERR_STATE) >>> - * >>> - * _note_: we enable the "error interrupt" >>> - * (FLEXCAN_CTRL_ERR_MSK), too. Otherwise we don't get any >>> - * warning or bus passive interrupts. >>> */ >>> reg_ctrl = flexcan_read(®s->ctrl); >>> reg_ctrl &= ~FLEXCAN_CTRL_TSYN; >>> reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | >>> - FLEXCAN_CTRL_ERR_STATE | FLEXCAN_CTRL_ERR_MSK; >>> + FLEXCAN_CTRL_ERR_STATE; >>> + /* >>> + * enable the "error interrupt" (FLEXCAN_CTRL_ERR_MSK), >>> + * on most Flexcan cores, too. Otherwise we don't get >>> + * any error warning or passive interrupts. >>> + */ >>> + if (priv->devtype_data->features & FLEXCAN_HAS_BROKEN_ERR_STATE || >>> + priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) >>> + reg_ctrl |= FLEXCAN_CTRL_ERR_MSK; >>> >>> /* save for later use */ >>> priv->reg_ctrl_default = reg_ctrl; >>> @@ -772,7 +780,7 @@ static int flexcan_chip_start(struct net_device >*dev) >>> flexcan_write(0x0, ®s->rx14mask); >>> flexcan_write(0x0, ®s->rx15mask); >>> >>> - if (priv->devtype_data->hw_ver >= 10) >>> + if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) >>> flexcan_write(0x0, ®s->rxfgmask); >>> >>> flexcan_transceiver_switch(priv, 1); @@ -954,6 +962,7 @@ static >>> void __devexit unregister_flexcandev(struct net_device *dev) >>> >>> static const struct of_device_id flexcan_of_match[] = { >>> { .compatible = "fsl,p1010-flexcan", .data = >>> &fsl_p1010_devtype_data, }, >>> + { .compatible = "fsl,imx28-flexcan", .data = >>> +&fsl_imx28_devtype_data, }, >>> { .compatible = "fsl,imx6q-flexcan", .data = >&fsl_imx6q_devtype_data, }, >>> { /* sentinel */ }, >>> }; >>> -- >>> 1.7.7.6 >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-can" >> in the body of a message to majordomo@vger.kernel.org More majordomo >> info at http://vger.kernel.org/majordomo-info.html >> >> > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Dong, On 10/08/2012 09:59 AM, Dong Aisheng-B29396 wrote: > Hi Wolfgang, > >> On 10/07/2012 05:09 AM, Shawn Guo wrote: >>> On Fri, Sep 28, 2012 at 03:17:15PM +0200, Wolfgang Grandegger wrote: >>>> Due to a bug in most Flexcan cores, the bus error interrupt needs to >>>> be enabled. Otherwise we don't get any error warning or passive >>>> interrupts. This is _not_ necessay for the i.MX28 and this patch >>>> disables bus error interrupts if "berr-reporting" is not requested. >>>> This avoids bus error flooding, which might harm, especially on >>>> low-end systems. >>>> >>>> To handle such quirks of the Flexcan cores, a hardware feature flag >>>> has been introduced, also replacing the "hw_ver" variable. So far >>>> nobody could tell what Flexcan core version is available on what >>>> Freescale SOC, apart from the i.MX6Q and P1010, and which bugs or >>>> features are present on the various "hw_rev". >>>> >>>> CC: Hui Wang <jason77.wang@gmail.com> >>>> CC: Shawn Guo <shawn.guo@linaro.org> >>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> >>>> --- >>>> >>>> Concerning the bug, I know that the i.MX35 does have it. Maybe other >>>> Flexcan cores than on the i.MX28 does *not* have it either. If you >>>> have a chance, please check on the P1010, i.MX6Q, i.MX51, i.MX53, >>>> etc. >>> >>> >From what I can tell, i.MX35, i.MX51 and i.MX53 use the same version, >>> so they should all have the bug. And for i.MX6Q, since it uses a >>> newer version even than i.MX28, I would believe it's affected by the bug. >>> But I'm copying Dong who should have better knowledge about this to >>> confirm. >> >> Thank for clarification. I have a i.MX6Q board but without CAN adapter :(, >> unfortunately. Otherwise I would try it out myself. >> > How did you verify this issue? I provoke state changes, e.g. by sending a message without connection to the bus. On the Mx28, the TWRN_INT/RWRN_INT/(BOFF_INT?) does trigger the corresponding interrupt. This does not work properly on some other cores, e.g. the Mx35. Therefore we enable ERR_INT for those cores to realize state changes. > I just checked our ic guy of flexcan, it seems he also had no sense of this issue. > > Below is some version info what I got: > Mx6s use FlexCAN3, with IP version 10.00.12.00 > Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 > Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 > Mx35 use FlexCAN2 (without glitch filter) , with IP version 03.00.00.00 > Mx25 use FlexCAN2 (without glitch filter), with IP version 03.00.00.00 > I'm not sure if mx6q has such issue. OK, we need to find that out experimentally. Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Wolfgang, >Hi Dong, > >On 10/08/2012 09:59 AM, Dong Aisheng-B29396 wrote: >> Hi Wolfgang, >> >>> On 10/07/2012 05:09 AM, Shawn Guo wrote: >>>> On Fri, Sep 28, 2012 at 03:17:15PM +0200, Wolfgang Grandegger wrote: >>>>> Due to a bug in most Flexcan cores, the bus error interrupt needs >>>>> to be enabled. Otherwise we don't get any error warning or passive >>>>> interrupts. This is _not_ necessay for the i.MX28 and this patch >>>>> disables bus error interrupts if "berr-reporting" is not requested. >>>>> This avoids bus error flooding, which might harm, especially on >>>>> low-end systems. >>>>> >>>>> To handle such quirks of the Flexcan cores, a hardware feature flag >>>>> has been introduced, also replacing the "hw_ver" variable. So far >>>>> nobody could tell what Flexcan core version is available on what >>>>> Freescale SOC, apart from the i.MX6Q and P1010, and which bugs or >>>>> features are present on the various "hw_rev". >>>>> >>>>> CC: Hui Wang <jason77.wang@gmail.com> >>>>> CC: Shawn Guo <shawn.guo@linaro.org> >>>>> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> >>>>> --- >>>>> >>>>> Concerning the bug, I know that the i.MX35 does have it. Maybe >>>>> other Flexcan cores than on the i.MX28 does *not* have it either. >>>>> If you have a chance, please check on the P1010, i.MX6Q, i.MX51, >>>>> i.MX53, etc. >>>> >>>> >From what I can tell, i.MX35, i.MX51 and i.MX53 use the same >>>> >version, >>>> so they should all have the bug. And for i.MX6Q, since it uses a >>>> newer version even than i.MX28, I would believe it's affected by the >bug. >>>> But I'm copying Dong who should have better knowledge about this to >>>> confirm. >>> >>> Thank for clarification. I have a i.MX6Q board but without CAN >>> adapter :(, unfortunately. Otherwise I would try it out myself. >>> >> How did you verify this issue? > >I provoke state changes, e.g. by sending a message without connection to >the bus. On the Mx28, the TWRN_INT/RWRN_INT/(BOFF_INT?) does trigger the >corresponding interrupt. This does not work properly on some other cores, >e.g. the Mx35. Therefore we enable ERR_INT for those cores to realize >state changes. Thanks for the info. >> I just checked our ic guy of flexcan, it seems he also had no sense of >this issue. >> >> Below is some version info what I got: >> Mx6s use FlexCAN3, with IP version 10.00.12.00 >> Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 >> Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 >> Mx35 use FlexCAN2 (without glitch filter) , with IP version >> 03.00.00.00 >> Mx25 use FlexCAN2 (without glitch filter), with IP version 03.00.00.00 >> I'm not sure if mx6q has such issue. > >OK, we need to find that out experimentally. > Our IC owner double checked the MX35 and MX53 IP and found the RX_WARN & TX_WARN Interrupt source actually are not connected to ARM. That means flexcan will not trigger interrupt to ARM core even RX_WARN or TX_WARN Happens. This may be the root cause that why you cannot see RX_WARN interrupt if not enable bus error interrupt on mx35. He also checked that mx6q has the rx/tx warning interrupt connected to arm. So we guess mx6q does not have this issue. Anyway, we can test to confirm. Regards Dong Aisheng -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/08/2012 11:13 AM, Dong Aisheng-B29396 wrote: >>> I just checked our ic guy of flexcan, it seems he also had no sense of >> this issue. >>> >>> Below is some version info what I got: >>> Mx6s use FlexCAN3, with IP version 10.00.12.00 >>> Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 >>> Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 >>> Mx35 use FlexCAN2 (without glitch filter) , with IP version >>> 03.00.00.00 >>> Mx25 use FlexCAN2 (without glitch filter), with IP version 03.00.00.00 >>> I'm not sure if mx6q has such issue. >> >> OK, we need to find that out experimentally. >> > Our IC owner double checked the MX35 and MX53 IP and found the RX_WARN & TX_WARN > Interrupt source actually are not connected to ARM. Does this mean it's a SoC problem, not a problem of the ip core? > That means flexcan will not trigger interrupt to ARM core even RX_WARN or TX_WARN > Happens. > This may be the root cause that why you cannot see RX_WARN interrupt if not enable > bus error interrupt on mx35. > He also checked that mx6q has the rx/tx warning interrupt connected to arm. > So we guess mx6q does not have this issue. > Anyway, we can test to confirm. What about mx25? Marc
>-----Original Message----- >From: Marc Kleine-Budde [mailto:mkl@pengutronix.de] >Sent: Monday, October 08, 2012 5:32 PM >To: Dong Aisheng-B29396 >Cc: Wolfgang Grandegger; Shawn Guo; Linux Netdev List; Linux-CAN; Hui Wang >Subject: Re: [PATCH] flexcan: disable bus error interrupts for the i.MX28 >Importance: High > >On 10/08/2012 11:13 AM, Dong Aisheng-B29396 wrote: >>>> I just checked our ic guy of flexcan, it seems he also had no sense >>>> of >>> this issue. >>>> >>>> Below is some version info what I got: >>>> Mx6s use FlexCAN3, with IP version 10.00.12.00 >>>> Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 >>>> Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 >>>> Mx35 use FlexCAN2 (without glitch filter) , with IP version >>>> 03.00.00.00 >>>> Mx25 use FlexCAN2 (without glitch filter), with IP version >>>> 03.00.00.00 I'm not sure if mx6q has such issue. >>> >>> OK, we need to find that out experimentally. >>> >> Our IC owner double checked the MX35 and MX53 IP and found the RX_WARN >> & TX_WARN Interrupt source actually are not connected to ARM. > >Does this mean it's a SoC problem, not a problem of the ip core? > It's not a problem of ip core, it's about how to use the IP. I do not know why some i.MX SoCs does not use rx/tx warn interrupts. >> That means flexcan will not trigger interrupt to ARM core even RX_WARN >> or TX_WARN Happens. >> This may be the root cause that why you cannot see RX_WARN interrupt >> if not enable bus error interrupt on mx35. >> He also checked that mx6q has the rx/tx warning interrupt connected to >arm. >> So we guess mx6q does not have this issue. >> Anyway, we can test to confirm. > >What about mx25? > For mx25 and mx28, he could not access it now. Will check tomorrow. Regards Dong Aisheng -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Wolfgang, >-----Original Message----- >From: Dong Aisheng-B29396 >Sent: Monday, October 08, 2012 5:44 PM >To: 'Marc Kleine-Budde' >Cc: Wolfgang Grandegger; Shawn Guo; Linux Netdev List; Linux-CAN; Hui Wang >Subject: RE: [PATCH] flexcan: disable bus error interrupts for the i.MX28 > >>-----Original Message----- >>From: Marc Kleine-Budde [mailto:mkl@pengutronix.de] >>Sent: Monday, October 08, 2012 5:32 PM >>To: Dong Aisheng-B29396 >>Cc: Wolfgang Grandegger; Shawn Guo; Linux Netdev List; Linux-CAN; Hui >>Wang >>Subject: Re: [PATCH] flexcan: disable bus error interrupts for the >>i.MX28 >>Importance: High >> >>On 10/08/2012 11:13 AM, Dong Aisheng-B29396 wrote: >>>>> I just checked our ic guy of flexcan, it seems he also had no sense >>>>> of >>>> this issue. >>>>> >>>>> Below is some version info what I got: >>>>> Mx6s use FlexCAN3, with IP version 10.00.12.00 >>>>> Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 >>>>> Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 >>>>> Mx35 use FlexCAN2 (without glitch filter) , with IP version >>>>> 03.00.00.00 >>>>> Mx25 use FlexCAN2 (without glitch filter), with IP version >>>>> 03.00.00.00 I'm not sure if mx6q has such issue. >>>> >>>> OK, we need to find that out experimentally. >>>> >>> Our IC owner double checked the MX35 and MX53 IP and found the >>> RX_WARN & TX_WARN Interrupt source actually are not connected to ARM. >> >>Does this mean it's a SoC problem, not a problem of the ip core? >> >It's not a problem of ip core, it's about how to use the IP. >I do not know why some i.MX SoCs does not use rx/tx warn interrupts. > >>> That means flexcan will not trigger interrupt to ARM core even >>> RX_WARN or TX_WARN Happens. >>> This may be the root cause that why you cannot see RX_WARN interrupt >>> if not enable bus error interrupt on mx35. >>> He also checked that mx6q has the rx/tx warning interrupt connected >>> to >>arm. >>> So we guess mx6q does not have this issue. >>> Anyway, we can test to confirm. >> >>What about mx25? >> >For mx25 and mx28, he could not access it now. >Will check tomorrow. > Just let you know: The checking result is Mx28 has rx/tx warning interrupt line connected while mx25 not. Looks align with what we guess. Regards Dong Aisheng -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/09/2012 01:52 PM, Dong Aisheng-B29396 wrote: > Hi Wolfgang, > >> -----Original Message----- >> From: Dong Aisheng-B29396 >> Sent: Monday, October 08, 2012 5:44 PM >> To: 'Marc Kleine-Budde' >> Cc: Wolfgang Grandegger; Shawn Guo; Linux Netdev List; Linux-CAN; Hui Wang >> Subject: RE: [PATCH] flexcan: disable bus error interrupts for the i.MX28 >> >>> -----Original Message----- >>> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de] >>> Sent: Monday, October 08, 2012 5:32 PM >>> To: Dong Aisheng-B29396 >>> Cc: Wolfgang Grandegger; Shawn Guo; Linux Netdev List; Linux-CAN; Hui >>> Wang >>> Subject: Re: [PATCH] flexcan: disable bus error interrupts for the >>> i.MX28 >>> Importance: High >>> >>> On 10/08/2012 11:13 AM, Dong Aisheng-B29396 wrote: >>>>>> I just checked our ic guy of flexcan, it seems he also had no sense >>>>>> of >>>>> this issue. >>>>>> >>>>>> Below is some version info what I got: >>>>>> Mx6s use FlexCAN3, with IP version 10.00.12.00 >>>>>> Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 >>>>>> Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 >>>>>> Mx35 use FlexCAN2 (without glitch filter) , with IP version >>>>>> 03.00.00.00 >>>>>> Mx25 use FlexCAN2 (without glitch filter), with IP version >>>>>> 03.00.00.00 I'm not sure if mx6q has such issue. >>>>> >>>>> OK, we need to find that out experimentally. >>>>> >>>> Our IC owner double checked the MX35 and MX53 IP and found the >>>> RX_WARN & TX_WARN Interrupt source actually are not connected to ARM. >>> >>> Does this mean it's a SoC problem, not a problem of the ip core? >>> >> It's not a problem of ip core, it's about how to use the IP. >> I do not know why some i.MX SoCs does not use rx/tx warn interrupts. >> >>>> That means flexcan will not trigger interrupt to ARM core even >>>> RX_WARN or TX_WARN Happens. >>>> This may be the root cause that why you cannot see RX_WARN interrupt >>>> if not enable bus error interrupt on mx35. >>>> He also checked that mx6q has the rx/tx warning interrupt connected >>>> to >>> arm. >>>> So we guess mx6q does not have this issue. >>>> Anyway, we can test to confirm. >>> >>> What about mx25? >>> >> For mx25 and mx28, he could not access it now. >> Will check tomorrow. >> > Just let you know: > The checking result is Mx28 has rx/tx warning interrupt line connected > while mx25 not. > Looks align with what we guess. OK, then I'm going to remove FLEXCAN_HAS_BROKEN_ERR_STATE for the mx6q as well in the next version of the patch. Thanks for taking care. Wolfgang -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/08/2012 09:59 AM, Dong Aisheng-B29396 wrote: [...] >>>> Concerning the bug, I know that the i.MX35 does have it. Maybe other >>>> Flexcan cores than on the i.MX28 does *not* have it either. If you >>>> have a chance, please check on the P1010, i.MX6Q, i.MX51, i.MX53, >>>> etc. >>> >>> >From what I can tell, i.MX35, i.MX51 and i.MX53 use the same version, >>> so they should all have the bug. And for i.MX6Q, since it uses a >>> newer version even than i.MX28, I would believe it's affected by the bug. >>> But I'm copying Dong who should have better knowledge about this to >>> confirm. >> >> Thank for clarification. I have a i.MX6Q board but without CAN adapter :(, >> unfortunately. Otherwise I would try it out myself. >> > How did you verify this issue? > I just checked our ic guy of flexcan, it seems he also had no sense of this issue. Wolfgang added this table to the driver code, which is a very good idea. I've some (nitpicky) questions :) > Below is some version info what I got: > Mx6s use FlexCAN3, with IP version 10.00.12.00 ^^^ Is this core different from the mx6q? Has the flexcan on mx6 a glitch filter? > Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 > Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 > Mx35 use FlexCAN2 (without glitch filter) , with IP version 03.00.00.00 > Mx25 use FlexCAN2 (without glitch filter), with IP version 03.00.00.00 Do you have access to the powerpc designs which have a flexcan core, too? I'm interested to complete the above table with powerpcs cores (p1010 and similar). Marc
On Wed, Oct 10, 2012 at 10:04:42PM +0200, Marc Kleine-Budde wrote: > On 10/08/2012 09:59 AM, Dong Aisheng-B29396 wrote: > [...] > > >>>> Concerning the bug, I know that the i.MX35 does have it. Maybe other > >>>> Flexcan cores than on the i.MX28 does *not* have it either. If you > >>>> have a chance, please check on the P1010, i.MX6Q, i.MX51, i.MX53, > >>>> etc. > >>> > >>> >From what I can tell, i.MX35, i.MX51 and i.MX53 use the same version, > >>> so they should all have the bug. And for i.MX6Q, since it uses a > >>> newer version even than i.MX28, I would believe it's affected by the bug. > >>> But I'm copying Dong who should have better knowledge about this to > >>> confirm. > >> > >> Thank for clarification. I have a i.MX6Q board but without CAN adapter :(, > >> unfortunately. Otherwise I would try it out myself. > >> > > How did you verify this issue? > > I just checked our ic guy of flexcan, it seems he also had no sense of this issue. > > Wolfgang added this table to the driver code, which is a very good idea. > I've some (nitpicky) questions :) > > > Below is some version info what I got: > > Mx6s use FlexCAN3, with IP version 10.00.12.00 > ^^^ > Is this core different from the mx6q? Yes, it's a single core named i.MX6 Solo. For details, please see: http://www.freescale.com/webapp/sps/site/taxonomy.jsp?code=IMX6X_SERIES > Has the flexcan on mx6 a glitch filter? > Yes. > > Mx53 use FlexCAN2 (with glitch filter), with IP version 03.00.00.00 > > Mx28 use FlexCAN2 (with glitch filter), with IP version 03.00.04.00 > > Mx35 use FlexCAN2 (without glitch filter) , with IP version 03.00.00.00 > > Mx25 use FlexCAN2 (without glitch filter), with IP version 03.00.00.00 > > Do you have access to the powerpc designs which have a flexcan core, > too? I'm interested to complete the above table with powerpcs cores > (p1010 and similar). Sorry, we don't have access. Regards Dong Aisheng > > 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 | > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index c5f1431..c78ecfc 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -144,6 +144,10 @@ #define FLEXCAN_MB_CODE_MASK (0xf0ffffff) +/* FLEXCAN hardware feature flags */ +#define FLEXCAN_HAS_V10_FEATURES BIT(1) /* For core version >= 10 */ +#define FLEXCAN_HAS_BROKEN_ERR_STATE BIT(2) /* Broken error state handling */ + /* Structure of the message buffer */ struct flexcan_mb { u32 can_ctrl; @@ -178,7 +182,7 @@ struct flexcan_regs { }; struct flexcan_devtype_data { - u32 hw_ver; /* hardware controller version */ + u32 features; /* hardware controller features */ }; struct flexcan_priv { @@ -197,11 +201,11 @@ struct flexcan_priv { }; static struct flexcan_devtype_data fsl_p1010_devtype_data = { - .hw_ver = 3, + .features = FLEXCAN_HAS_BROKEN_ERR_STATE, }; - +static struct flexcan_devtype_data fsl_imx28_devtype_data; static struct flexcan_devtype_data fsl_imx6q_devtype_data = { - .hw_ver = 10, + .features = FLEXCAN_HAS_V10_FEATURES | FLEXCAN_HAS_BROKEN_ERR_STATE, }; static const struct can_bittiming_const flexcan_bittiming_const = { @@ -741,15 +745,19 @@ static int flexcan_chip_start(struct net_device *dev) * enable tx and rx warning interrupt * enable bus off interrupt * (== FLEXCAN_CTRL_ERR_STATE) - * - * _note_: we enable the "error interrupt" - * (FLEXCAN_CTRL_ERR_MSK), too. Otherwise we don't get any - * warning or bus passive interrupts. */ reg_ctrl = flexcan_read(®s->ctrl); reg_ctrl &= ~FLEXCAN_CTRL_TSYN; reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF | - FLEXCAN_CTRL_ERR_STATE | FLEXCAN_CTRL_ERR_MSK; + FLEXCAN_CTRL_ERR_STATE; + /* + * enable the "error interrupt" (FLEXCAN_CTRL_ERR_MSK), + * on most Flexcan cores, too. Otherwise we don't get + * any error warning or passive interrupts. + */ + if (priv->devtype_data->features & FLEXCAN_HAS_BROKEN_ERR_STATE || + priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) + reg_ctrl |= FLEXCAN_CTRL_ERR_MSK; /* save for later use */ priv->reg_ctrl_default = reg_ctrl; @@ -772,7 +780,7 @@ static int flexcan_chip_start(struct net_device *dev) flexcan_write(0x0, ®s->rx14mask); flexcan_write(0x0, ®s->rx15mask); - if (priv->devtype_data->hw_ver >= 10) + if (priv->devtype_data->features & FLEXCAN_HAS_V10_FEATURES) flexcan_write(0x0, ®s->rxfgmask); flexcan_transceiver_switch(priv, 1); @@ -954,6 +962,7 @@ static void __devexit unregister_flexcandev(struct net_device *dev) static const struct of_device_id flexcan_of_match[] = { { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, }, + { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, }, { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, }, { /* sentinel */ }, };
Due to a bug in most Flexcan cores, the bus error interrupt needs to be enabled. Otherwise we don't get any error warning or passive interrupts. This is _not_ necessay for the i.MX28 and this patch disables bus error interrupts if "berr-reporting" is not requested. This avoids bus error flooding, which might harm, especially on low-end systems. To handle such quirks of the Flexcan cores, a hardware feature flag has been introduced, also replacing the "hw_ver" variable. So far nobody could tell what Flexcan core version is available on what Freescale SOC, apart from the i.MX6Q and P1010, and which bugs or features are present on the various "hw_rev". CC: Hui Wang <jason77.wang@gmail.com> CC: Shawn Guo <shawn.guo@linaro.org> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com> --- Concerning the bug, I know that the i.MX35 does have it. Maybe other Flexcan cores than on the i.MX28 does *not* have it either. If you have a chance, please check on the P1010, i.MX6Q, i.MX51, i.MX53, etc. Wolfgang. drivers/net/can/flexcan.c | 29 +++++++++++++++++++---------- 1 files changed, 19 insertions(+), 10 deletions(-)