* [PATCH v2] ipv4: Restore old dst_free() behavior.
From: Eric Dumazet @ 2012-07-31 11:08 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120730.223827.74792864437911339.davem@davemloft.net>
From: Eric Dumazet <edumazet@google.com>
On Mon, 2012-07-30 at 22:38 -0700, David Miller wrote:
> Eric, this is what I'd like to propose.
>
> It seems the problem you were likely running into was simply
> the fact that we were not inserting an RCU grace period for
> the dst_free() that we do when purging a FIB nexthop.
>
> So this reverts your change, and instead adds the necessary
> call_rcu_bh() wrapper around the dst_free() done in fib_semantics.c
>
> That makes it so that we don't need all of that inc_not_zero stuff for
> sockets, and the special dst flag. If we set the pointer to NULL,
> then do the dst_free() via RCU, we can test that refcount safely in
> dst_free() since it can only decrease at that point.
>
> What do you think? Does it pass your tests?
It doesnt.
But I believe I found why, thats the good news ;)
In the past, we used RCU only for input routes, thats why using
call_rcu_bh() was OK.
But now we alse cache output routes, we must use call_rcu(), because
on output path we use rcu_read_lock() 'only', in process context,
not from softirq handler.
If you dont mind, I would like to keep inet_sk_rx_dst_set() helper
because I believe its cleaner and I'll probably add IPv6 stuff on it
(the cookie thing)
Also I added __rcu attributes to nh_rth_output and nh_rth_input to
better document what is going on in this code.
Thanks
[PATCH v2] ipv4: Restore old dst_free() behavior
commit 404e0a8b6a55 (net: ipv4: fix RCU races on dst refcounts) tried
to solve a race but added a problem at device/fib dismantle time :
We really want to call dst_free() as soon as possible, even if sockets
still have dst in their cache.
dst_release() calls in free_fib_info_rcu() are not welcomed.
Root of the problem was that now we also cache output routes (in
nh_rth_output), we must use call_rcu() instead of call_rcu_bh() in
rt_free(), because output route lookups are done in process context.
Based on feedback and initial patch from David Miller (adding another
call_rcu_bh() call in fib, but it appears it was not the right fix)
I left the inet_sk_rx_dst_set() helper and added __rcu attributes
to nh_rth_output and nh_rth_input to better document what is going on in
this code.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/dst.h | 7 ++++++-
include/net/inet_sock.h | 10 +++-------
include/net/ip_fib.h | 4 ++--
net/core/dst.c | 26 +++++---------------------
net/decnet/dn_route.c | 6 ------
net/ipv4/fib_semantics.c | 21 +++++++++++++++++----
net/ipv4/route.c | 26 +++++++++++++++++---------
7 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index 31a9fd3..baf5978 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -61,7 +61,6 @@ struct dst_entry {
#define DST_NOPEER 0x0040
#define DST_FAKE_RTABLE 0x0080
#define DST_XFRM_TUNNEL 0x0100
-#define DST_RCU_FREE 0x0200
unsigned short pending_confirm;
@@ -383,6 +382,12 @@ static inline void dst_free(struct dst_entry *dst)
__dst_free(dst);
}
+static inline void dst_rcu_free(struct rcu_head *head)
+{
+ struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
+ dst_free(dst);
+}
+
static inline void dst_confirm(struct dst_entry *dst)
{
dst->pending_confirm = 1;
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index e3fd34c..83b567f 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -253,13 +253,9 @@ static inline void inet_sk_rx_dst_set(struct sock *sk, const struct sk_buff *skb
{
struct dst_entry *dst = skb_dst(skb);
- if (atomic_inc_not_zero(&dst->__refcnt)) {
- if (!(dst->flags & DST_RCU_FREE))
- dst->flags |= DST_RCU_FREE;
-
- sk->sk_rx_dst = dst;
- inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
- }
+ dst_hold(dst);
+ sk->sk_rx_dst = dst;
+ inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
}
#endif /* _INET_SOCK_H */
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index e69c3a4..e521a03 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -81,8 +81,8 @@ struct fib_nh {
__be32 nh_gw;
__be32 nh_saddr;
int nh_saddr_genid;
- struct rtable *nh_rth_output;
- struct rtable *nh_rth_input;
+ struct rtable __rcu *nh_rth_output;
+ struct rtable __rcu *nh_rth_input;
struct fnhe_hash_bucket *nh_exceptions;
};
diff --git a/net/core/dst.c b/net/core/dst.c
index d9e33eb..069d51d 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -258,15 +258,6 @@ again:
}
EXPORT_SYMBOL(dst_destroy);
-static void dst_rcu_destroy(struct rcu_head *head)
-{
- struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
-
- dst = dst_destroy(dst);
- if (dst)
- __dst_free(dst);
-}
-
void dst_release(struct dst_entry *dst)
{
if (dst) {
@@ -274,14 +265,10 @@ void dst_release(struct dst_entry *dst)
newrefcnt = atomic_dec_return(&dst->__refcnt);
WARN_ON(newrefcnt < 0);
- if (unlikely(dst->flags & (DST_NOCACHE | DST_RCU_FREE)) && !newrefcnt) {
- if (dst->flags & DST_RCU_FREE) {
- call_rcu_bh(&dst->rcu_head, dst_rcu_destroy);
- } else {
- dst = dst_destroy(dst);
- if (dst)
- __dst_free(dst);
- }
+ if (unlikely(dst->flags & DST_NOCACHE) && !newrefcnt) {
+ dst = dst_destroy(dst);
+ if (dst)
+ __dst_free(dst);
}
}
}
@@ -333,14 +320,11 @@ EXPORT_SYMBOL(__dst_destroy_metrics_generic);
*/
void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
{
- bool hold;
-
WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
/* If dst not in cache, we must take a reference, because
* dst_release() will destroy dst as soon as its refcount becomes zero
*/
- hold = (dst->flags & (DST_NOCACHE | DST_RCU_FREE)) == DST_NOCACHE;
- if (unlikely(hold)) {
+ if (unlikely(dst->flags & DST_NOCACHE)) {
dst_hold(dst);
skb_dst_set(skb, dst);
} else {
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 2671977..85a3604 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -184,12 +184,6 @@ static __inline__ unsigned int dn_hash(__le16 src, __le16 dst)
return dn_rt_hash_mask & (unsigned int)tmp;
}
-static inline void dst_rcu_free(struct rcu_head *head)
-{
- struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
- dst_free(dst);
-}
-
static inline void dnrt_free(struct dn_route *rt)
{
call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free);
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index e55171f..625cf18 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -161,6 +161,21 @@ static void free_nh_exceptions(struct fib_nh *nh)
kfree(hash);
}
+static void rt_nexthop_free(struct rtable __rcu **rtp)
+{
+ struct rtable *rt = rcu_dereference_protected(*rtp, 1);
+
+ if (!rt)
+ return;
+
+ /* Not even needed : RCU_INIT_POINTER(*rtp, NULL);
+ * because we waited an RCU grace period before calling
+ * free_fib_info_rcu()
+ */
+
+ dst_free(&rt->dst);
+}
+
/* Release a nexthop info record */
static void free_fib_info_rcu(struct rcu_head *head)
{
@@ -171,10 +186,8 @@ static void free_fib_info_rcu(struct rcu_head *head)
dev_put(nexthop_nh->nh_dev);
if (nexthop_nh->nh_exceptions)
free_nh_exceptions(nexthop_nh);
- if (nexthop_nh->nh_rth_output)
- dst_release(&nexthop_nh->nh_rth_output->dst);
- if (nexthop_nh->nh_rth_input)
- dst_release(&nexthop_nh->nh_rth_input->dst);
+ rt_nexthop_free(&nexthop_nh->nh_rth_output);
+ rt_nexthop_free(&nexthop_nh->nh_rth_input);
} endfor_nexthops(fi);
release_net(fi->fib_net);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d6eabcf..2bd1074 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1199,23 +1199,31 @@ restart:
fnhe->fnhe_stamp = jiffies;
}
+static inline void rt_free(struct rtable *rt)
+{
+ call_rcu(&rt->dst.rcu_head, dst_rcu_free);
+}
+
static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
{
- struct rtable *orig, *prev, **p = &nh->nh_rth_output;
+ struct rtable *orig, *prev, **p = (struct rtable **)&nh->nh_rth_output;
if (rt_is_input_route(rt))
- p = &nh->nh_rth_input;
+ p = (struct rtable **)&nh->nh_rth_input;
orig = *p;
- rt->dst.flags |= DST_RCU_FREE;
- dst_hold(&rt->dst);
prev = cmpxchg(p, orig, rt);
if (prev == orig) {
if (orig)
- dst_release(&orig->dst);
+ rt_free(orig);
} else {
- dst_release(&rt->dst);
+ /* Routes we intend to cache in the FIB nexthop have
+ * the DST_NOCACHE bit clear. However, if we are
+ * unsuccessful at storing this route into the cache
+ * we really need to set it.
+ */
+ rt->dst.flags |= DST_NOCACHE;
}
}
@@ -1412,7 +1420,7 @@ static int __mkroute_input(struct sk_buff *skb,
do_cache = false;
if (res->fi) {
if (!itag) {
- rth = FIB_RES_NH(*res).nh_rth_input;
+ rth = rcu_dereference(FIB_RES_NH(*res).nh_rth_input);
if (rt_cache_valid(rth)) {
skb_dst_set_noref(skb, &rth->dst);
goto out;
@@ -1574,7 +1582,7 @@ local_input:
do_cache = false;
if (res.fi) {
if (!itag) {
- rth = FIB_RES_NH(res).nh_rth_input;
+ rth = rcu_dereference(FIB_RES_NH(res).nh_rth_input);
if (rt_cache_valid(rth)) {
skb_dst_set_noref(skb, &rth->dst);
err = 0;
@@ -1742,7 +1750,7 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
if (fi) {
fnhe = find_exception(&FIB_RES_NH(*res), fl4->daddr);
if (!fnhe) {
- rth = FIB_RES_NH(*res).nh_rth_output;
+ rth = rcu_dereference(FIB_RES_NH(*res).nh_rth_output);
if (rt_cache_valid(rth)) {
dst_hold(&rth->dst);
return rth;
^ permalink raw reply related
* [ANNOUNCE] conntrack-tools 1.2.2 release
From: Pablo Neira Ayuso @ 2012-07-31 11:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, netfilter, netfilter-announce, lwn
[-- Attachment #1: Type: text/plain, Size: 337 bytes --]
Hi!
The Netfilter project proudly presents:
conntrack-tools 1.2.2
This release contains accumulated bugfixes.
See ChangeLog that comes attached to this email for more details.
You can download it from:
http://www.netfilter.org/projects/conntrack-tools/downloads.html
ftp://ftp.netfilter.org/pub/conntrack-tools/
Have fun!
[-- Attachment #2: changes-conntrack-tools-1.2.2.txt --]
[-- Type: text/plain, Size: 485 bytes --]
Jan Engelhardt (1):
update .gitignore
Pablo Neira Ayuso (7):
conntrackd: simplify TCP connection handling logic
conntrackd: fix compilation in src/parse.c
doc: fix documentation on ExpectationSync and H.323 helper
conntrackd: add bugtrap notice in case of flush while commit in progress
conntrackd: fix commit operation, needs to be synchronous
conntrackd: implement selective flushing for `-t' and `-F' commands
bump version to 1.2.2
^ permalink raw reply
* [ANNOUNCE] iptables 1.4.15 release
From: Pablo Neira Ayuso @ 2012-07-31 11:25 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, netfilter, netfilter-announce, lwn
[-- Attachment #1: Type: text/plain, Size: 407 bytes --]
Hi!
The Netfilter project proudly presents:
iptables 1.4.15
This release includes support for new features now present in the
Linux kernel 3.5 and one major bugfix (that shows up with gcc-4.7).
See ChangeLog that comes attached to this email for more details.
You can download it from:
http://www.netfilter.org/projects/iptables/downloads.html
ftp://ftp.netfilter.org/pub/iptables/
Have fun!
[-- Attachment #2: changes-iptables-1.4.15.txt --]
[-- Type: text/plain, Size: 805 bytes --]
Denys Fedoryshchenko (1):
libxt_recent: add --mask netmask
Eldad Zack (1):
libxt_recent: remove unused variable
Florian Westphal (2):
libxt_devgroup: add man page snippet
libxt_hashlimit: add support for byte-based operation
Hans Schillstrom (3):
extensions: add HMARK target
libxt_HMARK: fix output of iptables -L
libxt_HMARK: correct a number of errors introduced by Pablo's rework
Pablo Neira Ayuso (6):
libxtables: add xtables_ip[6]mask_to_cidr
libxt_HMARK: fix ct case example
iptables-restore: move code to add_param_to_argv, cleanup (fix gcc-4.7)
Revert "iptables-restore: move code to add_param_to_argv, cleanup (fix gcc-4.7)"
iptables-restore: fix parameter parsing (shows up with gcc-4.7)
bump version to 1.4.15
^ permalink raw reply
* Re: [PATCH 1/2] net: Allow to create links with given ifindex
From: Eric W. Biederman @ 2012-07-31 11:58 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <50179F66.1000604@parallels.com>
Pavel Emelyanov <xemul@parallels.com> writes:
> On 07/30/2012 02:56 PM, Eric W. Biederman wrote:
>> ebiederm@xmission.com (Eric W. Biederman) writes:
>>
>>> Pavel Emelyanov <xemul@parallels.com> writes:
>>>
>>>> Currently the RTM_NEWLINK results in -EOPNOTSUPP if the ifinfomsg->ifi_index
>>>> is not zero. I propose to allow requesting ifindices on link creation. This
>>>> is required by the checkpoint-restore to correctly restore a net namespace
>>>> (i.e. -- a container). The question what to do with pre-created devices such
>>>> as lo or sit fbdev is open, but for manually created devices this can be
>>>> solved by this patch.
>>>
>>> Have you walked through and found the locations where we still rely on
>>> ifindex being globally unique?
>>>
>>> Last time I was working in this area there were serveral places where
>>> things were indexed by just the interface index.
>>
>> If it is really safe to make ifindex per network namespace at this
>> point you can make dev_new_ifindex have a per network namespace base
>> counter, and that will fix your problems with the loopback device.
>
> Not it's not so unfortunately :(
>
> First, let's imagine that on host A the loopback device got registered as
> first device, but on host B for some reason some other device got registered
> first. In that case after migration from A to B the lo on B will have index
> equals 2. And there's no any strict requirement that lo's per net operations
> are registered first. Please, correct me if I'm wrong.
Actually there is a hard requirement that the loopback device be the
last device in a network namespace to be unregistered. We meet that
requirement by registering the loopback device first
"net/core/dev.c:net_dev_init()".
> Next. In fact, lo is not the only problem. Look at the e.g. sit versus ipgre
> fallback devices. Both gets created on netns creation and obtain whatever
> ifindices are generated for them. Even if we make ifidex per netns chances
> that sit gets registered _strictly_ before ipgre equal zero, since they are
> both modules.
True. However those fallback devices should no longer be needed,
and even if they are I think you can delete and recreate them.
Making lo the particularly interesting case.
>> Unless you have done the work to root out the last of dependencies on
>> ifindex being globally unique I think you will run into some operational
>> problems.
>
> I totally agree with that. Before doing this patch I revisited the ancient
> attempt to make ifindices per netns and checked the issues Dave and you
> discussed then -- I have looked through how the ifindices are used in the
> networking code and found no places where the system-wide uniqueness is still
> required. That's why I proposed this patch for inclusion. If you know the
> places I've missed, please let me know, I will work on it.
I took a quick look and I did not see anything. I saw places under
net/sched/ that looked a bit suspicious, and of course there are places
where we use oif and iff in some of the routing code that make we wonder
a bit. But if you have looked and if I have looked I think we are ok.
> Just an idea -- is it worth moving the possibility to have ifindidces intersect
> under CONFIG_<SOMETHING> (EXPERT/CHECKPOINT_RESTORE) to let wider audience check
> the code in real-life?
I think the best testing we are going to get diversity wise is to create
a per netns counter into dev_new_index when net-next opens up.
Having an ifindex that we can only set at netdevice creation time seems
reasonable.
Eric
^ permalink raw reply
* Re: [PATCH] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Olivier Sobrie @ 2012-07-31 13:06 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <5017ABC6.7030307@pengutronix.de>
On Tue, Jul 31, 2012 at 11:56:22AM +0200, Marc Kleine-Budde wrote:
> On 07/30/2012 03:33 PM, Olivier Sobrie wrote:
> > Hi Marc,
> >
> > On Mon, Jul 30, 2012 at 01:11:46PM +0200, Marc Kleine-Budde wrote:
> >> On 07/30/2012 07:32 AM, Olivier Sobrie wrote:
> >>> This driver provides support for several Kvaser CAN/USB devices.
> >>> Such kind of devices supports up to three can network interfaces.
> >>>
> >>> It has been tested with a Kvaser USB Leaf Light (one network interface)
> >>> connected to a pch_can interface.
> >>> The firmware version of the Kvaser device was 2.5.205.
> >>
> >> Please add linux-usb@vger.kernel.org to Cc for review of the USB part.
> >
> > Ok I'll do it when I send the new version of the patch.
> > But it might be a good idea to add an entry in the MAINTAINERS file so
> > that when someone sends a patch they are aware of this when the
> > get_maintainer script is invoked.
>
> Interesting Idea. We should discuss this here, however we should not
> bother the USB List when sending USB unrelated patches.
>
> >> Please combine .h and .c file. Please make use of netdev_LEVEL() for
> >> error printing, not dev_LEVEL().
> >
> > I'll combine the .c and .h.
> > I used the netdev_LEVEL() everywhere it was possible. It requires to
> > have access to a pointer to netdev which is not always possible;
> > that's the reason why I used dev_LEVEL().
>
> I see, you used it when channel is invalid. So you have obviously no netdev.
Indeed.
>
> >> Please review if all members of the struct kvaser_msg are properly
> >> aligned. You never access the struct kvaser_msg_* members directly, as
> >> they are unaligned. Please check for le16 and le32 access. You missed to
> >> convert the bitrate.
> >
> > Indeed. Thanks. I'll check if I didn't missed another one.
>
> Tnx
>
> >> Please check if your driver survives hot-unplugging while sending and
> >> receiving CAN frames at maximum laod.
> >
> > I tested this with two Kvaser sending frames with "cangen can0 -g 0 -i"
> > never saw a crash.
>
> Please test send sending and receiving at the same time.
Yes that's what I did; "cangen can0 -g 0 -i" on both sides.
>
> >> More comments inline,
> >> regards, Marc
> >>
> >>> List of Kvaser devices supported by the driver:
> >>> - Kvaser Leaf prototype (P010v2 and v3)
> >>> - Kvaser Leaf Light (P010v3)
> >>> - Kvaser Leaf Professional HS
> >>> - Kvaser Leaf SemiPro HS
> >>> - Kvaser Leaf Professional LS
> >>> - Kvaser Leaf Professional SWC
> >>> - Kvaser Leaf Professional LIN
> >>> - Kvaser Leaf SemiPro LS
> >>> - Kvaser Leaf SemiPro SWC
> >>> - Kvaser Memorator II, Prototype
> >>> - Kvaser Memorator II HS/HS
> >>> - Kvaser USBcan Professional HS/HS
> >>> - Kvaser Leaf Light GI
> >>> - Kvaser Leaf Professional HS (OBD-II connector)
> >>> - Kvaser Memorator Professional HS/LS
> >>> - Kvaser Leaf Light "China"
> >>> - Kvaser BlackBird SemiPro
> >>> - Kvaser OEM Mercury
> >>> - Kvaser OEM Leaf
> >>> - Kvaser USBcan R
> >>>
> >>> Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> >>> ---
> >>> drivers/net/can/usb/Kconfig | 33 ++
> >>> drivers/net/can/usb/Makefile | 1 +
> >>> drivers/net/can/usb/kvaser_usb.c | 1062 ++++++++++++++++++++++++++++++++++++++
> >>> drivers/net/can/usb/kvaser_usb.h | 237 +++++++++
> >>> 4 files changed, 1333 insertions(+)
> >>> create mode 100644 drivers/net/can/usb/kvaser_usb.c
> >>> create mode 100644 drivers/net/can/usb/kvaser_usb.h
> >>>
> >>> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
> >>> index 0a68768..578955f 100644
> >>> --- a/drivers/net/can/usb/Kconfig
> >>> +++ b/drivers/net/can/usb/Kconfig
> >>> @@ -13,6 +13,39 @@ config CAN_ESD_USB2
> >>> This driver supports the CAN-USB/2 interface
> >>> from esd electronic system design gmbh (http://www.esd.eu).
> >>>
> >>> +config CAN_KVASER_USB
> >>> + tristate "Kvaser CAN/USB interface"
> >>> + ---help---
> >>> + This driver adds support for Kvaser CAN/USB devices like Kvaser
> >>> + Leaf Light.
> >>> +
> >>> + The driver gives support for the following devices:
> >>> + - Kvaser Leaf prototype (P010v2 and v3)
> >>> + - Kvaser Leaf Light (P010v3)
> >>> + - Kvaser Leaf Professional HS
> >>> + - Kvaser Leaf SemiPro HS
> >>> + - Kvaser Leaf Professional LS
> >>> + - Kvaser Leaf Professional SWC
> >>> + - Kvaser Leaf Professional LIN
> >>> + - Kvaser Leaf SemiPro LS
> >>> + - Kvaser Leaf SemiPro SWC
> >>> + - Kvaser Memorator II, Prototype
> >>> + - Kvaser Memorator II HS/HS
> >>> + - Kvaser USBcan Professional HS/HS
> >>> + - Kvaser Leaf Light GI
> >>> + - Kvaser Leaf Professional HS (OBD-II connector)
> >>> + - Kvaser Memorator Professional HS/LS
> >>> + - Kvaser Leaf Light "China"
> >>> + - Kvaser BlackBird SemiPro
> >>> + - Kvaser OEM Mercury
> >>> + - Kvaser OEM Leaf
> >>> + - Kvaser USBcan R
> >>> +
> >>> + If unsure, say N.
> >>> +
> >>> + To compile this driver as a module, choose M here: the
> >>> + module will be called kvaser_usb.
> >>> +
> >>> config CAN_PEAK_USB
> >>> tristate "PEAK PCAN-USB/USB Pro interfaces"
> >>> ---help---
> >>> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> >>> index da6d1d3..80a2ee4 100644
> >>> --- a/drivers/net/can/usb/Makefile
> >>> +++ b/drivers/net/can/usb/Makefile
> >>> @@ -4,6 +4,7 @@
> >>>
> >>> obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
> >>> obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
> >>> +obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
> >>> obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/
> >>>
> >>> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> >>> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> >>> new file mode 100644
> >>> index 0000000..4965480
> >>> --- /dev/null
> >>> +++ b/drivers/net/can/usb/kvaser_usb.c
> >>> @@ -0,0 +1,1062 @@
> >>> +/*
> >>
> >> Please add a license statement and probably your copyright:
> >>
> >> * 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 version 2.
> >>
> >> You also should copy the copyright from the drivers you used:
> >>
> >>> + * Parts of this driver are based on the following:
> >>> + * - Kvaser linux leaf driver (version 4.78)
> >>
> >> I just downloaded their driver and noticed that it's quite sparse on
> >> stating the license the code is released under.
> >> "doc/HTMLhelp/copyright.htx" is quite restrictive, the word GPL occurs 3
> >> times, all in MODULE_LICENSE("GPL"). Running modinfo on the usbcan.ko
> >> shows "license: GPL"
> >
> > I'll add the license statement.
> > In fact it's the leaf.ko which is used for this device and it is under
> > GPL as modinfo said.
>
> I just talked to my boss and we're the same opinion, that
> MODULE_LICENSE("GPL") is a technical term and not relevant if the
> included license doesn't say a word about GPL. If the kvaser tarball
> violates the GPL, however is written on different sheet of paper (as we
> say in Germany).
>
> So I cannot put my S-o-b under this driver as long as we haven't talked
> to kvaser.
Ok I thought it was sufficient enough to have MODULE_LICENSE("GPL") in
the code to indicate it is a GPL driver. I'll ask Kvaser before sending
any new version of the patch.
>
> >>> + * - CAN driver for esd CAN-USB/2
> >>> + */
> >>> +
> >>> +#include <linux/init.h>
> >>> +#include <linux/completion.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/netdevice.h>
> >>> +#include <linux/usb.h>
> >>> +
> >>> +#include <linux/can.h>
> >>> +#include <linux/can/dev.h>
> >>> +#include <linux/can/error.h>
> >>> +
> >>> +#include "kvaser_usb.h"
> >>> +
> >>> +struct kvaser_usb_tx_urb_context {
> >>> + struct kvaser_usb_net_priv *priv;
> >>
> >> Huh - how does this work without forward declaration?
> >
> > It works.
>
> Yes, obviously :)
>
> > "In C and C++ it is possible to declare pointers to structs before
> > declaring their struct layout, provided the pointers are not
> > dereferenced--this is known as forward declaration."
> >
> > See http://www.linuxtopia.org/online_books/an_introduction_to_gcc/gccintro_94.html
>
> Thanks for the link.
> >>
> >>> + u32 echo_index;
> >>> + int dlc;
> >>> +};
> >>> +
> >>> +struct kvaser_usb {
> >>> + struct usb_device *udev;
> >>> + struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES];
> >>> +
> >>> + struct usb_anchor rx_submitted;
> >>> +
> >>> + u32 fw_version;
> >>> + unsigned int nchannels;
> >>> +
> >>> + bool rxinitdone;
> >>> +};
> >>> +
> >>> +struct kvaser_usb_net_priv {
> >>> + struct can_priv can;
> >>> +
> >>> + atomic_t active_tx_urbs;
> >>> + struct usb_anchor tx_submitted;
> >>> + struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS];
> >>> +
> >>> + int open_time;
> >>
> >> please remove open_time
> >
> > Ok.
> >
> >>
> >>> + struct completion start_stop_comp;
> >>> +
> >>> + struct kvaser_usb *dev;
> >>> + struct net_device *netdev;
> >>> + int channel;
> >>> + struct can_berr_counter bec;
> >>> +};
> >>> +
> >>> +static struct usb_device_id kvaser_usb_table[] = {
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID),
> >>> + .driver_info = KVASER_HAS_SILENT_MODE },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID) },
> >>> + { USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID) },
> >>> + { }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> >>> +
> >>> +static int kvaser_usb_send_msg(const struct kvaser_usb *dev,
> >>> + struct kvaser_msg *msg)
> >> inline?
> >
> > Ok.
> >
> >>> +{
> >>> + int actual_len;
> >>> +
> >>> + return usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, 1),
> >> ^
> >> Can you please introduce a #define for this.
> >
> > Ok. No problem.
> >
> >>
> >>> + msg, msg->len, &actual_len, USB_SEND_TIMEOUT);
> >>> +}
> >>> +
> >>> +static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
> >>> + struct kvaser_msg *msg)
> >>> +{
> >>> + struct kvaser_msg *tmp;
> >>> + void *buf;
> >>> + int actual_len;
> >>> + int err;
> >>> + int pos = 0;
> >>> +
> >>> + buf = kzalloc(RX_BUFFER_SIZE, GFP_KERNEL);
> >>> + if (!buf)
> >>> + return -ENOMEM;
> >>> +
> >>> + err = usb_bulk_msg(dev->udev, usb_rcvbulkpipe(dev->udev, 129),
> >> ^^^
> >> dito
> >
> > Ok too.
> >
> >>
> >>> + buf, RX_BUFFER_SIZE, &actual_len,
> >>> + USB_RECEIVE_TIMEOUT);
> >>> + if (err < 0) {
> >>> + kfree(buf);
> >>> + return err;
> >>> + }
> >>> +
> >>> + while (pos < actual_len) {
> >>
> >> Please check that pos + sizeof(*msg) is < actual_len, as you fill access
> >> it later.
> >
> > I'll instead perform a check on 'pos + tmp->len < actual_len' and copy
> > only tmp->len instead of sizeof(*msg).
> > Thanks.
>
> Even better, saves some bytes to be copied. Take care not to deref tmp,
> unless you checked that tmp is in valid memory.
Ok. I changed the loop by 'while (pos <= actual_len - MSG_HEADER_LEN)'
and then perform the check on 'pos + tmp->len < actual_len'.
>
> >>
> >>> + tmp = buf + pos;
> >>> +
> >>> + if (!tmp->len)
> >>> + break;
> >>> +
> >>> + if (tmp->id == id) {
> >>> + memcpy(msg, tmp, sizeof(*msg));
> >>> + kfree(buf);
> >>> + return 0;
> >>> + }
> >>> +
> >>> + pos += tmp->len;
> >>> + }
> >>> +
> >>> + kfree(buf);
> >>> +
> >>> + return -EINVAL;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
> >>> + u8 msg_id, int channel)
> >>> +{
> >>> + struct kvaser_msg msg;
> >>> + int err;
> >>> +
> >>> + msg.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
> >>> + msg.id = msg_id;
> >>> + msg.u.simple.channel = channel;
> >>> + msg.u.simple.tid = 0xff;
> >>
> >> Please use C99 struct initializer.
> >>
> >> struct kvaser_msg msg = {
> >> .len = ,
> >> .id =,
> >> };
> >
> > Ok.
> >
> >>
> >>
> >>> +
> >>> + err = kvaser_usb_send_msg(dev, &msg);
> >>
> >> just:
> >> return err;
> >
> > Ok.
> >
> >>
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> >>> +{
> >>> + struct kvaser_msg msg;
> >>> + int err;
> >>> +
> >>> + err = kvaser_usb_send_simple_msg(dev, CMD_GET_SOFTWARE_INFO, 0);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + err = kvaser_usb_wait_msg(dev, CMD_GET_SOFTWARE_INFO_REPLY, &msg);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> >>> +{
> >>> + struct kvaser_msg msg;
> >>> + int err;
> >>> +
> >>> + err = kvaser_usb_send_simple_msg(dev, CMD_GET_CARD_INFO, 0);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + err = kvaser_usb_wait_msg(dev, CMD_GET_CARD_INFO_REPLY, &msg);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + dev->nchannels = msg.u.cardinfo.nchannels;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> >>> + const struct kvaser_msg *msg)
> >>> +{
> >>> + struct net_device_stats *stats;
> >>> + struct kvaser_usb_tx_urb_context *context;
> >>> + struct kvaser_usb_net_priv *priv;
> >>> + u8 channel = msg->u.tx_acknowledge.channel;
> >>> + u8 tid = msg->u.tx_acknowledge.tid;
> >>> +
> >>> + if (channel >= dev->nchannels) {
> >>> + dev_err(dev->udev->dev.parent,
> >>> + "Invalid channel number (%d)\n", channel);
> >>> + return;
> >>> + }
> >>
> >> can you do a check for (channel >= dev->nchannels), in a central place?
> >> e.g. kvaser_usb_handle_message()?
> >
> > The problem is that channel is not always at the same place in the
> > messages I get from the hardware. 'tid' and 'channel' are inverted for
> > tx and rx frames.
> > e.g.
>
> Grr...who's written that firmware :D
>
> >
> > struct kvaser_msg_tx_can {
> > u8 channel;
> > u8 tid;
> > u8 msg[14];
> > u8 padding;
> > u8 flags;
> > } __packed;
> >
> > struct kvaser_msg_busparams {
> > u8 tid;
> > u8 channel;
> > __le32 bitrate;
> > u8 tseg1;
> > u8 tseg2;
> > u8 sjw;
> > u8 no_samp;
> > } __packed;
> >
> >>
> >>> +
> >>> + priv = dev->nets[channel];
> >>> +
> >>> + if (!netif_device_present(priv->netdev))
> >>> + return;
> >>> +
> >>> + stats = &priv->netdev->stats;
> >>> +
> >>> + context = &priv->tx_contexts[tid % MAX_TX_URBS];
> >>> +
> >>> + /*
> >>> + * It looks like the firmware never sets the flags field of the
> >>> + * tx_acknowledge frame and never reports a transmit failure.
> >>> + * If the can message can't be transmited (e.g. incompatible
> >>> + * bitrates), a frame CMD_CAN_ERROR_EVENT is sent (with a null
> >>> + * tid) and the firmware tries to transmit again the packet until
> >>> + * it succeeds. Once the packet is successfully transmitted, then
> >>> + * the tx_acknowledge frame is sent.
> >>> + */
> >>> +
> >>> + stats->tx_packets++;
> >>> + stats->tx_bytes += context->dlc;
> >>> + can_get_echo_skb(priv->netdev, context->echo_index);
> >>> +
> >>> + context->echo_index = MAX_TX_URBS;
> >>> + atomic_dec(&priv->active_tx_urbs);
> >>> +
> >>> + netif_wake_queue(priv->netdev);
> >>> +}
> >>> +
> >>> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >>> + const struct kvaser_msg *msg)
> >>> +{
> >>> + struct can_frame *cf;
> >>> + struct sk_buff *skb;
> >>> + struct net_device_stats *stats;
> >>> + struct kvaser_usb_net_priv *priv;
> >>> + u8 channel, status, txerr, rxerr;
> >>> +
> >>> + if (msg->id == CMD_CAN_ERROR_EVENT) {
> >>> + channel = msg->u.error_event.channel;
> >>> + status = msg->u.error_event.status;
> >>> + txerr = msg->u.error_event.tx_errors_count;
> >>> + rxerr = msg->u.error_event.rx_errors_count;
> >>> + } else {
> >>> + channel = msg->u.chip_state_event.channel;
> >>> + status = msg->u.chip_state_event.status;
> >>> + txerr = msg->u.chip_state_event.tx_errors_count;
> >>> + rxerr = msg->u.chip_state_event.rx_errors_count;
> >>> + }
> >>> +
> >>> + if (channel >= dev->nchannels) {
> >>> + dev_err(dev->udev->dev.parent,
> >>> + "Invalid channel number (%d)\n", channel);
> >>> + return;
> >>> + }
> >>> +
> >>> + priv = dev->nets[channel];
> >>> + stats = &priv->netdev->stats;
> >>> +
> >>> + skb = alloc_can_err_skb(priv->netdev, &cf);
> >>> + if (!skb) {
> >>> + stats->rx_dropped++;
> >>> + return;
> >>> + }
> >>> +
> >>> + if ((status & M16C_STATE_BUS_OFF) ||
> >>> + (status & M16C_STATE_BUS_RESET)) {
> >>> + priv->can.state = CAN_STATE_BUS_OFF;
> >>> + cf->can_id |= CAN_ERR_BUSOFF;
> >>> + can_bus_off(priv->netdev);
>
> you should increment priv->can.can_stats.bus_off
> What does the firmware do in this state? Does it automatically try to
> recover and try to send the outstanding frames?
Yes that's what I observed.
>
> If so, you should turn of the CAN interface, it possible. See:
> http://lxr.free-electrons.com/source/drivers/net/can/at91_can.c#L986
Ok I'll have a look at this.
>
> Please test Bus-Off behaviour:
> - setup working CAN network
> - short circuit CAN-H and CAN-L wires
> - start "candump any,0:0,#FFFFFFFF" on one shell
> - send one can frame on the other
>
> then
>
> - remove the short circuit
> - see if the can frame is transmitted to the other side
> - it should show up as an echo'ed CAN frame on the sender side
>
> Repeat the same test with disconnecting CAN-H and CAN-L from the other
> CAN station instead of short circuit.
>
> Please send the output from candump.
1) With the short circuit:
I perform the test you described. It showed that the Kvaser passes from
ERROR-WARNING to ERROR-PASSIVE and then BUS-OFF. But after going to the
state BUS-OFF it comes back to ERROR-WARNING.
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
...
can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME <-- bus off
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
...
can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
can1 123 [2] 11 22 <-- short circuit removed
I see the echo and on the other end I see the frame coming in.
By the way I see that the txerr and rxerr fields of the structure
kvaser_msg_error_event stay at 0.
2) With CAN-H and CAN-L disconnected:
I never see the bus going in OFF state. It stays in PASSIVE mode.
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
...
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
can1 123 [2] 11 22 <-- other end connected
>
> >>> + } else if (status & M16C_STATE_BUS_ERROR) {
> >>> + priv->can.state = CAN_STATE_ERROR_WARNING;
> >>> + priv->can.can_stats.error_warning++;
> >>> + } else if (status & M16C_STATE_BUS_PASSIVE) {
> >>> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
> >>> + priv->can.can_stats.error_passive++;
> >>> + } else {
> >>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >>> + cf->can_id |= CAN_ERR_PROT;
> >>> + cf->data[2] = CAN_ERR_PROT_ACTIVE;
> >>> + }
> >>> +
> >>> + if (msg->id == CMD_CAN_ERROR_EVENT) {
> >>> + u8 error_factor = msg->u.error_event.error_factor;
> >>> +
> >>> + priv->can.can_stats.bus_error++;
> >>> + stats->rx_errors++;
> >>> +
> >>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> >>> +
> >>> + if ((priv->can.state == CAN_STATE_ERROR_WARNING) ||
> >>> + (priv->can.state == CAN_STATE_ERROR_PASSIVE)) {
> >>> + cf->data[1] = (txerr > rxerr) ?
> >>> + CAN_ERR_CRTL_TX_PASSIVE
> >>> + : CAN_ERR_CRTL_RX_PASSIVE;
> >>> + }
> >>> +
> >>> + if (error_factor & M16C_EF_ACKE)
> >>> + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK |
> >>> + CAN_ERR_PROT_LOC_ACK_DEL);
> >>> + if (error_factor & M16C_EF_CRCE)
> >>> + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> >>> + CAN_ERR_PROT_LOC_CRC_DEL);
> >>> + if (error_factor & M16C_EF_FORME)
> >>> + cf->data[2] |= CAN_ERR_PROT_FORM;
> >>> + if (error_factor & M16C_EF_STFE)
> >>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> >>> + if (error_factor & M16C_EF_BITE0)
> >>> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> >>> + if (error_factor & M16C_EF_BITE1)
> >>> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> >>> + if (error_factor & M16C_EF_TRE)
> >>> + cf->data[2] |= CAN_ERR_PROT_TX;
> >>> + }
> >>> +
> >>> + cf->data[6] = txerr;
> >>> + cf->data[7] = rxerr;
> >>> +
> >>> + netif_rx(skb);
> >>> +
> >>> + priv->bec.txerr = txerr;
> >>> + priv->bec.rxerr = rxerr;
> >>> +
> >>> + stats->rx_packets++;
> >>> + stats->rx_bytes += cf->can_dlc;
> >>> +}
> >>> +
> >>> +static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> >>> + const struct kvaser_msg *msg)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv;
> >>> + struct can_frame *cf;
> >>> + struct sk_buff *skb;
> >>> + struct net_device_stats *stats;
> >>> + u8 channel = msg->u.rx_can.channel;
> >>> +
> >>> + if (channel >= dev->nchannels) {
> >>> + dev_err(dev->udev->dev.parent,
> >>> + "Invalid channel number (%d)\n", channel);
> >>> + return;
> >>> + }
> >>> +
> >>> + priv = dev->nets[channel];
> >>> + stats = &priv->netdev->stats;
> >>> +
> >>> + skb = alloc_can_skb(priv->netdev, &cf);
> >>> + if (skb == NULL) {
> >>> + stats->tx_dropped++;
> >>> + return;
> >>> + }
> >>> +
> >>> + cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> >>> + (msg->u.rx_can.msg[1] & 0x3f);
> >>> + cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> >>> +
> >>> + if (msg->id == CMD_RX_EXT_MESSAGE) {
> >>> + cf->can_id <<= 18;
> >>> + cf->can_id += ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
> >> |=
> >>
> >> is more appropriate here
> >
> > Ok.
> >
> >>
> >>> + ((msg->u.rx_can.msg[3] & 0xff) << 6) |
> >>> + (msg->u.rx_can.msg[4] & 0x3f);
> >>> + cf->can_id |= CAN_EFF_FLAG;
> >>> + }
> >>> +
> >>> + if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME) {
> >>> + cf->can_id |= CAN_RTR_FLAG;
> >>> + } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> >>> + MSG_FLAG_NERR)) {
> >>> + cf->can_id |= CAN_ERR_FLAG;
> >>> + cf->can_dlc = CAN_ERR_DLC;
> >>
> >> What kind of error is this? Can you set cf->data? What about the
> >> original cd->can_id? What about the stats->rx_*error* stats?
> >
> > Good question I've to take a look to this.
> >
> >>
> >>> + } else if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> >>> + cf->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL;
> >>> + cf->can_dlc = CAN_ERR_DLC;
> >>> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> >>> +
> >>> + stats->rx_over_errors++;
> >>> + stats->rx_errors++;
> >>> + } else if (!msg->u.rx_can.flag) {
> >>> + memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc);
> >>> + } else {
> >>> + kfree_skb(skb);
> >>> + return;
> >>> + }
> >>> +
> >>> + netif_rx(skb);
> >>> +
> >>> + stats->rx_packets++;
> >>> + stats->rx_bytes += cf->can_dlc;
> >>> +}
> >>> +
> >>> +static void kvaser_usb_start_stop_chip_reply(const struct kvaser_usb *dev,
> >>> + const struct kvaser_msg *msg)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv;
> >>> + u8 channel = msg->u.simple.channel;
> >>> +
> >>> + if (channel >= dev->nchannels) {
> >>> + dev_err(dev->udev->dev.parent,
> >>> + "Invalid channel number (%d)\n", channel);
> >>> + return;
> >>> + }
> >>> +
> >>> + priv = dev->nets[channel];
> >>> +
> >>> + complete(&priv->start_stop_comp);
> >>> +}
> >>> +
> >>> +static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
> >>> + const struct kvaser_msg *msg)
> >>> +{
> >>> + switch (msg->id) {
> >>> + case CMD_START_CHIP_REPLY:
> >>> + case CMD_STOP_CHIP_REPLY:
> >>> + kvaser_usb_start_stop_chip_reply(dev, msg);
> >>> + break;
> >>> +
> >>> + case CMD_RX_STD_MESSAGE:
> >>> + case CMD_RX_EXT_MESSAGE:
> >>> + kvaser_usb_rx_can_msg(dev, msg);
> >>> + break;
> >>> +
> >>> + case CMD_CHIP_STATE_EVENT:
> >>> + case CMD_CAN_ERROR_EVENT:
> >>> + kvaser_usb_rx_error(dev, msg);
> >>> + break;
> >>> +
> >>> + case CMD_TX_ACKNOWLEDGE:
> >>> + kvaser_usb_tx_acknowledge(dev, msg);
> >>> + break;
> >>> +
> >>> + default:
> >>> + dev_warn(dev->udev->dev.parent,
> >>> + "Unhandled message (%d)\n", msg->id);
> >>> + break;
> >>> + }
> >>> +}
> >>> +
> >>> +static void kvaser_usb_read_bulk_callback(struct urb *urb)
> >>> +{
> >>> + struct kvaser_usb *dev = urb->context;
> >>> + struct kvaser_msg *msg;
> >>> + int pos = 0;
> >>> + int err, i;
> >>> +
> >>> + switch (urb->status) {
> >>> + case 0:
> >>> + break;
> >>> + case -ENOENT:
> >>> + case -ESHUTDOWN:
> >>> + return;
> >>> + default:
> >>> + dev_info(dev->udev->dev.parent, "Rx URB aborted (%d)\n",
> >>> + urb->status);
> >>> + goto resubmit_urb;
> >>> + }
> >>> +
> >>> + while (pos < urb->actual_length) {
> >>
> >> please check here for pos + sizeof(*msg), too
> >
> > Same as above.
> >
> >>
> >>> + msg = urb->transfer_buffer + pos;
> >>> +
> >>> + if (!msg->len)
> >>> + break;
> >>> +
> >>> + kvaser_usb_handle_message(dev, msg);
> >>> +
> >>> + if (pos > urb->actual_length) {
> >>> + dev_err(dev->udev->dev.parent, "Format error\n");
> >>> + break;
> >>> + }
> >>> +
> >>> + pos += msg->len;
> >>> + }
> >>> +
> >>> +resubmit_urb:
> >>> + usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 129),
> >> ^^^
> >>
> >> use #define
> >
> > Ok.
> >
> >>
> >>> + urb->transfer_buffer, RX_BUFFER_SIZE,
> >>> + kvaser_usb_read_bulk_callback, dev);
> >>> +
> >>> + err = usb_submit_urb(urb, GFP_ATOMIC);
> >>> + if (err == -ENODEV) {
> >>> + for (i = 0; i < dev->nchannels; i++) {
> >>> + if (!dev->nets[i])
> >>> + continue;
> >>> +
> >>> + netif_device_detach(dev->nets[i]->netdev);
> >>> + }
> >>> + } else if (err) {
> >>> + dev_err(dev->udev->dev.parent,
> >>> + "Failed resubmitting read bulk urb: %d\n", err);
> >>> + }
> >>> +
> >>> + return;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev)
> >>> +{
> >>> + int i, err = 0;
> >>> +
> >>> + if (dev->rxinitdone)
> >>> + return 0;
> >>> +
> >>> + for (i = 0; i < MAX_RX_URBS; i++) {
> >>> + struct urb *urb = NULL;
> >>> + u8 *buf = NULL;
> >>> +
> >>> + urb = usb_alloc_urb(0, GFP_KERNEL);
> >>> + if (!urb) {
> >>> + dev_warn(dev->udev->dev.parent,
> >>> + "No memory left for URBs\n");
> >>> + err = -ENOMEM;
> >>> + break;
> >>> + }
> >>> +
> >>> + buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE,
> >>> + GFP_KERNEL, &urb->transfer_dma);
> >>> + if (!buf) {
> >>> + dev_warn(dev->udev->dev.parent,
> >>> + "No memory left for USB buffer\n");
> >>> + usb_free_urb(urb);
> >>> + err = -ENOMEM;
> >>> + break;
> >>> + }
> >>> +
> >>> + usb_fill_bulk_urb(urb, dev->udev,
> >>> + usb_rcvbulkpipe(dev->udev, 129),
> >>
> >> use #define
> >
> > Ok.
> >
> >>
> >>> + buf, RX_BUFFER_SIZE,
> >>> + kvaser_usb_read_bulk_callback, dev);
> >>> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> >>> + usb_anchor_urb(urb, &dev->rx_submitted);
> >>> +
> >>> + err = usb_submit_urb(urb, GFP_KERNEL);
> >>> + if (err) {
> >>> + usb_unanchor_urb(urb);
> >>> + usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
> >>> + urb->transfer_dma);
> >>> + break;
> >>> + }
> >>> +
> >>> + usb_free_urb(urb);
> >>> + }
> >>> +
> >>> + if (i == 0) {
> >>> + dev_warn(dev->udev->dev.parent,
> >>> + "Cannot setup read URBs, error %d\n", err);
> >>> + return err;
> >>> + } else if (i < MAX_RX_URBS) {
> >>> + dev_warn(dev->udev->dev.parent,
> >>> + "RX performances may be slow\n");
> >>> + }
> >>> +
> >>> + dev->rxinitdone = true;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
> >>> +{
> >>> + struct kvaser_msg msg;
> >>> +
> >>> + memset(&msg, 0x00, sizeof(msg));
> >>> + msg.id = CMD_SET_CTRL_MODE;
> >>> + msg.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_ctrl_mode);
> >>> + msg.u.ctrl_mode.tid = 0xff;
> >>> + msg.u.ctrl_mode.channel = priv->channel;
> >>
> >> please use C99 struct initializers
> >
> > Ok.
> >
> >>
> >>> +
> >>> + if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> >>> + msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
> >>> + else
> >>> + msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
> >>> +
> >>> + return kvaser_usb_send_msg(priv->dev, &msg);
> >>> +}
> >>> +
> >>> +static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
> >>> +{
> >>> + int err;
> >>> +
> >>> + init_completion(&priv->start_stop_comp);
> >>> +
> >>> + err = kvaser_usb_send_simple_msg(priv->dev, CMD_START_CHIP,
> >>> + priv->channel);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + if (!wait_for_completion_timeout(&priv->start_stop_comp,
> >>> + msecs_to_jiffies(START_TIMEOUT)))
> >>> + return -ETIMEDOUT;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_open(struct net_device *netdev)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >>> + struct kvaser_usb *dev = priv->dev;
> >>> + int err;
> >>> +
> >>> + err = open_candev(netdev);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + err = kvaser_usb_setup_rx_urbs(dev);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + err = kvaser_usb_set_opt_mode(priv);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + err = kvaser_usb_start_chip(priv);
> >>> + if (err) {
> >>> + netdev_warn(netdev, "Cannot start device, error %d\n", err);
> >>> + close_candev(netdev);
> >>> + return err;
> >>> + }
> >>> +
> >>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >>> + priv->open_time = jiffies;
> >>> + netif_start_queue(netdev);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> >>> +{
> >>> + int i;
> >>> +
> >>> + usb_kill_anchored_urbs(&priv->tx_submitted);
> >>> + atomic_set(&priv->active_tx_urbs, 0);
> >>> +
> >>> + for (i = 0; i < MAX_TX_URBS; i++)
> >> ARRAY_SIZE(priv->tx_contexts) instead of MAX_TX_URBS
> >
> > Ok.
> >
> >>> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> >>> +}
> >>> +
> >>> +static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
> >>> +{
> >>> + int i;
> >>> +
> >>> + usb_kill_anchored_urbs(&dev->rx_submitted);
> >>> +
> >>> + for (i = 0; i < MAX_NET_DEVICES; i++) {
> >> ARRAY_SIZE()
> >>> + struct kvaser_usb_net_priv *priv = dev->nets[i];
> >>> +
> >>> + if (priv)
> >>> + kvaser_usb_unlink_tx_urbs(priv);
> >>> + }
> >>> +}
> >>> +
> >>> +static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv)
> >>> +{
> >>> + int err;
> >>> +
> >>> + init_completion(&priv->start_stop_comp);
> >>> +
> >>> + err = kvaser_usb_send_simple_msg(priv->dev, CMD_STOP_CHIP,
> >>> + priv->channel);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + if (!wait_for_completion_timeout(&priv->start_stop_comp,
> >>> + msecs_to_jiffies(STOP_TIMEOUT)))
> >>> + return -ETIMEDOUT;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
> >>> +{
> >>> + struct kvaser_msg msg;
> >>> +
> >>> + memset(&msg, 0x00, sizeof(msg));
> >>> + msg.id = CMD_FLUSH_QUEUE;
> >>> + msg.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_flush_queue);
> >>> + msg.u.flush_queue.channel = priv->channel;
> >> C99 initialziers, please
> >
> > Ok.
> >
> >>> +
> >>> + return kvaser_usb_send_msg(priv->dev, &msg);
> >>> +}
> >>> +
> >>> +static int kvaser_usb_close(struct net_device *netdev)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >>> + int err;
> >>> +
> >>> + netif_stop_queue(netdev);
> >>> +
> >>> + err = kvaser_usb_flush_queue(priv);
> >>> + if (err)
> >>> + netdev_warn(netdev, "Cannot flush queue, error %d\n", err);
> >>> +
> >>> + err = kvaser_usb_stop_chip(priv);
> >>> + if (err) {
> >>> + netdev_warn(netdev, "Cannot stop device, error %d\n", err);
> >>> + return err;
> >>> + }
> >>> +
> >>> + kvaser_usb_unlink_tx_urbs(priv);
> >>> +
> >>> + priv->can.state = CAN_STATE_STOPPED;
> >>> + close_candev(priv->netdev);
> >>> + priv->open_time = 0;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static void kvaser_usb_write_bulk_callback(struct urb *urb)
> >>> +{
> >>> + struct kvaser_usb_tx_urb_context *context = urb->context;
> >>> + struct kvaser_usb_net_priv *priv;
> >>> + struct net_device *netdev;
> >>> +
> >>> + if (WARN_ON(!context))
> >>> + return;
> >>> +
> >>> + priv = context->priv;
> >>> + netdev = priv->netdev;
> >>> +
> >>> + usb_free_coherent(urb->dev, urb->transfer_buffer_length,
> >>> + urb->transfer_buffer, urb->transfer_dma);
> >>> +
> >>> + if (!netif_device_present(netdev))
> >>> + return;
> >>> +
> >>> + if (urb->status)
> >>> + netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
> >>> +
> >>> + netdev->trans_start = jiffies;
> >>
> >> Is trans_start needed? at least for non-usb devices it works without.
> >
> > I don't know, I'll try to figure this out.
> > I see it's used in the two others CAN/USB drivers, 'ems_usb.c' and
> > 'esd_usb2.c'
> >
> >>
> >>> +}
> >>> +
> >>> +static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> >>> + struct net_device *netdev)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >>> + struct kvaser_usb *dev = priv->dev;
> >>> + struct net_device_stats *stats = &netdev->stats;
> >>> + struct can_frame *cf = (struct can_frame *)skb->data;
> >>> + struct kvaser_usb_tx_urb_context *context = NULL;
> >>> + struct urb *urb;
> >>> + void *buf;
> >>> + struct kvaser_msg *msg;
> >>> + int i, err;
> >>> + int ret = NETDEV_TX_OK;
> >>> +
> >>> + if (can_dropped_invalid_skb(netdev, skb))
> >>> + return NETDEV_TX_OK;
> >>> +
> >>> + urb = usb_alloc_urb(0, GFP_ATOMIC);
> >>> + if (!urb) {
> >>> + netdev_err(netdev, "No memory left for URBs\n");
> >>> + stats->tx_dropped++;
> >>> + dev_kfree_skb(skb);
> >>> + goto nourbmem;
> >>> + }
> >>> +
> >>> + buf = usb_alloc_coherent(dev->udev, sizeof(struct kvaser_msg),
> >>> + GFP_ATOMIC, &urb->transfer_dma);
> >>> + if (!buf) {
> >>> + netdev_err(netdev, "No memory left for USB buffer\n");
> >>> + stats->tx_dropped++;
> >>> + dev_kfree_skb(skb);
> >>> + goto nobufmem;
> >>> + }
> >>> +
> >>> + msg = (struct kvaser_msg *)buf;
> >>> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
> >>> + msg->u.tx_can.flags = 0;
> >>> + msg->u.tx_can.channel = priv->channel;
> >>> +
> >>> + if (cf->can_id & CAN_EFF_FLAG) {
> >>> + msg->id = CMD_TX_EXT_MESSAGE;
> >>> + msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
> >>> + msg->u.tx_can.msg[1] = (cf->can_id >> 18) & 0x3f;
> >>> + msg->u.tx_can.msg[2] = (cf->can_id >> 14) & 0x0f;
> >>> + msg->u.tx_can.msg[3] = (cf->can_id >> 6) & 0xff;
> >>> + msg->u.tx_can.msg[4] = cf->can_id & 0x3f;
> >>> + } else {
> >>> + msg->id = CMD_TX_STD_MESSAGE;
> >>> + msg->u.tx_can.msg[0] = (cf->can_id >> 6) & 0x1f;
> >>> + msg->u.tx_can.msg[1] = cf->can_id & 0x3f;
> >>> + }
> >>> +
> >>> + msg->u.tx_can.msg[5] = cf->can_dlc;
> >>> + memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
> >>> +
> >>> + if (cf->can_id & CAN_RTR_FLAG)
> >>> + msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
> >>> +
> >>> + for (i = 0; i < MAX_TX_URBS; i++) {
> >> ARRAY_SIZE
> >
> > Ok.
> >
> >>> + if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
> >>> + context = &priv->tx_contexts[i];
> >>> + break;
> >>> + }
> >>> + }
> >>> +
> >>> + if (!context) {
> >>> + netdev_warn(netdev, "cannot find free context\n");
> >>> + ret = NETDEV_TX_BUSY;
> >>> + goto releasebuf;
> >>> + }
> >>> +
> >>> + context->priv = priv;
> >>> + context->echo_index = i;
> >>> + context->dlc = cf->can_dlc;
> >>> +
> >>> + msg->u.tx_can.tid = context->echo_index;
> >>> +
> >>> + usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 1),
> >>> + buf, msg->len,
> >>> + kvaser_usb_write_bulk_callback, context);
> >>> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> >>> + usb_anchor_urb(urb, &priv->tx_submitted);
> >>> +
> >>> + can_put_echo_skb(skb, netdev, context->echo_index);
> >>> +
> >>> + atomic_inc(&priv->active_tx_urbs);
> >>> +
> >>> + if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
> >>> + netif_stop_queue(netdev);
> >>> +
> >>> + err = usb_submit_urb(urb, GFP_ATOMIC);
> >>> + if (unlikely(err)) {
> >>> + can_free_echo_skb(netdev, context->echo_index);
> >>> +
> >>> + atomic_dec(&priv->active_tx_urbs);
> >>> + usb_unanchor_urb(urb);
> >>> +
> >>> + stats->tx_dropped++;
> >>> +
> >>> + if (err == -ENODEV)
> >>> + netif_device_detach(netdev);
> >>> + else
> >>> + netdev_warn(netdev, "Failed tx_urb %d\n", err);
> >>> +
> >>> + goto releasebuf;
> >>> + }
> >>> +
> >>> + netdev->trans_start = jiffies;
> >>> +
> >>> + usb_free_urb(urb);
> >>> +
> >>> + return NETDEV_TX_OK;
> >>> +
> >>> +releasebuf:
> >>> + usb_free_coherent(dev->udev, sizeof(struct kvaser_msg),
> >>> + buf, urb->transfer_dma);
> >>> +nobufmem:
> >>> + usb_free_urb(urb);
> >>> +nourbmem:
> >>> + return ret;
> >>> +}
> >>> +
> >>> +static const struct net_device_ops kvaser_usb_netdev_ops = {
> >>> + .ndo_open = kvaser_usb_open,
> >>> + .ndo_stop = kvaser_usb_close,
> >>> + .ndo_start_xmit = kvaser_usb_start_xmit,
> >>> +};
> >>> +
> >>> +static struct can_bittiming_const kvaser_usb_bittiming_const = {
> >>> + .name = "kvaser_usb",
> >>> + .tseg1_min = KVASER_USB_TSEG1_MIN,
> >>> + .tseg1_max = KVASER_USB_TSEG1_MAX,
> >>> + .tseg2_min = KVASER_USB_TSEG2_MIN,
> >>> + .tseg2_max = KVASER_USB_TSEG2_MAX,
> >>> + .sjw_max = KVASER_USB_SJW_MAX,
> >>> + .brp_min = KVASER_USB_BRP_MIN,
> >>> + .brp_max = KVASER_USB_BRP_MAX,
> >>> + .brp_inc = KVASER_USB_BRP_INC,
> >>> +};
> >>> +
> >>> +static int kvaser_usb_set_bittiming(struct net_device *netdev)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >>> + struct can_bittiming *bt = &priv->can.bittiming;
> >>> + struct kvaser_usb *dev = priv->dev;
> >>> + struct kvaser_msg msg;
> >>> +
> >>> + msg.id = CMD_SET_BUS_PARAMS;
> >>> + msg.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_busparams);
> >>> + msg.u.busparams.channel = priv->channel;
> >>> + msg.u.busparams.tid = 0xff;
> >>> + msg.u.busparams.bitrate = bt->bitrate;
> >>
> >> bitrate is le32
> >
> > Indeed ! I'll fix this.
> >
> >>
> >>> + msg.u.busparams.sjw = bt->sjw;
> >>> + msg.u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1;
> >>> + msg.u.busparams.tseg2 = bt->phase_seg2;
> >>
> >> C99 initializers, please
> >>
> >>> +
> >>> + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> >>> + msg.u.busparams.no_samp = 3;
> >>> + else
> >>> + msg.u.busparams.no_samp = 1;
> >>> +
> >>> + return kvaser_usb_send_msg(dev, &msg);
> >>> +}
> >>> +
> >>> +static int kvaser_usb_set_mode(struct net_device *netdev,
> >>> + enum can_mode mode)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >>> +
> >>> + if (!priv->open_time)
> >>> + return -EINVAL;
> >>> +
> >>> + switch (mode) {
> >>> + case CAN_MODE_START:
> >>> + if (netif_queue_stopped(netdev))
> >>> + netif_wake_queue(netdev);
> >>
> >> No need to restart your USB device?
> >
> > No. I don't think so.
> > The module continuously tries to transmit the frame and isn't stopped.
> > So there is no need to restart it if it has been explicitely stopped.
> >
> > When it cannot transmit, the module try again and sends continuously
> > CMD_CAN_ERROR_EVENT frames until it succeeds to transmit the frame.
> > If the device is stopped with the command CMD_STOP_CHIP then it stops
> > sending these CMD_CAN_ERROR_EVENT.
> > Should I handle this in another manner?
>
> If the firmware automatically recovers from busoff (like the at91 does),
> you should stop the chip it priv->can.restart_ms == 0 and let the chip
> continue working otherwise.
>
> >
> >>
> >>> + break;
> >>> + default:
> >>> + return -EOPNOTSUPP;
> >>> + }
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_get_berr_counter(const struct net_device *netdev,
> >>> + struct can_berr_counter *bec)
> >>> +{
> >>> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >>> +
> >>> + bec->txerr = priv->bec.txerr;
> >>> + bec->rxerr = priv->bec.rxerr;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_init_one(struct usb_interface *intf,
> >>> + const struct usb_device_id *id, int channel)
> >>> +{
> >>> + struct kvaser_usb *dev = usb_get_intfdata(intf);
> >>> + struct net_device *netdev;
> >>> + struct kvaser_usb_net_priv *priv;
> >>> + int i, err;
> >>> +
> >>> + netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
> >>> + if (!netdev) {
> >>> + dev_err(&intf->dev, "Cannot alloc candev\n");
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> + priv = netdev_priv(netdev);
> >>> +
> >>> + init_usb_anchor(&priv->tx_submitted);
> >>> + atomic_set(&priv->active_tx_urbs, 0);
> >>> +
> >>> + for (i = 0; i < MAX_TX_URBS; i++)
> >>> + priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> >>> +
> >>> + priv->dev = dev;
> >>> + priv->netdev = netdev;
> >>> + priv->channel = channel;
> >>> +
> >>> + priv->can.state = CAN_STATE_STOPPED;
> >>> + priv->can.clock.freq = CAN_USB_CLOCK;
> >>> + priv->can.bittiming_const = &kvaser_usb_bittiming_const;
> >>> + priv->can.do_set_bittiming = kvaser_usb_set_bittiming;
> >>> + priv->can.do_set_mode = kvaser_usb_set_mode;
> >>> + priv->can.do_get_berr_counter = kvaser_usb_get_berr_counter;
> >>> + priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
> >>> + if (id->driver_info & KVASER_HAS_SILENT_MODE)
> >>> + priv->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
> >>> +
> >>> + netdev->flags |= IFF_ECHO;
> >>> +
> >>> + netdev->netdev_ops = &kvaser_usb_netdev_ops;
> >>> +
> >>> + SET_NETDEV_DEV(netdev, &intf->dev);
> >>> +
> >>> + err = register_candev(netdev);
> >>> + if (err) {
> >>> + dev_err(&intf->dev, "Failed to register can device\n");
> >>> + free_candev(netdev);
> >>> + return err;
> >>> + }
> >>> +
> >>> + dev->nets[channel] = priv;
> >>> + netdev_info(netdev, "device %s registered\n", netdev->name);
> >>
> >> netdev_info should take care of printing the device's name.
> >
> > Ok.
> >
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int kvaser_usb_probe(struct usb_interface *intf,
> >>> + const struct usb_device_id *id)
> >>> +{
> >>> + struct kvaser_usb *dev;
> >>> + int err = -ENOMEM;
> >>> + int i;
> >>> +
> >>> + dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> >>
> >> Who will free dev on driver unload? Please make use of devm_kzalloc().
> >
> > Ok. kfree is missing is disconnect().
> > I'll replace it by devm_kzalloc() and devm_free().
>
> The beauty of devm_kzalloc is you don't have to call *_free, its
> automatically called if probe fails or when remove function has been called.
Cool :-)
>
> >
> >>
> >>> + if (!dev)
> >>> + return -ENOMEM;
> >>> +
> >>> + dev->udev = interface_to_usbdev(intf);
> >>> +
> >>> + init_usb_anchor(&dev->rx_submitted);
> >>> +
> >>> + usb_set_intfdata(intf, dev);
> >>> +
> >>> + if (kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, 0)) {
> >>> + dev_err(&intf->dev, "Cannot reset kvaser\n");
> >>> + goto error;
> >>> + }
> >>> +
> >>> + if (kvaser_usb_get_software_info(dev)) {
> >>> + dev_err(&intf->dev, "Cannot get software infos\n");
> >>> + goto error;
> >>> + }
> >>> +
> >>> + if (kvaser_usb_get_card_info(dev)) {
> >>> + dev_err(&intf->dev, "Cannot get card infos\n");
> >>> + goto error;
> >>> + }
> >>> +
> >>> + dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n",
> >>> + ((dev->fw_version >> 24) & 0xff),
> >>> + ((dev->fw_version >> 16) & 0xff),
> >>> + (dev->fw_version & 0xffff));
> >>> +
> >>> + for (i = 0; i < dev->nchannels; i++)
> >>> + kvaser_usb_init_one(intf, id, i);
> >>> +
> >>> + return 0;
> >>> +
> >>> +error:
> >>> + kfree(dev);
> >>> + return err;
> >>> +}
> >>> +
> >>> +static void kvaser_usb_disconnect(struct usb_interface *intf)
> >>> +{
> >>> + struct kvaser_usb *dev = usb_get_intfdata(intf);
> >>> + int i;
> >>> +
> >>> + usb_set_intfdata(intf, NULL);
> >>> +
> >>> + if (!dev)
> >>> + return;
> >>> +
> >>> + for (i = 0; i < dev->nchannels; i++) {
> >>> + if (!dev->nets[i])
> >>> + continue;
> >>> +
> >>> + unregister_netdev(dev->nets[i]->netdev);
> >>> + free_candev(dev->nets[i]->netdev);
> >>> + }
> >>> +
> >>> + kvaser_usb_unlink_all_urbs(dev);
> >>> +}
> >>> +
> >>> +static struct usb_driver kvaser_usb_driver = {
> >>> + .name = "kvaser_usb",
> >>> + .probe = kvaser_usb_probe,
> >>> + .disconnect = kvaser_usb_disconnect,
> >>> + .id_table = kvaser_usb_table
> >>> +};
> >>> +
> >>> +module_usb_driver(kvaser_usb_driver);
> >>> +
> >>> +MODULE_AUTHOR("Olivier Sobrie <olivier@sobrie.be>");
> >>> +MODULE_DESCRIPTION("Can driver for Kvaser CAN/USB devices");
> >>> +MODULE_LICENSE("GPL v2");
> >>> diff --git a/drivers/net/can/usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb.h
> >>> new file mode 100644
> >>> index 0000000..8e0b6ab
> >>> --- /dev/null
> >>> +++ b/drivers/net/can/usb/kvaser_usb.h
> >>> @@ -0,0 +1,237 @@
> >>> +#ifndef _KVASER_USB_H_
> >>> +#define _KVASER_USB_H_
> >>> +
> >>> +#define MAX_TX_URBS 16
> >> Please no tab between define and macro name
> >
> > Ok I didn't know it was not allowed... checkpatch didn't complain.
>
> It's allowed, but not used without tab it's more common, at least among
> CAN drivers.
>
> >
> >>> +#define MAX_RX_URBS 4
> >>> +#define START_TIMEOUT 1000
> >>> +#define STOP_TIMEOUT 1000
> >>> +#define USB_SEND_TIMEOUT 1000
> >>> +#define USB_RECEIVE_TIMEOUT 1000
> >>> +#define RX_BUFFER_SIZE 3072
> >>> +#define CAN_USB_CLOCK 8000000
> >>> +#define MAX_NET_DEVICES 3
> >>> +
> >>> +/* Kvaser USB devices */
> >>> +#define KVASER_VENDOR_ID 0x0bfd
> >>> +#define USB_LEAF_DEVEL_PRODUCT_ID 10
> >>> +#define USB_LEAF_LITE_PRODUCT_ID 11
> >>> +#define USB_LEAF_PRO_PRODUCT_ID 12
> >>> +#define USB_LEAF_SPRO_PRODUCT_ID 14
> >>> +#define USB_LEAF_PRO_LS_PRODUCT_ID 15
> >>> +#define USB_LEAF_PRO_SWC_PRODUCT_ID 16
> >>> +#define USB_LEAF_PRO_LIN_PRODUCT_ID 17
> >>> +#define USB_LEAF_SPRO_LS_PRODUCT_ID 18
> >>> +#define USB_LEAF_SPRO_SWC_PRODUCT_ID 19
> >>> +#define USB_MEMO2_DEVEL_PRODUCT_ID 22
> >>> +#define USB_MEMO2_HSHS_PRODUCT_ID 23
> >>> +#define USB_UPRO_HSHS_PRODUCT_ID 24
> >>> +#define USB_LEAF_LITE_GI_PRODUCT_ID 25
> >>> +#define USB_LEAF_PRO_OBDII_PRODUCT_ID 26
> >>> +#define USB_MEMO2_HSLS_PRODUCT_ID 27
> >>> +#define USB_LEAF_LITE_CH_PRODUCT_ID 28
> >>> +#define USB_BLACKBIRD_SPRO_PRODUCT_ID 29
> >>> +#define USB_OEM_MERCURY_PRODUCT_ID 34
> >>> +#define USB_OEM_LEAF_PRODUCT_ID 35
> >>> +#define USB_CAN_R_PRODUCT_ID 39
> >>> +
> >>> +/* USB devices features */
> >>> +#define KVASER_HAS_SILENT_MODE (1 << 0)
> >> pleae use BIT(0)
> >>> +
> >>> +/* Message header size */
> >>> +#define MSG_HEADER_LEN 2
> >>> +
> >>> +/* Can message flags */
> >>> +#define MSG_FLAG_ERROR_FRAME (1 << 0)
> >>> +#define MSG_FLAG_OVERRUN (1 << 1)
> >>> +#define MSG_FLAG_NERR (1 << 2)
> >>> +#define MSG_FLAG_WAKEUP (1 << 3)
> >>> +#define MSG_FLAG_REMOTE_FRAME (1 << 4)
> >>> +#define MSG_FLAG_RESERVED (1 << 5)
> >>> +#define MSG_FLAG_TX_ACK (1 << 6)
> >>> +#define MSG_FLAG_TX_REQUEST (1 << 7)
> >>> +
> >>> +/* Can states */
> >>> +#define M16C_STATE_BUS_RESET (1 << 0)
> >>> +#define M16C_STATE_BUS_ERROR (1 << 4)
> >>> +#define M16C_STATE_BUS_PASSIVE (1 << 5)
> >>> +#define M16C_STATE_BUS_OFF (1 << 6)
> >>> +
> >>> +/* Can msg ids */
> >>> +#define CMD_RX_STD_MESSAGE 12
> >>> +#define CMD_TX_STD_MESSAGE 13
> >>> +#define CMD_RX_EXT_MESSAGE 14
> >>> +#define CMD_TX_EXT_MESSAGE 15
> >>> +#define CMD_SET_BUS_PARAMS 16
> >>> +#define CMD_GET_BUS_PARAMS 17
> >>> +#define CMD_GET_BUS_PARAMS_REPLY 18
> >>> +#define CMD_GET_CHIP_STATE 19
> >>> +#define CMD_CHIP_STATE_EVENT 20
> >>> +#define CMD_SET_CTRL_MODE 21
> >>> +#define CMD_GET_CTRL_MODE 22
> >>> +#define CMD_GET_CTRL_MODE_REPLY 23
> >>> +#define CMD_RESET_CHIP 24
> >>> +#define CMD_RESET_CHIP_REPLY 25
> >>> +#define CMD_START_CHIP 26
> >>> +#define CMD_START_CHIP_REPLY 27
> >>> +#define CMD_STOP_CHIP 28
> >>> +#define CMD_STOP_CHIP_REPLY 29
> >>> +#define CMD_GET_CARD_INFO2 32
> >>> +#define CMD_GET_CARD_INFO 34
> >>> +#define CMD_GET_CARD_INFO_REPLY 35
> >>> +#define CMD_GET_SOFTWARE_INFO 38
> >>> +#define CMD_GET_SOFTWARE_INFO_REPLY 39
> >>> +#define CMD_ERROR_EVENT 45
> >>> +#define CMD_FLUSH_QUEUE 48
> >>> +#define CMD_TX_ACKNOWLEDGE 50
> >>> +#define CMD_CAN_ERROR_EVENT 51
> >>> +#define CMD_USB_THROTTLE 77
> >>> +
> >>> +/* error factors */
> >>> +#define M16C_EF_ACKE (1 << 0)
> >>> +#define M16C_EF_CRCE (1 << 1)
> >>> +#define M16C_EF_FORME (1 << 2)
> >>> +#define M16C_EF_STFE (1 << 3)
> >>> +#define M16C_EF_BITE0 (1 << 4)
> >>> +#define M16C_EF_BITE1 (1 << 5)
> >>> +#define M16C_EF_RCVE (1 << 6)
> >>> +#define M16C_EF_TRE (1 << 7)
> >>> +
> >>> +/* bittiming parameters */
> >>> +#define KVASER_USB_TSEG1_MIN 1
> >>> +#define KVASER_USB_TSEG1_MAX 16
> >>> +#define KVASER_USB_TSEG2_MIN 1
> >>> +#define KVASER_USB_TSEG2_MAX 8
> >>> +#define KVASER_USB_SJW_MAX 4
> >>> +#define KVASER_USB_BRP_MIN 1
> >>> +#define KVASER_USB_BRP_MAX 64
> >>> +#define KVASER_USB_BRP_INC 1
> >>> +
> >>> +/* ctrl modes */
> >>> +#define KVASER_CTRL_MODE_NORMAL 1
> >>> +#define KVASER_CTRL_MODE_SILENT 2
> >>> +#define KVASER_CTRL_MODE_SELFRECEPTION 3
> >>> +#define KVASER_CTRL_MODE_OFF 4
> >>> +
> >>> +struct kvaser_msg_simple {
> >>> + u8 tid;
> >>> + u8 channel;
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_cardinfo {
> >>> + u8 tid;
> >>> + u8 nchannels;
> >>> + __le32 serial_number;
> >>> + __le32 padding;
> >>> + __le32 clock_resolution;
> >>> + __le32 mfgdate;
> >>> + u8 ean[8];
> >>> + u8 hw_revision;
> >>> + u8 usb_hs_mode;
> >>> + __le16 padding2;
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_cardinfo2 {
> >>> + u8 tid;
> >>> + u8 channel;
> >>> + u8 pcb_id[24];
> >>> + __le32 oem_unlock_code;
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_softinfo {
> >>> + u8 tid;
> >>> + u8 channel;
> >>> + __le32 sw_options;
> >>> + __le32 fw_version;
> >>> + __le16 max_outstanding_tx;
> >>> + __le16 padding[9];
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_busparams {
> >>> + u8 tid;
> >>> + u8 channel;
> >>> + __le32 bitrate;
> >>> + u8 tseg1;
> >>> + u8 tseg2;
> >>> + u8 sjw;
> >>> + u8 no_samp;
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_tx_can {
> >>> + u8 channel;
> >>> + u8 tid;
> >>> + u8 msg[14];
> >>> + u8 padding;
> >>> + u8 flags;
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_rx_can {
> >>> + u8 channel;
> >>> + u8 flag;
> >>> + __le16 time[3];
> >>> + u8 msg[14];
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_chip_state_event {
> >>> + u8 tid;
> >>> + u8 channel;
> >>> + __le16 time[3];
> >>> + u8 tx_errors_count;
> >>> + u8 rx_errors_count;
> >>> + u8 status;
> >>> + u8 padding[3];
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_tx_acknowledge {
> >>> + u8 channel;
> >>> + u8 tid;
> >>> + __le16 time[3];
> >>> + u8 flags;
> >>> + u8 time_offset;
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_error_event {
> >>> + u8 tid;
> >>> + u8 flags;
> >>> + __le16 time[3];
> >>> + u8 channel;
> >>> + u8 padding;
> >>> + u8 tx_errors_count;
> >>> + u8 rx_errors_count;
> >>> + u8 status;
> >>> + u8 error_factor;
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_ctrl_mode {
> >>> + u8 tid;
> >>> + u8 channel;
> >>> + u8 ctrl_mode;
> >>> + u8 padding[3];
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg_flush_queue {
> >>> + u8 tid;
> >>> + u8 channel;
> >>> + u8 flags;
> >>> + u8 padding[3];
> >>> +} __packed;
> >>> +
> >>> +struct kvaser_msg {
> >>> + u8 len;
> >>> + u8 id;
> >>> + union {
> >>> + struct kvaser_msg_simple simple;
> >>> + struct kvaser_msg_cardinfo cardinfo;
> >>> + struct kvaser_msg_cardinfo2 cardinfo2;
> >>> + struct kvaser_msg_softinfo softinfo;
> >>> + struct kvaser_msg_busparams busparams;
> >>> + struct kvaser_msg_tx_can tx_can;
> >>> + struct kvaser_msg_rx_can rx_can;
> >>> + struct kvaser_msg_chip_state_event chip_state_event;
> >>> + struct kvaser_msg_tx_acknowledge tx_acknowledge;
> >>> + struct kvaser_msg_error_event error_event;
> >>> + struct kvaser_msg_ctrl_mode ctrl_mode;
> >>> + struct kvaser_msg_ctrl_mode flush_queue;
> >>> + } u;
> >>> +} __packed;
> >>> +
> >>> +#endif
> >>>
> >>
> >>
> >> --
> >> Pengutronix e.K. | Marc Kleine-Budde |
> >> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> >> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> >> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
> >>
> >
> > Thanks for the review.
> np,
>
> regards, Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
>
Thanks,
--
Olivier
^ permalink raw reply
* Re: [PATCH] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Marc Kleine-Budde @ 2012-07-31 13:27 UTC (permalink / raw)
To: Olivier Sobrie; +Cc: Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <20120731130650.GA23541@hposo>
[-- Attachment #1: Type: text/plain, Size: 10413 bytes --]
On 07/31/2012 03:06 PM, Olivier Sobrie wrote:
[...]
>>>> Please check if your driver survives hot-unplugging while sending and
>>>> receiving CAN frames at maximum laod.
>>>
>>> I tested this with two Kvaser sending frames with "cangen can0 -g 0 -i"
>>> never saw a crash.
>>
>> Please test send sending and receiving at the same time.
>
> Yes that's what I did; "cangen can0 -g 0 -i" on both sides.
Fine.
[...]
>>>>> --- /dev/null
>>>>> +++ b/drivers/net/can/usb/kvaser_usb.c
>>>>> @@ -0,0 +1,1062 @@
>>>>> +/*
>>>>
>>>> Please add a license statement and probably your copyright:
>>>>
>>>> * 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 version 2.
>>>>
>>>> You also should copy the copyright from the drivers you used:
>>>>
>>>>> + * Parts of this driver are based on the following:
>>>>> + * - Kvaser linux leaf driver (version 4.78)
>>>>
>>>> I just downloaded their driver and noticed that it's quite sparse on
>>>> stating the license the code is released under.
>>>> "doc/HTMLhelp/copyright.htx" is quite restrictive, the word GPL occurs 3
>>>> times, all in MODULE_LICENSE("GPL"). Running modinfo on the usbcan.ko
>>>> shows "license: GPL"
>>>
>>> I'll add the license statement.
>>> In fact it's the leaf.ko which is used for this device and it is under
>>> GPL as modinfo said.
>>
>> I just talked to my boss and we're the same opinion, that
>> MODULE_LICENSE("GPL") is a technical term and not relevant if the
>> included license doesn't say a word about GPL. If the kvaser tarball
>> violates the GPL, however is written on different sheet of paper (as we
>> say in Germany).
>>
>> So I cannot put my S-o-b under this driver as long as we haven't talked
>> to kvaser.
>
> Ok I thought it was sufficient enough to have MODULE_LICENSE("GPL") in
> the code to indicate it is a GPL driver. I'll ask Kvaser before sending
> any new version of the patch.
We can continue the review process, this problem has to be sorted out
before I can apply this patch to linux-can-next tree.
[...]
>>>>> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
>>>>> + const struct kvaser_msg *msg)
>>>>> +{
>>>>> + struct can_frame *cf;
>>>>> + struct sk_buff *skb;
>>>>> + struct net_device_stats *stats;
>>>>> + struct kvaser_usb_net_priv *priv;
>>>>> + u8 channel, status, txerr, rxerr;
>>>>> +
>>>>> + if (msg->id == CMD_CAN_ERROR_EVENT) {
>>>>> + channel = msg->u.error_event.channel;
>>>>> + status = msg->u.error_event.status;
>>>>> + txerr = msg->u.error_event.tx_errors_count;
>>>>> + rxerr = msg->u.error_event.rx_errors_count;
>>>>> + } else {
>>>>> + channel = msg->u.chip_state_event.channel;
>>>>> + status = msg->u.chip_state_event.status;
>>>>> + txerr = msg->u.chip_state_event.tx_errors_count;
>>>>> + rxerr = msg->u.chip_state_event.rx_errors_count;
>>>>> + }
>>>>> +
>>>>> + if (channel >= dev->nchannels) {
>>>>> + dev_err(dev->udev->dev.parent,
>>>>> + "Invalid channel number (%d)\n", channel);
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + priv = dev->nets[channel];
>>>>> + stats = &priv->netdev->stats;
>>>>> +
>>>>> + skb = alloc_can_err_skb(priv->netdev, &cf);
>>>>> + if (!skb) {
>>>>> + stats->rx_dropped++;
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + if ((status & M16C_STATE_BUS_OFF) ||
>>>>> + (status & M16C_STATE_BUS_RESET)) {
>>>>> + priv->can.state = CAN_STATE_BUS_OFF;
>>>>> + cf->can_id |= CAN_ERR_BUSOFF;
>>>>> + can_bus_off(priv->netdev);
>>
>> you should increment priv->can.can_stats.bus_off
>> What does the firmware do in this state? Does it automatically try to
>> recover and try to send the outstanding frames?
>
> Yes that's what I observed.
It's behaves like the at91_can.
>> If so, you should turn of the CAN interface, it possible. See:
>> http://lxr.free-electrons.com/source/drivers/net/can/at91_can.c#L986
>
> Ok I'll have a look at this.
>
>>
>> Please test Bus-Off behaviour:
>> - setup working CAN network
>> - short circuit CAN-H and CAN-L wires
>> - start "candump any,0:0,#FFFFFFFF" on one shell
>> - send one can frame on the other
>>
>> then
>>
>> - remove the short circuit
>> - see if the can frame is transmitted to the other side
>> - it should show up as an echo'ed CAN frame on the sender side
>>
>> Repeat the same test with disconnecting CAN-H and CAN-L from the other
>> CAN station instead of short circuit.
>>
>> Please send the output from candump.
>
> 1) With the short circuit:
>
> I perform the test you described. It showed that the Kvaser passes from
> ERROR-WARNING to ERROR-PASSIVE and then BUS-OFF. But after going to the
> state BUS-OFF it comes back to ERROR-WARNING.
>
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
Why don't we have any rx/tx numbers in the error frame?
> ...
> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME <-- bus off
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> ...
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 123 [2] 11 22 <-- short circuit removed
>
> I see the echo and on the other end I see the frame coming in.
> By the way I see that the txerr and rxerr fields of the structure
> kvaser_msg_error_event stay at 0.
>
> 2) With CAN-H and CAN-L disconnected:
>
> I never see the bus going in OFF state. It stays in PASSIVE mode.
>
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> ...
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 80 1B 00 00 00 00 ERRORFRAME
> can1 123 [2] 11 22 <-- other end connected
From the hardware point of view the short circuit and open end tests
look good. Please adjust the driver to turn off the CAN interface in
case of a bus off if restart_ms is 0.
>>>>> + } else if (status & M16C_STATE_BUS_ERROR) {
>>>>> + priv->can.state = CAN_STATE_ERROR_WARNING;
>>>>> + priv->can.can_stats.error_warning++;
>>>>> + } else if (status & M16C_STATE_BUS_PASSIVE) {
>>>>> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
>>>>> + priv->can.can_stats.error_passive++;
>>>>> + } else {
>>>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>>>> + cf->can_id |= CAN_ERR_PROT;
>>>>> + cf->data[2] = CAN_ERR_PROT_ACTIVE;
>>>>> + }
>>>>> +
>>>>> + if (msg->id == CMD_CAN_ERROR_EVENT) {
>>>>> + u8 error_factor = msg->u.error_event.error_factor;
>>>>> +
>>>>> + priv->can.can_stats.bus_error++;
>>>>> + stats->rx_errors++;
>>>>> +
>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>> +
>>>>> + if ((priv->can.state == CAN_STATE_ERROR_WARNING) ||
>>>>> + (priv->can.state == CAN_STATE_ERROR_PASSIVE)) {
>>>>> + cf->data[1] = (txerr > rxerr) ?
>>>>> + CAN_ERR_CRTL_TX_PASSIVE
>>>>> + : CAN_ERR_CRTL_RX_PASSIVE;
Please use CAN_ERR_CRTL_RX_WARNING, CAN_ERR_CRTL_TX_WARNING where
appropriate.
>>>>> + }
>>>>> +
>>>>> + if (error_factor & M16C_EF_ACKE)
>>>>> + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK |
>>>>> + CAN_ERR_PROT_LOC_ACK_DEL);
>>>>> + if (error_factor & M16C_EF_CRCE)
>>>>> + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
>>>>> + CAN_ERR_PROT_LOC_CRC_DEL);
>>>>> + if (error_factor & M16C_EF_FORME)
>>>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>>>>> + if (error_factor & M16C_EF_STFE)
>>>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>>>> + if (error_factor & M16C_EF_BITE0)
>>>>> + cf->data[2] |= CAN_ERR_PROT_BIT0;
>>>>> + if (error_factor & M16C_EF_BITE1)
>>>>> + cf->data[2] |= CAN_ERR_PROT_BIT1;
>>>>> + if (error_factor & M16C_EF_TRE)
>>>>> + cf->data[2] |= CAN_ERR_PROT_TX;
>>>>> + }
>>>>> +
>>>>> + cf->data[6] = txerr;
>>>>> + cf->data[7] = rxerr;
>>>>> +
>>>>> + netif_rx(skb);
>>>>> +
>>>>> + priv->bec.txerr = txerr;
>>>>> + priv->bec.rxerr = rxerr;
>>>>> +
>>>>> + stats->rx_packets++;
>>>>> + stats->rx_bytes += cf->can_dlc;
>>>>> +}
>>>>> +
[...]
>>>>> +static int kvaser_usb_get_berr_counter(const struct net_device *netdev,
>>>>> + struct can_berr_counter *bec)
>>>>> +{
>>>>> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>>>>> +
>>>>> + bec->txerr = priv->bec.txerr;
>>>>> + bec->rxerr = priv->bec.rxerr;
I think you can copy the struct like this:
*bec = priv->bec;
>>>>> +
>>>>> + return 0;
>>>>> +}
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] net: Allow to create links with given ifindex
From: Pavel Emelyanov @ 2012-07-31 13:30 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Linux Netdev List, David Miller
In-Reply-To: <878ve0dtw3.fsf@xmission.com>
>> First, let's imagine that on host A the loopback device got registered as
>> first device, but on host B for some reason some other device got registered
>> first. In that case after migration from A to B the lo on B will have index
>> equals 2. And there's no any strict requirement that lo's per net operations
>> are registered first. Please, correct me if I'm wrong.
>
> Actually there is a hard requirement that the loopback device be the
> last device in a network namespace to be unregistered. We meet that
> requirement by registering the loopback device first
> "net/core/dev.c:net_dev_init()".
Hm... Indeed, and this is good news!
>> Next. In fact, lo is not the only problem. Look at the e.g. sit versus ipgre
>> fallback devices. Both gets created on netns creation and obtain whatever
>> ifindices are generated for them. Even if we make ifidex per netns chances
>> that sit gets registered _strictly_ before ipgre equal zero, since they are
>> both modules.
>
> True. However those fallback devices should no longer be needed,
> and even if they are I think you can delete and recreate them.
Good idea! I will look at that direction.
> Making lo the particularly interesting case.
Yup, provided we can manually recreate those auto-created devices this solves
the issue.
>> Just an idea -- is it worth moving the possibility to have ifindidces intersect
>> under CONFIG_<SOMETHING> (EXPERT/CHECKPOINT_RESTORE) to let wider audience check
>> the code in real-life?
>
> I think the best testing we are going to get diversity wise is to create
> a per netns counter into dev_new_index when net-next opens up.
>
> Having an ifindex that we can only set at netdevice creation time seems
> reasonable.
OK, thank you, Eric.
> Eric
> .
>
Thanks,
Pavel
^ permalink raw reply
* Re: [PATCH] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Wolfgang Grandegger @ 2012-07-31 13:43 UTC (permalink / raw)
To: Olivier Sobrie; +Cc: Marc Kleine-Budde, linux-can, netdev
In-Reply-To: <20120731130650.GA23541@hposo>
On 07/31/2012 03:06 PM, Olivier Sobrie wrote:
> On Tue, Jul 31, 2012 at 11:56:22AM +0200, Marc Kleine-Budde wrote:
...
>> Please test Bus-Off behaviour:
>> - setup working CAN network
>> - short circuit CAN-H and CAN-L wires
>> - start "candump any,0:0,#FFFFFFFF" on one shell
>> - send one can frame on the other
>>
>> then
>>
>> - remove the short circuit
>> - see if the can frame is transmitted to the other side
>> - it should show up as an echo'ed CAN frame on the sender side
>>
>> Repeat the same test with disconnecting CAN-H and CAN-L from the other
>> CAN station instead of short circuit.
>>
>> Please send the output from candump.
>
> 1) With the short circuit:
>
> I perform the test you described. It showed that the Kvaser passes from
> ERROR-WARNING to ERROR-PASSIVE and then BUS-OFF. But after going to the
> state BUS-OFF it comes back to ERROR-WARNING.
You can use the option "-e" to get a human readable output.
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
A state change (0x10 here) should only be reported once.
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> ...
> can1 200000C8 [8] 00 00 90 00 00 00 00 00 ERRORFRAME <-- bus off
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> ...
> can1 20000088 [8] 00 10 90 00 00 00 00 00 ERRORFRAME
> can1 123 [2] 11 22 <-- short circuit removed
>
> I see the echo and on the other end I see the frame coming in.
> By the way I see that the txerr and rxerr fields of the structure
> kvaser_msg_error_event stay at 0.
If possible, they should be set for any CAN error message.
>
> 2) With CAN-H and CAN-L disconnected:
>
> I never see the bus going in OFF state. It stays in PASSIVE mode.
That is the correct behavior. You should get ACK slot (and not ACK
delimiter), IIRC.
I'm offline for the rest of the week. I will have a closer look next
week (to your next version(s)).
Wolfgang.
^ permalink raw reply
* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Bjorn Helgaas @ 2012-07-31 13:45 UTC (permalink / raw)
To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120730181542.GA987@jonmason-lab>
On Mon, Jul 30, 2012 at 12:15 PM, Jon Mason <jon.mason@intel.com> wrote:
> On Mon, Jul 30, 2012 at 10:50:13AM -0600, Bjorn Helgaas wrote:
>> On Sun, Jul 29, 2012 at 6:26 PM, Jon Mason <jon.mason@intel.com> wrote:
>> > A PCI-Express non-transparent bridge (NTB) is a point-to-point PCIe bus
>> > connecting 2 systems, providing electrical isolation between the two subsystems.
>> > A non-transparent bridge is functionally similar to a transparent bridge except
>> > that both sides of the bridge have their own independent address domains. The
>> > host on one side of the bridge will not have the visibility of the complete
>> > memory or I/O space on the other side of the bridge. To communicate across the
>> > non-transparent bridge, each NTB endpoint has one (or more) apertures exposed to
>> > the local system. Writes to these apertures are mirrored to memory on the
>> > remote system. Communications can also occur through the use of doorbell
>> > registers that initiate interrupts to the alternate domain, and scratch-pad
>> > registers accessible from both sides.
>> >
>> > The NTB device driver is needed to configure these memory windows, doorbell, and
>> > scratch-pad registers as well as use them in such a way as they can be turned
>> > into a viable communication channel to the remote system. ntb_hw.[ch]
>> > determines the usage model (NTB to NTB or NTB to Root Port) and abstracts away
>> > the underlying hardware to provide access and a common interface to the doorbell
>> > registers, scratch pads, and memory windows. These hardware interfaces are
>> > exported so that other, non-mainlined kernel drivers can access these.
>> > ntb_transport.[ch] also uses the exported interfaces in ntb_hw.[ch] to setup a
>> > communication channel(s) and provide a reliable way of transferring data from
>> > one side to the other, which it then exports so that "client" drivers can access
>> > them. These client drivers are used to provide a standard kernel interface
>> > (i.e., Ethernet device) to NTB, such that Linux can transfer data from one
>> > system to the other in a standard way.
>> >
>> > Signed-off-by: Jon Mason <jon.mason@intel.com>
>> > ---
>> > MAINTAINERS | 6 +
>> > drivers/Kconfig | 2 +
>> > drivers/Makefile | 1 +
>> > drivers/ntb/Kconfig | 13 +
>> > drivers/ntb/Makefile | 3 +
>> > drivers/ntb/ntb_hw.c | 1178 ++++++++++++++++++++++++++++++++++++
>> > drivers/ntb/ntb_hw.h | 206 +++++++
>> > drivers/ntb/ntb_regs.h | 150 +++++
>> > drivers/ntb/ntb_transport.c | 1387 +++++++++++++++++++++++++++++++++++++++++++
>> > include/linux/ntb.h | 92 +++
>>
>> Where will drivers for non-Intel NTBs fit in this hierarchy? It seems
>> a bit presumptuous to claim the generic "ntb" names just for Intel
>> devices.
>
> I've tried to make it all generic enough that non-Intel NTBs should plug in with
> minimal changes to ntb_hw.c. If their design is too divergent, then a slight
> redesign of ntb_hw.c might be necessary. But from what I've seen of other
> designs on the internet, they appear to be extremely similar. The transport and
> client drivers were written with the hardware abstracted away as much as
> possible to prevent the need to modify it for different hardware. If there is
> anything which is Intel hardware specific, I'd be happy to change it to make it
> more generic.
That makes sense from a technical point of view, but I think it's
going to cause maintenance issues. For example, assume PLX NTB
support is added. Will PLX be happy about having to convince you to
accept changes? Will Intel be happy about having to release a new
driver for their hardware just to incorporate a PLX bug fix? Will
users of PLX hardware accept a new driver release that only benefits
Intel users?
^ permalink raw reply
* Re: TCP stalls with 802.3ad + bridge + kvm guest
From: Peter Samuelson @ 2012-07-31 14:07 UTC (permalink / raw)
To: Jay Vosburgh, netdev; +Cc: jgoerzen
In-Reply-To: <26496.1343419205@death.nxdomain>
> > ixgbe [10 Gbit port] -- bonding [802.3ad] -- bridge -- KVM guest
> >
> >(There's also a VLAN layer, but I can reproduce this problem without
> >it.) It all works, except that with some flows in the KVM guest - I
> >can reproduce using smbclient - transfers keep stalling, such that I'm
> >averaging well under 1 MB/s. Should be more like 100 MB/s.
> >
> >Oddly, this only occurs when both the 802.3ad and KVM are used:
> >
> > Server Agg Client TCP stalls
> > --------------------------------------------------
> > external none KVM guest no
> > external 802.3ad KVM host no
> > KVM host 802.3ad KVM guest no
> > external 802.3ad KVM guest yes
[Jay Vosburgh]
> Does the "none" for Agg (the first line) mean no bonding at all?
Correct. 'None' is without the bonding driver, putting the eth
interface directly on the bridge. (With or without a VLAN layer.)
> Does the problem happen if the bond is a different mode
> (balance-xor, for example)?
This is taking me longer to test, as I have to also update the switch
port config, which required coordination with coworkers. I'll get that
info as soon as I can.
> Do the various stats on the host and guest show any drops?
> E.g., from "netstat -i" and "tc -s qdisc"
Yes, 'netstat -i' on the KVM host shows a few dropped RX packets on
bond0 - looks like it increments by 2 each time I do a test download of
my 20MB file. 'tc -s qdisc' on the KVM host shows nothing, and I see
no drops on the KVM guest side.
I'll get the switch reconfigured not to do LACP so I can test other
bonding modes, as you suggest. Thanks for the quick response! Sorry
my own followup was delayed....
Peter
^ permalink raw reply
* CPU: 0 Not tainted (3.1.9+ #1) when ifconfig rose0 down
From: Bernard Pidoux @ 2012-07-31 14:11 UTC (permalink / raw)
To: linux-hams, Linux Netdev List
Hi,
I observe systematically a kernel panic when I try to shutdown rose0
device using ifconfig rose0 down
This is happening on two very different ROSE implementation, one is on a
machine with x86-64 kernel 4.6.3 on an Intel core 2 duo CPU
the other is on a RaspBerry Pi with Raspbian and 3.1.9+ wheezy kernel
recompiled with AX.25 modules (ax25, rose, netrom, 6pack, kiss) enabled.
Here is an image of the screen dump :
http://f6bvp.org/photos/rose_device_event.JPG
It can be noticed that PC is at rose_device_event and
LR is at sock_def_wakeup
One thing to be noticed is that when I close before all ROSE and AX.25
applications, there are still a few populated sockets, probably for one
of the program did not close the sockets properly.
I that case, does rose module should accept to shutdown rose0 device ?
However, I guess that it should not create a kernel panic due to a
kernel NULL pointer.
I don't know what to do in order to debug that issue.
Bernard
^ permalink raw reply
* [PATCH iproute2 1/2] Add missing can.h
From: Rostislav Lisovy @ 2012-07-31 14:46 UTC (permalink / raw)
To: netdev; +Cc: linux-can, pisa, sojkam1, Rostislav Lisovy
Header file used when working with AF_CAN frames -- generated from
linux kernel 3.5+
Signed-off-by: Rostislav Lisovy <lisovy@gmail.com>
---
include/linux/can.h | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 161 insertions(+)
create mode 100644 include/linux/can.h
diff --git a/include/linux/can.h b/include/linux/can.h
new file mode 100644
index 0000000..018055e
--- /dev/null
+++ b/include/linux/can.h
@@ -0,0 +1,161 @@
+/*
+ * linux/can.h
+ *
+ * Definitions for CAN network layer (socket addr / CAN frame / CAN filter)
+ *
+ * Authors: Oliver Hartkopp <oliver.hartkopp@volkswagen.de>
+ * Urs Thuermann <urs.thuermann@volkswagen.de>
+ * Copyright (c) 2002-2007 Volkswagen Group Electronic Research
+ * All rights reserved.
+ *
+ */
+
+#ifndef CAN_H
+#define CAN_H
+
+#include <linux/types.h>
+#include <linux/socket.h>
+
+/* controller area network (CAN) kernel definitions */
+
+/* special address description flags for the CAN_ID */
+#define CAN_EFF_FLAG 0x80000000U /* EFF/SFF is set in the MSB */
+#define CAN_RTR_FLAG 0x40000000U /* remote transmission request */
+#define CAN_ERR_FLAG 0x20000000U /* error message frame */
+
+/* valid bits in CAN ID for frame formats */
+#define CAN_SFF_MASK 0x000007FFU /* standard frame format (SFF) */
+#define CAN_EFF_MASK 0x1FFFFFFFU /* extended frame format (EFF) */
+#define CAN_ERR_MASK 0x1FFFFFFFU /* omit EFF, RTR, ERR flags */
+
+/*
+ * Controller Area Network Identifier structure
+ *
+ * bit 0-28 : CAN identifier (11/29 bit)
+ * bit 29 : error message frame flag (0 = data frame, 1 = error message)
+ * bit 30 : remote transmission request flag (1 = rtr frame)
+ * bit 31 : frame format flag (0 = standard 11 bit, 1 = extended 29 bit)
+ */
+typedef __u32 canid_t;
+
+#define CAN_SFF_ID_BITS 11
+#define CAN_EFF_ID_BITS 29
+
+/*
+ * Controller Area Network Error Message Frame Mask structure
+ *
+ * bit 0-28 : error class mask (see include/linux/can/error.h)
+ * bit 29-31 : set to zero
+ */
+typedef __u32 can_err_mask_t;
+
+/* CAN payload length and DLC definitions according to ISO 11898-1 */
+#define CAN_MAX_DLC 8
+#define CAN_MAX_DLEN 8
+
+/* CAN FD payload length and DLC definitions according to ISO 11898-7 */
+#define CANFD_MAX_DLC 15
+#define CANFD_MAX_DLEN 64
+
+/**
+ * struct can_frame - basic CAN frame structure
+ * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
+ * @can_dlc: frame payload length in byte (0 .. 8) aka data length code
+ * N.B. the DLC field from ISO 11898-1 Chapter 8.4.2.3 has a 1:1
+ * mapping of the 'data length code' to the real payload length
+ * @data: CAN frame payload (up to 8 byte)
+ */
+struct can_frame {
+ canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
+ __u8 can_dlc; /* frame payload length in byte (0 .. CAN_MAX_DLEN) */
+ __u8 data[CAN_MAX_DLEN] __attribute__((aligned(8)));
+};
+
+/*
+ * defined bits for canfd_frame.flags
+ *
+ * As the default for CAN FD should be to support the high data rate in the
+ * payload section of the frame (HDR) and to support up to 64 byte in the
+ * data section (EDL) the bits are only set in the non-default case.
+ * Btw. as long as there's no real implementation for CAN FD network driver
+ * these bits are only preliminary.
+ *
+ * RX: NOHDR/NOEDL - info about received CAN FD frame
+ * ESI - bit from originating CAN controller
+ * TX: NOHDR/NOEDL - control per-frame settings if supported by CAN controller
+ * ESI - bit is set by local CAN controller
+ */
+#define CANFD_NOHDR 0x01 /* frame without high data rate */
+#define CANFD_NOEDL 0x02 /* frame without extended data length */
+#define CANFD_ESI 0x04 /* error state indicator */
+
+/**
+ * struct canfd_frame - CAN flexible data rate frame structure
+ * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
+ * @len: frame payload length in byte (0 .. CANFD_MAX_DLEN)
+ * @flags: additional flags for CAN FD
+ * @__res0: reserved / padding
+ * @__res1: reserved / padding
+ * @data: CAN FD frame payload (up to CANFD_MAX_DLEN byte)
+ */
+struct canfd_frame {
+ canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
+ __u8 len; /* frame payload length in byte */
+ __u8 flags; /* additional flags for CAN FD */
+ __u8 __res0; /* reserved / padding */
+ __u8 __res1; /* reserved / padding */
+ __u8 data[CANFD_MAX_DLEN] __attribute__((aligned(8)));
+};
+
+#define CAN_MTU (sizeof(struct can_frame))
+#define CANFD_MTU (sizeof(struct canfd_frame))
+
+/* particular protocols of the protocol family PF_CAN */
+#define CAN_RAW 1 /* RAW sockets */
+#define CAN_BCM 2 /* Broadcast Manager */
+#define CAN_TP16 3 /* VAG Transport Protocol v1.6 */
+#define CAN_TP20 4 /* VAG Transport Protocol v2.0 */
+#define CAN_MCNET 5 /* Bosch MCNet */
+#define CAN_ISOTP 6 /* ISO 15765-2 Transport Protocol */
+#define CAN_NPROTO 7
+
+#define SOL_CAN_BASE 100
+
+/**
+ * struct sockaddr_can - the sockaddr structure for CAN sockets
+ * @can_family: address family number AF_CAN.
+ * @can_ifindex: CAN network interface index.
+ * @can_addr: protocol specific address information
+ */
+struct sockaddr_can {
+ __kernel_sa_family_t can_family;
+ int can_ifindex;
+ union {
+ /* transport protocol class address information (e.g. ISOTP) */
+ struct { canid_t rx_id, tx_id; } tp;
+
+ /* reserved for future CAN protocols address information */
+ } can_addr;
+};
+
+/**
+ * struct can_filter - CAN ID based filter in can_register().
+ * @can_id: relevant bits of CAN ID which are not masked out.
+ * @can_mask: CAN mask (see description)
+ *
+ * Description:
+ * A filter matches, when
+ *
+ * <received_can_id> & mask == can_id & mask
+ *
+ * The filter can be inverted (CAN_INV_FILTER bit set in can_id) or it can
+ * filter for error message frames (CAN_ERR_FLAG bit set in mask).
+ */
+struct can_filter {
+ canid_t can_id;
+ canid_t can_mask;
+};
+
+#define CAN_INV_FILTER 0x20000000U /* to be set in can_filter.can_id */
+
+#endif /* CAN_H */
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2 2/2] em_canid: Ematch used to classify CAN frames according to their identifiers
From: Rostislav Lisovy @ 2012-07-31 14:46 UTC (permalink / raw)
To: netdev; +Cc: linux-can, pisa, sojkam1, Rostislav Lisovy
In-Reply-To: <1343745981-14248-1-git-send-email-lisovy@gmail.com>
This ematch enables effective filtering of CAN frames (AF_CAN) based
on CAN identifiers with masking of compared bits. Implementation
utilizes bitmap based classification for standard frame format (SFF)
which is optimized for minimal overhead.
Signed-off-by: Rostislav Lisovy <lisovy@gmail.com>
---
etc/iproute2/ematch_map | 1 +
include/linux/pkt_cls.h | 6 +-
tc/Makefile | 1 +
tc/em_canid.c | 191 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 197 insertions(+), 2 deletions(-)
create mode 100644 tc/em_canid.c
diff --git a/etc/iproute2/ematch_map b/etc/iproute2/ematch_map
index 7c6a281..8d08613 100644
--- a/etc/iproute2/ematch_map
+++ b/etc/iproute2/ematch_map
@@ -3,3 +3,4 @@
2 nbyte
3 u32
4 meta
+7 canid
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index defbde2..082eafa 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -451,8 +451,10 @@ enum {
#define TCF_EM_U32 3
#define TCF_EM_META 4
#define TCF_EM_TEXT 5
-#define TCF_EM_VLAN 6
-#define TCF_EM_MAX 6
+#define TCF_EM_VLAN 6
+#define TCF_EM_CANID 7
+#define TCF_EM_IPSET 8
+#define TCF_EM_MAX 8
enum {
TCF_EM_PROG_TC
diff --git a/tc/Makefile b/tc/Makefile
index 64d93ad..bfdcf9f 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -45,6 +45,7 @@ TCMODULES += p_udp.o
TCMODULES += em_nbyte.o
TCMODULES += em_cmp.o
TCMODULES += em_u32.o
+TCMODULES += em_canid.o
TCMODULES += em_meta.o
TCMODULES += q_mqprio.o
TCMODULES += q_codel.o
diff --git a/tc/em_canid.c b/tc/em_canid.c
new file mode 100644
index 0000000..16f6ed5
--- /dev/null
+++ b/tc/em_canid.c
@@ -0,0 +1,191 @@
+/*
+ * em_canid.c Ematch rule to match CAN frames according to their CAN identifiers
+ *
+ * This program is free software; you can distribute 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.
+ *
+ * Idea: Oliver Hartkopp <oliver.hartkopp@volkswagen.de>
+ * Copyright: (c) 2011 Czech Technical University in Prague
+ * (c) 2011 Volkswagen Group Research
+ * Authors: Michal Sojka <sojkam1@fel.cvut.cz>
+ * Pavel Pisa <pisa@cmp.felk.cvut.cz>
+ * Rostislav Lisovy <lisovy@gmail.cz>
+ * Funded by: Volkswagen Group Research
+ *
+ * Documentation: http://rtime.felk.cvut.cz/can/socketcan-qdisc-final.pdf
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+#include <errno.h>
+#include <linux/can.h>
+#include <inttypes.h>
+#include "m_ematch.h"
+
+#define EM_CANID_RULES_MAX 400 /* Main reason for this number is Nelink
+ message size limit equal to Single memory page size. When dump()
+ is invoked, there are even some ematch related headers sent from
+ kernel to userspace together with em_canid configuration --
+ 400*sizeof(struct can_filter) should fit without any problems */
+
+extern struct ematch_util canid_ematch_util;
+struct rules {
+ struct can_filter *rules_raw;
+ int rules_capacity; /* Size of array allocated for rules_raw */
+ int rules_cnt; /* Actual number of rules stored in rules_raw */
+};
+
+static void canid_print_usage(FILE *fd)
+{
+ fprintf(fd,
+ "Usage: canid(IDLIST)\n" \
+ "where: IDLIST := IDSPEC [ IDLIST ]\n" \
+ " IDSPEC := { ’sff’ CANID | ’eff’ CANID }\n" \
+ " CANID := ID[:MASK]\n" \
+ " ID, MASK := hexadecimal number (i.e. 0x123)\n" \
+ "Example: canid(sff 0x123 sff 0x124 sff 0x125:0xf)\n");
+}
+
+static int canid_parse_rule(struct rules *rules, struct bstr *a, int iseff)
+{
+ unsigned int can_id = 0;
+ unsigned int can_mask = 0;
+
+ if (sscanf(a->data, "%"SCNx32 ":" "%"SCNx32, &can_id, &can_mask) != 2) {
+ if (sscanf(a->data, "%"SCNx32, &can_id) != 1) {
+ return -1;
+ } else {
+ can_mask = (iseff) ? CAN_EFF_MASK : CAN_SFF_MASK;
+ }
+ }
+
+ /* Stretch rules array up to EM_CANID_RULES_MAX if necessary */
+ if (rules->rules_cnt == rules->rules_capacity) {
+ if (rules->rules_capacity <= EM_CANID_RULES_MAX/2) {
+ rules->rules_capacity *= 2;
+ rules->rules_raw = realloc(rules->rules_raw,
+ sizeof(struct can_filter) * rules->rules_capacity);
+ } else {
+ return -2;
+ }
+ }
+
+ rules->rules_raw[rules->rules_cnt].can_id =
+ can_id | ((iseff) ? CAN_EFF_FLAG : 0);
+ rules->rules_raw[rules->rules_cnt].can_mask =
+ can_mask | CAN_EFF_FLAG;
+
+ rules->rules_cnt++;
+
+ return 0;
+}
+
+static int canid_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
+ struct bstr *args)
+{
+ int iseff = 0;
+ int ret = 0;
+ struct rules rules = {
+ .rules_capacity = 25, /* Denominator of EM_CANID_RULES_MAX
+ Will be multiplied by 2 to calculate the size for realloc() */
+ .rules_cnt = 0
+ };
+
+#define PARSE_ERR(CARG, FMT, ARGS...) \
+ em_parse_error(EINVAL, args, CARG, &canid_ematch_util, FMT, ##ARGS)
+
+ if (args == NULL)
+ return PARSE_ERR(args, "canid: missing arguments");
+
+ rules.rules_raw = malloc(sizeof(struct can_filter) * rules.rules_capacity);
+ memset(rules.rules_raw, 0, sizeof(struct can_filter) * rules.rules_capacity);
+
+ do {
+ if (!bstrcmp(args, "sff")) {
+ iseff = 0;
+ } else if (!bstrcmp(args, "eff")) {
+ iseff = 1;
+ } else {
+ ret = PARSE_ERR(args, "canid: invalid key");
+ goto exit;
+ }
+
+ args = bstr_next(args);
+ if (args == NULL) {
+ ret = PARSE_ERR(args, "canid: missing argument");
+ goto exit;
+ }
+
+ ret = canid_parse_rule(&rules, args, iseff);
+ if (ret == -1) {
+ ret = PARSE_ERR(args, "canid: Improperly formed CAN ID & mask\n");
+ goto exit;
+ } else if (ret == -2) {
+ ret = PARSE_ERR(args, "canid: Too many arguments on input\n");
+ goto exit;
+ }
+ } while ((args = bstr_next(args)) != NULL);
+
+ addraw_l(n, MAX_MSG, hdr, sizeof(*hdr));
+ addraw_l(n, MAX_MSG, rules.rules_raw,
+ sizeof(struct can_filter) * rules.rules_cnt);
+
+#undef PARSE_ERR
+exit:
+ free(rules.rules_raw);
+ return ret;
+}
+
+static int canid_print_eopt(FILE *fd, struct tcf_ematch_hdr *hdr, void *data,
+ int data_len)
+{
+ struct can_filter *conf = data; /* Array with rules */
+ int rules_count;
+ int i;
+
+ rules_count = data_len / sizeof(struct can_filter);
+
+ for (i = 0; i < rules_count; i++) {
+ struct can_filter *pcfltr = &conf[i];
+
+ if (pcfltr->can_id & CAN_EFF_FLAG) {
+ if (pcfltr->can_mask == (CAN_EFF_FLAG | CAN_EFF_MASK))
+ fprintf(fd, "eff 0x%"PRIX32,
+ pcfltr->can_id & CAN_EFF_MASK);
+ else
+ fprintf(fd, "eff 0x%"PRIX32":0x%"PRIX32,
+ pcfltr->can_id & CAN_EFF_MASK,
+ pcfltr->can_mask & CAN_EFF_MASK);
+ } else {
+ if (pcfltr->can_mask == (CAN_EFF_FLAG | CAN_SFF_MASK))
+ fprintf(fd, "sff 0x%"PRIX32,
+ pcfltr->can_id & CAN_SFF_MASK);
+ else
+ fprintf(fd, "sff 0x%"PRIX32":0x%"PRIX32,
+ pcfltr->can_id & CAN_SFF_MASK,
+ pcfltr->can_mask & CAN_SFF_MASK);
+ }
+
+ if ((i + 1) < rules_count)
+ fprintf(fd, " ");
+ }
+
+ return 0;
+}
+
+struct ematch_util canid_ematch_util = {
+ .kind = "canid",
+ .kind_num = TCF_EM_CANID,
+ .parse_eopt = canid_parse_eopt,
+ .print_eopt = canid_print_eopt,
+ .print_usage = canid_print_usage
+};
--
1.7.10.4
^ permalink raw reply related
* [PATCH] ipv4: percpu nh_rth_output cache
From: Eric Dumazet @ 2012-07-31 15:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Alexander Duyck
From: Eric Dumazet <edumazet@google.com>
Input path is mostly run under RCU and doesnt touch dst refcnt
But output path on forwarding or UDP workloads hits
badly dst refcount, and we have lot of false sharing, for example
in ipv4_mtu() when reading rt->rt_pmtu
Using a percpu cache for nh_rth_output gives a nice performance
increase at a small cost.
24 udpflood test on my 24 cpu machine (dummy0 output device)
(each process sends 1.000.000 udp frames, 24 processes are started)
before : 5.24 s
after : 2.06 s
For reference, time on linux-3.5 : 6.60 s
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
On top on previous "ipv4: Restore old dst_free() behavior" patch
We probably can remove all paddings in struct dst_entry
include/net/ip_fib.h | 3 ++-
net/ipv4/fib_semantics.c | 19 ++++++++++++++++++-
net/ipv4/route.c | 20 +++++++++++++++-----
3 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index e521a03..a0f003d 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -21,6 +21,7 @@
#include <linux/rcupdate.h>
#include <net/fib_rules.h>
#include <net/inetpeer.h>
+#include <linux/percpu.h>
struct fib_config {
u8 fc_dst_len;
@@ -81,7 +82,7 @@ struct fib_nh {
__be32 nh_gw;
__be32 nh_saddr;
int nh_saddr_genid;
- struct rtable __rcu *nh_rth_output;
+ struct rtable * __percpu *nh_pcpu_rth_output;
struct rtable __rcu *nh_rth_input;
struct fnhe_hash_bucket *nh_exceptions;
};
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 625cf18..c8bf86e 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -176,6 +176,22 @@ static void rt_nexthop_free(struct rtable __rcu **rtp)
dst_free(&rt->dst);
}
+static void rt_nexthop_free_cpus(struct rtable * __percpu *rtp)
+{
+ int cpu;
+
+ if (!rtp)
+ return;
+
+ for_each_possible_cpu(cpu) {
+ struct rtable *rt = *per_cpu_ptr(rtp, cpu);
+
+ if (rt)
+ dst_free(&rt->dst);
+ }
+ free_percpu(rtp);
+}
+
/* Release a nexthop info record */
static void free_fib_info_rcu(struct rcu_head *head)
{
@@ -186,7 +202,7 @@ static void free_fib_info_rcu(struct rcu_head *head)
dev_put(nexthop_nh->nh_dev);
if (nexthop_nh->nh_exceptions)
free_nh_exceptions(nexthop_nh);
- rt_nexthop_free(&nexthop_nh->nh_rth_output);
+ rt_nexthop_free_cpus(nexthop_nh->nh_pcpu_rth_output);
rt_nexthop_free(&nexthop_nh->nh_rth_input);
} endfor_nexthops(fi);
@@ -817,6 +833,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
fi->fib_nhs = nhs;
change_nexthops(fi) {
nexthop_nh->nh_parent = fi;
+ nexthop_nh->nh_pcpu_rth_output = alloc_percpu(struct rtable *);
} endfor_nexthops(fi)
if (cfg->fc_mx) {
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 2bd1074..d5efcfe 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1206,11 +1206,16 @@ static inline void rt_free(struct rtable *rt)
static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
{
- struct rtable *orig, *prev, **p = (struct rtable **)&nh->nh_rth_output;
+ struct rtable *orig, *prev, **p;
- if (rt_is_input_route(rt))
+ if (rt_is_input_route(rt)) {
p = (struct rtable **)&nh->nh_rth_input;
-
+ } else {
+ if (!nh->nh_pcpu_rth_output)
+ goto nocache;
+ p = per_cpu_ptr(nh->nh_pcpu_rth_output,
+ raw_smp_processor_id());
+ }
orig = *p;
prev = cmpxchg(p, orig, rt);
@@ -1223,6 +1228,7 @@ static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
* unsuccessful at storing this route into the cache
* we really need to set it.
*/
+nocache:
rt->dst.flags |= DST_NOCACHE;
}
}
@@ -1749,8 +1755,12 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
fnhe = NULL;
if (fi) {
fnhe = find_exception(&FIB_RES_NH(*res), fl4->daddr);
- if (!fnhe) {
- rth = rcu_dereference(FIB_RES_NH(*res).nh_rth_output);
+ if (!fnhe && FIB_RES_NH(*res).nh_pcpu_rth_output) {
+ struct rtable **prth;
+
+ prth = per_cpu_ptr(FIB_RES_NH(*res).nh_pcpu_rth_output,
+ raw_smp_processor_id());
+ rth = rcu_dereference(*(__force __rcu struct rtable **)prth);
if (rt_cache_valid(rth)) {
dst_hold(&rth->dst);
return rth;
^ permalink raw reply related
* Re: CPU: 0 Not tainted (3.1.9+ #1) when ifconfig rose0 down
From: Bernard Pidoux @ 2012-07-31 15:27 UTC (permalink / raw)
To: linux-hams, Linux Netdev List
In-Reply-To: <5017E786.5000102@free.fr>
Here is a complementary observation.
Trying to remove rose module with rmmod rose did not create any kernel
panic.
However, there is an endless message from the kernel saying :
Message from syslogd@raspberrypi at Jul 31 17:22:40 ...
kernel:[ 831.579007] unregister_netdevice: waiting for rose0 to
become free. Usage count = 23
Message from syslogd@raspberrypi at Jul 31 17:22:50 ...
kernel:[ 841.739390] unregister_netdevice: waiting for rose0 to
become free. Usage count = 23
Message from syslogd@raspberrypi at Jul 31 17:23:00 ...
kernel:[ 851.899758] unregister_netdevice: waiting for rose0 to
become free. Usage count = 23
.....
As observed at many occasions, count number seems to be random ! and
the same message keeps going without any change of count number.
At the same time, there is no possibility to recover the command line on
any console.
However I could loggin via ssh and I noticed that rose0 device is
actually no more in the ifconfig list.
If I try to remove rose with rmmod rose I get :
root@raspberrypi:/home/pi# rmmod rose
libkmod: ERROR ../libkmod/libkmod-module.c:753
kmod_module_remove_module: could not remove 'rose': Device or resource busy
Error: could not remove module rose: Device or resource busy
Does this help ?
On 31/07/2012 16:11, Bernard Pidoux wrote:
> Hi,
>
> I observe systematically a kernel panic when I try to shutdown rose0
> device using ifconfig rose0 down
>
> This is happening on two very different ROSE implementation, one is on a
> machine with x86-64 kernel 4.6.3 on an Intel core 2 duo CPU
> the other is on a RaspBerry Pi with Raspbian and 3.1.9+ wheezy kernel
> recompiled with AX.25 modules (ax25, rose, netrom, 6pack, kiss) enabled.
>
> Here is an image of the screen dump :
>
> http://f6bvp.org/photos/rose_device_event.JPG
>
> It can be noticed that PC is at rose_device_event and
> LR is at sock_def_wakeup
>
> One thing to be noticed is that when I close before all ROSE and AX.25
> applications, there are still a few populated sockets, probably for one
> of the program did not close the sockets properly.
>
> I that case, does rose module should accept to shutdown rose0 device ?
> However, I guess that it should not create a kernel panic due to a
> kernel NULL pointer.
>
> I don't know what to do in order to debug that issue.
>
> Bernard
>
^ permalink raw reply
* Re: [PATCH] ipv4: percpu nh_rth_output cache
From: Eric Dumazet @ 2012-07-31 15:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Alexander Duyck
In-Reply-To: <1343748166.21269.304.camel@edumazet-glaptop>
On Tue, 2012-07-31 at 17:22 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> + p = per_cpu_ptr(nh->nh_pcpu_rth_output,
> + raw_smp_processor_id());
> + }
Note : this can use :
p = __this_cpu_ptr(nh->nh_pcpu_rth_output);
and same thing in __mkroute_output()
prth = __this_cpu_ptr(FIB_RES_NH(*res).nh_pcpu_rth_output);
^ permalink raw reply
* [PATCH v2] ipv4: percpu nh_rth_output cache
From: Eric Dumazet @ 2012-07-31 15:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Alexander Duyck
From: Eric Dumazet <edumazet@google.com>
Input path is mostly run under RCU and doesnt touch dst refcnt
But output path on forwarding or UDP workloads hits
badly dst refcount, and we have lot of false sharing, for example
in ipv4_mtu() when reading rt->rt_pmtu
Using a percpu cache for nh_rth_output gives a nice performance
increase at a small cost.
24 udpflood test on my 24 cpu machine (dummy0 output device)
(each process sends 1.000.000 udp frames, 24 processes are started)
before : 5.24 s
after : 2.06 s
For reference, time on linux-3.5 : 6.60 s
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
v2: use __this_cpu_ptr() and slighly better annotations to avoid
ugly casts
On top on previous "ipv4: Restore old dst_free() behavior" patch
We probably can remove all paddings in struct dst_entry
include/net/ip_fib.h | 3 ++-
net/ipv4/fib_semantics.c | 20 +++++++++++++++++++-
net/ipv4/route.c | 18 +++++++++++++-----
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index e521a03..e331746 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -21,6 +21,7 @@
#include <linux/rcupdate.h>
#include <net/fib_rules.h>
#include <net/inetpeer.h>
+#include <linux/percpu.h>
struct fib_config {
u8 fc_dst_len;
@@ -81,7 +82,7 @@ struct fib_nh {
__be32 nh_gw;
__be32 nh_saddr;
int nh_saddr_genid;
- struct rtable __rcu *nh_rth_output;
+ struct rtable __rcu * __percpu *nh_pcpu_rth_output;
struct rtable __rcu *nh_rth_input;
struct fnhe_hash_bucket *nh_exceptions;
};
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 625cf18..fe2ca02 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -176,6 +176,23 @@ static void rt_nexthop_free(struct rtable __rcu **rtp)
dst_free(&rt->dst);
}
+static void rt_nexthop_free_cpus(struct rtable __rcu * __percpu *rtp)
+{
+ int cpu;
+
+ if (!rtp)
+ return;
+
+ for_each_possible_cpu(cpu) {
+ struct rtable *rt;
+
+ rt = rcu_dereference_protected(*per_cpu_ptr(rtp, cpu), 1);
+ if (rt)
+ dst_free(&rt->dst);
+ }
+ free_percpu(rtp);
+}
+
/* Release a nexthop info record */
static void free_fib_info_rcu(struct rcu_head *head)
{
@@ -186,7 +203,7 @@ static void free_fib_info_rcu(struct rcu_head *head)
dev_put(nexthop_nh->nh_dev);
if (nexthop_nh->nh_exceptions)
free_nh_exceptions(nexthop_nh);
- rt_nexthop_free(&nexthop_nh->nh_rth_output);
+ rt_nexthop_free_cpus(nexthop_nh->nh_pcpu_rth_output);
rt_nexthop_free(&nexthop_nh->nh_rth_input);
} endfor_nexthops(fi);
@@ -817,6 +834,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
fi->fib_nhs = nhs;
change_nexthops(fi) {
nexthop_nh->nh_parent = fi;
+ nexthop_nh->nh_pcpu_rth_output = alloc_percpu(struct rtable __rcu *);
} endfor_nexthops(fi)
if (cfg->fc_mx) {
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 2bd1074..4f6276c 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1206,11 +1206,15 @@ static inline void rt_free(struct rtable *rt)
static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
{
- struct rtable *orig, *prev, **p = (struct rtable **)&nh->nh_rth_output;
+ struct rtable *orig, *prev, **p;
- if (rt_is_input_route(rt))
+ if (rt_is_input_route(rt)) {
p = (struct rtable **)&nh->nh_rth_input;
-
+ } else {
+ if (!nh->nh_pcpu_rth_output)
+ goto nocache;
+ p = (struct rtable **)__this_cpu_ptr(nh->nh_pcpu_rth_output);
+ }
orig = *p;
prev = cmpxchg(p, orig, rt);
@@ -1223,6 +1227,7 @@ static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
* unsuccessful at storing this route into the cache
* we really need to set it.
*/
+nocache:
rt->dst.flags |= DST_NOCACHE;
}
}
@@ -1749,8 +1754,11 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
fnhe = NULL;
if (fi) {
fnhe = find_exception(&FIB_RES_NH(*res), fl4->daddr);
- if (!fnhe) {
- rth = rcu_dereference(FIB_RES_NH(*res).nh_rth_output);
+ if (!fnhe && FIB_RES_NH(*res).nh_pcpu_rth_output) {
+ struct rtable __rcu **prth;
+
+ prth = __this_cpu_ptr(FIB_RES_NH(*res).nh_pcpu_rth_output);
+ rth = rcu_dereference(*prth);
if (rt_cache_valid(rth)) {
dst_hold(&rth->dst);
return rth;
^ permalink raw reply related
* Re: [PATCH] mac80211: use eth_broadcast_addr
From: Joe Perches @ 2012-07-31 15:46 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Johannes Berg, netdev
In-Reply-To: <1343744471-5910-1-git-send-email-johannes@sipsolutions.net>
On Tue, 2012-07-31 at 16:21 +0200, Johannes Berg wrote:
> Instead of memset().
Maybe just do them all?
git ls-files net drivers/net |
xargs perl -p -i -e 's/\bmemset\s*\(\s*([^,]+),\s*(?i:0xff|255)\s*\,\s*(ETH_ALEN|6)\s*\)/eth_broadcast_addr\(\1\)/g'
^ permalink raw reply
* [PATCH v2 0/6] mv643xx Ethernet DT support and CSB1724 board support.
From: Ian Molton @ 2012-07-31 15:45 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev
Respin of the previous patch. All comments have been addressed.
Addressed:
* Typos
* Docs
* Automatic clock detection / setup
* PHY addresses are not | 0x80 anymore
- which is a horrid way for the driver to be doing things but hey...
* property added to DT bindings for tx_csum_limit
* GFP_ATOMIC allocations demoted to GFP_KERNEL (cpoypaste error)
* devide order in .dts(i) files fixed
Outstanding:
* Should the tx_csum_limit issue be addressed by:
- a new device name
- detecting the host platform
- a property (present solution).
Ian Molton (6):
Initial csb1724 board support (FDT)
mv643xx.c: Remove magic numbers.
mv643xx.c: Add basic device tree support.
kirkwood: Add a clock setup helper for mv643xx ethernet.
csb1724: Enable device tree based mv643xx ethernet support.
DT: Convert all kirkwood boards with mv643xx that use DT
Documentation/devicetree/bindings/net/mv643xx.txt | 75 +++++++++++++
arch/arm/boot/dts/kirkwood-csb1724.dts | 49 +++++++++
arch/arm/boot/dts/kirkwood-dnskw.dtsi | 9 ++
arch/arm/boot/dts/kirkwood-dreamplug.dts | 18 +++
arch/arm/boot/dts/kirkwood-goflexnet.dts | 8 ++
arch/arm/boot/dts/kirkwood-ib62x0.dts | 10 ++
arch/arm/boot/dts/kirkwood-iconnect.dts | 10 ++
arch/arm/boot/dts/kirkwood-lsxl.dtsi | 17 +++
arch/arm/boot/dts/kirkwood-ts219-6281.dts | 8 +-
arch/arm/boot/dts/kirkwood-ts219-6282.dts | 8 +-
arch/arm/boot/dts/kirkwood-ts219.dtsi | 3 +
arch/arm/boot/dts/kirkwood.dtsi | 33 ++++++
arch/arm/configs/csb1724_defconfig | 92 ++++++++++++++++
arch/arm/mach-kirkwood/Kconfig | 7 ++
arch/arm/mach-kirkwood/Makefile | 1 +
arch/arm/mach-kirkwood/Makefile.boot | 1 +
arch/arm/mach-kirkwood/board-csb1724.c | 60 ++++++++++
arch/arm/mach-kirkwood/board-dnskw.c | 7 +-
arch/arm/mach-kirkwood/board-dreamplug.c | 13 +--
arch/arm/mach-kirkwood/board-dt.c | 11 ++
arch/arm/mach-kirkwood/board-goflexnet.c | 7 +-
arch/arm/mach-kirkwood/board-ib62x0.c | 7 +-
arch/arm/mach-kirkwood/board-iconnect.c | 7 +-
arch/arm/mach-kirkwood/board-lsxl.c | 13 +--
arch/arm/mach-kirkwood/board-ts219.c | 10 +-
arch/arm/mach-kirkwood/common.c | 22 ++++
arch/arm/mach-kirkwood/common.h | 9 ++
drivers/net/ethernet/marvell/mv643xx_eth.c | 122 ++++++++++++++++++---
28 files changed, 565 insertions(+), 72 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/mv643xx.txt
create mode 100644 arch/arm/boot/dts/kirkwood-csb1724.dts
create mode 100644 arch/arm/configs/csb1724_defconfig
create mode 100644 arch/arm/mach-kirkwood/board-csb1724.c
--
1.7.9.5
^ permalink raw reply
* [PATCH v2 4/6] kirkwood: Add a clock setup helper for mv643xx ethernet.
From: Ian Molton @ 2012-07-31 15:45 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev
In-Reply-To: <1343749529-17571-1-git-send-email-ian.molton@codethink.co.uk>
This patch adds an ethernet setup helper function allowing the mv643xx
clock to be kept enabled so that the MAC address(es) are not lost.
Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
arch/arm/mach-kirkwood/board-dt.c | 2 ++
arch/arm/mach-kirkwood/common.c | 22 ++++++++++++++++++++++
arch/arm/mach-kirkwood/common.h | 3 +++
3 files changed, 27 insertions(+)
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index 9816b85..1bcae22 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -97,6 +97,8 @@ static void __init kirkwood_dt_init(void)
of_platform_populate(NULL, kirkwood_dt_match_table,
kirkwood_auxdata_lookup, NULL);
+
+ kirkwood_eth_clock_fixup();
}
static const char *kirkwood_dt_board_compat[] = {
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index c4b64ad..57b91cf 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -18,6 +18,7 @@
#include <linux/clk-provider.h>
#include <linux/spinlock.h>
#include <linux/mv643xx_i2c.h>
+#include <linux/of.h>
#include <net/dsa.h>
#include <asm/page.h>
#include <asm/timex.h>
@@ -293,6 +294,27 @@ void __init kirkwood_ehci_init(void)
orion_ehci_init(USB_PHYS_BASE, IRQ_KIRKWOOD_USB, EHCI_PHY_NA);
}
+/* Fixup ethernet clocks for DT based kirkwood platforms.
+ * This is required because if the clock is not kept running, the
+ * Interface will forget its MAC address.
+ */
+#ifdef CONFIG_OF
+void __init kirkwood_eth_clock_fixup(void)
+{
+ struct device_node *np;
+
+ np = of_find_node_by_name(NULL, "egiga0");
+ if (np && of_device_is_available(np))
+ clk_prepare_enable(ge0);
+ of_node_put(np);
+
+ np = of_find_node_by_name(NULL, "egiga1");
+ if (np && of_device_is_available(np))
+ clk_prepare_enable(ge1);
+ of_node_put(np);
+
+}
+#endif
/*****************************************************************************
* GE00
diff --git a/arch/arm/mach-kirkwood/common.h b/arch/arm/mach-kirkwood/common.h
index 8aab1ae..7183718 100644
--- a/arch/arm/mach-kirkwood/common.h
+++ b/arch/arm/mach-kirkwood/common.h
@@ -36,6 +36,9 @@ void kirkwood_enable_pcie(void);
void kirkwood_pcie_id(u32 *dev, u32 *rev);
void kirkwood_ehci_init(void);
+#ifdef CONFIG_OF
+void kirkwood_eth_clock_fixup(void);
+#endif
void kirkwood_ge00_init(struct mv643xx_eth_platform_data *eth_data);
void kirkwood_ge01_init(struct mv643xx_eth_platform_data *eth_data);
void kirkwood_ge00_switch_init(struct dsa_platform_data *d, int irq);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 1/6] Initial csb1724 board support (FDT)
From: Ian Molton @ 2012-07-31 15:45 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev
In-Reply-To: <1343749529-17571-1-git-send-email-ian.molton@codethink.co.uk>
This patch adds support for the csb1724 SoM.
It includes serial and SATA support.
Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
arch/arm/boot/dts/kirkwood-csb1724.dts | 30 ++++++++++++++++
arch/arm/configs/csb1724_defconfig | 47 +++++++++++++++++++++++++
arch/arm/mach-kirkwood/Kconfig | 7 ++++
arch/arm/mach-kirkwood/Makefile | 1 +
arch/arm/mach-kirkwood/Makefile.boot | 1 +
arch/arm/mach-kirkwood/board-csb1724.c | 59 ++++++++++++++++++++++++++++++++
arch/arm/mach-kirkwood/board-dt.c | 4 +++
arch/arm/mach-kirkwood/common.h | 6 ++++
8 files changed, 155 insertions(+)
create mode 100644 arch/arm/boot/dts/kirkwood-csb1724.dts
create mode 100644 arch/arm/configs/csb1724_defconfig
create mode 100644 arch/arm/mach-kirkwood/board-csb1724.c
diff --git a/arch/arm/boot/dts/kirkwood-csb1724.dts b/arch/arm/boot/dts/kirkwood-csb1724.dts
new file mode 100644
index 0000000..44dfe9a
--- /dev/null
+++ b/arch/arm/boot/dts/kirkwood-csb1724.dts
@@ -0,0 +1,30 @@
+/dts-v1/;
+
+/include/ "kirkwood.dtsi"
+
+/ {
+ model = "Cogent CSB1724-88F-628X SoM";
+ compatible = "cogent,csb1724", "marvell,kirkwood-88f6281", "marvell,kirkwood";
+
+ memory {
+ device_type = "memory";
+ reg = <0x00000000 0x20000000>;
+ };
+
+ chosen {
+ bootargs = "console=ttyS0,115200n8 earlyprintk";
+ };
+
+ ocp@f1000000 {
+ serial@12000 {
+ clock-frequency = <200000000>;
+ status = "ok";
+ };
+
+ sata@80000 {
+ nr-ports = <2>;
+ status = "ok";
+ };
+ };
+
+};
diff --git a/arch/arm/configs/csb1724_defconfig b/arch/arm/configs/csb1724_defconfig
new file mode 100644
index 0000000..927b269
--- /dev/null
+++ b/arch/arm/configs/csb1724_defconfig
@@ -0,0 +1,47 @@
+CONFIG_ARCH_KIRKWOOD=y
+CONFIG_ARCH_KIRKWOOD_DT=y
+CONFIG_MACH_CSB1724_DT=y
+CONFIG_EMBEDDED=y
+CONFIG_EXPERT=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_ARM_APPENDED_DTB=y
+CONFIG_VFP=y
+CONFIG_AEABI=y
+CONFIG_OABI_COMPAT=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_ATA=y
+CONFIG_ATA_SFF=y
+CONFIG_ATA_BMDMA=y
+CONFIG_SATA_MV=y
+CONFIG_MSDOS_FS=y
+CONFIG_VFAT_FS=y
+CONFIG_TMPFS=y
+CONFIG_NLS_CODEPAGE_437=y
+CONFIG_NLS_CODEPAGE_850=y
+CONFIG_NLS_ISO8859_1=y
+CONFIG_NLS_ISO8859_2=y
+CONFIG_NLS_UTF8=y
+CONFIG_MAGIC_SYSRQ=y
+CONFIG_DEBUG_FS=y
+CONFIG_TIMER_STATS=y
+CONFIG_DEBUG_INFO=y
+CONFIG_DEBUG_USER=y
+CONFIG_DEBUG_LL=y
+CONFIG_EARLY_PRINTK=y
+CONFIG_DTC=y
+CONFIG_SCSI=y
+CONFIG_BLK_DEV_SD=y
+CONFIG_EXT4_FS=y
+CONFIG_SYSVIPC=y
+CONFIG_SYSVIPC_SYSCTL=y
+CONFIG_TICK_ONESHOT=y
+CONFIG_NO_HZ=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_MODULES=y
+CONFIG_MODULE_UNLOAD=y
+CONFIG_PROC_DEVICETREE=y
+CONFIG_BLK_DEV_LOOP=y
+CONFIG_BLK_DEV_LOOP_MIN_COUNT=8
+CONFIG_DYNAMIC_DEBUG=y
diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
index ca5c15a..6d51c50 100644
--- a/arch/arm/mach-kirkwood/Kconfig
+++ b/arch/arm/mach-kirkwood/Kconfig
@@ -109,6 +109,13 @@ config MACH_LSXL_DT
Buffalo Linkstation LS-XHL & LS-CHLv2 devices, using
Flattened Device Tree.
+config MACH_CSB1724_DT
+ bool "Cogent CSB1724 SoM (Flattened Device Tree)"
+ select ARCH_KIRKWOOD_DT
+ help
+ Say 'Y' here if you want your kernel to support the
+ Cogent CSB1724 SoM, using Flattened Device Tree.
+
config MACH_TS219
bool "QNAP TS-110, TS-119, TS-119P+, TS-210, TS-219, TS-219P and TS-219P+ Turbo NAS"
help
diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
index 055c85a..665ed63 100644
--- a/arch/arm/mach-kirkwood/Makefile
+++ b/arch/arm/mach-kirkwood/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_MACH_IB62X0_DT) += board-ib62x0.o
obj-$(CONFIG_MACH_TS219_DT) += board-ts219.o tsx1x-common.o
obj-$(CONFIG_MACH_GOFLEXNET_DT) += board-goflexnet.o
obj-$(CONFIG_MACH_LSXL_DT) += board-lsxl.o
+obj-$(CONFIG_MACH_CSB1724_DT) += board-csb1724.o
diff --git a/arch/arm/mach-kirkwood/Makefile.boot b/arch/arm/mach-kirkwood/Makefile.boot
index 2a576ab..899bc80 100644
--- a/arch/arm/mach-kirkwood/Makefile.boot
+++ b/arch/arm/mach-kirkwood/Makefile.boot
@@ -2,6 +2,7 @@
params_phys-y := 0x00000100
initrd_phys-y := 0x00800000
+dtb-$(CONFIG_MACH_CSB1724_DT) += kirkwood-csb1724.dtb
dtb-$(CONFIG_MACH_DREAMPLUG_DT) += kirkwood-dreamplug.dtb
dtb-$(CONFIG_MACH_DLINK_KIRKWOOD_DT) += kirkwood-dns320.dtb
dtb-$(CONFIG_MACH_DLINK_KIRKWOOD_DT) += kirkwood-dns325.dtb
diff --git a/arch/arm/mach-kirkwood/board-csb1724.c b/arch/arm/mach-kirkwood/board-csb1724.c
new file mode 100644
index 0000000..979112d
--- /dev/null
+++ b/arch/arm/mach-kirkwood/board-csb1724.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2012 (C), Ian Molton <ian.molton@codethink.co.uk>
+ *
+ * arch/arm/mach-kirkwood/board-csb1724.c
+ *
+ * Cogent csb1724 Board Init for drivers not converted to
+ * flattened device tree yet.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include "mpp.h"
+
+static unsigned int csb1724_mpp_config[] __initdata = {
+ MPP0_NF_IO2,
+ MPP1_NF_IO3,
+ MPP2_NF_IO4,
+ MPP3_NF_IO5,
+ MPP4_NF_IO6,
+ MPP5_NF_IO7,
+ MPP8_TW0_SDA,
+ MPP9_TW0_SCK,
+ MPP12_SD_CLK,
+ MPP13_SD_CMD,
+ MPP14_SD_D0,
+ MPP15_SD_D1,
+ MPP16_SD_D2,
+ MPP17_SD_D3,
+ MPP18_NF_IO0,
+ MPP19_NF_IO1,
+ MPP20_GE1_TXD0,
+ MPP21_GE1_TXD1,
+ MPP22_GE1_TXD2,
+ MPP23_GE1_TXD3,
+ MPP24_GE1_RXD0,
+ MPP25_GE1_RXD1,
+ MPP26_GE1_RXD2,
+ MPP27_GE1_RXD3,
+ MPP30_GE1_RXCTL,
+ MPP31_GE1_RXCLK,
+ MPP32_GE1_TCLKOUT,
+ MPP33_GE1_TXCTL,
+ MPP34_SATA1_ACTn,
+ MPP35_SATA0_ACTn,
+ MPP36_TW1_SDA,
+ MPP37_TW1_SCK,
+};
+
+void __init csb1724_init(void)
+{
+ /*
+ * Basic setup. Needs to be called early.
+ */
+ kirkwood_mpp_conf(csb1724_mpp_config);
+}
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index e4eb450..7679f7f 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -87,6 +87,9 @@ static void __init kirkwood_dt_init(void)
if (of_machine_is_compatible("buffalo,lsxl"))
lsxl_init();
+ if (of_machine_is_compatible("cogent,csb1724"))
+ csb1724_init();
+
of_platform_populate(NULL, kirkwood_dt_match_table,
kirkwood_auxdata_lookup, NULL);
}
@@ -100,6 +103,7 @@ static const char *kirkwood_dt_board_compat[] = {
"qnap,ts219",
"seagate,goflexnet",
"buffalo,lsxl",
+ "cogent,csb1724",
NULL
};
diff --git a/arch/arm/mach-kirkwood/common.h b/arch/arm/mach-kirkwood/common.h
index 304dd1a..8aab1ae 100644
--- a/arch/arm/mach-kirkwood/common.h
+++ b/arch/arm/mach-kirkwood/common.h
@@ -94,6 +94,12 @@ void lsxl_init(void);
static inline void lsxl_init(void) {};
#endif
+#ifdef CONFIG_MACH_CSB1724_DT
+void csb1724_init(void);
+#else
+static inline void csb1724_init(void) {};
+#endif
+
/* early init functions not converted to fdt yet */
char *kirkwood_id(void);
void kirkwood_l2_init(void);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 3/6] mv643xx.c: Add basic device tree support.
From: Ian Molton @ 2012-07-31 15:45 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev
In-Reply-To: <1343749529-17571-1-git-send-email-ian.molton@codethink.co.uk>
This patch adds basic device tree support to the mv643xx ethernet driver.
It should be enough for most current users of the device, and should allow
a fairly painless migration once proper support for clk devices is available
to those platforms.
Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
Documentation/devicetree/bindings/net/mv643xx.txt | 75 +++++++++++++
arch/arm/mach-kirkwood/board-dt.c | 5 +
drivers/net/ethernet/marvell/mv643xx_eth.c | 119 ++++++++++++++++++---
3 files changed, 185 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/mv643xx.txt
diff --git a/Documentation/devicetree/bindings/net/mv643xx.txt b/Documentation/devicetree/bindings/net/mv643xx.txt
new file mode 100644
index 0000000..2727f79
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mv643xx.txt
@@ -0,0 +1,75 @@
+mv643xx related nodes.
+
+marvell,mdio-mv643xx:
+
+Required properties:
+
+ - interrupts : <a> where a is the SMI interrupt number.
+ - reg : the base address and size of the controllers register space.
+
+Optional properties:
+ - shared_smi : on some chips, the second PHY is "shared", meaning it is
+ really accessed via the first SMI controller. It is passed in this
+ way due to the present structure of the driver, which requires the
+ base address for the MAC to be passed in via the SMI controllers
+ platform data.
+ - tx_csum_limit : on some devices, this option is required for proper
+ operation wrt. jumbo frames.
+
+
+Example:
+
+smi0: mdio@72000 {
+ compatible = "marvell,mdio-mv643xx";
+ reg = <0x72000 0x4000>;
+ interrupts = <46>;
+ tx_csum_limit = <1600>;
+ status = "disabled";
+};
+
+smi1: mdio@76000 {
+ compatible = "marvell,mdio-mv643xx";
+ reg = <0x76000 0x4000>;
+ interrupts = <47>;
+ shared_smi = <&smi0>;
+ tx_csum_limit = <1600>;
+ status = "disabled";
+};
+
+
+
+marvell,mv643xx-eth:
+
+Required properties:
+ - interrupts : the port interrupt number.
+ - mdio : phandle of the smi device as drescribed above
+
+Optional properties:
+ - port_number : the port number on this bus.
+ - phy_addr : the PHY address.
+ - reg : should match the mdio reg this device is attached to.
+ this is a required hack for now due to the way the
+ driver is constructed. This allows the device clock to be
+ kept running so that the MAC is not lost after boot.
+
+
+Example:
+
+egiga0 {
+ compatible = "marvell,mv643xx-eth";
+ reg = <0x72000 0x4000>;
+ mdio = <&smi0>;
+ port_number = <0>;
+ phy_addr = <0x80>;
+ interrupts = <11>;
+};
+
+egiga1 {
+ compatible = "marvell,mv643xx-eth";
+ reg = <0x76000 0x4000>;
+ mdio = <&smi1>;
+ port_number = <0>;
+ phy_addr = <0x81>;
+ interrupts = <15>;
+};
+
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index 7679f7f..9816b85 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -14,6 +14,7 @@
#include <linux/init.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/mv643xx_eth.h>
#include <linux/kexec.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
@@ -33,6 +34,10 @@ struct of_dev_auxdata kirkwood_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("marvell,orion-wdt", 0xf1020300, "orion_wdt", NULL),
OF_DEV_AUXDATA("marvell,orion-sata", 0xf1080000, "sata_mv.0", NULL),
OF_DEV_AUXDATA("marvell,orion-nand", 0xf4000000, "orion_nand", NULL),
+ OF_DEV_AUXDATA("marvell,mv643xx", 0xf1072000, MV643XX_ETH_NAME ".0",
+ NULL),
+ OF_DEV_AUXDATA("marvell,mv643xx", 0xf1076000, MV643XX_ETH_NAME ".1",
+ NULL),
{},
};
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 92497eb..733c69f 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -48,6 +48,9 @@
#include <linux/ethtool.h>
#include <linux/platform_device.h>
#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_irq.h>
#include <linux/kernel.h>
#include <linux/spinlock.h>
#include <linux/workqueue.h>
@@ -2601,11 +2604,11 @@ static void infer_hw_params(struct mv643xx_eth_shared_private *msp)
static int mv643xx_eth_shared_probe(struct platform_device *pdev)
{
static int mv643xx_eth_version_printed;
- struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
+ struct mv643xx_eth_shared_platform_data *pd;
struct mv643xx_eth_shared_private *msp;
const struct mbus_dram_target_info *dram;
struct resource *res;
- int ret;
+ int ret, irq = -1;
if (!mv643xx_eth_version_printed++)
pr_notice("MV-643xx 10/100/1000 ethernet driver version %s\n",
@@ -2625,6 +2628,26 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
if (msp->base == NULL)
goto out_free;
+ if (pdev->dev.of_node) {
+ struct device_node *np = NULL;
+
+ /* when all users of this driver use FDT, we can remove this */
+ pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+ if (!pd) {
+ dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+ goto out_free;
+ }
+
+ of_property_read_u32(pdev->dev.of_node,
+ "tx_csum_limit", &pd->tx_csum_limit);
+
+ np = of_parse_phandle(pdev->dev.of_node, "shared_smi", 0);
+ if (np)
+ pd->shared_smi = of_find_device_by_node(np);
+
+ } else {
+ pd = pdev->dev.platform_data;
+ }
/*
* Set up and register SMI bus.
*/
@@ -2654,15 +2677,22 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
/*
* Check whether the error interrupt is hooked up.
*/
- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (res != NULL) {
+ if (pdev->dev.of_node) {
+ irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+ } else {
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (res)
+ irq = res->start;
+ }
+
+ if (irq != -1) {
int err;
- err = request_irq(res->start, mv643xx_eth_err_irq,
+ err = request_irq(irq, mv643xx_eth_err_irq,
IRQF_SHARED, "mv643xx_eth", msp);
if (!err) {
writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
- msp->err_interrupt = res->start;
+ msp->err_interrupt = irq;
}
}
@@ -2675,6 +2705,10 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
msp->tx_csum_limit = (pd != NULL && pd->tx_csum_limit) ?
pd->tx_csum_limit : 9 * 1024;
+
+ if (pdev->dev.of_node)
+ kfree(pd); /* If we created a fake pd, free it now */
+
infer_hw_params(msp);
platform_set_drvdata(pdev, msp);
@@ -2708,12 +2742,21 @@ static int mv643xx_eth_shared_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_OF
+static struct of_device_id mv_mdio_dt_ids[] __devinitdata = {
+ { .compatible = "marvell,mdio-mv643xx", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mv_mdio_dt_ids);
+#endif
+
static struct platform_driver mv643xx_eth_shared_driver = {
.probe = mv643xx_eth_shared_probe,
.remove = mv643xx_eth_shared_remove,
.driver = {
.name = MV643XX_ETH_SHARED_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(mv_mdio_dt_ids),
},
};
@@ -2873,7 +2916,36 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
struct resource *res;
int err;
- pd = pdev->dev.platform_data;
+ if (pdev->dev.of_node) {
+ struct device_node *np = NULL;
+
+ /* when all users of this driver use FDT, we can remove this */
+ pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+ if (!pd) {
+ dev_dbg(&pdev->dev, "Could not allocate platform data\n");
+ return -ENOMEM;
+ }
+
+ of_property_read_u32(pdev->dev.of_node,
+ "port_number", &pd->port_number);
+
+ if(!of_property_read_u32(pdev->dev.of_node,
+ "phy_addr", &pd->phy_addr))
+ pd->phy_addr = MV643XX_ETH_PHY_ADDR(pd->phy_addr);
+ else
+ pd->phy_addr = MV643XX_ETH_PHY_ADDR_DEFAULT;
+
+ np = of_parse_phandle(pdev->dev.of_node, "mdio", 0);
+ if (np) {
+ pd->shared = of_find_device_by_node(np);
+ } else {
+ kfree(pd);
+ return -ENODEV;
+ }
+ } else {
+ pd = pdev->dev.platform_data;
+ }
+
if (pd == NULL) {
dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
return -ENODEV;
@@ -2881,12 +2953,15 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
if (pd->shared == NULL) {
dev_err(&pdev->dev, "no mv643xx_eth_platform_data->shared\n");
- return -ENODEV;
+ err = -ENODEV;
+ goto out_free_pd;
}
dev = alloc_etherdev_mq(sizeof(struct mv643xx_eth_private), 8);
- if (!dev)
- return -ENOMEM;
+ if (!dev) {
+ err = -ENOMEM;
+ goto out_free_pd;
+ }
mp = netdev_priv(dev);
platform_set_drvdata(pdev, mp);
@@ -2923,6 +2998,8 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
init_pscr(mp, pd->speed, pd->duplex);
+ if (pdev->dev.of_node)
+ kfree(pd); /* If we created a fake pd, free it now */
mib_counters_clear(mp);
@@ -2942,10 +3019,13 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
mp->rx_oom.data = (unsigned long)mp;
mp->rx_oom.function = oom_timer_wrapper;
-
- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- BUG_ON(!res);
- dev->irq = res->start;
+ if (pdev->dev.of_node) {
+ dev->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+ } else {
+ res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ BUG_ON(!res);
+ dev->irq = res->start;
+ }
dev->netdev_ops = &mv643xx_eth_netdev_ops;
@@ -2991,6 +3071,8 @@ out:
}
#endif
free_netdev(dev);
+out_free_pd:
+ kfree(pd);
return err;
}
@@ -3030,6 +3112,14 @@ static void mv643xx_eth_shutdown(struct platform_device *pdev)
port_reset(mp);
}
+#ifdef CONFIG_OF
+static struct of_device_id mv_eth_dt_ids[] __devinitdata = {
+ { .compatible = "marvell,mv643xx-eth", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mv_eth_dt_ids);
+#endif
+
static struct platform_driver mv643xx_eth_driver = {
.probe = mv643xx_eth_probe,
.remove = mv643xx_eth_remove,
@@ -3037,6 +3127,7 @@ static struct platform_driver mv643xx_eth_driver = {
.driver = {
.name = MV643XX_ETH_NAME,
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(mv_eth_dt_ids),
},
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 5/6] csb1724: Enable device tree based mv643xx ethernet support.
From: Ian Molton @ 2012-07-31 15:45 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev
In-Reply-To: <1343749529-17571-1-git-send-email-ian.molton@codethink.co.uk>
This patch enables mv643xx based ethernet built into the SoM on the
csb1724, via flattened device tree.
Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
arch/arm/boot/dts/kirkwood-csb1724.dts | 19 ++++++++++++++
arch/arm/boot/dts/kirkwood.dtsi | 33 +++++++++++++++++++++++
arch/arm/configs/csb1724_defconfig | 45 ++++++++++++++++++++++++++++++++
arch/arm/mach-kirkwood/board-csb1724.c | 1 +
4 files changed, 98 insertions(+)
diff --git a/arch/arm/boot/dts/kirkwood-csb1724.dts b/arch/arm/boot/dts/kirkwood-csb1724.dts
index 44dfe9a..f43f8dd 100644
--- a/arch/arm/boot/dts/kirkwood-csb1724.dts
+++ b/arch/arm/boot/dts/kirkwood-csb1724.dts
@@ -25,6 +25,25 @@
nr-ports = <2>;
status = "ok";
};
+
+ smi0: mdio@72000 {
+ status = "ok";
+ };
+
+ smi1: mdio@76000 {
+ status = "ok";
+ };
+
+ egiga0 {
+ phy_addr = <0>;
+ status = "ok";
+ };
+
+ egiga1 {
+ phy_addr = <1>;
+ status = "ok";
+ };
+
};
};
diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index cef9616..f5f1f92 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -76,6 +76,39 @@
status = "okay";
};
+ smi0: mdio@72000 {
+ compatible = "marvell,mdio-mv643xx";
+ reg = <0x72000 0x4000>;
+ interrupts = <46>;
+ tx_csum_limit = <1600>;
+ status = "disabled";
+ };
+
+ egiga0 {
+ compatible = "marvell,mv643xx-eth";
+ reg = <0x72000 0x4000>;
+ mdio = <&smi0>;
+ interrupts = <11>;
+ status = "disabled";
+ };
+
+ smi1: mdio@76000 {
+ compatible = "marvell,mdio-mv643xx";
+ reg = <0x76000 0x4000>;
+ interrupts = <47>;
+ shared_smi = <&smi0>;
+ tx_csum_limit = <1600>;
+ status = "disabled";
+ };
+
+ egiga1 {
+ compatible = "marvell,mv643xx-eth";
+ reg = <0x76000 0x4000>;
+ mdio = <&smi1>;
+ interrupts = <15>;
+ status = "disabled";
+ };
+
sata@80000 {
compatible = "marvell,orion-sata";
reg = <0x80000 0x5000>;
diff --git a/arch/arm/configs/csb1724_defconfig b/arch/arm/configs/csb1724_defconfig
index 927b269..fbea657 100644
--- a/arch/arm/configs/csb1724_defconfig
+++ b/arch/arm/configs/csb1724_defconfig
@@ -45,3 +45,48 @@ CONFIG_PROC_DEVICETREE=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_LOOP_MIN_COUNT=8
CONFIG_DYNAMIC_DEBUG=y
+CONFIG_NET=y
+CONFIG_INET=y
+CONFIG_NETDEVICES=y
+CONFIG_MV643XX_ETH=y
+CONFIG_PACKET=y
+CONFIG_UNIX=y
+CONFIG_IPV6=n
+CONFIG_NET_VENDOR_3COM=n
+CONFIG_NET_VENDOR_ADAPTEC=n
+CONFIG_NET_VENDOR_ALTEON=n
+CONFIG_NET_VENDOR_AMD=n
+CONFIG_NET_VENDOR_ATHEROS=n
+CONFIG_NET_VENDOR_BROADCOM=n
+CONFIG_NET_VENDOR_BROCADE=n
+CONFIG_NET_VENDOR_CHELSIO=n
+CONFIG_NET_VENDOR_CIRRUS=n
+CONFIG_NET_VENDOR_CISCO=n
+CONFIG_NET_VENDOR_DEC=n
+CONFIG_NET_VENDOR_HP=n
+CONFIG_NET_VENDOR_DLINK=n
+CONFIG_NET_VENDOR_EMULEX=n
+CONFIG_NET_VENDOR_EXAR=n
+CONFIG_NET_VENDOR_FARADAY=n
+CONFIG_NET_VENDOR_INTEL=n
+CONFIG_NET_VENDOR_MELLANOX=n
+CONFIG_NET_VENDOR_MICREL=n
+CONFIG_NET_VENDOR_MYRI=n
+CONFIG_NET_VENDOR_NATSEMI=n
+CONFIG_NET_VENDOR_NVIDIA=n
+CONFIG_NET_VENDOR_OKI=n
+CONFIG_NET_PACKET_ENGINE=n
+CONFIG_NET_VENDOR_QLOGIC=n
+CONFIG_NET_VENDOR_REALTEK=n
+CONFIG_NET_VENDOR_RDC=n
+CONFIG_NET_VENDOR_SEEQ=n
+CONFIG_NET_VENDOR_SILAN=n
+CONFIG_NET_VENDOR_SIS=n
+CONFIG_NET_VENDOR_SMSC=n
+CONFIG_NET_VENDOR_STMICRO=n
+CONFIG_NET_VENDOR_SUN=n
+CONFIG_NET_VENDOR_TEHUTI=n
+CONFIG_NET_VENDOR_TI=n
+CONFIG_NET_VENDOR_VIA=n
+CONFIG_NET_VENDOR_WIZNET=n
+
diff --git a/arch/arm/mach-kirkwood/board-csb1724.c b/arch/arm/mach-kirkwood/board-csb1724.c
index 979112d..9c58b92 100644
--- a/arch/arm/mach-kirkwood/board-csb1724.c
+++ b/arch/arm/mach-kirkwood/board-csb1724.c
@@ -13,6 +13,7 @@
#include <linux/kernel.h>
#include <linux/init.h>
+#include "common.h"
#include "mpp.h"
static unsigned int csb1724_mpp_config[] __initdata = {
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 2/6] mv643xx.c: Remove magic numbers.
From: Ian Molton @ 2012-07-31 15:45 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev
In-Reply-To: <1343749529-17571-1-git-send-email-ian.molton@codethink.co.uk>
replace magic number with RX_CSUM_WITH_HEADER.
Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
drivers/net/ethernet/marvell/mv643xx_eth.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 4fbba57..92497eb 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -86,6 +86,7 @@ static char mv643xx_eth_driver_version[] = "1.4";
* port #0, 0x0800 for port #1, and 0x0c00 for port #2.
*/
#define PORT_CONFIG 0x0000
+#define RX_CSUM_WITH_HEADER 0x02000000
#define UNICAST_PROMISCUOUS_MODE 0x00000001
#define PORT_CONFIG_EXT 0x0004
#define MAC_ADDR_LOW 0x0014
@@ -1607,7 +1608,7 @@ mv643xx_eth_set_features(struct net_device *dev, netdev_features_t features)
struct mv643xx_eth_private *mp = netdev_priv(dev);
bool rx_csum = features & NETIF_F_RXCSUM;
- wrlp(mp, PORT_CONFIG, rx_csum ? 0x02000000 : 0x00000000);
+ wrlp(mp, PORT_CONFIG, rx_csum ? RX_CSUM_WITH_HEADER : 0x00000000);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 6/6] DT: Convert all kirkwood boards with mv643xx that use DT
From: Ian Molton @ 2012-07-31 15:45 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: andrew, thomas.petazzoni, ben.dooks, arnd, netdev
In-Reply-To: <1343749529-17571-1-git-send-email-ian.molton@codethink.co.uk>
This patch converts all present DT capable kirkwood board configurations
to use DT to configure the mv643xx ethernet controller.
Signed-off-by: Ian Molton <ian.molton@codethink.co.uk>
---
arch/arm/boot/dts/kirkwood-dnskw.dtsi | 9 +++++++++
arch/arm/boot/dts/kirkwood-dreamplug.dts | 18 ++++++++++++++++++
arch/arm/boot/dts/kirkwood-goflexnet.dts | 8 ++++++++
arch/arm/boot/dts/kirkwood-ib62x0.dts | 10 ++++++++++
arch/arm/boot/dts/kirkwood-iconnect.dts | 10 ++++++++++
arch/arm/boot/dts/kirkwood-lsxl.dtsi | 17 +++++++++++++++++
arch/arm/boot/dts/kirkwood-ts219-6281.dts | 8 +++++++-
arch/arm/boot/dts/kirkwood-ts219-6282.dts | 8 +++++++-
arch/arm/boot/dts/kirkwood-ts219.dtsi | 3 +++
arch/arm/mach-kirkwood/board-dnskw.c | 7 +------
arch/arm/mach-kirkwood/board-dreamplug.c | 13 ++-----------
arch/arm/mach-kirkwood/board-goflexnet.c | 7 +------
arch/arm/mach-kirkwood/board-ib62x0.c | 7 +------
arch/arm/mach-kirkwood/board-iconnect.c | 7 +------
arch/arm/mach-kirkwood/board-lsxl.c | 13 ++-----------
arch/arm/mach-kirkwood/board-ts219.c | 10 +---------
16 files changed, 98 insertions(+), 57 deletions(-)
diff --git a/arch/arm/boot/dts/kirkwood-dnskw.dtsi b/arch/arm/boot/dts/kirkwood-dnskw.dtsi
index 7408655..214fe0b 100644
--- a/arch/arm/boot/dts/kirkwood-dnskw.dtsi
+++ b/arch/arm/boot/dts/kirkwood-dnskw.dtsi
@@ -65,5 +65,14 @@
reg = <0x7b00000 0x500000>;
};
};
+
+ smi0: mdio@72000 {
+ status = "ok";
+ };
+
+ egiga0 {
+ phy_addr = <8>;
+ status = "ok";
+ };
};
};
diff --git a/arch/arm/boot/dts/kirkwood-dreamplug.dts b/arch/arm/boot/dts/kirkwood-dreamplug.dts
index 26e281f..c27ed1c 100644
--- a/arch/arm/boot/dts/kirkwood-dreamplug.dts
+++ b/arch/arm/boot/dts/kirkwood-dreamplug.dts
@@ -53,6 +53,24 @@
status = "okay";
nr-ports = <1>;
};
+
+ smi0: mdio@72000 {
+ status = "ok";
+ };
+
+ smi1: mdio@76000 {
+ status = "ok";
+ };
+
+ egiga0 {
+ phy_addr = <0>;
+ status = "ok";
+ };
+
+ egiga1 {
+ phy_addr = <1>;
+ status = "ok";
+ };
};
gpio-leds {
diff --git a/arch/arm/boot/dts/kirkwood-goflexnet.dts b/arch/arm/boot/dts/kirkwood-goflexnet.dts
index 7c8238f..f03dbd0 100644
--- a/arch/arm/boot/dts/kirkwood-goflexnet.dts
+++ b/arch/arm/boot/dts/kirkwood-goflexnet.dts
@@ -50,6 +50,14 @@
nr-ports = <2>;
};
+ smi0: mdio@72000 {
+ status = "ok";
+ };
+
+ egiga0 {
+ phy_addr = <0>;
+ status = "ok";
+ };
};
gpio-leds {
compatible = "gpio-leds";
diff --git a/arch/arm/boot/dts/kirkwood-ib62x0.dts b/arch/arm/boot/dts/kirkwood-ib62x0.dts
index 66794ed..8c462a1 100644
--- a/arch/arm/boot/dts/kirkwood-ib62x0.dts
+++ b/arch/arm/boot/dts/kirkwood-ib62x0.dts
@@ -45,6 +45,16 @@
};
};
+
+ smi0: mdio@72000 {
+ status = "ok";
+ };
+
+ egiga0 {
+ phy_addr = <8>;
+ status = "ok";
+ };
+
};
gpio_keys {
diff --git a/arch/arm/boot/dts/kirkwood-iconnect.dts b/arch/arm/boot/dts/kirkwood-iconnect.dts
index 52d9470..9fc82be 100644
--- a/arch/arm/boot/dts/kirkwood-iconnect.dts
+++ b/arch/arm/boot/dts/kirkwood-iconnect.dts
@@ -30,6 +30,16 @@
clock-frequency = <200000000>;
status = "ok";
};
+
+ smi0: mdio@72000 {
+ status = "ok";
+ };
+
+ egiga0 {
+ phy_addr = <b>;
+ status = "ok";
+ };
+
};
gpio-leds {
compatible = "gpio-leds";
diff --git a/arch/arm/boot/dts/kirkwood-lsxl.dtsi b/arch/arm/boot/dts/kirkwood-lsxl.dtsi
index 8ac51c0..2f47661 100644
--- a/arch/arm/boot/dts/kirkwood-lsxl.dtsi
+++ b/arch/arm/boot/dts/kirkwood-lsxl.dtsi
@@ -40,6 +40,23 @@
};
};
};
+ smi0: mdio@72000 {
+ status = "ok";
+ };
+
+ smi1: mdio@76000 {
+ status = "ok";
+ };
+
+ egiga0 {
+ phy_addr = <0>;
+ status = "ok";
+ };
+
+ egiga1 {
+ phy_addr = <8>;
+ status = "ok";
+ };
};
gpio_keys {
diff --git a/arch/arm/boot/dts/kirkwood-ts219-6281.dts b/arch/arm/boot/dts/kirkwood-ts219-6281.dts
index ccbf327..4ca49b5 100644
--- a/arch/arm/boot/dts/kirkwood-ts219-6281.dts
+++ b/arch/arm/boot/dts/kirkwood-ts219-6281.dts
@@ -3,6 +3,12 @@
/include/ "kirkwood-ts219.dtsi"
/ {
+ ocp@f1000000 {
+ egiga0 {
+ phy_addr = <8>;
+ status = "ok";
+ };
+ };
gpio_keys {
compatible = "gpio-keys";
#address-cells = <1>;
@@ -18,4 +24,4 @@
gpios = <&gpio0 16 1>;
};
};
-};
\ No newline at end of file
+};
diff --git a/arch/arm/boot/dts/kirkwood-ts219-6282.dts b/arch/arm/boot/dts/kirkwood-ts219-6282.dts
index fbe9932..40f3c61 100644
--- a/arch/arm/boot/dts/kirkwood-ts219-6282.dts
+++ b/arch/arm/boot/dts/kirkwood-ts219-6282.dts
@@ -3,6 +3,12 @@
/include/ "kirkwood-ts219.dtsi"
/ {
+ ocp@f1000000 {
+ egiga0 {
+ phy_addr = <0>;
+ status = "ok";
+ };
+ };
gpio_keys {
compatible = "gpio-keys";
#address-cells = <1>;
@@ -18,4 +24,4 @@
gpios = <&gpio1 5 1>;
};
};
-};
\ No newline at end of file
+};
diff --git a/arch/arm/boot/dts/kirkwood-ts219.dtsi b/arch/arm/boot/dts/kirkwood-ts219.dtsi
index 64ea27c..06caf41 100644
--- a/arch/arm/boot/dts/kirkwood-ts219.dtsi
+++ b/arch/arm/boot/dts/kirkwood-ts219.dtsi
@@ -74,5 +74,8 @@
status = "okay";
nr-ports = <2>;
};
+ smi0: mdio@72000 {
+ status = "ok";
+ };
};
};
diff --git a/arch/arm/mach-kirkwood/board-dnskw.c b/arch/arm/mach-kirkwood/board-dnskw.c
index 4ab3506..4d8216b 100644
--- a/arch/arm/mach-kirkwood/board-dnskw.c
+++ b/arch/arm/mach-kirkwood/board-dnskw.c
@@ -15,7 +15,6 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
#include <linux/of.h>
#include <linux/gpio.h>
#include <linux/input.h>
@@ -29,10 +28,6 @@
#include "common.h"
#include "mpp.h"
-static struct mv643xx_eth_platform_data dnskw_ge00_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(8),
-};
-
static unsigned int dnskw_mpp_config[] __initdata = {
MPP13_UART1_TXD, /* Custom ... */
MPP14_UART1_RXD, /* ... Controller (DNS-320 only) */
@@ -112,7 +107,7 @@ void __init dnskw_init(void)
kirkwood_mpp_conf(dnskw_mpp_config);
kirkwood_ehci_init();
- kirkwood_ge00_init(&dnskw_ge00_data);
+ kirkwood_ge00_init(NULL);
platform_device_register(&dnskw_fan_device);
diff --git a/arch/arm/mach-kirkwood/board-dreamplug.c b/arch/arm/mach-kirkwood/board-dreamplug.c
index aeb234d..b97a112 100644
--- a/arch/arm/mach-kirkwood/board-dreamplug.c
+++ b/arch/arm/mach-kirkwood/board-dreamplug.c
@@ -15,7 +15,6 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_fdt.h>
@@ -34,14 +33,6 @@
#include "common.h"
#include "mpp.h"
-static struct mv643xx_eth_platform_data dreamplug_ge00_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(0),
-};
-
-static struct mv643xx_eth_platform_data dreamplug_ge01_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(1),
-};
-
static struct mvsdio_platform_data dreamplug_mvsdio_data = {
/* unfortunately the CD signal has not been connected */
};
@@ -65,7 +56,7 @@ void __init dreamplug_init(void)
kirkwood_mpp_conf(dreamplug_mpp_config);
kirkwood_ehci_init();
- kirkwood_ge00_init(&dreamplug_ge00_data);
- kirkwood_ge01_init(&dreamplug_ge01_data);
+ kirkwood_ge00_init(NULL);
+ kirkwood_ge01_init(NULL);
kirkwood_sdio_init(&dreamplug_mvsdio_data);
}
diff --git a/arch/arm/mach-kirkwood/board-goflexnet.c b/arch/arm/mach-kirkwood/board-goflexnet.c
index 413e2c8..be7437d 100644
--- a/arch/arm/mach-kirkwood/board-goflexnet.c
+++ b/arch/arm/mach-kirkwood/board-goflexnet.c
@@ -20,7 +20,6 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_fdt.h>
@@ -36,10 +35,6 @@
#include "common.h"
#include "mpp.h"
-static struct mv643xx_eth_platform_data goflexnet_ge00_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(0),
-};
-
static unsigned int goflexnet_mpp_config[] __initdata = {
MPP29_GPIO, /* USB Power Enable */
MPP47_GPIO, /* LED Orange */
@@ -67,5 +62,5 @@ void __init goflexnet_init(void)
pr_err("can't setup GPIO 29 (USB Power Enable)\n");
kirkwood_ehci_init();
- kirkwood_ge00_init(&goflexnet_ge00_data);
+ kirkwood_ge00_init(NULL);
}
diff --git a/arch/arm/mach-kirkwood/board-ib62x0.c b/arch/arm/mach-kirkwood/board-ib62x0.c
index cfc47f8..0a29183 100644
--- a/arch/arm/mach-kirkwood/board-ib62x0.c
+++ b/arch/arm/mach-kirkwood/board-ib62x0.c
@@ -16,7 +16,6 @@
#include <linux/platform_device.h>
#include <linux/mtd/partitions.h>
#include <linux/ata_platform.h>
-#include <linux/mv643xx_eth.h>
#include <linux/gpio.h>
#include <linux/input.h>
#include <asm/mach-types.h>
@@ -27,10 +26,6 @@
#define IB62X0_GPIO_POWER_OFF 24
-static struct mv643xx_eth_platform_data ib62x0_ge00_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(8),
-};
-
static unsigned int ib62x0_mpp_config[] __initdata = {
MPP0_NF_IO2,
MPP1_NF_IO3,
@@ -62,7 +57,7 @@ void __init ib62x0_init(void)
kirkwood_mpp_conf(ib62x0_mpp_config);
kirkwood_ehci_init();
- kirkwood_ge00_init(&ib62x0_ge00_data);
+ kirkwood_ge00_init(NULL);
if (gpio_request(IB62X0_GPIO_POWER_OFF, "ib62x0:power:off") == 0 &&
gpio_direction_output(IB62X0_GPIO_POWER_OFF, 0) == 0)
pm_power_off = ib62x0_power_off;
diff --git a/arch/arm/mach-kirkwood/board-iconnect.c b/arch/arm/mach-kirkwood/board-iconnect.c
index d7a9198..220f0d4 100644
--- a/arch/arm/mach-kirkwood/board-iconnect.c
+++ b/arch/arm/mach-kirkwood/board-iconnect.c
@@ -17,7 +17,6 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/mtd/partitions.h>
-#include <linux/mv643xx_eth.h>
#include <linux/gpio.h>
#include <linux/input.h>
#include <linux/gpio_keys.h>
@@ -26,10 +25,6 @@
#include "common.h"
#include "mpp.h"
-static struct mv643xx_eth_platform_data iconnect_ge00_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(11),
-};
-
static unsigned int iconnect_mpp_config[] __initdata = {
MPP12_GPIO,
MPP35_GPIO,
@@ -92,7 +87,7 @@ void __init iconnect_init(void)
kirkwood_nand_init(ARRAY_AND_SIZE(iconnect_nand_parts), 25);
kirkwood_ehci_init();
- kirkwood_ge00_init(&iconnect_ge00_data);
+ kirkwood_ge00_init(NULL);
platform_device_register(&iconnect_button_device);
}
diff --git a/arch/arm/mach-kirkwood/board-lsxl.c b/arch/arm/mach-kirkwood/board-lsxl.c
index 83d8975..60331d1 100644
--- a/arch/arm/mach-kirkwood/board-lsxl.c
+++ b/arch/arm/mach-kirkwood/board-lsxl.c
@@ -18,7 +18,6 @@
#include <linux/ata_platform.h>
#include <linux/spi/flash.h>
#include <linux/spi/spi.h>
-#include <linux/mv643xx_eth.h>
#include <linux/gpio.h>
#include <linux/gpio-fan.h>
#include <linux/input.h>
@@ -28,14 +27,6 @@
#include "common.h"
#include "mpp.h"
-static struct mv643xx_eth_platform_data lsxl_ge00_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(0),
-};
-
-static struct mv643xx_eth_platform_data lsxl_ge01_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(8),
-};
-
static unsigned int lsxl_mpp_config[] __initdata = {
MPP10_GPO, /* HDD Power Enable */
MPP11_GPIO, /* USB Vbus Enable */
@@ -126,8 +117,8 @@ void __init lsxl_init(void)
gpio_set_value(LSXL_GPIO_HDD_POWER, 1);
kirkwood_ehci_init();
- kirkwood_ge00_init(&lsxl_ge00_data);
- kirkwood_ge01_init(&lsxl_ge01_data);
+ kirkwood_ge00_init(NULL);
+ kirkwood_ge01_init(NULL);
platform_device_register(&lsxl_fan_device);
/* register power-off method */
diff --git a/arch/arm/mach-kirkwood/board-ts219.c b/arch/arm/mach-kirkwood/board-ts219.c
index 1750e68..7e7fe6c 100644
--- a/arch/arm/mach-kirkwood/board-ts219.c
+++ b/arch/arm/mach-kirkwood/board-ts219.c
@@ -18,7 +18,6 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/platform_device.h>
-#include <linux/mv643xx_eth.h>
#include <linux/ata_platform.h>
#include <linux/gpio_keys.h>
#include <linux/input.h>
@@ -29,10 +28,6 @@
#include "mpp.h"
#include "tsx1x-common.h"
-static struct mv643xx_eth_platform_data qnap_ts219_ge00_data = {
- .phy_addr = MV643XX_ETH_PHY_ADDR(8),
-};
-
static unsigned int qnap_ts219_mpp_config[] __initdata = {
MPP0_SPI_SCn,
MPP1_SPI_MOSI,
@@ -62,10 +57,7 @@ void __init qnap_dt_ts219_init(void)
kirkwood_mpp_conf(qnap_ts219_mpp_config);
kirkwood_pcie_id(&dev, &rev);
- if (dev == MV88F6282_DEV_ID)
- qnap_ts219_ge00_data.phy_addr = MV643XX_ETH_PHY_ADDR(0);
-
- kirkwood_ge00_init(&qnap_ts219_ge00_data);
+ kirkwood_ge00_init(NULL);
kirkwood_ehci_init();
pm_power_off = qnap_tsx1x_power_off;
--
1.7.9.5
^ 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