diff mbox

[V3] CAN: Add Flexcan CAN controller driver

Message ID 20090729082010.GZ2714@pengutronix.de
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Sascha Hauer July 29, 2009, 8:20 a.m. UTC
And another go...

Sascha


From 9f6f6fb67d57d082e056dec8121f1423c1b4fa0b Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 21 Jul 2009 10:47:19 +0200
Subject: [PATCH] CAN: Add Flexcan CAN driver

This core is found on some Freescale SoCs and also some Coldfire
SoCs. Support for Coldfire is missing though at the moment as
they have an older revision of the core which does not have RX FIFO
support.

V3: integrated comments from Oliver Hartkopp
V2: integrated comments from Wolfgang Grandegger

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/can/Kconfig   |    6 +
 drivers/net/can/Makefile  |    1 +
 drivers/net/can/flexcan.c |  719 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 726 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/flexcan.c

Comments

Oliver Hartkopp July 29, 2009, 8:45 a.m. UTC | #1
Sascha Hauer wrote:
> And another go...
> 
> Sascha
> 
> 
> From 9f6f6fb67d57d082e056dec8121f1423c1b4fa0b Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Tue, 21 Jul 2009 10:47:19 +0200
> Subject: [PATCH] CAN: Add Flexcan CAN driver
> 
> This core is found on some Freescale SoCs and also some Coldfire
> SoCs. Support for Coldfire is missing though at the moment as
> they have an older revision of the core which does not have RX FIFO
> support.
> 
> V3: integrated comments from Oliver Hartkopp
> V2: integrated comments from Wolfgang Grandegger
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Thanks Sascha!

The register access and the transition between the struct can_frame became
much clearer now.

Looks good to me.

Best regards,
Oliver
--
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
Wolfgang Grandegger July 29, 2009, 9:09 a.m. UTC | #2
Sascha Hauer wrote:
> And another go...
> 
> Sascha
> 
> 
>>From 9f6f6fb67d57d082e056dec8121f1423c1b4fa0b Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Tue, 21 Jul 2009 10:47:19 +0200
> Subject: [PATCH] CAN: Add Flexcan CAN driver
> 
> This core is found on some Freescale SoCs and also some Coldfire
> SoCs. Support for Coldfire is missing though at the moment as
> they have an older revision of the core which does not have RX FIFO
> support.
> 
> V3: integrated comments from Oliver Hartkopp
> V2: integrated comments from Wolfgang Grandegger
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/net/can/Kconfig   |    6 +
>  drivers/net/can/Makefile  |    1 +
>  drivers/net/can/flexcan.c |  719 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 726 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/can/flexcan.c
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 33821a8..99c6da4 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -74,6 +74,12 @@ config CAN_KVASER_PCI
>  	  This driver is for the the PCIcanx and PCIcan cards (1, 2 or
>  	  4 channel) from Kvaser (http://www.kvaser.com).
>  
> +config CAN_FLEXCAN
> +	tristate "Support for Freescale FLEXCAN based chips"
> +	depends on CAN_DEV
> +	---help---
> +	  Say Y here if you want to support for Freescale FlexCAN.
> +
>  config CAN_DEBUG_DEVICES
>  	bool "CAN devices debugging messages"
>  	depends on CAN
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index 523a941..25f2032 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -8,5 +8,6 @@ obj-$(CONFIG_CAN_DEV)		+= can-dev.o
>  can-dev-y			:= dev.o
>  
>  obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
> +obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> new file mode 100644
> index 0000000..77661b3
> --- /dev/null
> +++ b/drivers/net/can/flexcan.c
[...]
> +static void flexcan_error(struct net_device *ndev, u32 stat)
> +{
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	enum can_state state = priv->can.state;
> +	int error_warning = 0, rx_errors = 0, tx_errors = 0;
> +
> +	skb = dev_alloc_skb(sizeof(struct can_frame));
> +	if (!skb)
> +		return;
> +
> +	skb->dev = ndev;
> +	skb->protocol = __constant_htons(ETH_P_CAN);
> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
> +
> +	cf = (struct can_frame *)skb_put(skb, sizeof(*cf));
> +	memset(cf, 0, sizeof(*cf));
> +
> +	cf->can_id = CAN_ERR_FLAG;
> +	cf->can_dlc = CAN_ERR_DLC;
> +
> +	if (stat & ERRSTAT_RWRNINT) {
> +		error_warning = 1;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> +	}
> +
> +	if (stat & ERRSTAT_TWRNINT) {
> +		error_warning = 1;
> +		cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> +	}

What is the meaning of this error warning interrupt? It does *not*
change the state.

> +	switch ((stat >> 4) & 0x3) {
> +	case 0:
> +		state = CAN_STATE_ERROR_ACTIVE;
> +		break;
> +	case 1:
> +		state = CAN_STATE_ERROR_PASSIVE;
> +		break;
> +	default:
> +		state = CAN_STATE_BUS_OFF;
> +		break;
> +	}

I'm still not happy with the error message generation. If a state change
to error passive happens, it should be signaled to the user. Here my ideas:

	if (state != priv->can.state) {
		if (state == CAN_STATE_ERROR_WARNING) {
			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
			priv->can.can_stats.error_warning++;
		} else if (state == CAN_STATE_ERROR_PASSIVE) {
			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
			priv->can.can_stats.error_passive++;
		}

It might have missed something, though.

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
Sascha Hauer July 30, 2009, 8:37 a.m. UTC | #3
On Wed, Jul 29, 2009 at 11:09:14AM +0200, Wolfgang Grandegger wrote:
> Sascha Hauer wrote:
> > And another go...
> > 
> > Sascha
> > 
> > 
> >>From 9f6f6fb67d57d082e056dec8121f1423c1b4fa0b Mon Sep 17 00:00:00 2001
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Date: Tue, 21 Jul 2009 10:47:19 +0200
> > Subject: [PATCH] CAN: Add Flexcan CAN driver
> > 
> > This core is found on some Freescale SoCs and also some Coldfire
> > SoCs. Support for Coldfire is missing though at the moment as
> > they have an older revision of the core which does not have RX FIFO
> > support.
> > 
> > V3: integrated comments from Oliver Hartkopp
> > V2: integrated comments from Wolfgang Grandegger
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/net/can/Kconfig   |    6 +
> >  drivers/net/can/Makefile  |    1 +
> >  drivers/net/can/flexcan.c |  719 +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 726 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/net/can/flexcan.c
> > 
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> > index 33821a8..99c6da4 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -74,6 +74,12 @@ config CAN_KVASER_PCI
> >  	  This driver is for the the PCIcanx and PCIcan cards (1, 2 or
> >  	  4 channel) from Kvaser (http://www.kvaser.com).
> >  
> > +config CAN_FLEXCAN
> > +	tristate "Support for Freescale FLEXCAN based chips"
> > +	depends on CAN_DEV
> > +	---help---
> > +	  Say Y here if you want to support for Freescale FlexCAN.
> > +
> >  config CAN_DEBUG_DEVICES
> >  	bool "CAN devices debugging messages"
> >  	depends on CAN
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> > index 523a941..25f2032 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -8,5 +8,6 @@ obj-$(CONFIG_CAN_DEV)		+= can-dev.o
> >  can-dev-y			:= dev.o
> >  
> >  obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
> > +obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
> >  
> >  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > new file mode 100644
> > index 0000000..77661b3
> > --- /dev/null
> > +++ b/drivers/net/can/flexcan.c
> [...]
> > +static void flexcan_error(struct net_device *ndev, u32 stat)
> > +{
> > +	struct can_frame *cf;
> > +	struct sk_buff *skb;
> > +	struct flexcan_priv *priv = netdev_priv(ndev);
> > +	struct net_device_stats *stats = &ndev->stats;
> > +	enum can_state state = priv->can.state;
> > +	int error_warning = 0, rx_errors = 0, tx_errors = 0;
> > +
> > +	skb = dev_alloc_skb(sizeof(struct can_frame));
> > +	if (!skb)
> > +		return;
> > +
> > +	skb->dev = ndev;
> > +	skb->protocol = __constant_htons(ETH_P_CAN);
> > +	skb->ip_summed = CHECKSUM_UNNECESSARY;
> > +
> > +	cf = (struct can_frame *)skb_put(skb, sizeof(*cf));
> > +	memset(cf, 0, sizeof(*cf));
> > +
> > +	cf->can_id = CAN_ERR_FLAG;
> > +	cf->can_dlc = CAN_ERR_DLC;
> > +
> > +	if (stat & ERRSTAT_RWRNINT) {
> > +		error_warning = 1;
> > +		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> > +	}
> > +
> > +	if (stat & ERRSTAT_TWRNINT) {
> > +		error_warning = 1;
> > +		cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> > +	}
> 
> What is the meaning of this error warning interrupt? It does *not*
> change the state.

This TWRNINT means that the Tx error counter has transitioned from < 96
to >= 96 (analog the RWRNINT for the Rx error counter). Unfortunately
the state bits handled below do not reflect this error warning state.

So this interrupt signals us that we have been in error warning state
once but we have no possibility to detect if we are still in error
warning state. I don't know how to handle this correctly. I could
disable this interrupt and ignore it completely.

> 
> > +	switch ((stat >> 4) & 0x3) {
> > +	case 0:
> > +		state = CAN_STATE_ERROR_ACTIVE;
> > +		break;
> > +	case 1:
> > +		state = CAN_STATE_ERROR_PASSIVE;
> > +		break;
> > +	default:
> > +		state = CAN_STATE_BUS_OFF;
> > +		break;
> > +	}
> 
> I'm still not happy with the error message generation. If a state change
> to error passive happens, it should be signaled to the user. Here my ideas:
> 
> 	if (state != priv->can.state) {
> 		if (state == CAN_STATE_ERROR_WARNING) {
> 			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> 			priv->can.can_stats.error_warning++;
> 		} else if (state == CAN_STATE_ERROR_PASSIVE) {
> 			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> 			priv->can.can_stats.error_passive++;
> 		}
> 
> It might have missed something, though.

Sorry, I missed that from your previous mail. As we can't properly
detect ERROR_WARNING state this would reduce to:

	if (state != priv->can.state && state == CAN_STATE_ERROR_WARNING) {
		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
		priv->can.can_stats.error_warning++;
	}

Sascha
Wolfgang Grandegger July 30, 2009, 8:51 a.m. UTC | #4
Sascha Hauer wrote:
> On Wed, Jul 29, 2009 at 11:09:14AM +0200, Wolfgang Grandegger wrote:
>> Sascha Hauer wrote:
>>> And another go...
>>>
>>> Sascha
>>>
>>>
>>> >From 9f6f6fb67d57d082e056dec8121f1423c1b4fa0b Mon Sep 17 00:00:00 2001
>>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>> Date: Tue, 21 Jul 2009 10:47:19 +0200
>>> Subject: [PATCH] CAN: Add Flexcan CAN driver
>>>
>>> This core is found on some Freescale SoCs and also some Coldfire
>>> SoCs. Support for Coldfire is missing though at the moment as
>>> they have an older revision of the core which does not have RX FIFO
>>> support.
>>>
>>> V3: integrated comments from Oliver Hartkopp
>>> V2: integrated comments from Wolfgang Grandegger
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>  drivers/net/can/Kconfig   |    6 +
>>>  drivers/net/can/Makefile  |    1 +
>>>  drivers/net/can/flexcan.c |  719 +++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 726 insertions(+), 0 deletions(-)
>>>  create mode 100644 drivers/net/can/flexcan.c
>>>
>>> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
>>> index 33821a8..99c6da4 100644
>>> --- a/drivers/net/can/Kconfig
>>> +++ b/drivers/net/can/Kconfig
>>> @@ -74,6 +74,12 @@ config CAN_KVASER_PCI
>>>  	  This driver is for the the PCIcanx and PCIcan cards (1, 2 or
>>>  	  4 channel) from Kvaser (http://www.kvaser.com).
>>>  
>>> +config CAN_FLEXCAN
>>> +	tristate "Support for Freescale FLEXCAN based chips"
>>> +	depends on CAN_DEV
>>> +	---help---
>>> +	  Say Y here if you want to support for Freescale FlexCAN.
>>> +
>>>  config CAN_DEBUG_DEVICES
>>>  	bool "CAN devices debugging messages"
>>>  	depends on CAN
>>> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
>>> index 523a941..25f2032 100644
>>> --- a/drivers/net/can/Makefile
>>> +++ b/drivers/net/can/Makefile
>>> @@ -8,5 +8,6 @@ obj-$(CONFIG_CAN_DEV)		+= can-dev.o
>>>  can-dev-y			:= dev.o
>>>  
>>>  obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
>>> +obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>>>  
>>>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
>>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>> new file mode 100644
>>> index 0000000..77661b3
>>> --- /dev/null
>>> +++ b/drivers/net/can/flexcan.c
>> [...]
>>> +static void flexcan_error(struct net_device *ndev, u32 stat)
>>> +{
>>> +	struct can_frame *cf;
>>> +	struct sk_buff *skb;
>>> +	struct flexcan_priv *priv = netdev_priv(ndev);
>>> +	struct net_device_stats *stats = &ndev->stats;
>>> +	enum can_state state = priv->can.state;
>>> +	int error_warning = 0, rx_errors = 0, tx_errors = 0;
>>> +
>>> +	skb = dev_alloc_skb(sizeof(struct can_frame));
>>> +	if (!skb)
>>> +		return;
>>> +
>>> +	skb->dev = ndev;
>>> +	skb->protocol = __constant_htons(ETH_P_CAN);
>>> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
>>> +
>>> +	cf = (struct can_frame *)skb_put(skb, sizeof(*cf));
>>> +	memset(cf, 0, sizeof(*cf));
>>> +
>>> +	cf->can_id = CAN_ERR_FLAG;
>>> +	cf->can_dlc = CAN_ERR_DLC;
>>> +
>>> +	if (stat & ERRSTAT_RWRNINT) {
>>> +		error_warning = 1;
>>> +		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>>> +	}
>>> +
>>> +	if (stat & ERRSTAT_TWRNINT) {
>>> +		error_warning = 1;
>>> +		cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
>>> +	}
>> What is the meaning of this error warning interrupt? It does *not*
>> change the state.
> 
> This TWRNINT means that the Tx error counter has transitioned from < 96
> to >= 96 (analog the RWRNINT for the Rx error counter). Unfortunately
> the state bits handled below do not reflect this error warning state.
> 
> So this interrupt signals us that we have been in error warning state
> once but we have no possibility to detect if we are still in error
> warning state. I don't know how to handle this correctly. I could
> disable this interrupt and ignore it completely.
> 
>>> +	switch ((stat >> 4) & 0x3) {
>>> +	case 0:
>>> +		state = CAN_STATE_ERROR_ACTIVE;
>>> +		break;
>>> +	case 1:
>>> +		state = CAN_STATE_ERROR_PASSIVE;
>>> +		break;
>>> +	default:
>>> +		state = CAN_STATE_BUS_OFF;
>>> +		break;
>>> +	}
>> I'm still not happy with the error message generation. If a state change
>> to error passive happens, it should be signaled to the user. Here my ideas:
>>
>> 	if (state != priv->can.state) {
>> 		if (state == CAN_STATE_ERROR_WARNING) {
>> 			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>> 			priv->can.can_stats.error_warning++;
>> 		} else if (state == CAN_STATE_ERROR_PASSIVE) {
>> 			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
>> 			priv->can.can_stats.error_passive++;
>> 		}
>>
>> It might have missed something, though.
> 
> Sorry, I missed that from your previous mail. As we can't properly
> detect ERROR_WARNING state this would reduce to:
> 
> 	if (state != priv->can.state && state == CAN_STATE_ERROR_WARNING) {
> 		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> 		priv->can.can_stats.error_warning++;
> 	}

Why you can't detect error warning? When the [RT]WRNINT comes, the error
waning state has entered. By looking to the manual, the state changes to
error warning and bus-off are directly signaled via interrupt. A change
to error passive might be visible when bus error ints occurs. You could
add some dev_dbg's to the "if (stat & ERRSTAT_[RT]WRNINT)" and "switch"
blocks above and check with sending a message with no cable connected.

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
Sascha Hauer July 30, 2009, 9:27 a.m. UTC | #5
On Thu, Jul 30, 2009 at 10:51:26AM +0200, Wolfgang Grandegger wrote:
> Sascha Hauer wrote:
> > On Wed, Jul 29, 2009 at 11:09:14AM +0200, Wolfgang Grandegger wrote:
> >> Sascha Hauer wrote:
> >>> And another go...
> >>>
> >>> Sascha
> >>>
> >>>
> >>> >From 9f6f6fb67d57d082e056dec8121f1423c1b4fa0b Mon Sep 17 00:00:00 2001
> >>> From: Sascha Hauer <s.hauer@pengutronix.de>
> >>> Date: Tue, 21 Jul 2009 10:47:19 +0200
> >>> Subject: [PATCH] CAN: Add Flexcan CAN driver
> >>>
> >>> This core is found on some Freescale SoCs and also some Coldfire
> >>> SoCs. Support for Coldfire is missing though at the moment as
> >>> they have an older revision of the core which does not have RX FIFO
> >>> support.
> >>>
> >>> V3: integrated comments from Oliver Hartkopp
> >>> V2: integrated comments from Wolfgang Grandegger
> >>>
> >>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >>> ---
> >>>  drivers/net/can/Kconfig   |    6 +
> >>>  drivers/net/can/Makefile  |    1 +
> >>>  drivers/net/can/flexcan.c |  719 +++++++++++++++++++++++++++++++++++++++++++++
> >>>  3 files changed, 726 insertions(+), 0 deletions(-)
> >>>  create mode 100644 drivers/net/can/flexcan.c
> >>>
> >>> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> >>> index 33821a8..99c6da4 100644
> >>> --- a/drivers/net/can/Kconfig
> >>> +++ b/drivers/net/can/Kconfig
> >>> @@ -74,6 +74,12 @@ config CAN_KVASER_PCI
> >>>  	  This driver is for the the PCIcanx and PCIcan cards (1, 2 or
> >>>  	  4 channel) from Kvaser (http://www.kvaser.com).
> >>>  
> >>> +config CAN_FLEXCAN
> >>> +	tristate "Support for Freescale FLEXCAN based chips"
> >>> +	depends on CAN_DEV
> >>> +	---help---
> >>> +	  Say Y here if you want to support for Freescale FlexCAN.
> >>> +
> >>>  config CAN_DEBUG_DEVICES
> >>>  	bool "CAN devices debugging messages"
> >>>  	depends on CAN
> >>> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> >>> index 523a941..25f2032 100644
> >>> --- a/drivers/net/can/Makefile
> >>> +++ b/drivers/net/can/Makefile
> >>> @@ -8,5 +8,6 @@ obj-$(CONFIG_CAN_DEV)		+= can-dev.o
> >>>  can-dev-y			:= dev.o
> >>>  
> >>>  obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
> >>> +obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
> >>>  
> >>>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> >>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> >>> new file mode 100644
> >>> index 0000000..77661b3
> >>> --- /dev/null
> >>> +++ b/drivers/net/can/flexcan.c
> >> [...]
> >>> +static void flexcan_error(struct net_device *ndev, u32 stat)
> >>> +{
> >>> +	struct can_frame *cf;
> >>> +	struct sk_buff *skb;
> >>> +	struct flexcan_priv *priv = netdev_priv(ndev);
> >>> +	struct net_device_stats *stats = &ndev->stats;
> >>> +	enum can_state state = priv->can.state;
> >>> +	int error_warning = 0, rx_errors = 0, tx_errors = 0;
> >>> +
> >>> +	skb = dev_alloc_skb(sizeof(struct can_frame));
> >>> +	if (!skb)
> >>> +		return;
> >>> +
> >>> +	skb->dev = ndev;
> >>> +	skb->protocol = __constant_htons(ETH_P_CAN);
> >>> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
> >>> +
> >>> +	cf = (struct can_frame *)skb_put(skb, sizeof(*cf));
> >>> +	memset(cf, 0, sizeof(*cf));
> >>> +
> >>> +	cf->can_id = CAN_ERR_FLAG;
> >>> +	cf->can_dlc = CAN_ERR_DLC;
> >>> +
> >>> +	if (stat & ERRSTAT_RWRNINT) {
> >>> +		error_warning = 1;
> >>> +		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> >>> +	}
> >>> +
> >>> +	if (stat & ERRSTAT_TWRNINT) {
> >>> +		error_warning = 1;
> >>> +		cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> >>> +	}
> >> What is the meaning of this error warning interrupt? It does *not*
> >> change the state.
> > 
> > This TWRNINT means that the Tx error counter has transitioned from < 96
> > to >= 96 (analog the RWRNINT for the Rx error counter). Unfortunately
> > the state bits handled below do not reflect this error warning state.
> > 
> > So this interrupt signals us that we have been in error warning state
> > once but we have no possibility to detect if we are still in error
> > warning state. I don't know how to handle this correctly. I could
> > disable this interrupt and ignore it completely.
> > 
> >>> +	switch ((stat >> 4) & 0x3) {
> >>> +	case 0:
> >>> +		state = CAN_STATE_ERROR_ACTIVE;
> >>> +		break;
> >>> +	case 1:
> >>> +		state = CAN_STATE_ERROR_PASSIVE;
> >>> +		break;
> >>> +	default:
> >>> +		state = CAN_STATE_BUS_OFF;
> >>> +		break;
> >>> +	}
> >> I'm still not happy with the error message generation. If a state change
> >> to error passive happens, it should be signaled to the user. Here my ideas:
> >>
> >> 	if (state != priv->can.state) {
> >> 		if (state == CAN_STATE_ERROR_WARNING) {
> >> 			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> >> 			priv->can.can_stats.error_warning++;
> >> 		} else if (state == CAN_STATE_ERROR_PASSIVE) {
> >> 			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> >> 			priv->can.can_stats.error_passive++;
> >> 		}
> >>
> >> It might have missed something, though.
> > 
> > Sorry, I missed that from your previous mail. As we can't properly
> > detect ERROR_WARNING state this would reduce to:
> > 
> > 	if (state != priv->can.state && state == CAN_STATE_ERROR_WARNING) {
> > 		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> > 		priv->can.can_stats.error_warning++;
> > 	}
> 
> Why you can't detect error warning? When the [RT]WRNINT comes, the error
> waning state has entered. By looking to the manual, the state changes to
> error warning and bus-off are directly signaled via interrupt. A change
> to error passive might be visible when bus error ints occurs. You could
> add some dev_dbg's to the "if (stat & ERRSTAT_[RT]WRNINT)" and "switch"
> blocks above and check with sending a message with no cable connected.

Read what I've written above. We get an interrupt on entering
error warning, but the bits reflecting the actual state only give error
active, error passive or bus off, but *not* error warning.

We could read the error counter and do something like

	switch ((stat >> 4) & 0x3) {
	case 0:
		state = CAN_STATE_ERROR_ACTIVE;
		break;
	case 1:
		state = CAN_STATE_ERROR_PASSIVE;
		break;
	default:
		state = CAN_STATE_BUS_OFF;
		break;
	}

	errcnt = readl(&regs->errcnt);
	rxerr = (errcnt >> 8) & 0xff;
	txerr = errcnt & 0xff;
	if ((rxerr >= 96 || txerr >= 96) && state == CAN_STATE_ERROR_ACTIVE)
		state = CAN_STATE_ERROR_WARNING;

Sascha
Wolfgang Grandegger July 30, 2009, 12:38 p.m. UTC | #6
Sascha Hauer wrote:
> On Thu, Jul 30, 2009 at 10:51:26AM +0200, Wolfgang Grandegger wrote:
>> Sascha Hauer wrote:
>>> On Wed, Jul 29, 2009 at 11:09:14AM +0200, Wolfgang Grandegger wrote:
>>>> Sascha Hauer wrote:
>>>>> And another go...
>>>>>
>>>>> Sascha
>>>>>
>>>>>
>>>>> >From 9f6f6fb67d57d082e056dec8121f1423c1b4fa0b Mon Sep 17 00:00:00 2001
>>>>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>>>> Date: Tue, 21 Jul 2009 10:47:19 +0200
>>>>> Subject: [PATCH] CAN: Add Flexcan CAN driver
>>>>>
>>>>> This core is found on some Freescale SoCs and also some Coldfire
>>>>> SoCs. Support for Coldfire is missing though at the moment as
>>>>> they have an older revision of the core which does not have RX FIFO
>>>>> support.
>>>>>
>>>>> V3: integrated comments from Oliver Hartkopp
>>>>> V2: integrated comments from Wolfgang Grandegger
>>>>>
>>>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>>>> ---
>>>>>  drivers/net/can/Kconfig   |    6 +
>>>>>  drivers/net/can/Makefile  |    1 +
>>>>>  drivers/net/can/flexcan.c |  719 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 726 insertions(+), 0 deletions(-)
>>>>>  create mode 100644 drivers/net/can/flexcan.c
>>>>>
>>>>> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
>>>>> index 33821a8..99c6da4 100644
>>>>> --- a/drivers/net/can/Kconfig
>>>>> +++ b/drivers/net/can/Kconfig
>>>>> @@ -74,6 +74,12 @@ config CAN_KVASER_PCI
>>>>>  	  This driver is for the the PCIcanx and PCIcan cards (1, 2 or
>>>>>  	  4 channel) from Kvaser (http://www.kvaser.com).
>>>>>  
>>>>> +config CAN_FLEXCAN
>>>>> +	tristate "Support for Freescale FLEXCAN based chips"
>>>>> +	depends on CAN_DEV
>>>>> +	---help---
>>>>> +	  Say Y here if you want to support for Freescale FlexCAN.
>>>>> +
>>>>>  config CAN_DEBUG_DEVICES
>>>>>  	bool "CAN devices debugging messages"
>>>>>  	depends on CAN
>>>>> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
>>>>> index 523a941..25f2032 100644
>>>>> --- a/drivers/net/can/Makefile
>>>>> +++ b/drivers/net/can/Makefile
>>>>> @@ -8,5 +8,6 @@ obj-$(CONFIG_CAN_DEV)		+= can-dev.o
>>>>>  can-dev-y			:= dev.o
>>>>>  
>>>>>  obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
>>>>> +obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>>>>>  
>>>>>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
>>>>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>>>> new file mode 100644
>>>>> index 0000000..77661b3
>>>>> --- /dev/null
>>>>> +++ b/drivers/net/can/flexcan.c
>>>> [...]
>>>>> +static void flexcan_error(struct net_device *ndev, u32 stat)
>>>>> +{
>>>>> +	struct can_frame *cf;
>>>>> +	struct sk_buff *skb;
>>>>> +	struct flexcan_priv *priv = netdev_priv(ndev);
>>>>> +	struct net_device_stats *stats = &ndev->stats;
>>>>> +	enum can_state state = priv->can.state;
>>>>> +	int error_warning = 0, rx_errors = 0, tx_errors = 0;
>>>>> +
>>>>> +	skb = dev_alloc_skb(sizeof(struct can_frame));
>>>>> +	if (!skb)
>>>>> +		return;
>>>>> +
>>>>> +	skb->dev = ndev;
>>>>> +	skb->protocol = __constant_htons(ETH_P_CAN);
>>>>> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
>>>>> +
>>>>> +	cf = (struct can_frame *)skb_put(skb, sizeof(*cf));
>>>>> +	memset(cf, 0, sizeof(*cf));
>>>>> +
>>>>> +	cf->can_id = CAN_ERR_FLAG;
>>>>> +	cf->can_dlc = CAN_ERR_DLC;
>>>>> +
>>>>> +	if (stat & ERRSTAT_RWRNINT) {
>>>>> +		error_warning = 1;
>>>>> +		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>>>>> +	}
>>>>> +
>>>>> +	if (stat & ERRSTAT_TWRNINT) {
>>>>> +		error_warning = 1;
>>>>> +		cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
>>>>> +	}
>>>> What is the meaning of this error warning interrupt? It does *not*
>>>> change the state.
>>> This TWRNINT means that the Tx error counter has transitioned from < 96
>>> to >= 96 (analog the RWRNINT for the Rx error counter). Unfortunately
>>> the state bits handled below do not reflect this error warning state.
>>>
>>> So this interrupt signals us that we have been in error warning state
>>> once but we have no possibility to detect if we are still in error
>>> warning state. I don't know how to handle this correctly. I could
>>> disable this interrupt and ignore it completely.
>>>
>>>>> +	switch ((stat >> 4) & 0x3) {
>>>>> +	case 0:
>>>>> +		state = CAN_STATE_ERROR_ACTIVE;
>>>>> +		break;
>>>>> +	case 1:
>>>>> +		state = CAN_STATE_ERROR_PASSIVE;
>>>>> +		break;
>>>>> +	default:
>>>>> +		state = CAN_STATE_BUS_OFF;
>>>>> +		break;
>>>>> +	}
>>>> I'm still not happy with the error message generation. If a state change
>>>> to error passive happens, it should be signaled to the user. Here my ideas:
>>>>
>>>> 	if (state != priv->can.state) {
>>>> 		if (state == CAN_STATE_ERROR_WARNING) {
>>>> 			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>>>> 			priv->can.can_stats.error_warning++;
>>>> 		} else if (state == CAN_STATE_ERROR_PASSIVE) {
>>>> 			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
>>>> 			priv->can.can_stats.error_passive++;
>>>> 		}
>>>>
>>>> It might have missed something, though.
>>> Sorry, I missed that from your previous mail. As we can't properly
>>> detect ERROR_WARNING state this would reduce to:
>>>
>>> 	if (state != priv->can.state && state == CAN_STATE_ERROR_WARNING) {
>>> 		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>>> 		priv->can.can_stats.error_warning++;
>>> 	}
>> Why you can't detect error warning? When the [RT]WRNINT comes, the error
>> waning state has entered. By looking to the manual, the state changes to
>> error warning and bus-off are directly signaled via interrupt. A change
>> to error passive might be visible when bus error ints occurs. You could
>> add some dev_dbg's to the "if (stat & ERRSTAT_[RT]WRNINT)" and "switch"
>> blocks above and check with sending a message with no cable connected.
> 
> Read what I've written above. We get an interrupt on entering
> error warning, but the bits reflecting the actual state only give error
> active, error passive or bus off, but *not* error warning.

You could check the error counters.to verify the real state.

> We could read the error counter and do something like
> 
> 	switch ((stat >> 4) & 0x3) {
> 	case 0:
> 		state = CAN_STATE_ERROR_ACTIVE;
> 		break;
> 	case 1:
> 		state = CAN_STATE_ERROR_PASSIVE;
> 		break;
> 	default:
> 		state = CAN_STATE_BUS_OFF;
> 		break;
> 	}
> 
> 	errcnt = readl(&regs->errcnt);
> 	rxerr = (errcnt >> 8) & 0xff;
> 	txerr = errcnt & 0xff;
> 	if ((rxerr >= 96 || txerr >= 96) && state == CAN_STATE_ERROR_ACTIVE)
> 		state = CAN_STATE_ERROR_WARNING;

Yes, or do not handle the state in the "case 0" above appropriately. The
CAN spec only specifies the bus error states error active, error passive
and bus off. The warning is somehow optional but available on most CAN
controllers. Nevertheless, we should create a message if the ISR
realizes a state changes.

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
Wolfgang Grandegger July 30, 2009, 8:22 p.m. UTC | #7
Wolfgang Grandegger wrote:
> Sascha Hauer wrote:
[...]
>> 	errcnt = readl(&regs->errcnt);
>> 	rxerr = (errcnt >> 8) & 0xff;
>> 	txerr = errcnt & 0xff;
>> 	if ((rxerr >= 96 || txerr >= 96) && state == CAN_STATE_ERROR_ACTIVE)
>> 		state = CAN_STATE_ERROR_WARNING;
> 
> Yes, or do not handle the state in the "case 0" above appropriately. The
> CAN spec only specifies the bus error states error active, error passive
> and bus off. The warning is somehow optional but available on most CAN
> controllers. Nevertheless, we should create a message if the ISR
> realizes a state changes.

FYI, I will be on holiday for the next 1.5 weeks.

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
Sascha Hauer July 31, 2009, 11:20 a.m. UTC | #8
On Thu, Jul 30, 2009 at 10:22:11PM +0200, Wolfgang Grandegger wrote:
> Wolfgang Grandegger wrote:
> > Sascha Hauer wrote:
> [...]
> >> 	errcnt = readl(&regs->errcnt);
> >> 	rxerr = (errcnt >> 8) & 0xff;
> >> 	txerr = errcnt & 0xff;
> >> 	if ((rxerr >= 96 || txerr >= 96) && state == CAN_STATE_ERROR_ACTIVE)
> >> 		state = CAN_STATE_ERROR_WARNING;
> > 
> > Yes, or do not handle the state in the "case 0" above appropriately. The
> > CAN spec only specifies the bus error states error active, error passive
> > and bus off. The warning is somehow optional but available on most CAN
> > controllers. Nevertheless, we should create a message if the ISR
> > realizes a state changes.
> 
> FYI, I will be on holiday for the next 1.5 weeks.

Lucky you ;)

I just did some tests with the CAN cable disconnected. It turns out that
I get flooded by interrupts caused by ACK errors. The system is
completely unresponsive then. Unfortunately the ACK error interrupt
can't be disabled seperately, so we have to disable error interrupts
completely once we get an ACK error. I'm thinking about setting up a
timer and poll the error status register then.

Sascha
Wolfgang Grandegger Aug. 2, 2009, 7:22 p.m. UTC | #9
Sascha Hauer wrote:
> On Thu, Jul 30, 2009 at 10:22:11PM +0200, Wolfgang Grandegger wrote:
>> Wolfgang Grandegger wrote:
>>> Sascha Hauer wrote:
>> [...]
>>>> 	errcnt = readl(&regs->errcnt);
>>>> 	rxerr = (errcnt >> 8) & 0xff;
>>>> 	txerr = errcnt & 0xff;
>>>> 	if ((rxerr >= 96 || txerr >= 96) && state == CAN_STATE_ERROR_ACTIVE)
>>>> 		state = CAN_STATE_ERROR_WARNING;
>>> Yes, or do not handle the state in the "case 0" above appropriately. The
>>> CAN spec only specifies the bus error states error active, error passive
>>> and bus off. The warning is somehow optional but available on most CAN
>>> controllers. Nevertheless, we should create a message if the ISR
>>> realizes a state changes.
>> FYI, I will be on holiday for the next 1.5 weeks.
> 
> Lucky you ;)

A last answer before I go really offline...

> I just did some tests with the CAN cable disconnected. It turns out that
> I get flooded by interrupts caused by ACK errors. The system is
> completely unresponsive then. Unfortunately the ACK error interrupt
> can't be disabled seperately, so we have to disable error interrupts
> completely once we get an ACK error. I'm thinking about setting up a
> timer and poll the error status register then.

I suspect that you are using 1MB/s. That's the infamous bus error
flooding we realize with the SJA1000 on low-end systems as well. The
right solution is using the NAPI (RX polling) interface.

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

Patch

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 33821a8..99c6da4 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -74,6 +74,12 @@  config CAN_KVASER_PCI
 	  This driver is for the the PCIcanx and PCIcan cards (1, 2 or
 	  4 channel) from Kvaser (http://www.kvaser.com).
 
+config CAN_FLEXCAN
+	tristate "Support for Freescale FLEXCAN based chips"
+	depends on CAN_DEV
+	---help---
+	  Say Y here if you want to support for Freescale FlexCAN.
+
 config CAN_DEBUG_DEVICES
 	bool "CAN devices debugging messages"
 	depends on CAN
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 523a941..25f2032 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -8,5 +8,6 @@  obj-$(CONFIG_CAN_DEV)		+= can-dev.o
 can-dev-y			:= dev.o
 
 obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
+obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
new file mode 100644
index 0000000..77661b3
--- /dev/null
+++ b/drivers/net/can/flexcan.c
@@ -0,0 +1,719 @@ 
+/*
+ * flexcan.c - FLEXCAN CAN controller driver
+ *
+ * Copyright (c) 2005-2006 Varma Electronics Oy
+ * Copyright (c) 2009 Sascha Hauer, Pengutronix
+ *
+ * Based on code originally by Andrey Volkov <avolkov@varma-el.com>
+ *
+ * LICENCE:
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/can/netlink.h>
+#include <linux/can/error.h>
+#include <linux/interrupt.h>
+#include <linux/netdevice.h>
+#include <linux/if_ether.h>
+#include <linux/can/dev.h>
+#include <linux/if_arp.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/list.h>
+#include <linux/can.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME "flexcan"
+
+/* FLEXCAN module configuration register (CANMCR) bits */
+#define CANMCR_MDIS				(1 << 31)
+#define CANMCR_FRZ				(1 << 30)
+#define CANMCR_FEN				(1 << 29)
+#define CANMCR_HALT				(1 << 28)
+#define CANMCR_SOFTRST				(1 << 25)
+#define CANMCR_FRZACK				(1 << 24)
+#define CANMCR_SUPV				(1 << 23)
+#define CANMCR_SRX_DIS				(1 << 17)
+#define CANMCR_MAXMB(x)				((x) & 0x0f)
+#define CANMCR_IDAM_A				(0 << 8)
+#define CANMCR_IDAM_B				(1 << 8)
+#define CANMCR_IDAM_C				(2 << 8)
+
+/* FLEXCAN control register (CANCTRL) bits */
+#define CANCTRL_PRESDIV(x)			(((x) & 0xff) << 24)
+#define CANCTRL_RJW(x)				(((x) & 0x03) << 22)
+#define CANCTRL_PSEG1(x)			(((x) & 0x07) << 19)
+#define CANCTRL_PSEG2(x)			(((x) & 0x07) << 16)
+#define CANCTRL_BOFFMSK				(1 << 15)
+#define CANCTRL_ERRMSK				(1 << 14)
+#define CANCTRL_CLKSRC				(1 << 13)
+#define CANCTRL_LPB				(1 << 12)
+#define CANCTRL_TWRN_MSK			(1 << 11)
+#define CANCTRL_RWRN_MSK			(1 << 10)
+#define CANCTRL_SAMP				(1 << 7)
+#define CANCTRL_BOFFREC				(1 << 6)
+#define CANCTRL_TSYNC				(1 << 5)
+#define CANCTRL_LBUF				(1 << 4)
+#define CANCTRL_LOM				(1 << 3)
+#define CANCTRL_PROPSEG(x)			((x) & 0x07)
+
+/* FLEXCAN error counter register (ERRCNT) bits */
+#define ERRCNT_REXECTR(x)			(((x) & 0xff) << 8)
+#define ERRCNT_TXECTR(x)			((x) & 0xff)
+
+/* FLEXCAN error and status register (ERRSTAT) bits */
+#define ERRSTAT_TWRNINT				(1 << 17)
+#define ERRSTAT_RWRNINT				(1 << 16)
+#define ERRSTAT_BIT1ERR				(1 << 15)
+#define ERRSTAT_BIT0ERR				(1 << 14)
+#define ERRSTAT_ACKERR				(1 << 13)
+#define ERRSTAT_CRCERR				(1 << 12)
+#define ERRSTAT_FRMERR				(1 << 11)
+#define ERRSTAT_STFERR				(1 << 10)
+#define ERRSTAT_TXWRN				(1 << 9)
+#define ERRSTAT_RXWRN				(1 << 8)
+#define ERRSTAT_IDLE 				(1 << 7)
+#define ERRSTAT_TXRX				(1 << 6)
+#define ERRSTAT_FLTCONF_MASK			(3 << 4)
+#define ERRSTAT_FLTCONF_ERROR_ACTIVE		(0 << 4)
+#define ERRSTAT_FLTCONF_ERROR_PASSIVE		(1 << 4)
+#define ERRSTAT_FLTCONF_ERROR_BUS_OFF		(2 << 4)
+#define ERRSTAT_BOFFINT				(1 << 2)
+#define ERRSTAT_ERRINT          		(1 << 1)
+#define ERRSTAT_WAKINT				(1 << 0)
+#define ERRSTAT_INT	(ERRSTAT_BOFFINT | ERRSTAT_ERRINT | ERRSTAT_TWRNINT | \
+		ERRSTAT_RWRNINT)
+
+/* FLEXCAN interrupt flag register (IFLAG) bits */
+#define IFLAG_BUF(x)				(1 << (x))
+#define IFLAG_RX_FIFO_OVERFLOW			(1 << 7)
+#define IFLAG_RX_FIFO_WARN			(1 << 6)
+#define IFLAG_RX_FIFO_AVAILABLE			(1 << 5)
+
+/* FLEXCAN message buffers */
+#define MB_CNT_CODE(x)				(((x) & 0xf) << 24)
+#define MB_CNT_SRR				(1 << 22)
+#define MB_CNT_IDE				(1 << 21)
+#define MB_CNT_RTR				(1 << 20)
+#define MB_CNT_LENGTH(x)			(((x) & 0xf) << 16)
+#define MB_CNT_TIMESTAMP(x)			((x) & 0xffff)
+
+#define MB_ID_STD				(0x7ff << 18)
+#define MB_ID_EXT				0x1fffffff
+#define MB_CODE_MASK				0xf0ffffff
+
+/* Structure of the message buffer */
+struct flexcan_mb {
+	u32	can_ctrl;
+	u32	can_id;
+	u32	data[2];
+};
+
+/* Structure of the hardware registers */
+struct flexcan_regs {
+	u32	canmcr;		/* 0x00 */
+	u32	canctrl;	/* 0x04 */
+	u32	timer;		/* 0x08 */
+	u32	reserved1;	/* 0x0c */
+	u32 	rxgmask;	/* 0x10 */
+	u32 	rx14mask;	/* 0x14 */
+	u32 	rx15mask;	/* 0x18 */
+	u32	errcnt;		/* 0x1c */
+	u32 	errstat;	/* 0x20 */
+	u32	imask2;		/* 0x24 */
+	u32	imask1;		/* 0x28 */
+	u32	iflag2;		/* 0x2c */
+	u32 	iflag1;		/* 0x30 */
+	u32	reserved4[19];
+	struct	flexcan_mb cantxfg[64];
+};
+
+struct flexcan_priv {
+	struct can_priv can;
+	void __iomem *base;
+
+	struct net_device *dev;
+	struct clk *clk;
+};
+
+static struct can_bittiming_const flexcan_bittiming_const = {
+	.name = DRIVER_NAME,
+	.tseg1_min = 1,
+	.tseg1_max = 16,
+	.tseg2_min = 2,
+	.tseg2_max = 8,
+	.sjw_max = 4,
+	.brp_min = 1,
+	.brp_max = 256,
+	.brp_inc = 1,
+};
+
+#define TX_BUF_ID	8
+
+static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct can_frame *frame = (struct can_frame *)skb->data;
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 can_id;
+	u32 ctrl = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
+
+	netif_stop_queue(dev);
+
+	if (frame->can_id & CAN_EFF_FLAG) {
+		can_id = frame->can_id & CAN_EFF_MASK;
+		ctrl |= MB_CNT_IDE | MB_CNT_SRR;
+	} else {
+		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
+	}
+
+	if (frame->can_id & CAN_RTR_FLAG)
+		ctrl |= MB_CNT_RTR;
+
+	if (frame->can_dlc > 0) {
+		u32 data;
+		data = frame->data[0] << 24;
+		data |= frame->data[1] << 16;
+		data |= frame->data[2] << 8;
+		data |= frame->data[3];
+		writel(data, &regs->cantxfg[TX_BUF_ID].data[0]);
+	}
+	if (frame->can_dlc > 3) {
+		u32 data;
+		data = frame->data[4] << 24;
+		data |= frame->data[5] << 16;
+		data |= frame->data[6] << 8;
+		data |= frame->data[7];
+		writel(data, &regs->cantxfg[TX_BUF_ID].data[1]);
+	}
+
+	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
+	writel(ctrl, &regs->cantxfg[TX_BUF_ID].can_ctrl);
+
+	kfree_skb(skb);
+
+	return NETDEV_TX_OK;
+}
+
+static void flexcan_rx_frame(struct net_device *ndev,
+		struct flexcan_mb __iomem *mb)
+{
+	struct net_device_stats *stats = &ndev->stats;
+	struct sk_buff *skb;
+	struct can_frame *frame;
+	int ctrl, length;
+	u32 id;
+
+	ctrl = readl(&mb->can_ctrl);
+	length = (ctrl >> 16) & 0x0f;
+	if (length > 8) {
+		stats->rx_dropped++;
+		return;
+	}
+
+	skb = dev_alloc_skb(sizeof(struct can_frame));
+	if (!skb) {
+		stats->rx_dropped++;
+		return;
+	}
+
+	frame = (struct can_frame *)skb_put(skb,
+			sizeof(struct can_frame));
+
+	frame->can_dlc = length;
+	id = readl(&mb->can_id);
+
+	if (ctrl & MB_CNT_IDE) {
+		frame->can_id = id & CAN_EFF_MASK;
+		frame->can_id |= CAN_EFF_FLAG;
+	} else {
+		frame->can_id  = (id >> 18) & CAN_SFF_MASK;
+	}
+
+	if (ctrl & MB_CNT_RTR)
+		frame->can_id |= CAN_RTR_FLAG;
+
+	if (length > 0) {
+		u32 data = readl(&mb->data[0]);
+		frame->data[0] = (data >> 24) & 0xff;
+		frame->data[1] = (data >> 16) & 0xff;
+		frame->data[2] = (data >> 8) & 0xff;
+		frame->data[3] = data & 0xff;
+	}
+	if (length > 3) {
+		u32 data = readl(&mb->data[1]);
+		frame->data[4] = (data >> 24) & 0xff;
+		frame->data[5] = (data >> 16) & 0xff;
+		frame->data[6] = (data >> 8) & 0xff;
+		frame->data[7] = data & 0xff;
+	}
+
+	stats->rx_packets++;
+	stats->rx_bytes += frame->can_dlc;
+	skb->dev = ndev;
+	skb->protocol = __constant_htons(ETH_P_CAN);
+	skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	netif_rx(skb);
+}
+
+static void flexcan_error(struct net_device *ndev, u32 stat)
+{
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct net_device_stats *stats = &ndev->stats;
+	enum can_state state = priv->can.state;
+	int error_warning = 0, rx_errors = 0, tx_errors = 0;
+
+	skb = dev_alloc_skb(sizeof(struct can_frame));
+	if (!skb)
+		return;
+
+	skb->dev = ndev;
+	skb->protocol = __constant_htons(ETH_P_CAN);
+	skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	cf = (struct can_frame *)skb_put(skb, sizeof(*cf));
+	memset(cf, 0, sizeof(*cf));
+
+	cf->can_id = CAN_ERR_FLAG;
+	cf->can_dlc = CAN_ERR_DLC;
+
+	if (stat & ERRSTAT_RWRNINT) {
+		error_warning = 1;
+		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
+	}
+
+	if (stat & ERRSTAT_TWRNINT) {
+		error_warning = 1;
+		cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
+	}
+
+	switch ((stat >> 4) & 0x3) {
+	case 0:
+		state = CAN_STATE_ERROR_ACTIVE;
+		break;
+	case 1:
+		state = CAN_STATE_ERROR_PASSIVE;
+		break;
+	default:
+		state = CAN_STATE_BUS_OFF;
+		break;
+	}
+
+	if (stat & ERRSTAT_BOFFINT) {
+		cf->can_id |= CAN_ERR_BUSOFF;
+		can_bus_off(ndev);
+	}
+
+	if (stat & ERRSTAT_BIT1ERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_BIT1;
+	}
+
+	if (stat & ERRSTAT_BIT0ERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_BIT0;
+	}
+
+	if (stat & ERRSTAT_FRMERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_FORM;
+	}
+
+	if (stat & ERRSTAT_STFERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_STUFF;
+	}
+
+
+	if (stat & ERRSTAT_ACKERR) {
+		tx_errors = 1;
+		cf->can_id |= CAN_ERR_ACK;
+	}
+
+	if (error_warning)
+		priv->can.can_stats.error_warning++;
+	if (rx_errors)
+		stats->rx_errors++;
+	if (tx_errors)
+		stats->tx_errors++;
+
+	priv->can.state = state;
+
+	netif_rx(skb);
+
+	ndev->last_rx = jiffies;
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+}
+
+static irqreturn_t flexcan_isr(int irq, void *dev_id)
+{
+	struct net_device *ndev = dev_id;
+	struct net_device_stats *stats = &ndev->stats;
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 iflags, errstat;
+
+	errstat = readl(&regs->errstat);
+	if (errstat & ERRSTAT_INT) {
+		flexcan_error(ndev, errstat);
+		writel(errstat & ERRSTAT_INT, &regs->errstat);
+	}
+
+	iflags = readl(&regs->iflag1);
+
+	if (iflags & IFLAG_RX_FIFO_OVERFLOW) {
+		writel(IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
+		stats->rx_over_errors++;
+		stats->rx_errors++;
+	}
+
+	while (iflags & IFLAG_RX_FIFO_AVAILABLE) {
+		struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
+
+		flexcan_rx_frame(ndev, mb);
+		writel(IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+		readl(&regs->timer);
+		iflags = readl(&regs->iflag1);
+	}
+
+	if (iflags & (1 << TX_BUF_ID)) {
+		stats->tx_packets++;
+		writel((1 << TX_BUF_ID), &regs->iflag1);
+		netif_wake_queue(ndev);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int flexcan_set_bittiming(struct net_device *ndev)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct can_bittiming *bt = &priv->can.bittiming;
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->canctrl);
+	reg &= ~(CANCTRL_SAMP | CANCTRL_PRESDIV(0xff) |
+			CANCTRL_PSEG1(7) | CANCTRL_PSEG2(7));
+	reg |= CANCTRL_PRESDIV(bt->brp - 1) |
+		CANCTRL_PSEG1(bt->phase_seg1 - 1) |
+		CANCTRL_PSEG2(bt->phase_seg2 - 1) |
+		CANCTRL_RJW(3) |
+		CANCTRL_PROPSEG(bt->prop_seg - 1);
+	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+		reg |= CANCTRL_SAMP;
+	writel(reg, &regs->canctrl);
+
+	dev_dbg(&ndev->dev, "setting canctrl=0x%08x\n", reg);
+
+	return 0;
+}
+
+static int flexcan_open(struct net_device *ndev)
+{
+	int ret, i;
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	ret = open_candev(ndev);
+	if (ret)
+		return ret;
+
+	ret = request_irq(ndev->irq, flexcan_isr, 0, DRIVER_NAME, ndev);
+	if (ret)
+		goto err_irq;
+
+	clk_enable(priv->clk);
+
+	reg = CANMCR_SOFTRST | CANMCR_HALT;
+	writel(CANMCR_SOFTRST, &regs->canmcr);
+	udelay(10);
+
+	if (readl(&regs->canmcr) & CANMCR_SOFTRST) {
+		dev_err(&ndev->dev, "Failed to softreset can module.\n");
+		ret = -ENODEV;
+		goto err_reset;
+	}
+
+	/* Enable error and bus off interrupt */
+	reg = readl(&regs->canctrl);
+	reg |= CANCTRL_CLKSRC | CANCTRL_ERRMSK | CANCTRL_BOFFMSK |
+		CANCTRL_BOFFREC | CANCTRL_TWRN_MSK | CANCTRL_TWRN_MSK;
+	writel(reg, &regs->canctrl);
+
+	/* Set lowest buffer transmitted first */
+	reg |= CANCTRL_LBUF;
+	writel(reg, &regs->canctrl);
+
+	for (i = 0; i < 64; i++) {
+		writel(0, &regs->cantxfg[i].can_ctrl);
+		writel(0, &regs->cantxfg[i].can_id);
+		writel(0, &regs->cantxfg[i].data[0]);
+		writel(0, &regs->cantxfg[i].data[1]);
+
+		/* Put MB into rx queue */
+		writel(MB_CNT_CODE(0x04), &regs->cantxfg[i].can_ctrl);
+	}
+
+	/* acceptance mask/acceptance code (accept everything) */
+	writel(0x0, &regs->rxgmask);
+	writel(0x0, &regs->rx14mask);
+	writel(0x0, &regs->rx15mask);
+
+	/* Enable flexcan module */
+	reg = readl(&regs->canmcr);
+	reg &= ~(CANMCR_MDIS | CANMCR_FRZ);
+	reg |= CANMCR_IDAM_C | CANMCR_FEN;
+	writel(reg, &regs->canmcr);
+
+	/* Synchronize with the can bus */
+	reg &= ~CANMCR_HALT;
+	writel(reg, &regs->canmcr);
+
+	/* Enable interrupts */
+	writel(IFLAG_RX_FIFO_OVERFLOW | IFLAG_RX_FIFO_AVAILABLE |
+			IFLAG_BUF(TX_BUF_ID),
+			&regs->imask1);
+
+	netif_start_queue(ndev);
+
+	return 0;
+
+err_reset:
+	free_irq(ndev->irq, ndev);
+err_irq:
+	close_candev(ndev);
+
+	return ret;
+}
+
+static int flexcan_close(struct net_device *ndev)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+
+	netif_stop_queue(ndev);
+
+	/* Disable all interrupts */
+	writel(0, &regs->imask1);
+	free_irq(ndev->irq, ndev);
+
+	close_candev(ndev);
+
+	/* Disable module */
+	writel(CANMCR_MDIS, &regs->canmcr);
+	clk_disable(priv->clk);
+	return 0;
+}
+
+static int flexcan_set_mode(struct net_device *ndev, enum can_mode mode)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	switch (mode) {
+	case CAN_MODE_START:
+		reg = readl(&regs->canctrl);
+		reg &= ~CANCTRL_BOFFREC;
+		writel(reg, &regs->canctrl);
+		reg |= CANCTRL_BOFFREC;
+		writel(reg, &regs->canctrl);
+		priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+		if (netif_queue_stopped(ndev))
+			netif_wake_queue(ndev);
+		break;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static const struct net_device_ops flexcan_netdev_ops = {
+       .ndo_open		= flexcan_open,
+       .ndo_stop		= flexcan_close,
+       .ndo_start_xmit		= flexcan_start_xmit,
+};
+
+static int register_flexcandev(struct net_device *ndev)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->canmcr);
+	reg &= ~CANMCR_MDIS;
+	writel(reg, &regs->canmcr);
+	udelay(100);
+	reg |= CANMCR_FRZ | CANMCR_HALT | CANMCR_FEN;
+	writel(reg, &regs->canmcr);
+
+	reg = readl(&regs->canmcr);
+
+	/* Currently we only support newer versions of this core featuring
+	 * a RX FIFO. Older cores found on some Coldfire derivates are not
+	 * yet supported.
+	 */
+	if (!(reg & CANMCR_FEN)) {
+		dev_err(&ndev->dev, "Could not enable RX FIFO, unsupported "
+				"core");
+		return -ENODEV;
+	}
+
+	ndev->flags |= IFF_ECHO; /* we support local echo in hardware */
+	ndev->netdev_ops = &flexcan_netdev_ops;
+
+	return register_candev(ndev);
+}
+
+static void unregister_flexcandev(struct net_device *ndev)
+{
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->canmcr);
+	reg |= CANMCR_FRZ | CANMCR_HALT | CANMCR_MDIS;
+	writel(reg, &regs->canmcr);
+
+	unregister_candev(ndev);
+}
+
+static int __devinit flexcan_probe(struct platform_device *pdev)
+{
+	struct resource *mem;
+	struct net_device *ndev;
+	struct flexcan_priv *priv;
+	u32 mem_size;
+	int ret;
+
+	ndev = alloc_candev(sizeof(struct flexcan_priv));
+	if (!ndev)
+		return -ENOMEM;
+
+	priv = netdev_priv(ndev);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	ndev->irq = platform_get_irq(pdev, 0);
+	if (!mem || !ndev->irq) {
+		ret = -ENODEV;
+		goto failed_req;
+	}
+
+	mem_size = resource_size(mem);
+
+	if (!request_mem_region(mem->start, mem_size, DRIVER_NAME)) {
+		ret = -EBUSY;
+		goto failed_req;
+	}
+
+	SET_NETDEV_DEV(ndev, &pdev->dev);
+
+	priv->base = ioremap(mem->start, mem_size);
+	if (!priv->base) {
+		ret = -ENOMEM;
+		goto failed_map;
+	}
+
+	priv->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		ret = PTR_ERR(priv->clk);
+		goto failed_clock;
+	}
+	priv->can.clock.freq = clk_get_rate(priv->clk);
+
+	platform_set_drvdata(pdev, ndev);
+
+	priv->can.do_set_bittiming = flexcan_set_bittiming;
+	priv->can.bittiming_const = &flexcan_bittiming_const;
+	priv->can.do_set_mode = flexcan_set_mode;
+
+	ret = register_flexcandev(ndev);
+	if (ret)
+		goto failed_register;
+
+	return 0;
+
+failed_register:
+	clk_put(priv->clk);
+failed_clock:
+	iounmap(priv->base);
+failed_map:
+	release_mem_region(mem->start, mem_size);
+failed_req:
+	free_candev(ndev);
+
+	return ret;
+}
+
+static int __devexit flexcan_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct flexcan_priv *priv = netdev_priv(ndev);
+	struct resource *mem;
+
+	unregister_flexcandev(ndev);
+	platform_set_drvdata(pdev, NULL);
+	iounmap(priv->base);
+	clk_put(priv->clk);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(mem->start, resource_size(mem));
+	free_candev(ndev);
+
+	return 0;
+}
+
+static struct platform_driver flexcan_driver = {
+	.driver = {
+		   .name = DRIVER_NAME,
+		   },
+	.probe = flexcan_probe,
+	.remove = __devexit_p(flexcan_remove),
+};
+
+static int __init flexcan_init(void)
+{
+	return platform_driver_register(&flexcan_driver);
+}
+
+static void __exit flexcan_exit(void)
+{
+	platform_driver_unregister(&flexcan_driver);
+}
+
+module_init(flexcan_init);
+module_exit(flexcan_exit);
+
+MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("CAN port driver for flexcan based chip");