diff mbox

[V2] CAN: Add Flexcan CAN controller driver

Message ID 20090728120624.GS2714@pengutronix.de
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Sascha Hauer July 28, 2009, 12:06 p.m. UTC
Hi,

Here is the second version of the flexcan driver.

Sascha


From a1ea20bc862778f2b64efc18d21d05e5debb9562 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.

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 |  692 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 699 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/flexcan.c

Comments

Oliver Hartkopp July 28, 2009, 1:21 p.m. UTC | #1
Sascha Hauer wrote:
> Hi,
> 
> Here is the second version of the flexcan driver.

Hi Sascha,

some more points i forgot to mention, sorry ...


> +/* Structure of the message buffer */
> +struct flexcan_mb {
> +	u32	can_dlc;
> +	u32	can_id;
> +	u32	data[2];
> +};

This looks really hackish and does not reflect the structure of a flexcan
message buffer! The data is 'u8' and the name of 'dlc' for the
description/flag register is bad.


> +
> +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 dlc = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);

Naming this variable 'dlc' does not hit the point. See below.

> +	u32 *can_data;

Really this needs to be fixed up by defining a proper mailbox struct.


> +
> +	netif_stop_queue(dev);
> +
> +	if (frame->can_id & CAN_EFF_FLAG) {
> +		can_id = frame->can_id & MB_ID_EXT;

Please use CAN_EFF_MASK here.


> +		dlc |= MB_CNT_IDE | MB_CNT_SRR;
> +	} else {
> +		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
> +	}

Just nitpicking for Kernel coding style:
remove the last '{' and '}' pair.

> +
> +	if (frame->can_id & CAN_RTR_FLAG)
> +		dlc |= MB_CNT_RTR;
> +
> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
> +
> +	can_data = (u32 *)frame->data;
> +	writel(cpu_to_be32(*can_data), &regs->cantxfg[TX_BUF_ID].data[0]);
> +	writel(cpu_to_be32(*(can_data + 1)), &regs->cantxfg[TX_BUF_ID].data[1]);

IMHO it is not really transparent, that this is a correct handling to copy the
  can_frame.data[] on all architectures. I bet creating a for-statement
regarding the dlc is not slower and makes really clear, what's going on here.

> +
> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> +
> +	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 = readl(&mb->can_dlc);

'ctrl' is much better than 'dlc' naming above :-)

> +	int length = (ctrl >> 16) & 0x0f;

is probably not enough ... use %9 or an additional check.

> +	u32 id;
> +
> +	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) & MB_ID_EXT;

like above use CAN_EFF_MASK

or at least rename MB_ID_EXT to MB_ID_EXT_MASK

> +
> +	if (ctrl & MB_CNT_IDE) {
> +		frame->can_id = id & CAN_EFF_MASK;
> +		frame->can_id |= CAN_EFF_FLAG;
> +		if (ctrl & MB_CNT_RTR)
> +			frame->can_id |= CAN_RTR_FLAG;
> +	} else {
> +		frame->can_id  = id >> 18;
> +		if (ctrl & MB_CNT_RTR)
> +			frame->can_id |= CAN_RTR_FLAG;
> +	}
> +
> +	*(u32 *)frame->data = be32_to_cpu(readl(&mb->data[0]));
> +	*((u32 *)frame->data + 1) = be32_to_cpu(readl(&mb->data[1]));

Same as above.

Please write readable an maintainable code and let the compiler do his job.

Thanks,
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
Sascha Hauer July 28, 2009, 1:37 p.m. UTC | #2
Hi Oliver,

On Tue, Jul 28, 2009 at 03:21:40PM +0200, Oliver Hartkopp wrote:
> Sascha Hauer wrote:
> > Hi,
> > 
> > Here is the second version of the flexcan driver.
> 
> Hi Sascha,
> 
> some more points i forgot to mention, sorry ...
> 
> 
> > +/* Structure of the message buffer */
> > +struct flexcan_mb {
> > +	u32	can_dlc;
> > +	u32	can_id;
> > +	u32	data[2];
> > +};
> 
> This looks really hackish and does not reflect the structure of a flexcan
> message buffer! The data is 'u8' and the name of 'dlc' for the
> description/flag register is bad.
> 

see below..

> 
> > +
> > +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 dlc = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
> 
> Naming this variable 'dlc' does not hit the point. See below.
> 
> > +	u32 *can_data;
> 
> Really this needs to be fixed up by defining a proper mailbox struct.
> 
> 
> > +
> > +	netif_stop_queue(dev);
> > +
> > +	if (frame->can_id & CAN_EFF_FLAG) {
> > +		can_id = frame->can_id & MB_ID_EXT;
> 
> Please use CAN_EFF_MASK here.

I used MX_ID_EXT intentionally because it it flexcan specific and just
happens to be the same as CAN_EFF_MASK. I can change it if you like.

> 
> 
> > +		dlc |= MB_CNT_IDE | MB_CNT_SRR;
> > +	} else {
> > +		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
> > +	}
> 
> Just nitpicking for Kernel coding style:
> remove the last '{' and '}' pair.

No, Documentation/CondingStyle suggests that if one branch needs braces
the other branch should use them, too.

> 
> > +
> > +	if (frame->can_id & CAN_RTR_FLAG)
> > +		dlc |= MB_CNT_RTR;
> > +
> > +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> > +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
> > +
> > +	can_data = (u32 *)frame->data;
> > +	writel(cpu_to_be32(*can_data), &regs->cantxfg[TX_BUF_ID].data[0]);
> > +	writel(cpu_to_be32(*(can_data + 1)), &regs->cantxfg[TX_BUF_ID].data[1]);
> 
> IMHO it is not really transparent, that this is a correct handling to copy the
>   can_frame.data[] on all architectures. I bet creating a for-statement
> regarding the dlc is not slower and makes really clear, what's going on here.

This is indeed a problem here. The original Coldfire code I used as a
template used a loop around unsigned char * which did the wrong thing
for me. So yes, this is not generic here, but I have no idea how the
generic code looks like. As Coldfire is big endian this doesn't seem
that wrong.

> 
> > +
> > +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> > +
> > +	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 = readl(&mb->can_dlc);
> 
> 'ctrl' is much better than 'dlc' naming above :-)

ok, will change it above.

> 
> > +	int length = (ctrl >> 16) & 0x0f;
> 
> is probably not enough ... use %9 or an additional check.

ok, I'll add a sanity check.

> 
> > +	u32 id;
> > +
> > +	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) & MB_ID_EXT;
> 
> like above use CAN_EFF_MASK
> 
> or at least rename MB_ID_EXT to MB_ID_EXT_MASK
> 
> > +
> > +	if (ctrl & MB_CNT_IDE) {
> > +		frame->can_id = id & CAN_EFF_MASK;
> > +		frame->can_id |= CAN_EFF_FLAG;
> > +		if (ctrl & MB_CNT_RTR)
> > +			frame->can_id |= CAN_RTR_FLAG;
> > +	} else {
> > +		frame->can_id  = id >> 18;
> > +		if (ctrl & MB_CNT_RTR)
> > +			frame->can_id |= CAN_RTR_FLAG;
> > +	}
> > +
> > +	*(u32 *)frame->data = be32_to_cpu(readl(&mb->data[0]));
> > +	*((u32 *)frame->data + 1) = be32_to_cpu(readl(&mb->data[1]));
> 
> Same as above.
> 
> Please write readable an maintainable code and let the compiler do his job.
> 
> Thanks,
> Oliver
>
Oliver Hartkopp July 28, 2009, 1:50 p.m. UTC | #3
Sascha Hauer wrote:
> Hi Oliver,
> 
> On Tue, Jul 28, 2009 at 03:21:40PM +0200, Oliver Hartkopp wrote:
>> Sascha Hauer wrote:
>>> Hi,
>>>
>>> Here is the second version of the flexcan driver.
>> Hi Sascha,
>>
>> some more points i forgot to mention, sorry ...
>>
>>
>>> +/* Structure of the message buffer */
>>> +struct flexcan_mb {
>>> +	u32	can_dlc;
>>> +	u32	can_id;
>>> +	u32	data[2];
>>> +};
>> This looks really hackish and does not reflect the structure of a flexcan
>> message buffer! The data is 'u8' and the name of 'dlc' for the
>> description/flag register is bad.
>>
> 
> see below..

Especially can_dlc, can_id and data[] are known from struct can_frame which
really can confuse here ...

> 
>>> +
>>> +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 dlc = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
>> Naming this variable 'dlc' does not hit the point. See below.
>>
>>> +	u32 *can_data;
>> Really this needs to be fixed up by defining a proper mailbox struct.
>>
>>
>>> +
>>> +	netif_stop_queue(dev);
>>> +
>>> +	if (frame->can_id & CAN_EFF_FLAG) {
>>> +		can_id = frame->can_id & MB_ID_EXT;
>> Please use CAN_EFF_MASK here.
> 
> I used MX_ID_EXT intentionally because it it flexcan specific and just
> happens to be the same as CAN_EFF_MASK. I can change it if you like.

Yes, i've seen that. I would tend to use CAN_EFF_MASK here as you apply it on
frame->can_id.

When you get it from the controller MB_ID_EXT_MASK would be the better one.

> 
>>
>>> +		dlc |= MB_CNT_IDE | MB_CNT_SRR;
>>> +	} else {
>>> +		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
>>> +	}
>> Just nitpicking for Kernel coding style:
>> remove the last '{' and '}' pair.
> 
> No, Documentation/CondingStyle suggests that if one branch needs braces
> the other branch should use them, too.

Sorry. Didn't know that.

> 
>>> +
>>> +	if (frame->can_id & CAN_RTR_FLAG)
>>> +		dlc |= MB_CNT_RTR;
>>> +
>>> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
>>> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
>>> +
>>> +	can_data = (u32 *)frame->data;
>>> +	writel(cpu_to_be32(*can_data), &regs->cantxfg[TX_BUF_ID].data[0]);
>>> +	writel(cpu_to_be32(*(can_data + 1)), &regs->cantxfg[TX_BUF_ID].data[1]);
>> IMHO it is not really transparent, that this is a correct handling to copy the
>>   can_frame.data[] on all architectures. I bet creating a for-statement
>> regarding the dlc is not slower and makes really clear, what's going on here.
> 
> This is indeed a problem here. The original Coldfire code I used as a
> template used a loop around unsigned char * which did the wrong thing
> for me.

This could be a good starting point for an investigation ;-)

> So yes, this is not generic here, but I have no idea how the
> generic code looks like. As Coldfire is big endian this doesn't seem
> that wrong.

I would try to define a proper flexcan_mb struct like

struct flexcan_mb {
	u8 code;
	u8 ctrl;
	u16 timestamp;
	u32 id;
	u8 data[8];
}

And then see how it looks like ;-)

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
Oliver Hartkopp July 28, 2009, 2:12 p.m. UTC | #4
Oliver Hartkopp wrote:
> Sascha Hauer wrote:

>>>> +
>>>> +	if (frame->can_id & CAN_RTR_FLAG)
>>>> +		dlc |= MB_CNT_RTR;
>>>> +
>>>> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
>>>> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);

Are you sure, that this is correct?

Indeed i wonder, if it would make sense to skip the entire struct flexcan_mb
approach and fiddle byte-by-byte inside the registers ...

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
Sascha Hauer July 28, 2009, 2:18 p.m. UTC | #5
On Tue, Jul 28, 2009 at 03:50:48PM +0200, Oliver Hartkopp wrote:
> Sascha Hauer wrote:
> > Hi Oliver,
> > 
> > On Tue, Jul 28, 2009 at 03:21:40PM +0200, Oliver Hartkopp wrote:
> >> Sascha Hauer wrote:
> >>> Hi,
> >>>
> >>> Here is the second version of the flexcan driver.
> >> Hi Sascha,
> >>
> >> some more points i forgot to mention, sorry ...
> >>
> >>
> >>> +/* Structure of the message buffer */
> >>> +struct flexcan_mb {
> >>> +	u32	can_dlc;
> >>> +	u32	can_id;
> >>> +	u32	data[2];
> >>> +};
> >> This looks really hackish and does not reflect the structure of a flexcan
> >> message buffer! The data is 'u8' and the name of 'dlc' for the
> >> description/flag register is bad.
> >>
> > 
> > see below..
> 
> Especially can_dlc, can_id and data[] are known from struct can_frame which
> really can confuse here ...
> 
> > 
> >>> +
> >>> +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 dlc = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
> >> Naming this variable 'dlc' does not hit the point. See below.
> >>
> >>> +	u32 *can_data;
> >> Really this needs to be fixed up by defining a proper mailbox struct.
> >>
> >>
> >>> +
> >>> +	netif_stop_queue(dev);
> >>> +
> >>> +	if (frame->can_id & CAN_EFF_FLAG) {
> >>> +		can_id = frame->can_id & MB_ID_EXT;
> >> Please use CAN_EFF_MASK here.
> > 
> > I used MX_ID_EXT intentionally because it it flexcan specific and just
> > happens to be the same as CAN_EFF_MASK. I can change it if you like.
> 
> Yes, i've seen that. I would tend to use CAN_EFF_MASK here as you apply it on
> frame->can_id.
> 
> When you get it from the controller MB_ID_EXT_MASK would be the better one.
> 
> > 
> >>
> >>> +		dlc |= MB_CNT_IDE | MB_CNT_SRR;
> >>> +	} else {
> >>> +		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
> >>> +	}
> >> Just nitpicking for Kernel coding style:
> >> remove the last '{' and '}' pair.
> > 
> > No, Documentation/CondingStyle suggests that if one branch needs braces
> > the other branch should use them, too.
> 
> Sorry. Didn't know that.
> 
> > 
> >>> +
> >>> +	if (frame->can_id & CAN_RTR_FLAG)
> >>> +		dlc |= MB_CNT_RTR;
> >>> +
> >>> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> >>> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
> >>> +
> >>> +	can_data = (u32 *)frame->data;
> >>> +	writel(cpu_to_be32(*can_data), &regs->cantxfg[TX_BUF_ID].data[0]);
> >>> +	writel(cpu_to_be32(*(can_data + 1)), &regs->cantxfg[TX_BUF_ID].data[1]);
> >> IMHO it is not really transparent, that this is a correct handling to copy the
> >>   can_frame.data[] on all architectures. I bet creating a for-statement
> >> regarding the dlc is not slower and makes really clear, what's going on here.
> > 
> > This is indeed a problem here. The original Coldfire code I used as a
> > template used a loop around unsigned char * which did the wrong thing
> > for me.
> 
> This could be a good starting point for an investigation ;-)
> 
> > So yes, this is not generic here, but I have no idea how the
> > generic code looks like. As Coldfire is big endian this doesn't seem
> > that wrong.
> 
> I would try to define a proper flexcan_mb struct like
> 
> struct flexcan_mb {
> 	u8 code;
> 	u8 ctrl;
> 	u16 timestamp;
> 	u32 id;
> 	u8 data[8];
> }

I can't properly define it like this because I have a little endian
system and u8 code is on offset 3 on my hardware. I just checked it
because I always get confused with endian problems ;)

I hope you're not suggesting me to do something like

#ifdef __LITTLE_ENDIAN
struct flexcan_mb {
	u16 timestamp;
	u8 ctrl;
	u8 code;
	u32 id;
	u8 data[8];
}
#else
struct flexcan_mb {
	u8 code;
	u8 ctrl;
	u16 timestamp;
	u32 id;
	u8 data[8];
}
#endif

(which would still require endian specific handling for the actual CAN
data)

> 
> And then see how it looks like ;-)

Well, I would have to do a le/be conversion manually whereas cpu_to_be32
*should* do the right thing. I don't have any ColdFire hardware to test
though.

The more I think about it the more I think that my original code does
the right thing(tm)

Sascha
Sascha Hauer July 28, 2009, 2:24 p.m. UTC | #6
On Tue, Jul 28, 2009 at 04:12:10PM +0200, Oliver Hartkopp wrote:
> Oliver Hartkopp wrote:
> > Sascha Hauer wrote:
> 
> >>>> +
> >>>> +	if (frame->can_id & CAN_RTR_FLAG)
> >>>> +		dlc |= MB_CNT_RTR;
> >>>> +
> >>>> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> >>>> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
> 
> Are you sure, that this is correct?

Yes, I am sure, at least on my hardware.

> 
> Indeed i wonder, if it would make sense to skip the entire struct flexcan_mb
> approach and fiddle byte-by-byte inside the registers ...

You'll have to to 32 bit accesses to get it right on little and big
endian.

what we can do is:

writel(can_data[0] << 24 | can_data[1] << 16 | can_data[2] << 8 | can_data[3],
	msg_buf + 0x8);

Sascha
Oliver Hartkopp July 28, 2009, 2:48 p.m. UTC | #7
Sascha Hauer wrote:
> On Tue, Jul 28, 2009 at 04:12:10PM +0200, Oliver Hartkopp wrote:
>> Oliver Hartkopp wrote:
>>> Sascha Hauer wrote:
>>>>>> +
>>>>>> +	if (frame->can_id & CAN_RTR_FLAG)
>>>>>> +		dlc |= MB_CNT_RTR;
>>>>>> +
>>>>>> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
>>>>>> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
>> Are you sure, that this is correct?
> 
> Yes, I am sure, at least on my hardware.

I looked into the writel() macro which does a cpu_to_le32() to the value -
sorry that i did not check this before ...

> 
>> Indeed i wonder, if it would make sense to skip the entire struct flexcan_mb
>> approach and fiddle byte-by-byte inside the registers ...
> 
> You'll have to to 32 bit accesses to get it right on little and big
> endian.
> 
> what we can do is:
> 
> writel(can_data[0] << 24 | can_data[1] << 16 | can_data[2] << 8 | can_data[3],
> 	msg_buf + 0x8);

This looks easier to understand and makes things clear, when you look into the
specification.

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
Sascha Hauer July 28, 2009, 2:53 p.m. UTC | #8
On Tue, Jul 28, 2009 at 04:48:34PM +0200, Oliver Hartkopp wrote:
> Sascha Hauer wrote:
> > On Tue, Jul 28, 2009 at 04:12:10PM +0200, Oliver Hartkopp wrote:
> >> Oliver Hartkopp wrote:
> >>> Sascha Hauer wrote:
> >>>>>> +
> >>>>>> +	if (frame->can_id & CAN_RTR_FLAG)
> >>>>>> +		dlc |= MB_CNT_RTR;
> >>>>>> +
> >>>>>> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
> >>>>>> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
> >> Are you sure, that this is correct?
> > 
> > Yes, I am sure, at least on my hardware.
> 
> I looked into the writel() macro which does a cpu_to_le32() to the value -
> sorry that i did not check this before ...
> 
> > 
> >> Indeed i wonder, if it would make sense to skip the entire struct flexcan_mb
> >> approach and fiddle byte-by-byte inside the registers ...
> > 
> > You'll have to to 32 bit accesses to get it right on little and big
> > endian.
> > 
> > what we can do is:
> > 
> > writel(can_data[0] << 24 | can_data[1] << 16 | can_data[2] << 8 | can_data[3],
> > 	msg_buf + 0x8);
> 
> This looks easier to understand and makes things clear, when you look into the
> specification.

Ok, I'll change it like this.

Sascha
Wolfgang Grandegger July 28, 2009, 7:41 p.m. UTC | #9
Sascha Hauer wrote:
> On Tue, Jul 28, 2009 at 04:48:34PM +0200, Oliver Hartkopp wrote:
>> Sascha Hauer wrote:
>>> On Tue, Jul 28, 2009 at 04:12:10PM +0200, Oliver Hartkopp wrote:
>>>> Oliver Hartkopp wrote:
>>>>> Sascha Hauer wrote:
>>>>>>>> +
>>>>>>>> +	if (frame->can_id & CAN_RTR_FLAG)
>>>>>>>> +		dlc |= MB_CNT_RTR;
>>>>>>>> +
>>>>>>>> +	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
>>>>>>>> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
>>>> Are you sure, that this is correct?
>>> Yes, I am sure, at least on my hardware.
>> I looked into the writel() macro which does a cpu_to_le32() to the value -
>> sorry that i did not check this before ...
>>
>>>> Indeed i wonder, if it would make sense to skip the entire struct flexcan_mb
>>>> approach and fiddle byte-by-byte inside the registers ...
>>> You'll have to to 32 bit accesses to get it right on little and big
>>> endian.
>>>
>>> what we can do is:
>>>
>>> writel(can_data[0] << 24 | can_data[1] << 16 | can_data[2] << 8 | can_data[3],
>>> 	msg_buf + 0x8);
>> This looks easier to understand and makes things clear, when you look into the
>> specification.
> 
> Ok, I'll change it like this.

OK, even if I do not really share Oliver's concerns. The second write
could be suppressed if dlc <= 4.

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..8a2e820
--- /dev/null
+++ b/drivers/net/can/flexcan.c
@@ -0,0 +1,692 @@ 
+/*
+ * 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_dlc;
+	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 dlc = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
+	u32 *can_data;
+
+	netif_stop_queue(dev);
+
+	if (frame->can_id & CAN_EFF_FLAG) {
+		can_id = frame->can_id & MB_ID_EXT;
+		dlc |= MB_CNT_IDE | MB_CNT_SRR;
+	} else {
+		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
+	}
+
+	if (frame->can_id & CAN_RTR_FLAG)
+		dlc |= MB_CNT_RTR;
+
+	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
+	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
+
+	can_data = (u32 *)frame->data;
+	writel(cpu_to_be32(*can_data), &regs->cantxfg[TX_BUF_ID].data[0]);
+	writel(cpu_to_be32(*(can_data + 1)), &regs->cantxfg[TX_BUF_ID].data[1]);
+
+	writel(dlc, &regs->cantxfg[TX_BUF_ID].can_dlc);
+
+	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 = readl(&mb->can_dlc);
+	int length = (ctrl >> 16) & 0x0f;
+	u32 id;
+
+	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) & MB_ID_EXT;
+
+	if (ctrl & MB_CNT_IDE) {
+		frame->can_id = id & CAN_EFF_MASK;
+		frame->can_id |= CAN_EFF_FLAG;
+		if (ctrl & MB_CNT_RTR)
+			frame->can_id |= CAN_RTR_FLAG;
+	} else {
+		frame->can_id  = id >> 18;
+		if (ctrl & MB_CNT_RTR)
+			frame->can_id |= CAN_RTR_FLAG;
+	}
+
+	*(u32 *)frame->data = be32_to_cpu(readl(&mb->data[0]));
+	*((u32 *)frame->data + 1) = be32_to_cpu(readl(&mb->data[1]));
+
+	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_dlc);
+		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_dlc);
+	}
+
+	/* 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");