* Re: [PATCH] Add Support for Freescale FlexCAN CAN controller
From: Oliver Hartkopp @ 2009-07-27 10:52 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Socketcan-core, Linux Netdev List
In-Reply-To: <4A6D7C7E.9050703@grandegger.com>
Wolfgang Grandegger wrote:
> Oliver Hartkopp wrote:
>> Wolfgang Grandegger wrote:
>>> Sascha Hauer wrote:
>>>> On Fri, Jul 24, 2009 at 04:55:02PM +0200, Wolfgang Grandegger wrote:
>>
>>> [...]
>>>>>> + kfree_skb(skb);
>>>>> Support for echo skb using can_put/get_echo_skb() is missing. It should
>>>>> not be a big deal to add it.
>>>> In fact it's not missing, but the hardware is configured to receive its
>>>> own packets, so this isn't needed.
>>> But the user may disable IFF_ECHO, which should be handled somehow.
>> Really?
>>
>> IMO IFF_ECHO indicates the capability of the network driver to perform the MAC
>> layer frame echo on driver level.
>>
>> Only when you have a CAN driver that does not have this capability (the 'bad'
>> non default case) this flag is cleared.
>>
>> There is no need to provide a mode that can switch this functionality 'on' and
>> 'off'. IFF_ECHO is just an indication provided by the driver.
>>
>> When the FlexCAN controller can 'receive' a sent frame, so that it reflects
>> the correct message order on the bus, this is perfect.
>>
>> In this case Sascha should set the IFF_ECHO flag and he's done.
>
> I thought the user can toggle the IFF_ECHO flag with the "ip" utility,
> but obviously I'm wrong.
AFAIK the only driver that currently allows to switch this flag is the vcan
driver - but this is for testing purposes only and can be configured by the
module commandline.
I assume having the echo functionality runtime configurable is only effort and
complicates the driver ...
Best regards,
Oliver
^ permalink raw reply
* Re: [PATCH] IPVS: logging sizeof(struct ip_vs_conn) on startup
From: Hannes Eder @ 2009-07-27 10:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, lvs-users, malcolm, linux-kernel, David Miller,
Simon Horman
In-Reply-To: <4A6D7285.9080909@gmail.com>
On Mon, Jul 27, 2009 at 11:25, Eric Dumazet<eric.dumazet@gmail.com> wrote:
> Well, ip_vs_conn_cache uses SLAB_HWCACHE_ALIGN, so this should be rounded up
> to L1 cache size. Or add "at least" like in line 1080
> of net/netfilter/ipvs/ip_vs_conn.c :
>
> IP_VS_DBG(0, "Each connection entry needs %Zd bytes at least\n",
> sizeof(struct ip_vs_conn));
Good point. Is there an easy way to figure out how much memory is
wasted for alignment by the slab allocator? Otherwise I vote for the
"at least" approach.
-Hannes
^ permalink raw reply
* Re: [PATCH] IPVS: Add handling of incoming ICMPV6_PKT_TOOBIG messages
From: Julius Volz @ 2009-07-27 10:19 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, eric.dumazet, lvs-devel, netdev, robert.gallagher
In-Reply-To: <20090727023754.GC723@verge.net.au>
On Mon, Jul 27, 2009 at 4:37 AM, Simon Horman<horms@verge.net.au> wrote:
> On Sun, Jul 26, 2009 at 07:19:05PM -0700, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Fri, 24 Jul 2009 06:25:25 +0200
>>
>> > I see no reference to ICMPV6_PKT_TOOBIG in this patch, so ChangeLog
>> > might be misleading or uncomplete, since other ICMPV6 message types
>> > (ICMPV6_DEST_UNREACH/ICMPV6_TIME_EXCEED) will also be forwarded/handled
>> > ?
>>
>> Agreed, this commit message could use a tidy. What the patch actually is
>> doing is adding the handling of ipv6 icmp messages at all.
>>
>> Simon could you clarify the commit message a bit and resubmit?
>
> Will do.
Oops, yes. For some weird reason, only the specific problem that
caused the patch was in my head when writing that commit message. Feel
free to edit it to include ICMPv6 handling for in-out packets in
general.
Julius
^ permalink raw reply
* Re: [PATCH] Add Support for Freescale FlexCAN CAN controller
From: Wolfgang Grandegger @ 2009-07-27 10:07 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Socketcan-core, Linux Netdev List
In-Reply-To: <4A6D76B7.8030105@hartkopp.net>
Oliver Hartkopp wrote:
> Wolfgang Grandegger wrote:
>> Sascha Hauer wrote:
>>> On Fri, Jul 24, 2009 at 04:55:02PM +0200, Wolfgang Grandegger wrote:
>
>
>> [...]
>>>>> + kfree_skb(skb);
>>>> Support for echo skb using can_put/get_echo_skb() is missing. It should
>>>> not be a big deal to add it.
>>> In fact it's not missing, but the hardware is configured to receive its
>>> own packets, so this isn't needed.
>> But the user may disable IFF_ECHO, which should be handled somehow.
>
> Really?
>
> IMO IFF_ECHO indicates the capability of the network driver to perform the MAC
> layer frame echo on driver level.
>
> Only when you have a CAN driver that does not have this capability (the 'bad'
> non default case) this flag is cleared.
>
> There is no need to provide a mode that can switch this functionality 'on' and
> 'off'. IFF_ECHO is just an indication provided by the driver.
>
> When the FlexCAN controller can 'receive' a sent frame, so that it reflects
> the correct message order on the bus, this is perfect.
>
> In this case Sascha should set the IFF_ECHO flag and he's done.
I thought the user can toggle the IFF_ECHO flag with the "ip" utility,
but obviously I'm wrong.
Wolfgang.
^ permalink raw reply
* Re: tty_register_device NULL pointer dereference in 2.6.31-rc4
From: Dave Young @ 2009-07-27 9:59 UTC (permalink / raw)
To: Alan Cox
Cc: Marcel Holtmann, Oliver Hartkopp, Linux Netdev List,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20090725131046.0f076f37-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
On Sat, Jul 25, 2009 at 01:10:46PM +0100, Alan Cox wrote:
> > > tty_register_device appears to have been called with a NULL pointer. Not
> > > sure why however.
> >
> > if that is the pointer for the struct device, then that used to be fine
> > in the past. Not all RFCOMM device have a parent when they are created.
>
> The tty layer doesn't care about the struct device really. Nothing there
> has changed. The NULL passed appears to be the driver argument.
Agree with you, because in rfcomm_init, rfcomm thread run before tty initilized, the following patch may fix the problem. oliver, could you verify it it fix your problem?
---
rfcomm tty may be used before rfcomm_tty_driver initilized,
reporting in:
http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2
make 3 changes:
1. remove #ifdef in rfcomm/core.c,
make it blank function when rfcomm tty not selected in rfcomm.h
2. tune the rfcomm_init error patch to ensure
tty driver initilized before any usage.
3. remove __exit for rfcomm_cleanup_sockets
because above change need call it in a __init function.
CC: Alan Cox <alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
Reported-by: Oliver Hartkopp <oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Signed-off-by: Dave Young <hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
include/net/bluetooth/rfcomm.h | 13 ++++++++++++-
net/bluetooth/rfcomm/core.c | 31 +++++++++++++++++++++----------
net/bluetooth/rfcomm/sock.c | 2 +-
3 files changed, 34 insertions(+), 12 deletions(-)
--- linux-2.6.orig/include/net/bluetooth/rfcomm.h 2009-04-09 16:23:03.000000000 +0800
+++ linux-2.6/include/net/bluetooth/rfcomm.h 2009-07-27 17:14:43.000000000 +0800
@@ -355,7 +355,18 @@ struct rfcomm_dev_list_req {
};
int rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
+
+#ifdef CONFIG_BT_RFCOMM_TTY
int rfcomm_init_ttys(void);
void rfcomm_cleanup_ttys(void);
-
+#else
+static inline int rfcomm_init_ttys(void)
+{
+ return 0;
+}
+static inline int rfcomm_cleanup_ttys(void)
+{
+ return 0;
+}
+#endif
#endif /* __RFCOMM_H */
--- linux-2.6.orig/net/bluetooth/rfcomm/core.c 2009-06-16 17:39:32.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/core.c 2009-07-27 17:24:27.000000000 +0800
@@ -2080,28 +2080,41 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
/* ---- Initialization ---- */
static int __init rfcomm_init(void)
{
+ int ret;
+
l2cap_load();
+ ret = rfcomm_init_sockets();
+ if (ret)
+ goto out_sock;
+
+ ret = rfcomm_init_ttys();
+ if (ret)
+ goto out_tty;
+
hci_register_cb(&rfcomm_cb);
rfcomm_thread = kthread_run(rfcomm_run, NULL, "krfcommd");
if (IS_ERR(rfcomm_thread)) {
- hci_unregister_cb(&rfcomm_cb);
- return PTR_ERR(rfcomm_thread);
+ ret = PTR_ERR(rfcomm_thread);
+ goto out_thread;
}
if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
BT_ERR("Failed to create RFCOMM info file");
- rfcomm_init_sockets();
-
-#ifdef CONFIG_BT_RFCOMM_TTY
- rfcomm_init_ttys();
-#endif
-
BT_INFO("RFCOMM ver %s", VERSION);
return 0;
+
+out_thread:
+ hci_unregister_cb(&rfcomm_cb);
+out_tty:
+ rfcomm_cleanup_ttys();
+out_sock:
+ rfcomm_cleanup_sockets();
+
+ return ret;
}
static void __exit rfcomm_exit(void)
@@ -2112,9 +2125,7 @@ static void __exit rfcomm_exit(void)
kthread_stop(rfcomm_thread);
-#ifdef CONFIG_BT_RFCOMM_TTY
rfcomm_cleanup_ttys();
-#endif
rfcomm_cleanup_sockets();
}
--- linux-2.6.orig/net/bluetooth/rfcomm/sock.c 2009-04-09 16:23:04.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/sock.c 2009-07-27 17:39:43.000000000 +0800
@@ -1132,7 +1132,7 @@ error:
return err;
}
-void __exit rfcomm_cleanup_sockets(void)
+void rfcomm_cleanup_sockets(void)
{
class_remove_file(bt_class, &class_attr_rfcomm);
^ permalink raw reply
* Re: [PATCH] Add Support for Freescale FlexCAN CAN controller
From: Oliver Hartkopp @ 2009-07-27 9:43 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Sascha Hauer, Socketcan-core, Linux Netdev List
In-Reply-To: <4A6D727B.7030304@grandegger.com>
Wolfgang Grandegger wrote:
> Sascha Hauer wrote:
>> On Fri, Jul 24, 2009 at 04:55:02PM +0200, Wolfgang Grandegger wrote:
> [...]
>>>> + kfree_skb(skb);
>>> Support for echo skb using can_put/get_echo_skb() is missing. It should
>>> not be a big deal to add it.
>> In fact it's not missing, but the hardware is configured to receive its
>> own packets, so this isn't needed.
>
> But the user may disable IFF_ECHO, which should be handled somehow.
Really?
IMO IFF_ECHO indicates the capability of the network driver to perform the MAC
layer frame echo on driver level.
Only when you have a CAN driver that does not have this capability (the 'bad'
non default case) this flag is cleared.
There is no need to provide a mode that can switch this functionality 'on' and
'off'. IFF_ECHO is just an indication provided by the driver.
When the FlexCAN controller can 'receive' a sent frame, so that it reflects
the correct message order on the bus, this is perfect.
In this case Sascha should set the IFF_ECHO flag and he's done.
Regards,
Oliver
^ permalink raw reply
* Re: [PATCH] IPVS: logging sizeof(struct ip_vs_conn) on startup
From: Eric Dumazet @ 2009-07-27 9:25 UTC (permalink / raw)
To: Hannes Eder; +Cc: netdev, lvs-users, malcolm, linux-kernel
In-Reply-To: <20090727090741.16509.35291.stgit@jazzy.zrh.corp.google.com>
Hannes Eder a écrit :
> No more guessing, how much memory is used by IPVS for a connection.
>
> Signed-off-by: Hannes Eder <heder@google.com>
> ---
> net/netfilter/ipvs/ip_vs_core.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index b021464..5b0997c 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1475,7 +1475,8 @@ static int __init ip_vs_init(void)
> goto cleanup_conn;
> }
>
> - IP_VS_INFO("ipvs loaded.\n");
> + IP_VS_INFO("ipvs loaded (using %Zd bytes/connection).\n",
> + sizeof(struct ip_vs_conn));
> return ret;
>
> cleanup_conn:
Well, ip_vs_conn_cache uses SLAB_HWCACHE_ALIGN, so this should be rounded up
to L1 cache size. Or add "at least" like in line 1080
of net/netfilter/ipvs/ip_vs_conn.c :
IP_VS_DBG(0, "Each connection entry needs %Zd bytes at least\n",
sizeof(struct ip_vs_conn));
^ permalink raw reply
* Re: [PATCH] Add Support for Freescale FlexCAN CAN controller
From: Wolfgang Grandegger @ 2009-07-27 9:25 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Socketcan-core, Linux Netdev List
In-Reply-To: <20090727062559.GP2714@pengutronix.de>
Sascha Hauer wrote:
> On Fri, Jul 24, 2009 at 04:55:02PM +0200, Wolfgang Grandegger wrote:
>> Hi Sascha,
>>
>> Sascha Hauer wrote:
>>> Hi,
>>>
>>> This patch adds support for the Freescale FlexCAN CAN controller.
>>> The driver has been tested on an i.MX25 SoC with bitrates up to
>>> 1Mbit, remote frames and standard and extenden frames.
>>>
>>> Please review and consider for inclusion.
>> See below...
>>
>>> Sascha
>>>
>>>
>>> >From 94a607390f0d7b181e2ffdfe16a05f3aaca15ab9 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] Add Flexcan CAN driver
>> The prefix "can:" would be nice here.
>
> Ok, I'll add it next round.
>
>>> 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.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>> drivers/net/can/Kconfig | 6 +
>>> drivers/net/can/Makefile | 1 +
>>> drivers/net/can/flexcan.c | 696 +++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 703 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
[...]
>>> + kfree_skb(skb);
>> Support for echo skb using can_put/get_echo_skb() is missing. It should
>> not be a big deal to add it.
>
> In fact it's not missing, but the hardware is configured to receive its
> own packets, so this isn't needed.
But the user may disable IFF_ECHO, which should be handled somehow.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] IPVS: logging sizeof(struct ip_vs_conn) on startup
From: Simon Horman @ 2009-07-27 9:22 UTC (permalink / raw)
To: Hannes Eder; +Cc: netdev, lvs-users, malcolm, linux-kernel, David Miller
In-Reply-To: <20090727090741.16509.35291.stgit@jazzy.zrh.corp.google.com>
On Mon, Jul 27, 2009 at 11:08:34AM +0200, Hannes Eder wrote:
> No more guessing, how much memory is used by IPVS for a connection.
I like that.
> Signed-off-by: Hannes Eder <heder@google.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
> ---
> net/netfilter/ipvs/ip_vs_core.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index b021464..5b0997c 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1475,7 +1475,8 @@ static int __init ip_vs_init(void)
> goto cleanup_conn;
> }
>
> - IP_VS_INFO("ipvs loaded.\n");
> + IP_VS_INFO("ipvs loaded (using %Zd bytes/connection).\n",
> + sizeof(struct ip_vs_conn));
> return ret;
>
> cleanup_conn:
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* [PATCH net-next-2.6] net: ethtool_op_get_rx_csum() should be public and exported
From: Eric Dumazet @ 2009-07-27 9:18 UTC (permalink / raw)
To: David Miller; +Cc: sri, netdev
In-Reply-To: <20090726.194957.162341485.davem@davemloft.net>
David Miller a écrit :
> From: Sridhar Samudrala <sri@us.ibm.com>
> Date: Wed, 22 Jul 2009 16:38:22 -0700
>
>> [PATCH net-next-2.6] ethtool: add device independent rx_csum and get_flags routines
>>
>> This helps avoid error messages with ethtool -k on devices that
>> don't provide device specific routines.
>>
>> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
>
> Applied.
I wonder if ethtool_op_get_rx_csum() should be static or is meant to be used by drivers ?
I guess it should be public and exported like ethtool_op_get_tx_csum()
[PATCH] net: ethtool_op_get_rx_csum() should be public and exported
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/ethtool.h | 1 +
net/core/ethtool.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 9b660bd..90c4a36 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -368,6 +368,7 @@ struct net_device;
/* Some generic methods drivers may use in their ethtool_ops */
u32 ethtool_op_get_link(struct net_device *dev);
+u32 ethtool_op_get_rx_csum(struct net_device *dev);
u32 ethtool_op_get_tx_csum(struct net_device *dev);
int ethtool_op_set_tx_csum(struct net_device *dev, u32 data);
int ethtool_op_set_tx_hw_csum(struct net_device *dev, u32 data);
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index cf36ff4..44e5711 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -34,11 +34,13 @@ u32 ethtool_op_get_rx_csum(struct net_device *dev)
{
return (dev->features & NETIF_F_ALL_CSUM) != 0;
}
+EXPORT_SYMBOL(ethtool_op_get_rx_csum);
u32 ethtool_op_get_tx_csum(struct net_device *dev)
{
return (dev->features & NETIF_F_ALL_CSUM) != 0;
}
+EXPORT_SYMBOL(ethtool_op_get_tx_csum);
int ethtool_op_set_tx_csum(struct net_device *dev, u32 data)
{
@@ -1125,7 +1127,6 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
EXPORT_SYMBOL(ethtool_op_get_link);
EXPORT_SYMBOL(ethtool_op_get_sg);
EXPORT_SYMBOL(ethtool_op_get_tso);
-EXPORT_SYMBOL(ethtool_op_get_tx_csum);
EXPORT_SYMBOL(ethtool_op_set_sg);
EXPORT_SYMBOL(ethtool_op_set_tso);
EXPORT_SYMBOL(ethtool_op_set_tx_csum);
^ permalink raw reply related
* [PATCH] IPVS: logging sizeof(struct ip_vs_conn) on startup
From: Hannes Eder @ 2009-07-27 9:08 UTC (permalink / raw)
To: netdev; +Cc: lvs-users, malcolm, linux-kernel
No more guessing, how much memory is used by IPVS for a connection.
Signed-off-by: Hannes Eder <heder@google.com>
---
net/netfilter/ipvs/ip_vs_core.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index b021464..5b0997c 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1475,7 +1475,8 @@ static int __init ip_vs_init(void)
goto cleanup_conn;
}
- IP_VS_INFO("ipvs loaded.\n");
+ IP_VS_INFO("ipvs loaded (using %Zd bytes/connection).\n",
+ sizeof(struct ip_vs_conn));
return ret;
cleanup_conn:
^ permalink raw reply related
* Re: [PATCH] Add Support for Freescale FlexCAN CAN controller
From: Wolfgang Grandegger @ 2009-07-27 8:30 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Socketcan-core, Linux Netdev List
In-Reply-To: <20090727062559.GP2714@pengutronix.de>
Sascha Hauer wrote:
> On Fri, Jul 24, 2009 at 04:55:02PM +0200, Wolfgang Grandegger wrote:
>> Hi Sascha,
>>
>> Sascha Hauer wrote:
>>> Hi,
>>>
>>> This patch adds support for the Freescale FlexCAN CAN controller.
>>> The driver has been tested on an i.MX25 SoC with bitrates up to
>>> 1Mbit, remote frames and standard and extenden frames.
>>>
>>> Please review and consider for inclusion.
>> See below...
>>
>>> Sascha
[snip]
>>> +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, ®s->cantxfg[TX_BUF_ID].can_dlc);
>>> + writel(can_id, ®s->cantxfg[TX_BUF_ID].can_id);
>>> +
>>> + can_data = (u32 *)frame->data;
>>> + writel(cpu_to_be32(*can_data), ®s->cantxfg[TX_BUF_ID].data[0]);
>>> + writel(cpu_to_be32(*(can_data + 1)), ®s->cantxfg[TX_BUF_ID].data[1]);
>>> +
>>> + writel(dlc, ®s->cantxfg[TX_BUF_ID].can_dlc);
>>> +
>>> + kfree_skb(skb);
>> Support for echo skb using can_put/get_echo_skb() is missing. It should
>> not be a big deal to add it.
>
> In fact it's not missing, but the hardware is configured to receive its
> own packets, so this isn't needed.
Ah, OK, I assume that the message is looped back not before it went out
to the bus to ensure proper time ordering.
>>> +
>>> + 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;
>>> +
>>> + skb = dev_alloc_skb(sizeof(struct can_frame));
>>> + if (!skb)
>>> + return;
>>> +
>>> + skb->dev = ndev;
>>> + skb->protocol = htons(ETH_P_CAN);
>> skb->protocol = __constant_htons(ETH_P_CAN);
>> skb->ip_summed = CHECKSUM_UNNECESSARY;
>>
>> as above?!
>
> Ok
>
>>> +
>>> + 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;
>>> + state = CAN_STATE_ERROR_WARNING;
>>> + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>>> + }
>>> +
>>> + if (stat & ERRSTAT_TWRNINT) {
>>> + error_warning = 1;
>>> + state = CAN_STATE_ERROR_WARNING;
>>> + cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
>>> + }
>>> +
>>> + switch ((stat >> 4) & 0x3) {
>>> + case 0:
>>> + state = CAN_STATE_ERROR_ACTIVE;
>>> + break;
>> Does the device recover autmatically from the bus-off state? Can
>> automatic recovery be configured (switched off?).
>
> The device *can* be configured to automatically recover from bus off,
> but I didn't use this feature to be more conform to the Linux CAN API.
Good.
>>> + case 1:
>>> + state = CAN_STATE_ERROR_PASSIVE;
>>> + break;
>>> + default:
>>> + state = CAN_STATE_BUS_OFF;
>>> + break;
>>> + }
>> You seem to handle a state change to error passive like a change to
>> error warning.
>>
>>> + if (stat & ERRSTAT_BOFFINT) {
>>> + cf->can_id |= CAN_ERR_BUSOFF;
>>> + can_bus_off(ndev);
>>> + }
>>> +
>>> + if (stat & ERRSTAT_BIT1ERR) {
>> rx_error = 1; ???
>>
>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> + cf->data[2] |= CAN_ERR_PROT_BIT1;
>>> + }
>>> +
>>> + if (stat & ERRSTAT_BIT0ERR) {
>> rx_error = 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) {
>>> + rx_errors = 1;
>>> + cf->can_id |= CAN_ERR_ACK;
>> Is ACK error a RX error?
>>
>>> + }
>>> +
>>> + if (error_warning)
>>> + priv->can.can_stats.error_warning++;
>> What about priv->can.can_stats.error_passive;
>>
>>> + if (rx_errors)
>>> + stats->rx_errors++;
>>> + if (cf->can_id & CAN_ERR_BUSERROR)
>>> + priv->can.can_stats.bus_error++;
>> It gets incremented in can_bus_off() already!
>
> ok, I will rework the error handling.
>
>>> + 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(®s->errstat);
>>> + if (errstat & ERRSTAT_INT) {
>>> + flexcan_error(ndev, errstat);
>>> + writel(errstat & ERRSTAT_INT, ®s->errstat);
>>> + }
>>> +
>>> + iflags = readl(®s->iflag1);
>>> +
>>> + if (iflags & IFLAG_RX_FIFO_OVERFLOW) {
>>> + writel(IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1);
>>> + stats->rx_over_errors++;
>> stats->rx_errors++; ???
>>
>>> + }
>>> +
>>> + while (iflags & IFLAG_RX_FIFO_AVAILABLE) {
>>> + struct flexcan_mb __iomem *mb = ®s->cantxfg[0];
>>> +
>>> + flexcan_rx_frame(ndev, mb);
>>> + writel(IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1);
>>> + readl(®s->timer);
>>> + iflags = readl(®s->iflag1);
>>> + }
>>> +
>>> + if (iflags & (1 << TX_BUF_ID)) {
>>> + stats->tx_packets++;
>>> + writel((1 << TX_BUF_ID), ®s->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;
>>> + int ret = 0;
>>> + u32 reg;
>>> +
>>> + dev_dbg(&ndev->dev, "%s: infreq: %ld brp: %d p1: %d p2: %d sample: %d"
>>> + " sjw: %d prop: %d\n",
>>> + __func__, clk_get_rate(priv->clk), bt->brp,
>>> + bt->phase_seg1, bt->phase_seg2, bt->sample_point,
>>> + bt->sjw, bt->prop_seg);
>> Could you replace this dev_dbg() in favor of a
>>
>> dev_info(dev->dev.parent, "setting CANCTRL=0x%x\n", reg);
>>
>> before returning.
>
> The dev_dbg is redundant as the output of 'ip' already shows this
> information. But shouldn't this be a dev_dbg, too?
For SJA1000 and MSCAN we currently use dev_info() here. I found it
useful to show the bittiming registers as there is no other method to
retrieve them, but that's debatable.
[...]
>>> +MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_DESCRIPTION("CAN port driver for flexcan based chip");
>> Apart from that, it already looks quite good.
>>
>> Thanks for your contribution.
>
> Thanks for review, I will send an updated version soon.
Wolfgang.
^ permalink raw reply
* Re: [RFC] Idea about increasing efficency of skb allocation in network devices
From: Brice Goglin @ 2009-07-27 8:27 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, nhorman, netdev
In-Reply-To: <4A6D5E1E.3090907@gmail.com>
Eric Dumazet wrote:
>> Is there an easy way to get this NUMA node from the application socket
>> descriptor?
>>
>
> Thats not easy, this information can change for every packet (think of
> bonding setups, whith aggregation of devices on different NUMA nodes)
>
If we return a mask of cpus near the NIC, we could return the mask
containing cpus that are close to any of the devices that were
aggregated in this bonding setup.
If no bonding, it's fine. If bonding, the behavior looks acceptable to me.
> We could add a getsockopt() call to peek this information from the next
> data to be read from socket (returns node id where skb data is sitting,
> hoping that NIC driver hadnt copybreak it (ie : allocate a small skb and
> copy the device provided data on it before feeding packet to network stack))
>
>
>
>> Also, one question that was raised at the Linux Symposium is: how do you
>> know which processors run the receive queue for a specific connection ?
>> It would be nice to have a way to retrieve such information in the
>> application to avoid inter-node and inter-core/cache traffic.
>>
>
> All this depends on the fact you have multiqueue devices or not, and
> trafic spreads on all queues or not.
>
Again, on a per-connection basis, you should know whether your packets
are going through a single queue or to all of them? If going to a single
queue, return a mask of cpus near this exact queue. If going to multiple
queues (or if you don't know), just sumup the cpumask of all queues.
Brice
^ permalink raw reply
* Re: net-next-2.6 phonet causes build error
From: Eric Dumazet @ 2009-07-27 8:09 UTC (permalink / raw)
To: Rémi Denis-Courmont
Cc: ext Eric Dumazet, Linux Netdev List, David S. Miller
In-Reply-To: <200907271106.02456.remi.denis-courmont@nokia.com>
Rémi Denis-Courmont a écrit :
> On Monday 27 July 2009 10:03:06 ext Eric Dumazet wrote:
>> Rémi Denis-Courmont a écrit :
>>> On Sunday 26 July 2009 21:43:11 ext Oliver Hartkopp wrote:
>>>> i got this build error from the latest net-next-2.6:
>>>>
>>>> net/phonet/pn_dev.c: In function ‘phonet_init_net’:
>>>> net/phonet/pn_dev.c:221: error: implicit declaration of function
>>>> ‘proc_net_fops_create’
>>>> net/phonet/pn_dev.c: In function ‘phonet_exit_net’:
>>>> net/phonet/pn_dev.c:242: error: implicit declaration of function
>>>> ‘proc_net_remove’
>>> Oops. This makes me wonder why I did not get that one here on my test
>>> builds???
>> I also have this warning :
>>
>> net/phonet/pn_dev.c: In function `phonet_device_get':
>> net/phonet/pn_dev.c:99: warning: 'dev' might be used uninitialized in this
>> function
>
> Right. This is indeed a bug - thanks. Either my gcc version is bad at that
> stuff or I have missing debug options :(
>
I currently use gcc-3.4.6, and current net-next-2.6 doesnt compile for me
(but linux-2.6 does compile properly so I just know this is already fixed upstream)
UPD include/linux/compile.h
CC init/version.o
LD init/built-in.o
LD .tmp_vmlinux1
drivers/built-in.o(.text+0x86b17): In function `igdng_compute_m_n':
: undefined reference to `__udivdi3'
drivers/built-in.o(.init.text+0x3c44): In function `con_init':
: undefined reference to `.L1443'
make: *** [.tmp_vmlinux1] Erreur 1
No problem with gcc-4.4.0
^ permalink raw reply
* Re: net-next-2.6 phonet causes build error
From: Rémi Denis-Courmont @ 2009-07-27 8:06 UTC (permalink / raw)
To: ext Eric Dumazet; +Cc: Linux Netdev List, David S. Miller
In-Reply-To: <4A6D512A.1050504@gmail.com>
On Monday 27 July 2009 10:03:06 ext Eric Dumazet wrote:
> Rémi Denis-Courmont a écrit :
> > On Sunday 26 July 2009 21:43:11 ext Oliver Hartkopp wrote:
> >> i got this build error from the latest net-next-2.6:
> >>
> >> net/phonet/pn_dev.c: In function ‘phonet_init_net’:
> >> net/phonet/pn_dev.c:221: error: implicit declaration of function
> >> ‘proc_net_fops_create’
> >> net/phonet/pn_dev.c: In function ‘phonet_exit_net’:
> >> net/phonet/pn_dev.c:242: error: implicit declaration of function
> >> ‘proc_net_remove’
> >
> > Oops. This makes me wonder why I did not get that one here on my test
> > builds???
>
> I also have this warning :
>
> net/phonet/pn_dev.c: In function `phonet_device_get':
> net/phonet/pn_dev.c:99: warning: 'dev' might be used uninitialized in this
> function
Right. This is indeed a bug - thanks. Either my gcc version is bad at that
stuff or I have missing debug options :(
> What about following patch ?
>
> [PATCH] phonet: phonet_device_get() fix
>
> net/phonet/pn_dev.c: In function `phonet_device_get':
> net/phonet/pn_dev.c:99: warning: 'dev' might be used uninitialized in this
> function
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> ---
>
> diff --git a/net/phonet/pn_dev.c b/net/phonet/pn_dev.c
> index b0d6ddd..c2b77a6 100644
> --- a/net/phonet/pn_dev.c
> +++ b/net/phonet/pn_dev.c
> @@ -96,7 +96,7 @@ struct net_device *phonet_device_get(struct net *net)
> {
> struct phonet_device_list *pndevs = phonet_device_list(net);
> struct phonet_device *pnd;
> - struct net_device *dev;
> + struct net_device *dev = NULL;
>
> spin_lock_bh(&pndevs->lock);
> list_for_each_entry(pnd, &pndevs->list, list) {
--
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki
^ permalink raw reply
* Re: [RFC] Idea about increasing efficency of skb allocation in network devices
From: Eric Dumazet @ 2009-07-27 7:58 UTC (permalink / raw)
To: Brice Goglin; +Cc: David Miller, nhorman, netdev
In-Reply-To: <4A6D52FF.2030008@inria.fr>
Brice Goglin a écrit :
> David Miller wrote:
>> From: Neil Horman <nhorman@tuxdriver.com>
>> Date: Sun, 26 Jul 2009 20:36:09 -0400
>>
>>
>>> Since Network devices dma their memory into a provided DMA
>>> buffer (which can usually be at an arbitrary location, as they must
>>> cross potentially several pci busses to reach any memory location),
>>> I'm postulating that it would increase our receive path efficiency
>>> to provide a hint to the driver layer as to which node to allocate
>>> an skb data buffer on. This hint would be determined by a feedback
>>> mechanism. I was thinking that we could provide a callback function
>>> via the skb, that accepted the skb and the originating net_device.
>>> This callback can track statistics on which numa nodes consume
>>> (read: copy data from) skbs that were produced by specific net
>>> devices. Then, when in the future that netdevice allocates a new
>>> skb (perhaps via netdev_alloc_skb), we can use that statistical
>>> profile to determine if the data buffer should be allocated on the
>>> local node, or on a remote node instead.
>>>
>> No matter what, you will do an inter-node memory operation.
>>
>> Unless, the consumer NUMA node is the same as the one the
>> device is on.
>>
>> Because since the device is on a NUMA node, if you DMA remotely
>> you've eaten the NUMA cost already.
>>
>> If you always DMA to the device's NUMA node (what we try to do now) at
>> least the is the possibility of eliminating cross-NUMA traffic.
>>
>> Better to move the application or stack processing towards the NUMA
>> node the network device is on, I think.
>>
>
> Is there an easy way to get this NUMA node from the application socket
> descriptor?
Thats not easy, this information can change for every packet (think of
bonding setups, whith aggregation of devices on different NUMA nodes)
We could add a getsockopt() call to peek this information from the next
data to be read from socket (returns node id where skb data is sitting,
hoping that NIC driver hadnt copybreak it (ie : allocate a small skb and
copy the device provided data on it before feeding packet to network stack))
> Also, one question that was raised at the Linux Symposium is: how do you
> know which processors run the receive queue for a specific connection ?
> It would be nice to have a way to retrieve such information in the
> application to avoid inter-node and inter-core/cache traffic.
All this depends on the fact you have multiqueue devices or not, and
trafic spreads on all queues or not.
Assuming you have single queue device, only current way to handle
this is to do the reverse thinking.
Ie, bind NIC interrupts to the appropriate set of cpus, and
possibly bind user apps threads dealing with network trafic to same set.
Only background or cpu hungry threads should be allowed to run
on foreigns nodes.
^ permalink raw reply
* Re: [RFC] Idea about increasing efficency of skb allocation in network devices
From: Brice Goglin @ 2009-07-27 7:10 UTC (permalink / raw)
To: David Miller; +Cc: nhorman, netdev
In-Reply-To: <20090726.180254.202825489.davem@davemloft.net>
David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Sun, 26 Jul 2009 20:36:09 -0400
>
>
>> Since Network devices dma their memory into a provided DMA
>> buffer (which can usually be at an arbitrary location, as they must
>> cross potentially several pci busses to reach any memory location),
>> I'm postulating that it would increase our receive path efficiency
>> to provide a hint to the driver layer as to which node to allocate
>> an skb data buffer on. This hint would be determined by a feedback
>> mechanism. I was thinking that we could provide a callback function
>> via the skb, that accepted the skb and the originating net_device.
>> This callback can track statistics on which numa nodes consume
>> (read: copy data from) skbs that were produced by specific net
>> devices. Then, when in the future that netdevice allocates a new
>> skb (perhaps via netdev_alloc_skb), we can use that statistical
>> profile to determine if the data buffer should be allocated on the
>> local node, or on a remote node instead.
>>
>
> No matter what, you will do an inter-node memory operation.
>
> Unless, the consumer NUMA node is the same as the one the
> device is on.
>
> Because since the device is on a NUMA node, if you DMA remotely
> you've eaten the NUMA cost already.
>
> If you always DMA to the device's NUMA node (what we try to do now) at
> least the is the possibility of eliminating cross-NUMA traffic.
>
> Better to move the application or stack processing towards the NUMA
> node the network device is on, I think.
>
Is there an easy way to get this NUMA node from the application socket
descriptor?
Also, one question that was raised at the Linux Symposium is: how do you
know which processors run the receive queue for a specific connection ?
It would be nice to have a way to retrieve such information in the
application to avoid inter-node and inter-core/cache traffic.
Brice
^ permalink raw reply
* Re: net-next-2.6 phonet causes build error
From: Eric Dumazet @ 2009-07-27 7:03 UTC (permalink / raw)
To: Rémi Denis-Courmont; +Cc: Linux Netdev List, David S. Miller
In-Reply-To: <200907270927.55728.remi.denis-courmont@nokia.com>
Rémi Denis-Courmont a écrit :
> On Sunday 26 July 2009 21:43:11 ext Oliver Hartkopp wrote:
>> i got this build error from the latest net-next-2.6:
>>
>> net/phonet/pn_dev.c: In function ‘phonet_init_net’:
>> net/phonet/pn_dev.c:221: error: implicit declaration of function
>> ‘proc_net_fops_create’
>> net/phonet/pn_dev.c: In function ‘phonet_exit_net’:
>> net/phonet/pn_dev.c:242: error: implicit declaration of function
>> ‘proc_net_remove’
>
> Oops. This makes me wonder why I did not get that one here on my test
> builds???
>
I also have this warning :
net/phonet/pn_dev.c: In function `phonet_device_get':
net/phonet/pn_dev.c:99: warning: 'dev' might be used uninitialized in this function
What about following patch ?
[PATCH] phonet: phonet_device_get() fix
net/phonet/pn_dev.c: In function `phonet_device_get':
net/phonet/pn_dev.c:99: warning: 'dev' might be used uninitialized in this function
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/phonet/pn_dev.c b/net/phonet/pn_dev.c
index b0d6ddd..c2b77a6 100644
--- a/net/phonet/pn_dev.c
+++ b/net/phonet/pn_dev.c
@@ -96,7 +96,7 @@ struct net_device *phonet_device_get(struct net *net)
{
struct phonet_device_list *pndevs = phonet_device_list(net);
struct phonet_device *pnd;
- struct net_device *dev;
+ struct net_device *dev = NULL;
spin_lock_bh(&pndevs->lock);
list_for_each_entry(pnd, &pndevs->list, list) {
^ permalink raw reply related
* Re: [PATCH] Add Support for Freescale FlexCAN CAN controller
From: Sascha Hauer @ 2009-07-27 6:25 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Linux Netdev List, Socketcan-core
In-Reply-To: <4A69CB46.1090704@grandegger.com>
On Fri, Jul 24, 2009 at 04:55:02PM +0200, Wolfgang Grandegger wrote:
> Hi Sascha,
>
> Sascha Hauer wrote:
> > Hi,
> >
> > This patch adds support for the Freescale FlexCAN CAN controller.
> > The driver has been tested on an i.MX25 SoC with bitrates up to
> > 1Mbit, remote frames and standard and extenden frames.
> >
> > Please review and consider for inclusion.
>
> See below...
>
> >
> > Sascha
> >
> >
> >>From 94a607390f0d7b181e2ffdfe16a05f3aaca15ab9 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] Add Flexcan CAN driver
>
> The prefix "can:" would be nice here.
Ok, I'll add it next round.
>
> > 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.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > drivers/net/can/Kconfig | 6 +
> > drivers/net/can/Makefile | 1 +
> > drivers/net/can/flexcan.c | 696 +++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 703 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..3738898
> > --- /dev/null
> > +++ b/drivers/net/can/flexcan.c
> > @@ -0,0 +1,696 @@
> > +/*
> > + * 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)
> > +Cou
> > +/* 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, ®s->cantxfg[TX_BUF_ID].can_dlc);
> > + writel(can_id, ®s->cantxfg[TX_BUF_ID].can_id);
> > +
> > + can_data = (u32 *)frame->data;
> > + writel(cpu_to_be32(*can_data), ®s->cantxfg[TX_BUF_ID].data[0]);
> > + writel(cpu_to_be32(*(can_data + 1)), ®s->cantxfg[TX_BUF_ID].data[1]);
> > +
> > + writel(dlc, ®s->cantxfg[TX_BUF_ID].can_dlc);
> > +
> > + kfree_skb(skb);
>
> Support for echo skb using can_put/get_echo_skb() is missing. It should
> not be a big deal to add it.
In fact it's not missing, but the hardware is configured to receive its
own packets, so this isn't needed.
>
> > +
> > + 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;
> > +
> > + skb = dev_alloc_skb(sizeof(struct can_frame));
> > + if (!skb)
> > + return;
> > +
> > + skb->dev = ndev;
> > + skb->protocol = htons(ETH_P_CAN);
>
> skb->protocol = __constant_htons(ETH_P_CAN);
> skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> as above?!
Ok
>
> > +
> > + 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;
> > + state = CAN_STATE_ERROR_WARNING;
> > + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> > + }
> > +
> > + if (stat & ERRSTAT_TWRNINT) {
> > + error_warning = 1;
> > + state = CAN_STATE_ERROR_WARNING;
> > + cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> > + }
> > +
> > + switch ((stat >> 4) & 0x3) {
> > + case 0:
> > + state = CAN_STATE_ERROR_ACTIVE;
> > + break;
>
> Does the device recover autmatically from the bus-off state? Can
> automatic recovery be configured (switched off?).
The device *can* be configured to automatically recover from bus off,
but I didn't use this feature to be more conform to the Linux CAN API.
>
> > + case 1:
> > + state = CAN_STATE_ERROR_PASSIVE;
> > + break;
> > + default:
> > + state = CAN_STATE_BUS_OFF;
> > + break;
> > + }
>
> You seem to handle a state change to error passive like a change to
> error warning.
>
> > + if (stat & ERRSTAT_BOFFINT) {
> > + cf->can_id |= CAN_ERR_BUSOFF;
> > + can_bus_off(ndev);
> > + }
> > +
> > + if (stat & ERRSTAT_BIT1ERR) {
>
> rx_error = 1; ???
>
> > + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> > + cf->data[2] |= CAN_ERR_PROT_BIT1;
> > + }
> > +
> > + if (stat & ERRSTAT_BIT0ERR) {
>
> rx_error = 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) {
> > + rx_errors = 1;
> > + cf->can_id |= CAN_ERR_ACK;
>
> Is ACK error a RX error?
>
> > + }
> > +
> > + if (error_warning)
> > + priv->can.can_stats.error_warning++;
>
> What about priv->can.can_stats.error_passive;
>
> > + if (rx_errors)
> > + stats->rx_errors++;
> > + if (cf->can_id & CAN_ERR_BUSERROR)
> > + priv->can.can_stats.bus_error++;
>
> It gets incremented in can_bus_off() already!
ok, I will rework the error handling.
>
> > + 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(®s->errstat);
> > + if (errstat & ERRSTAT_INT) {
> > + flexcan_error(ndev, errstat);
> > + writel(errstat & ERRSTAT_INT, ®s->errstat);
> > + }
> > +
> > + iflags = readl(®s->iflag1);
> > +
> > + if (iflags & IFLAG_RX_FIFO_OVERFLOW) {
> > + writel(IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1);
> > + stats->rx_over_errors++;
>
> stats->rx_errors++; ???
>
> > + }
> > +
> > + while (iflags & IFLAG_RX_FIFO_AVAILABLE) {
> > + struct flexcan_mb __iomem *mb = ®s->cantxfg[0];
> > +
> > + flexcan_rx_frame(ndev, mb);
> > + writel(IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1);
> > + readl(®s->timer);
> > + iflags = readl(®s->iflag1);
> > + }
> > +
> > + if (iflags & (1 << TX_BUF_ID)) {
> > + stats->tx_packets++;
> > + writel((1 << TX_BUF_ID), ®s->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;
> > + int ret = 0;
> > + u32 reg;
> > +
> > + dev_dbg(&ndev->dev, "%s: infreq: %ld brp: %d p1: %d p2: %d sample: %d"
> > + " sjw: %d prop: %d\n",
> > + __func__, clk_get_rate(priv->clk), bt->brp,
> > + bt->phase_seg1, bt->phase_seg2, bt->sample_point,
> > + bt->sjw, bt->prop_seg);
>
> Could you replace this dev_dbg() in favor of a
>
> dev_info(dev->dev.parent, "setting CANCTRL=0x%x\n", reg);
>
> before returning.
The dev_dbg is redundant as the output of 'ip' already shows this
information. But shouldn't this be a dev_dbg, too?
>
> > + reg = readl(®s->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, ®s->canctrl);
> > +
> > + return ret;
> > +}
> > +
> > +
>
> Two empty lines!
>
> > +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, IRQF_DISABLED,
> > + DRIVER_NAME, ndev);
>
> Do you really need IRQF_DISABLED?
No.
>
> > + if (ret)
> > + goto err_irq;
> > +
> > + clk_enable(priv->clk);
> > +
> > + reg = CANMCR_SOFTRST | CANMCR_HALT;
> > + writel(CANMCR_SOFTRST, ®s->canmcr);
> > + udelay(10);
> > +
> > + if (readl(®s->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(®s->canctrl);
> > + reg |= CANCTRL_CLKSRC | CANCTRL_ERRMSK | CANCTRL_BOFFMSK |
> > + CANCTRL_BOFFREC | CANCTRL_TWRN_MSK | CANCTRL_TWRN_MSK;
> > + writel(reg, ®s->canctrl);
> > +
> > + /* Set lowest buffer transmitted first */
> > + reg |= CANCTRL_LBUF;
> > + writel(reg, ®s->canctrl);
> > +
> > + for (i = 0; i < 64; i++) {
> > + writel(0, ®s->cantxfg[i].can_dlc);
> > + writel(0, ®s->cantxfg[i].can_id);
> > + writel(0, ®s->cantxfg[i].data[0]);
> > + writel(0, ®s->cantxfg[i].data[1]);
> > +
> > + /* Put MB into rx queue */
> > + writel(MB_CNT_CODE(0x04), ®s->cantxfg[i].can_dlc);
> > + }
> > +
> > + /* acceptance mask/acceptance code (accept everything) */
> > + writel(0x0, ®s->rxgmask);
> > + writel(0x0, ®s->rx14mask);
> > + writel(0x0, ®s->rx15mask);
> > +
> > + /* Enable flexcan module */
> > + reg = readl(®s->canmcr);
> > + reg &= ~(CANMCR_MDIS | CANMCR_FRZ);
> > + reg |= CANMCR_IDAM_C | CANMCR_FEN;
> > + writel(reg, ®s->canmcr);
> > +
> > + /* Synchronize with the can bus */
> > + reg &= ~CANMCR_HALT;
> > + writel(reg, ®s->canmcr);
> > +
> > + /* Enable interrupts */
> > + writel(IFLAG_RX_FIFO_OVERFLOW | IFLAG_RX_FIFO_AVAILABLE |
> > + IFLAG_BUF(TX_BUF_ID),
> > + ®s->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, ®s->imask1);
> > + free_irq(ndev->irq, ndev);
> > +
> > + close_candev(ndev);
> > +
> > + /* Disable module */
> > + writel(CANMCR_MDIS, ®s->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(®s->canctrl);
> > + reg &= ~CANCTRL_BOFFREC;
> > + writel(reg, ®s->canctrl);
> > + reg |= CANCTRL_BOFFREC;
> > + writel(reg, ®s->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(®s->canmcr);
> > + reg &= ~CANMCR_MDIS;
> > + writel(reg, ®s->canmcr);
> > + udelay(100);
> > + reg |= CANMCR_FRZ | CANMCR_HALT | CANMCR_FEN;
> > + writel(reg, ®s->canmcr);
> > +
> > + reg = readl(®s->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");
>
> Line too long!
ok
>
> > + 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(®s->canmcr);
> > + reg |= CANMCR_FRZ | CANMCR_HALT | CANMCR_MDIS;
> > + writel(reg, ®s->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");
>
> Apart from that, it already looks quite good.
>
> Thanks for your contribution.
Thanks for review, I will send an updated version soon.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: net-next-2.6 phonet causes build error
From: Rémi Denis-Courmont @ 2009-07-27 6:27 UTC (permalink / raw)
To: Linux Netdev List
In-Reply-To: <4A6CA3BF.7010709@hartkopp.net>
On Sunday 26 July 2009 21:43:11 ext Oliver Hartkopp wrote:
> i got this build error from the latest net-next-2.6:
>
> net/phonet/pn_dev.c: In function ‘phonet_init_net’:
> net/phonet/pn_dev.c:221: error: implicit declaration of function
> ‘proc_net_fops_create’
> net/phonet/pn_dev.c: In function ‘phonet_exit_net’:
> net/phonet/pn_dev.c:242: error: implicit declaration of function
> ‘proc_net_remove’
Oops. This makes me wonder why I did not get that one here on my test
builds???
--
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki
^ permalink raw reply
* Re: [net-2.6 PATCH] ixgbe: fix for 82599 errata marking UDP checksum errors
From: Waskiewicz Jr, Peter P @ 2009-07-27 6:11 UTC (permalink / raw)
To: David Miller
Cc: Kirsher, Jeffrey T, netdev@vger.kernel.org, gospo@redhat.com,
Skidmore, Donald C
In-Reply-To: <20090726.202154.00452824.davem@davemloft.net>
On Sun, 26 Jul 2009, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Thu, 23 Jul 2009 21:00:39 -0700
>
> > From: Don Skidmore <donald.c.skidmore@intel.com>
> >
> > There is an 82599 errata that UDP frames with a zero checksum are
> > incorrectly marked as checksum invalid by the hardware. This was
> > leading to misleading hw_csum_rx_error counts. This patch adds a
> > test around this counter increase for this condition.
> >
> > Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> Applied, thanks.
>
> But I wonder. When you compute a checksum, and it's zero,
> you have to use the complement as the checksum value. How
> does that relate to the problem being tackled here?
The problem is our hardware has an errata. If the packet data contains no
checksum, or no checksum computed, our hardware doesn't recognize it. So
when it tries to compute the checksum on Rx, it doesn't just figure out
that it needs to use the one's complement. So it's not recognizing the
fact the checksum is either not computed, or computes to zero.
The only place we've seen this show up is receiving from pktgen, where no
checksum is computed. It is a really stupid HW errata, and is just an
annoyance really (our bad checksum counter soars when doing pktgen stress
testing).
Cheers,
-PJ Waskiewicz
^ permalink raw reply
* [PATCH NEXT 12/12] netxen: update version to 4.0.41
From: Dhananjay Phadke @ 2009-07-27 6:07 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1248674867-18821-1-git-send-email-dhananjay@netxen.com>
Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
drivers/net/netxen/netxen_nic.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index 9527f31..d4ac151 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -57,8 +57,8 @@
#define _NETXEN_NIC_LINUX_MAJOR 4
#define _NETXEN_NIC_LINUX_MINOR 0
-#define _NETXEN_NIC_LINUX_SUBVERSION 30
-#define NETXEN_NIC_LINUX_VERSIONID "4.0.30"
+#define _NETXEN_NIC_LINUX_SUBVERSION 41
+#define NETXEN_NIC_LINUX_VERSIONID "4.0.41"
#define NETXEN_VERSION_CODE(a, b, c) (((a) << 24) + ((b) << 16) + (c))
#define _major(v) (((v) >> 24) & 0xff)
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 11/12] netxen: support for ethtool set ringparam
From: Dhananjay Phadke @ 2009-07-27 6:07 UTC (permalink / raw)
To: davem; +Cc: netdev, Amit Kumar Salecha
In-Reply-To: <1248674867-18821-1-git-send-email-dhananjay@netxen.com>
Add support for ethtool -G to tune rx and tx ring sizes
per interface basis.
This is only supported for NX3031 based cards.
Signed-off-by: Amit Kumar Salecha <amit@netxen.com>
Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
drivers/net/netxen/netxen_nic.h | 4 +++
drivers/net/netxen/netxen_nic_ethtool.c | 44 +++++++++++++++++++++++++++++-
drivers/net/netxen/netxen_nic_main.c | 45 ++++++++++++++++++++++++-------
3 files changed, 82 insertions(+), 11 deletions(-)
diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index d01cc09..9527f31 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -1427,6 +1427,10 @@ struct net_device_stats *netxen_nic_get_stats(struct net_device *netdev);
void netxen_nic_update_cmd_producer(struct netxen_adapter *adapter,
struct nx_host_tx_ring *tx_ring);
+/* Functions from netxen_nic_main.c */
+u32 netxen_validate_ringparam(u32 val, u32 min, u32 max, char *);
+int netxen_nic_reset_context(struct netxen_adapter *);
+
/*
* NetXen Board information
*/
diff --git a/drivers/net/netxen/netxen_nic_ethtool.c b/drivers/net/netxen/netxen_nic_ethtool.c
index 80d286d..ec93aa7 100644
--- a/drivers/net/netxen/netxen_nic_ethtool.c
+++ b/drivers/net/netxen/netxen_nic_ethtool.c
@@ -490,7 +490,8 @@ netxen_nic_get_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom,
}
static void
-netxen_nic_get_ringparam(struct net_device *dev, struct ethtool_ringparam *ring)
+netxen_nic_get_ringparam(struct net_device *dev,
+ struct ethtool_ringparam *ring)
{
struct netxen_adapter *adapter = netdev_priv(dev);
@@ -513,6 +514,46 @@ netxen_nic_get_ringparam(struct net_device *dev, struct ethtool_ringparam *ring)
ring->rx_mini_pending = 0;
}
+static int
+netxen_nic_set_ringparam(struct net_device *dev,
+ struct ethtool_ringparam *ring)
+{
+ struct netxen_adapter *adapter = netdev_priv(dev);
+ u16 max_rcv_desc = MAX_RCV_DESCRIPTORS_10G;
+ u16 max_jumbo_desc = MAX_JUMBO_RCV_DESCRIPTORS_10G;
+ u16 num_rxd, num_jumbo_rxd, num_txd;
+
+ if (NX_IS_REVISION_P2(adapter->ahw.revision_id))
+ return -EOPNOTSUPP;
+
+ if (ring->rx_mini_pending)
+ return -EOPNOTSUPP;
+
+ if (adapter->ahw.port_type == NETXEN_NIC_GBE) {
+ max_rcv_desc = MAX_RCV_DESCRIPTORS_1G;
+ max_jumbo_desc = MAX_JUMBO_RCV_DESCRIPTORS_10G;
+ }
+
+ num_rxd = netxen_validate_ringparam(ring->rx_pending,
+ MIN_RCV_DESCRIPTORS, max_rcv_desc, "rx");
+
+ num_jumbo_rxd = netxen_validate_ringparam(ring->rx_jumbo_pending,
+ MIN_JUMBO_DESCRIPTORS, max_jumbo_desc, "rx jumbo");
+
+ num_txd = netxen_validate_ringparam(ring->tx_pending,
+ MIN_CMD_DESCRIPTORS, MAX_CMD_DESCRIPTORS, "tx");
+
+ if (num_rxd == adapter->num_rxd && num_txd == adapter->num_txd &&
+ num_jumbo_rxd == adapter->num_jumbo_rxd)
+ return 0;
+
+ adapter->num_rxd = num_rxd;
+ adapter->num_jumbo_rxd = num_jumbo_rxd;
+ adapter->num_txd = num_txd;
+
+ return netxen_nic_reset_context(adapter);
+}
+
static void
netxen_nic_get_pauseparam(struct net_device *dev,
struct ethtool_pauseparam *pause)
@@ -894,6 +935,7 @@ struct ethtool_ops netxen_nic_ethtool_ops = {
.get_eeprom_len = netxen_nic_get_eeprom_len,
.get_eeprom = netxen_nic_get_eeprom,
.get_ringparam = netxen_nic_get_ringparam,
+ .set_ringparam = netxen_nic_set_ringparam,
.get_pauseparam = netxen_nic_get_pauseparam,
.set_pauseparam = netxen_nic_set_pauseparam,
.set_tx_csum = ethtool_op_set_tx_csum,
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 41ab99d..894b1fd 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -930,6 +930,9 @@ netxen_nic_attach(struct netxen_adapter *adapter)
struct nx_host_rds_ring *rds_ring;
struct nx_host_tx_ring *tx_ring;
+ if (adapter->is_up == NETXEN_ADAPTER_UP_MAGIC)
+ return 0;
+
err = netxen_init_firmware(adapter);
if (err != 0) {
printk(KERN_ERR "Failed to init firmware\n");
@@ -993,6 +996,9 @@ err_out_free_sw:
static void
netxen_nic_detach(struct netxen_adapter *adapter)
{
+ if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+ return;
+
netxen_free_hw_resources(adapter);
netxen_release_rx_buffers(adapter);
netxen_nic_free_irq(adapter);
@@ -1001,6 +1007,30 @@ netxen_nic_detach(struct netxen_adapter *adapter)
adapter->is_up = 0;
}
+int
+netxen_nic_reset_context(struct netxen_adapter *adapter)
+{
+ int err = 0;
+ struct net_device *netdev = adapter->netdev;
+
+ if (adapter->is_up == NETXEN_ADAPTER_UP_MAGIC) {
+
+ if (netif_running(netdev))
+ netxen_nic_down(adapter, netdev);
+
+ netxen_nic_detach(adapter);
+
+ err = netxen_nic_attach(adapter);
+ if (err)
+ goto done;
+
+ if (netif_running(netdev))
+ err = netxen_nic_up(adapter, netdev);
+ }
+done:
+ return err;
+}
+
static int
netxen_setup_netdev(struct netxen_adapter *adapter,
struct net_device *netdev)
@@ -1222,9 +1252,7 @@ static void __devexit netxen_nic_remove(struct pci_dev *pdev)
unregister_netdev(netdev);
- if (adapter->is_up == NETXEN_ADAPTER_UP_MAGIC) {
- netxen_nic_detach(adapter);
- }
+ netxen_nic_detach(adapter);
if (adapter->portnum == 0)
netxen_free_dummy_dma(adapter);
@@ -1256,8 +1284,7 @@ netxen_nic_suspend(struct pci_dev *pdev, pm_message_t state)
if (netif_running(netdev))
netxen_nic_down(adapter, netdev);
- if (adapter->is_up == NETXEN_ADAPTER_UP_MAGIC)
- netxen_nic_detach(adapter);
+ netxen_nic_detach(adapter);
pci_save_state(pdev);
@@ -1318,11 +1345,9 @@ static int netxen_nic_open(struct net_device *netdev)
if (adapter->driver_mismatch)
return -EIO;
- if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC) {
- err = netxen_nic_attach(adapter);
- if (err)
- return err;
- }
+ err = netxen_nic_attach(adapter);
+ if (err)
+ return err;
err = netxen_nic_up(adapter, netdev);
if (err)
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 08/12] netxen: refactor net_device setup code
From: Dhananjay Phadke @ 2009-07-27 6:07 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1248674867-18821-1-git-send-email-dhananjay@netxen.com>
Move all net_device initialization into one function
netxen_setup_netdev().
Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
drivers/net/netxen/netxen_nic_main.c | 138 ++++++++++++++++++---------------
1 files changed, 75 insertions(+), 63 deletions(-)
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 8d4c30a..8a4e6ce 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -86,7 +86,7 @@ static int netxen_nic_open(struct net_device *netdev);
static int netxen_nic_close(struct net_device *netdev);
static int netxen_nic_xmit_frame(struct sk_buff *, struct net_device *);
static void netxen_tx_timeout(struct net_device *netdev);
-static void netxen_tx_timeout_task(struct work_struct *work);
+static void netxen_reset_task(struct work_struct *work);
static void netxen_watchdog(unsigned long);
static int netxen_nic_poll(struct napi_struct *napi, int budget);
#ifdef CONFIG_NET_POLL_CONTROLLER
@@ -202,7 +202,7 @@ netxen_napi_add(struct netxen_adapter *adapter, struct net_device *netdev)
struct netxen_recv_context *recv_ctx = &adapter->recv_ctx;
if (netxen_alloc_sds_rings(recv_ctx, adapter->max_sds_rings))
- return 1;
+ return -ENOMEM;
for (ring = 0; ring < adapter->max_sds_rings; ring++) {
sds_ring = &recv_ctx->sds_rings[ring];
@@ -1001,6 +1001,68 @@ netxen_nic_detach(struct netxen_adapter *adapter)
adapter->is_up = 0;
}
+static int
+netxen_setup_netdev(struct netxen_adapter *adapter,
+ struct net_device *netdev)
+{
+ int err = 0;
+ struct pci_dev *pdev = adapter->pdev;
+
+ adapter->rx_csum = 1;
+ adapter->mc_enabled = 0;
+ if (NX_IS_REVISION_P3(adapter->ahw.revision_id))
+ adapter->max_mc_count = 38;
+ else
+ adapter->max_mc_count = 16;
+
+ netdev->netdev_ops = &netxen_netdev_ops;
+ netdev->watchdog_timeo = 2*HZ;
+
+ netxen_nic_change_mtu(netdev, netdev->mtu);
+
+ SET_ETHTOOL_OPS(netdev, &netxen_nic_ethtool_ops);
+
+ netdev->features |= (NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO);
+ netdev->features |= (NETIF_F_GRO);
+ netdev->vlan_features |= (NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO);
+
+ if (NX_IS_REVISION_P3(adapter->ahw.revision_id)) {
+ netdev->features |= (NETIF_F_IPV6_CSUM | NETIF_F_TSO6);
+ netdev->vlan_features |= (NETIF_F_IPV6_CSUM | NETIF_F_TSO6);
+ }
+
+ if (adapter->pci_using_dac) {
+ netdev->features |= NETIF_F_HIGHDMA;
+ netdev->vlan_features |= NETIF_F_HIGHDMA;
+ }
+
+ netdev->irq = adapter->msix_entries[0].vector;
+
+ err = netxen_napi_add(adapter, netdev);
+ if (err)
+ return err;
+
+ init_timer(&adapter->watchdog_timer);
+ adapter->watchdog_timer.function = &netxen_watchdog;
+ adapter->watchdog_timer.data = (unsigned long)adapter;
+ INIT_WORK(&adapter->watchdog_task, netxen_watchdog_task);
+ INIT_WORK(&adapter->tx_timeout_task, netxen_reset_task);
+
+ if (netxen_read_mac_addr(adapter))
+ dev_warn(&pdev->dev, "failed to read mac addr\n");
+
+ netif_carrier_off(netdev);
+ netif_stop_queue(netdev);
+
+ err = register_netdev(netdev);
+ if (err) {
+ dev_err(&pdev->dev, "failed to register net device\n");
+ return err;
+ }
+
+ return 0;
+}
+
static int __devinit
netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
@@ -1038,9 +1100,8 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
netdev = alloc_etherdev(sizeof(struct netxen_adapter));
if(!netdev) {
- printk(KERN_ERR"%s: Failed to allocate memory for the "
- "device block.Check system memory resource"
- " usage.\n", netxen_nic_driver_name);
+ dev_err(&pdev->dev, "failed to allocate net_device\n");
+ err = -ENOMEM;
goto err_out_free_res;
}
@@ -1068,38 +1129,10 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* This will be reset for mezz cards */
adapter->portnum = pci_func_id;
- adapter->rx_csum = 1;
- adapter->mc_enabled = 0;
- if (NX_IS_REVISION_P3(revision_id))
- adapter->max_mc_count = 38;
- else
- adapter->max_mc_count = 16;
-
- netdev->netdev_ops = &netxen_netdev_ops;
- netdev->watchdog_timeo = 2*HZ;
-
- netxen_nic_change_mtu(netdev, netdev->mtu);
-
- SET_ETHTOOL_OPS(netdev, &netxen_nic_ethtool_ops);
-
- netdev->features |= (NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO);
- netdev->features |= (NETIF_F_GRO);
- netdev->vlan_features |= (NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO);
-
- if (NX_IS_REVISION_P3(revision_id)) {
- netdev->features |= (NETIF_F_IPV6_CSUM | NETIF_F_TSO6);
- netdev->vlan_features |= (NETIF_F_IPV6_CSUM | NETIF_F_TSO6);
- }
-
- if (adapter->pci_using_dac) {
- netdev->features |= NETIF_F_HIGHDMA;
- netdev->vlan_features |= NETIF_F_HIGHDMA;
- }
- if (netxen_nic_get_board_info(adapter) != 0) {
- printk("%s: Error getting board config info.\n",
- netxen_nic_driver_name);
- err = -EIO;
+ err = netxen_nic_get_board_info(adapter);
+ if (err) {
+ dev_err(&pdev->dev, "Error getting board config info.\n");
goto err_out_iounmap;
}
@@ -1119,6 +1152,7 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err = netxen_start_firmware(adapter, 1);
if (err)
goto err_out_iounmap;
+
/*
* See if the firmware gave us a virtual-physical port mapping.
*/
@@ -1133,31 +1167,9 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
netxen_setup_intr(adapter);
- netdev->irq = adapter->msix_entries[0].vector;
-
- if (netxen_napi_add(adapter, netdev))
- goto err_out_disable_msi;
-
- init_timer(&adapter->watchdog_timer);
- adapter->watchdog_timer.function = &netxen_watchdog;
- adapter->watchdog_timer.data = (unsigned long)adapter;
- INIT_WORK(&adapter->watchdog_task, netxen_watchdog_task);
- INIT_WORK(&adapter->tx_timeout_task, netxen_tx_timeout_task);
-
- err = netxen_read_mac_addr(adapter);
+ err = netxen_setup_netdev(adapter, netdev);
if (err)
- dev_warn(&pdev->dev, "failed to read mac addr\n");
-
- netif_carrier_off(netdev);
- netif_stop_queue(netdev);
-
- if ((err = register_netdev(netdev))) {
- printk(KERN_ERR "%s: register_netdev failed port #%d"
- " aborting\n", netxen_nic_driver_name,
- adapter->portnum);
- err = -EIO;
goto err_out_disable_msi;
- }
pci_set_drvdata(pdev, adapter);
@@ -1676,10 +1688,13 @@ static void netxen_tx_timeout(struct net_device *netdev)
{
struct netxen_adapter *adapter = (struct netxen_adapter *)
netdev_priv(netdev);
+
+ dev_err(&netdev->dev, "transmit timeout, resetting.\n");
+
SCHEDULE_WORK(&adapter->tx_timeout_task);
}
-static void netxen_tx_timeout_task(struct work_struct *work)
+static void netxen_reset_task(struct work_struct *work)
{
struct netxen_adapter *adapter =
container_of(work, struct netxen_adapter, tx_timeout_task);
@@ -1687,9 +1702,6 @@ static void netxen_tx_timeout_task(struct work_struct *work)
if (!netif_running(adapter->netdev))
return;
- printk(KERN_ERR "%s %s: transmit timeout, resetting.\n",
- netxen_nic_driver_name, adapter->netdev->name);
-
netxen_napi_disable(adapter);
adapter->netdev->trans_start = jiffies;
--
1.6.0.2
^ permalink raw reply related
* [PATCH NEXT 05/12] netxen: refactor tso code
From: Dhananjay Phadke @ 2009-07-27 6:07 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1248674867-18821-1-git-send-email-dhananjay@netxen.com>
o move all tso / checksum offload code into netxen_tso_check().
o optimize the tso header copy into simple loop.
o clean up unnecessary unions from cmd_desc_type0 struct.
Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
drivers/net/netxen/netxen_nic.h | 51 +++---------
drivers/net/netxen/netxen_nic_main.c | 152 ++++++++++++++++------------------
2 files changed, 84 insertions(+), 119 deletions(-)
diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index 3368af0..3a4cbd5 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -316,56 +316,29 @@ struct netxen_ring_ctx {
cpu_to_le16(((_flags) & 0x7f) | (((_opcode) & 0x3f) << 7))
#define netxen_set_tx_frags_len(_desc, _frags, _len) \
- (_desc)->num_of_buffers_total_length = \
+ (_desc)->nfrags__length = \
cpu_to_le32(((_frags) & 0xff) | (((_len) & 0xffffff) << 8))
struct cmd_desc_type0 {
u8 tcp_hdr_offset; /* For LSO only */
u8 ip_hdr_offset; /* For LSO only */
- /* Bit pattern: 0-6 flags, 7-12 opcode, 13-15 unused */
- __le16 flags_opcode;
- /* Bit pattern: 0-7 total number of segments,
- 8-31 Total size of the packet */
- __le32 num_of_buffers_total_length;
- union {
- struct {
- __le32 addr_low_part2;
- __le32 addr_high_part2;
- };
- __le64 addr_buffer2;
- };
+ __le16 flags_opcode; /* 15:13 unused, 12:7 opcode, 6:0 flags */
+ __le32 nfrags__length; /* 31:8 total len, 7:0 frag count */
+
+ __le64 addr_buffer2;
- __le16 reference_handle; /* changed to u16 to add mss */
- __le16 mss; /* passed by NDIS_PACKET for LSO */
- /* Bit pattern 0-3 port, 0-3 ctx id */
- u8 port_ctxid;
+ __le16 reference_handle;
+ __le16 mss;
+ u8 port_ctxid; /* 7:4 ctxid 3:0 port */
u8 total_hdr_length; /* LSO only : MAC+IP+TCP Hdr size */
__le16 conn_id; /* IPSec offoad only */
- union {
- struct {
- __le32 addr_low_part3;
- __le32 addr_high_part3;
- };
- __le64 addr_buffer3;
- };
- union {
- struct {
- __le32 addr_low_part1;
- __le32 addr_high_part1;
- };
- __le64 addr_buffer1;
- };
+ __le64 addr_buffer3;
+ __le64 addr_buffer1;
__le16 buffer_length[4];
- union {
- struct {
- __le32 addr_low_part4;
- __le32 addr_high_part4;
- };
- __le64 addr_buffer4;
- };
+ __le64 addr_buffer4;
__le64 unused;
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index fb976cb..381bd8f 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -1310,13 +1310,18 @@ static int netxen_nic_close(struct net_device *netdev)
return 0;
}
-static bool netxen_tso_check(struct net_device *netdev,
- struct cmd_desc_type0 *desc, struct sk_buff *skb)
+static void
+netxen_tso_check(struct net_device *netdev,
+ struct nx_host_tx_ring *tx_ring,
+ struct cmd_desc_type0 *first_desc,
+ struct sk_buff *skb)
{
- bool tso = false;
u8 opcode = TX_ETHER_PKT;
__be16 protocol = skb->protocol;
u16 flags = 0;
+ u32 producer;
+ int copied, offset, copy_len, hdr_len = 0, tso = 0;
+ struct cmd_desc_type0 *hwdesc;
if (protocol == cpu_to_be16(ETH_P_8021Q)) {
struct vlan_ethhdr *vh = (struct vlan_ethhdr *)skb->data;
@@ -1327,13 +1332,14 @@ static bool netxen_tso_check(struct net_device *netdev,
if ((netdev->features & (NETIF_F_TSO | NETIF_F_TSO6)) &&
skb_shinfo(skb)->gso_size > 0) {
- desc->mss = cpu_to_le16(skb_shinfo(skb)->gso_size);
- desc->total_hdr_length =
- skb_transport_offset(skb) + tcp_hdrlen(skb);
+ hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
+
+ first_desc->mss = cpu_to_le16(skb_shinfo(skb)->gso_size);
+ first_desc->total_hdr_length = hdr_len;
opcode = (protocol == cpu_to_be16(ETH_P_IPV6)) ?
TX_TCP_LSO6 : TX_TCP_LSO;
- tso = true;
+ tso = 1;
} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
u8 l4proto;
@@ -1354,10 +1360,39 @@ static bool netxen_tso_check(struct net_device *netdev,
opcode = TX_UDPV6_PKT;
}
}
- desc->tcp_hdr_offset = skb_transport_offset(skb);
- desc->ip_hdr_offset = skb_network_offset(skb);
- netxen_set_tx_flags_opcode(desc, flags, opcode);
- return tso;
+ first_desc->tcp_hdr_offset = skb_transport_offset(skb);
+ first_desc->ip_hdr_offset = skb_network_offset(skb);
+ netxen_set_tx_flags_opcode(first_desc, flags, opcode);
+
+ if (!tso)
+ return;
+
+ /* For LSO, we need to copy the MAC/IP/TCP headers into
+ * the descriptor ring
+ */
+ producer = tx_ring->producer;
+ copied = 0;
+ offset = 2;
+
+ while (copied < hdr_len) {
+
+ copy_len = min((int)sizeof(struct cmd_desc_type0) - offset,
+ (hdr_len - copied));
+
+ hwdesc = &tx_ring->desc_head[producer];
+ tx_ring->cmd_buf_arr[producer].skb = NULL;
+
+ skb_copy_from_linear_data_offset(skb, copied,
+ (char *)hwdesc + offset, copy_len);
+
+ copied += copy_len;
+ offset = 0;
+
+ producer = get_next_index(producer, tx_ring->num_desc);
+ }
+
+ tx_ring->producer = producer;
+ barrier();
}
static void
@@ -1381,9 +1416,8 @@ netxen_clean_tx_dma_mapping(struct pci_dev *pdev,
static inline void
netxen_clear_cmddesc(u64 *desc)
{
- int i;
- for (i = 0; i < 8; i++)
- desc[i] = 0ULL;
+ desc[0] = 0ULL;
+ desc[2] = 0ULL;
}
static int
@@ -1391,18 +1425,18 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
{
struct netxen_adapter *adapter = netdev_priv(netdev);
struct nx_host_tx_ring *tx_ring = adapter->tx_ring;
- unsigned int first_seg_len = skb->len - skb->data_len;
+ struct skb_frag_struct *frag;
struct netxen_cmd_buffer *pbuf;
struct netxen_skb_frag *buffrag;
- struct cmd_desc_type0 *hwdesc;
- struct pci_dev *pdev = adapter->pdev;
+ struct cmd_desc_type0 *hwdesc, *first_desc;
+ struct pci_dev *pdev;
dma_addr_t temp_dma;
int i, k;
+ unsigned long offset;
u32 producer;
- int frag_count, no_of_desc;
+ int len, frag_count, no_of_desc;
u32 num_txd = tx_ring->num_desc;
- bool is_tso = false;
frag_count = skb_shinfo(skb)->nr_frags + 1;
@@ -1416,32 +1450,30 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
producer = tx_ring->producer;
- hwdesc = &tx_ring->desc_head[producer];
- netxen_clear_cmddesc((u64 *)hwdesc);
- pbuf = &tx_ring->cmd_buf_arr[producer];
+ pdev = adapter->pdev;
+ len = skb->len - skb->data_len;
- is_tso = netxen_tso_check(netdev, hwdesc, skb);
+ temp_dma = pci_map_single(pdev, skb->data, len, PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(pdev, temp_dma))
+ goto drop_packet;
+ pbuf = &tx_ring->cmd_buf_arr[producer];
pbuf->skb = skb;
pbuf->frag_count = frag_count;
- buffrag = &pbuf->frag_array[0];
- temp_dma = pci_map_single(pdev, skb->data, first_seg_len,
- PCI_DMA_TODEVICE);
- if (pci_dma_mapping_error(pdev, temp_dma))
- goto drop_packet;
+ buffrag = &pbuf->frag_array[0];
buffrag->dma = temp_dma;
- buffrag->length = first_seg_len;
+ buffrag->length = len;
+
+ first_desc = hwdesc = &tx_ring->desc_head[producer];
+ netxen_clear_cmddesc((u64 *)hwdesc);
netxen_set_tx_frags_len(hwdesc, frag_count, skb->len);
netxen_set_tx_port(hwdesc, adapter->portnum);
- hwdesc->buffer_length[0] = cpu_to_le16(first_seg_len);
- hwdesc->addr_buffer1 = cpu_to_le64(buffrag->dma);
+ hwdesc->buffer_length[0] = cpu_to_le16(len);
+ hwdesc->addr_buffer1 = cpu_to_le64(temp_dma);
for (i = 1, k = 1; i < frag_count; i++, k++) {
- struct skb_frag_struct *frag;
- int len, temp_len;
- unsigned long offset;
/* move to next desc. if there is a need */
if ((i & 0x3) == 0) {
@@ -1452,11 +1484,11 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
pbuf = &tx_ring->cmd_buf_arr[producer];
pbuf->skb = NULL;
}
+ buffrag = &pbuf->frag_array[i];
frag = &skb_shinfo(skb)->frags[i - 1];
len = frag->size;
offset = frag->page_offset;
- temp_len = len;
temp_dma = pci_map_page(pdev, frag->page, offset,
len, PCI_DMA_TODEVICE);
if (pci_dma_mapping_error(pdev, temp_dma)) {
@@ -1464,11 +1496,10 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
goto drop_packet;
}
- buffrag++;
buffrag->dma = temp_dma;
- buffrag->length = temp_len;
+ buffrag->length = len;
- hwdesc->buffer_length[k] = cpu_to_le16(temp_len);
+ hwdesc->buffer_length[k] = cpu_to_le16(len);
switch (k) {
case 0:
hwdesc->addr_buffer1 = cpu_to_le64(temp_dma);
@@ -1483,53 +1514,14 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
hwdesc->addr_buffer4 = cpu_to_le64(temp_dma);
break;
}
- frag++;
}
- producer = get_next_index(producer, num_txd);
+ tx_ring->producer = get_next_index(producer, num_txd);
- /* For LSO, we need to copy the MAC/IP/TCP headers into
- * the descriptor ring
- */
- if (is_tso) {
- int hdr_len, first_hdr_len, more_hdr;
- hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
- if (hdr_len > (sizeof(struct cmd_desc_type0) - 2)) {
- first_hdr_len = sizeof(struct cmd_desc_type0) - 2;
- more_hdr = 1;
- } else {
- first_hdr_len = hdr_len;
- more_hdr = 0;
- }
- /* copy the MAC/IP/TCP headers to the cmd descriptor list */
- hwdesc = &tx_ring->desc_head[producer];
- pbuf = &tx_ring->cmd_buf_arr[producer];
- pbuf->skb = NULL;
-
- /* copy the first 64 bytes */
- memcpy(((void *)hwdesc) + 2,
- (void *)(skb->data), first_hdr_len);
- producer = get_next_index(producer, num_txd);
-
- if (more_hdr) {
- hwdesc = &tx_ring->desc_head[producer];
- pbuf = &tx_ring->cmd_buf_arr[producer];
- pbuf->skb = NULL;
- /* copy the next 64 bytes - should be enough except
- * for pathological case
- */
- skb_copy_from_linear_data_offset(skb, first_hdr_len,
- hwdesc,
- (hdr_len -
- first_hdr_len));
- producer = get_next_index(producer, num_txd);
- }
- }
-
- tx_ring->producer = producer;
- adapter->stats.txbytes += skb->len;
+ netxen_tso_check(netdev, tx_ring, first_desc, skb);
netxen_nic_update_cmd_producer(adapter, tx_ring);
+ adapter->stats.txbytes += skb->len;
adapter->stats.xmitcalled++;
return NETDEV_TX_OK;
--
1.6.0.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox