From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Grandegger Subject: Re: [PATCH v4 4/6] can: flexcan: Consolidate and unify state change handling. Date: Mon, 01 Dec 2014 12:37:03 +0100 Message-ID: <6a5747c2d61b278876b6c3154c6a3130@grandegger.com> References: <3103cd54-1421-4594-855f-7bfca8f4ef49@GRBSR0089.marel.net> <547B7CDD.2080202@grandegger.com> <20141201110923.29602.38961@shannon> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: Received: from pluto.manitu.net ([217.11.48.9]:39367 "EHLO pluto.manitu.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753021AbaLALhI (ORCPT ); Mon, 1 Dec 2014 06:37:08 -0500 In-Reply-To: <20141201110923.29602.38961@shannon> Sender: linux-can-owner@vger.kernel.org List-ID: To: Andri Yngvason Cc: linux-can@vger.kernel.org, mkl@pengutronix.de On Mon, 1 Dec 2014 11:09:23 +0000, Andri Yngvason wrote: > Quoting Wolfgang Grandegger (2014-11-30 20:23:57) >> On 11/28/2014 01:12 PM, Andri Yngvason wrote: >> > Replacing error state change handling with the new mechanism. >> > >> > Signed-off-by: Andri Yngvason >> > --- >> > Changes made since last proposal: >> > can: flexcan: add FLEXCAN_HAS_BROKEN_ERR_STATE for i.MX6 >> > can: flexcan: adapt to newer can_change_state >> > >> > drivers/net/can/flexcan.c | 103 >> > +++++++++------------------------------------- >> > 1 file changed, 19 insertions(+), 84 deletions(-) >> > >> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c >> > index 60f86bd..9f91735 100644 >> > --- a/drivers/net/can/flexcan.c >> > +++ b/drivers/net/can/flexcan.c >> > @@ -266,7 +266,7 @@ static struct flexcan_devtype_data >> > fsl_p1010_devtype_data = { >> > }; >> > static struct flexcan_devtype_data fsl_imx28_devtype_data; >> > static struct flexcan_devtype_data fsl_imx6q_devtype_data = { >> > - .features = FLEXCAN_HAS_V10_FEATURES, >> > + .features = FLEXCAN_HAS_V10_FEATURES | >> > FLEXCAN_HAS_BROKEN_ERR_STATE, >> >> Oops, this change is not related to the subject! Anyway, did it cure >> your problems with state handling. Is it required for all i.MX6 cores >> then? >> > Yeah, I accidentally squashed this; intended to make this a separate > patch. This > helps. It has the same effect as enabling berr-reporting. However, this > only > cures one of the problems. This does not help with the case where no one > else is > sending on the bus. > > This problem is not limited to i.MX6. This is a problem for ALL FlexCAN > cores. > This is a design flaw in FlexCAN cores. Maybe they did this part on a > monday > after a weekend of binge-drinking. In any case, they really messed up with > this > one. #define FLEXCAN_HAS_BROKEN_ERR_STATE BIT(2) /* [TR]WRN_INT not connected */ They actually forgot to connect the [TR]WRN_INT line. Don't know what they did the days before ;). > I suggest that we remove the FLEXCAN_HAS_BROKEN_ERR_STATE flag and turn on > error > interrupts permanently because they ALL have broken error state. Hm, how did you come to this conclusion? You just tested on an i.MX6, right? Anyway, the bus error reporting can cause very high CPU load and it should be off when it's not needed. Wolfgang.