* RE: [PATCH] e1000e: workaround missing power down mii control bit on 82571
From: Allan, Bruce W @ 2010-12-17 15:53 UTC (permalink / raw)
To: Arthur Jones; +Cc: Ben Hutchings, Kirsher, Jeffrey T, netdev@vger.kernel.org
In-Reply-To: <20101217140453.GS18990@ajones-laptop.nbttech.com>
>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>Behalf Of Arthur Jones
>Sent: Friday, December 17, 2010 6:05 AM
>To: Allan, Bruce W
>Cc: Ben Hutchings; Kirsher, Jeffrey T; netdev@vger.kernel.org
>Subject: Re: [PATCH] e1000e: workaround missing power down mii control bit on
>82571
>
>That's not what I saw. I saw the bit always
>read back zero, even right after I set it.
>
>Have you tried writing the power down bit and
>reading it back?
>
>Arthur
Yes, and it worked as expected.
^ permalink raw reply
* [RFC PATCH] net_sched: sch_sfq: better struct layouts
From: Eric Dumazet @ 2010-12-17 16:52 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, netdev, Jarek Poplawski
In-Reply-To: <1292504932.2883.110.camel@edumazet-laptop>
Le jeudi 16 décembre 2010 à 14:08 +0100, Eric Dumazet a écrit :
> Le mercredi 15 décembre 2010 à 18:03 +0100, Patrick McHardy a écrit :
> > On 15.12.2010 17:55, Eric Dumazet wrote:
>
> > > I was thinking in allowing more packets per SFQ (but keep the 126 active
> > > flows limit), what do you think ?
> >
> > I keep forgetting why this limit exists, let me try to figure
> > it out once more :)
>
> I took a look, and found we already are at max, unless we change
> sfq_index type (from u8 to u16)
>
> SFQ_SLOTS is 128 (not really exist, but this is the number of slots)
> SFQ_DEPTH is 128 (max depth for one slot)
> Constraints are :
> SFQ_SLOTS < 256
> SFQ_DEPTH < 256
> SFQ_SLOTS + SFQ_DEPTH < 256, because of the dep[] array.
>
> We could avoid the "struct sk_buff_head" structure with its un-needed
> spinlock, and eventually group data for a given slot in a structure to
> reduce number of cache lines we touch...
>
> struct sfq_slot {
> struct sk_buff *first;
> struct sk_buff *last;
> u8 qlen;
> sfq_index next; /* dequeue chain */
> u16 hash;
> short allot;
> /* 16bit hole */
> };
>
> This would save 768 bytes on x86_64 (and much more if LOCKDEP is used)
>
>
Here is a preliminary patch, shrinking sizeof(struct sfq_sched_data)
from 0x14f8 (or more if spinlocks are bigger) to 0x1180 bytes, and
reduce text size as well.
text data bss dec hex filename
4821 152 0 4973 136d old/net/sched/sch_sfq.o
4651 136 0 4787 12b3 new/net/sched/sch_sfq.o
All data for a slot/flow is now grouped in a compact and cache friendly
structure :
struct sfq_slot {
struct sk_buff *skblist_next;
struct sk_buff *skblist_prev;
sfq_index qlen; /* number of skbs in skblist */
sfq_index next; /* next slot in sfq chain */
unsigned short hash; /* hash value (index in ht[]) */
short allot; /* credit for this slot */
struct sfq_head anchor; /* anchor in dep[] chains */
};
net/sched/sch_sfq.c | 223 +++++++++++++++++++++++-------------------
1 file changed, 125 insertions(+), 98 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 3cf478d..28968eb 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -69,25 +69,40 @@
This implementation limits maximal queue length to 128;
maximal mtu to 2^15-1; number of hash buckets to 1024.
The only goal of this restrictions was that all data
- fit into one 4K page :-). Struct sfq_sched_data is
- organized in anti-cache manner: all the data for a bucket
- are scattered over different locations. This is not good,
- but it allowed me to put it into 4K.
+ fit into one 4K page on 32bit arches.
It is easy to increase these values, but not in flight. */
-#define SFQ_DEPTH 128
+#define SFQ_DEPTH 128 /* max number of packets per slot (per flow) */
+#define SFQ_SLOTS 128 /* max number of flows */
+#define EMPTY_SLOT 255
#define SFQ_HASH_DIVISOR 1024
-/* This type should contain at least SFQ_DEPTH*2 values */
+/* This type should contain at least SFQ_DEPTH + SFQ_SLOTS values */
typedef unsigned char sfq_index;
+/*
+ * We dont use pointers to save space.
+ * Small indexes [0 ... SFQ_SLOTS - 1] are 'pointers' to slots[] array
+ * while following values [SFQ_SLOTS ... SFQ_SLOTS + SFQ_DEPTH - 1]
+ * are 'pointers' to dep[] array
+ */
struct sfq_head
{
sfq_index next;
sfq_index prev;
};
+struct sfq_slot {
+ struct sk_buff *skblist_next;
+ struct sk_buff *skblist_prev;
+ sfq_index qlen; /* number of skbs in skblist */
+ sfq_index next; /* next slot in sfq chain */
+ unsigned short hash; /* hash value (index in ht[]) */
+ short allot; /* credit for this slot */
+ struct sfq_head anchor; /* anchor in dep[] chains */
+};
+
struct sfq_sched_data
{
/* Parameters */
@@ -99,17 +114,24 @@ struct sfq_sched_data
struct tcf_proto *filter_list;
struct timer_list perturb_timer;
u32 perturbation;
- sfq_index tail; /* Index of current slot in round */
- sfq_index max_depth; /* Maximal depth */
+ sfq_index max_depth; /* depth of longest slot */
+ struct sfq_slot *tail; /* current slot in round */
sfq_index ht[SFQ_HASH_DIVISOR]; /* Hash table */
- sfq_index next[SFQ_DEPTH]; /* Active slots link */
- short allot[SFQ_DEPTH]; /* Current allotment per slot */
- unsigned short hash[SFQ_DEPTH]; /* Hash value indexed by slots */
- struct sk_buff_head qs[SFQ_DEPTH]; /* Slot queue */
- struct sfq_head dep[SFQ_DEPTH*2]; /* Linked list of slots, indexed by depth */
+ struct sfq_slot slots[SFQ_SLOTS];
+ struct sfq_head dep[SFQ_DEPTH]; /* Linked list of slots, indexed by depth */
};
+/*
+ * sfq_head are either in a sfq_slot or in dep[] array
+ */
+static inline struct sfq_head *get_head(struct sfq_sched_data *q, sfq_index val)
+{
+ if (val < SFQ_SLOTS)
+ return &q->slots[val].anchor;
+ return &q->dep[val - SFQ_SLOTS];
+}
+
static __inline__ unsigned sfq_fold_hash(struct sfq_sched_data *q, u32 h, u32 h1)
{
return jhash_2words(h, h1, q->perturbation) & (SFQ_HASH_DIVISOR - 1);
@@ -200,30 +222,37 @@ static unsigned int sfq_classify(struct sk_buff *skb, struct Qdisc *sch,
return 0;
}
+/*
+ * x : slot number [0 .. SFQ_SLOTS - 1]
+ */
static inline void sfq_link(struct sfq_sched_data *q, sfq_index x)
{
sfq_index p, n;
- int d = q->qs[x].qlen + SFQ_DEPTH;
+ int qlen = q->slots[x].qlen;
- p = d;
- n = q->dep[d].next;
- q->dep[x].next = n;
- q->dep[x].prev = p;
- q->dep[p].next = q->dep[n].prev = x;
+ p = qlen + SFQ_SLOTS;
+ n = q->dep[qlen].next;
+
+ q->slots[x].anchor.next = n;
+ q->slots[x].anchor.prev = p;
+
+ q->dep[qlen].next = x; /* get_head(q, p)->next = x */
+ get_head(q, n)->prev = x;
}
static inline void sfq_dec(struct sfq_sched_data *q, sfq_index x)
{
sfq_index p, n;
+ int d;
- n = q->dep[x].next;
- p = q->dep[x].prev;
- q->dep[p].next = n;
- q->dep[n].prev = p;
+ n = q->slots[x].anchor.next;
+ p = q->slots[x].anchor.prev;
+ get_head(q, p)->next = n;
+ get_head(q, n)->prev = p;
- if (n == p && q->max_depth == q->qs[x].qlen + 1)
+ d = q->slots[x].qlen--;
+ if (n == p && q->max_depth == d)
q->max_depth--;
-
sfq_link(q, x);
}
@@ -232,34 +261,36 @@ static inline void sfq_inc(struct sfq_sched_data *q, sfq_index x)
sfq_index p, n;
int d;
- n = q->dep[x].next;
- p = q->dep[x].prev;
- q->dep[p].next = n;
- q->dep[n].prev = p;
- d = q->qs[x].qlen;
+ n = q->slots[x].anchor.next;
+ p = q->slots[x].anchor.prev;
+ get_head(q, p)->next = n;
+ get_head(q, n)->prev = p;
+
+ d = ++q->slots[x].qlen;
if (q->max_depth < d)
q->max_depth = d;
-
sfq_link(q, x);
}
static unsigned int sfq_drop(struct Qdisc *sch)
{
struct sfq_sched_data *q = qdisc_priv(sch);
- sfq_index d = q->max_depth;
+ sfq_index x, d = q->max_depth;
struct sk_buff *skb;
unsigned int len;
+ struct sfq_slot *slot;
- /* Queue is full! Find the longest slot and
- drop a packet from it */
-
+ /* Queue is full! Find the longest slot and drop tail packet from it */
if (d > 1) {
- sfq_index x = q->dep[d + SFQ_DEPTH].next;
- skb = q->qs[x].prev;
+ x = q->dep[d].next;
+ slot = &q->slots[x];
+drop:
+ skb = slot->skblist_prev;
+ slot->skblist_prev = skb->prev;
len = qdisc_pkt_len(skb);
- __skb_unlink(skb, &q->qs[x]);
- kfree_skb(skb);
sfq_dec(q, x);
+ skb->next = skb->prev = NULL;
+ kfree_skb(skb);
sch->q.qlen--;
sch->qstats.drops++;
sch->qstats.backlog -= len;
@@ -268,19 +299,11 @@ static unsigned int sfq_drop(struct Qdisc *sch)
if (d == 1) {
/* It is difficult to believe, but ALL THE SLOTS HAVE LENGTH 1. */
- d = q->next[q->tail];
- q->next[q->tail] = q->next[d];
- q->allot[q->next[d]] += q->quantum;
- skb = q->qs[d].prev;
- len = qdisc_pkt_len(skb);
- __skb_unlink(skb, &q->qs[d]);
- kfree_skb(skb);
- sfq_dec(q, d);
- sch->q.qlen--;
- q->ht[q->hash[d]] = SFQ_DEPTH;
- sch->qstats.drops++;
- sch->qstats.backlog -= len;
- return len;
+ x = q->tail->next;
+ slot = &q->slots[x];
+ q->tail->next = slot->next;
+ q->ht[slot->hash] = EMPTY_SLOT;
+ goto drop;
}
return 0;
@@ -292,6 +315,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
struct sfq_sched_data *q = qdisc_priv(sch);
unsigned int hash;
sfq_index x;
+ struct sfq_slot *slot;
int uninitialized_var(ret);
hash = sfq_classify(skb, sch, &ret);
@@ -304,31 +328,36 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
hash--;
x = q->ht[hash];
- if (x == SFQ_DEPTH) {
- q->ht[hash] = x = q->dep[SFQ_DEPTH].next;
- q->hash[x] = hash;
+ slot = &q->slots[x];
+ if (x == EMPTY_SLOT) {
+ x = q->dep[0].next; /* get a free slot */
+ q->ht[hash] = x;
+ slot = &q->slots[x];
+ slot->hash = hash;
+ slot->skblist_next = slot->skblist_prev = (struct sk_buff *)slot;
}
- /* If selected queue has length q->limit, this means that
- * all another queues are empty and that we do simple tail drop,
+ /* If selected queue has length q->limit, do simple tail drop,
* i.e. drop _this_ packet.
*/
- if (q->qs[x].qlen >= q->limit)
+ if (slot->qlen >= q->limit)
return qdisc_drop(skb, sch);
sch->qstats.backlog += qdisc_pkt_len(skb);
- __skb_queue_tail(&q->qs[x], skb);
+ skb->prev = slot->skblist_prev;
+ skb->next = (struct sk_buff *)slot;
+ slot->skblist_prev->next = skb;
+ slot->skblist_prev = skb;
sfq_inc(q, x);
- if (q->qs[x].qlen == 1) { /* The flow is new */
- if (q->tail == SFQ_DEPTH) { /* It is the first flow */
- q->tail = x;
- q->next[x] = x;
- q->allot[x] = q->quantum;
+ if (slot->qlen == 1) { /* The flow is new */
+ if (q->tail == NULL) { /* It is the first flow */
+ slot->next = x;
} else {
- q->next[x] = q->next[q->tail];
- q->next[q->tail] = x;
- q->tail = x;
+ slot->next = q->tail->next;
+ q->tail->next = x;
}
+ q->tail = slot;
+ slot->allot = q->quantum;
}
if (++sch->q.qlen <= q->limit) {
sch->bstats.bytes += qdisc_pkt_len(skb);
@@ -344,14 +373,12 @@ static struct sk_buff *
sfq_peek(struct Qdisc *sch)
{
struct sfq_sched_data *q = qdisc_priv(sch);
- sfq_index a;
/* No active slots */
- if (q->tail == SFQ_DEPTH)
+ if (q->tail == NULL)
return NULL;
- a = q->next[q->tail];
- return skb_peek(&q->qs[a]);
+ return q->slots[q->tail->next].skblist_next;
}
static struct sk_buff *
@@ -359,34 +386,35 @@ sfq_dequeue(struct Qdisc *sch)
{
struct sfq_sched_data *q = qdisc_priv(sch);
struct sk_buff *skb;
- sfq_index a, old_a;
+ sfq_index a, next_a;
+ struct sfq_slot *slot;
/* No active slots */
- if (q->tail == SFQ_DEPTH)
+ if (q->tail == NULL)
return NULL;
- a = old_a = q->next[q->tail];
-
+ a = q->tail->next;
+ slot = &q->slots[a];
/* Grab packet */
- skb = __skb_dequeue(&q->qs[a]);
+ skb = slot->skblist_next;
+ slot->skblist_next = skb->next;
+ skb->next = skb->prev = NULL;
sfq_dec(q, a);
sch->q.qlen--;
sch->qstats.backlog -= qdisc_pkt_len(skb);
- /* Is the slot empty? */
- if (q->qs[a].qlen == 0) {
- q->ht[q->hash[a]] = SFQ_DEPTH;
- a = q->next[a];
- if (a == old_a) {
- q->tail = SFQ_DEPTH;
+ /* Is the slot now empty? */
+ if (slot->qlen == 0) {
+ q->ht[slot->hash] = EMPTY_SLOT;
+ next_a = slot->next;
+ if (a == next_a) {
+ q->tail = NULL; /* no more active slots */
return skb;
}
- q->next[q->tail] = a;
- q->allot[a] += q->quantum;
- } else if ((q->allot[a] -= qdisc_pkt_len(skb)) <= 0) {
- q->tail = a;
- a = q->next[a];
- q->allot[a] += q->quantum;
+ q->tail->next = next_a;
+ } else if ((slot->allot -= qdisc_pkt_len(skb)) <= 0) {
+ q->tail = slot;
+ slot->allot += q->quantum;
}
return skb;
}
@@ -450,17 +478,16 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
init_timer_deferrable(&q->perturb_timer);
for (i = 0; i < SFQ_HASH_DIVISOR; i++)
- q->ht[i] = SFQ_DEPTH;
+ q->ht[i] = EMPTY_SLOT;
for (i = 0; i < SFQ_DEPTH; i++) {
- skb_queue_head_init(&q->qs[i]);
- q->dep[i + SFQ_DEPTH].next = i + SFQ_DEPTH;
- q->dep[i + SFQ_DEPTH].prev = i + SFQ_DEPTH;
+ q->dep[i].next = i + SFQ_SLOTS;
+ q->dep[i].prev = i + SFQ_SLOTS;
}
q->limit = SFQ_DEPTH - 1;
q->max_depth = 0;
- q->tail = SFQ_DEPTH;
+ q->tail = NULL;
if (opt == NULL) {
q->quantum = psched_mtu(qdisc_dev(sch));
q->perturb_period = 0;
@@ -471,7 +498,7 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
return err;
}
- for (i = 0; i < SFQ_DEPTH; i++)
+ for (i = 0; i < SFQ_SLOTS; i++)
sfq_link(q, i);
return 0;
}
@@ -547,9 +574,9 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
struct gnet_dump *d)
{
struct sfq_sched_data *q = qdisc_priv(sch);
- sfq_index idx = q->ht[cl-1];
- struct gnet_stats_queue qs = { .qlen = q->qs[idx].qlen };
- struct tc_sfq_xstats xstats = { .allot = q->allot[idx] };
+ const struct sfq_slot *slot = &q->slots[q->ht[cl - 1]];
+ struct gnet_stats_queue qs = { .qlen = slot->qlen };
+ struct tc_sfq_xstats xstats = { .allot = slot->allot };
if (gnet_stats_copy_queue(d, &qs) < 0)
return -1;
@@ -565,7 +592,7 @@ static void sfq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
return;
for (i = 0; i < SFQ_HASH_DIVISOR; i++) {
- if (q->ht[i] == SFQ_DEPTH ||
+ if (q->ht[i] == EMPTY_SLOT ||
arg->count < arg->skip) {
arg->count++;
continue;
^ permalink raw reply related
* Re: [net-next-2.6 PATCH 1/4] net: implement mechanism for HW based QOS
From: John Fastabend @ 2010-12-17 16:54 UTC (permalink / raw)
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, hadi@cyberus.ca, shemminger@vyatta.com,
tgraf@infradead.org, eric.dumazet@gmail.com,
nhorman@tuxdriver.com
In-Reply-To: <20101217153439.12170.39538.stgit@jf-dev1-dcblab>
On 12/17/2010 7:34 AM, John Fastabend wrote:
> This patch provides a mechanism for lower layer devices to
> steer traffic using skb->priority to tx queues. This allows
> for hardware based QOS schemes to use the default qdisc without
> incurring the penalties related to global state and the qdisc
> lock. While reliably receiving skbs on the correct tx ring
> to avoid head of line blocking resulting from shuffling in
> the LLD. Finally, all the goodness from txq caching and xps/rps
> can still be leveraged.
>
> Many drivers and hardware exist with the ability to implement
> QOS schemes in the hardware but currently these drivers tend
> to rely on firmware to reroute specific traffic, a driver
> specific select_queue or the queue_mapping action in the
> qdisc.
>
> By using select_queue for this drivers need to be updated for
> each and every traffic type and we lose the goodness of much
> of the upstream work. Firmware solutions are inherently
> inflexible. And finally if admins are expected to build a
> qdisc and filter rules to steer traffic this requires knowledge
> of how the hardware is currently configured. The number of tx
> queues and the queue offsets may change depending on resources.
> Also this approach incurs all the overhead of a qdisc with filters.
>
> With the mechanism in this patch users can set skb priority using
> expected methods ie setsockopt() or the stack can set the priority
> directly. Then the skb will be steered to the correct tx queues
> aligned with hardware QOS traffic classes. In the normal case with
> a single traffic class and all queues in this class everything
> works as is until the LLD enables multiple tcs.
>
> To steer the skb we mask out the lower 4 bits of the priority
> and allow the hardware to configure upto 15 distinct classes
> of traffic. This is expected to be sufficient for most applications
> at any rate it is more then the 8021Q spec designates and is
> equal to the number of prio bands currently implemented in
> the default qdisc.
>
> This in conjunction with a userspace application such as
> lldpad can be used to implement 8021Q transmission selection
> algorithms one of these algorithms being the extended transmission
> selection algorithm currently being used for DCB.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>
This conflicts with Vladislav Zolotarov's patch
[PATCH net-next 1/9] Take the distribution range definition out of skb_tx_hash()
The conflict is easily resolved, but I'll post a v2 of my patch to make it clean.
--John.
^ permalink raw reply
* [net-next-2.6 PATCH v2] net: implement mechanism for HW based QOS
From: John Fastabend @ 2010-12-17 16:56 UTC (permalink / raw)
To: davem; +Cc: netdev, hadi, shemminger, tgraf, eric.dumazet, nhorman
This patch provides a mechanism for lower layer devices to
steer traffic using skb->priority to tx queues. This allows
for hardware based QOS schemes to use the default qdisc without
incurring the penalties related to global state and the qdisc
lock. While reliably receiving skbs on the correct tx ring
to avoid head of line blocking resulting from shuffling in
the LLD. Finally, all the goodness from txq caching and xps/rps
can still be leveraged.
Many drivers and hardware exist with the ability to implement
QOS schemes in the hardware but currently these drivers tend
to rely on firmware to reroute specific traffic, a driver
specific select_queue or the queue_mapping action in the
qdisc.
By using select_queue for this drivers need to be updated for
each and every traffic type and we lose the goodness of much
of the upstream work. Firmware solutions are inherently
inflexible. And finally if admins are expected to build a
qdisc and filter rules to steer traffic this requires knowledge
of how the hardware is currently configured. The number of tx
queues and the queue offsets may change depending on resources.
Also this approach incurs all the overhead of a qdisc with filters.
With the mechanism in this patch users can set skb priority using
expected methods ie setsockopt() or the stack can set the priority
directly. Then the skb will be steered to the correct tx queues
aligned with hardware QOS traffic classes. In the normal case with
a single traffic class and all queues in this class everything
works as is until the LLD enables multiple tcs.
To steer the skb we mask out the lower 4 bits of the priority
and allow the hardware to configure upto 15 distinct classes
of traffic. This is expected to be sufficient for most applications
at any rate it is more then the 8021Q spec designates and is
equal to the number of prio bands currently implemented in
the default qdisc.
This in conjunction with a userspace application such as
lldpad can be used to implement 8021Q transmission selection
algorithms one of these algorithms being the extended transmission
selection algorithm currently being used for DCB.
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
include/linux/netdevice.h | 60 +++++++++++++++++++++++++++++++++++++++++++++
net/core/dev.c | 10 +++++++-
2 files changed, 69 insertions(+), 1 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cc916c5..c5d7949 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -646,6 +646,12 @@ struct xps_dev_maps {
(nr_cpu_ids * sizeof(struct xps_map *)))
#endif /* CONFIG_XPS */
+/* HW offloaded queuing disciplines txq count and offset maps */
+struct netdev_tc_txq {
+ u16 count;
+ u16 offset;
+};
+
/*
* This structure defines the management hooks for network devices.
* The following hooks can be defined; unless noted otherwise, they are
@@ -1146,6 +1152,9 @@ struct net_device {
/* Data Center Bridging netlink ops */
const struct dcbnl_rtnl_ops *dcbnl_ops;
#endif
+ u8 num_tc;
+ struct netdev_tc_txq tc_to_txq[16];
+ u8 prio_tc_map[16];
#if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
/* max exchange id for FCoE LRO by ddp */
@@ -1162,6 +1171,57 @@ struct net_device {
#define NETDEV_ALIGN 32
static inline
+int netdev_get_prio_tc_map(const struct net_device *dev, u32 prio)
+{
+ return dev->prio_tc_map[prio & 15];
+}
+
+static inline
+int netdev_set_prio_tc_map(struct net_device *dev, u8 prio, u8 tc)
+{
+ if (tc >= dev->num_tc)
+ return -EINVAL;
+
+ dev->prio_tc_map[prio & 15] = tc & 15;
+ return 0;
+}
+
+static inline
+void netdev_reset_tc(struct net_device *dev)
+{
+ dev->num_tc = 0;
+ memset(dev->tc_to_txq, 0, sizeof(dev->tc_to_txq));
+ memset(dev->prio_tc_map, 0, sizeof(dev->prio_tc_map));
+}
+
+static inline
+int netdev_set_tc_queue(struct net_device *dev, u8 tc, u16 count, u16 offset)
+{
+ if (tc >= dev->num_tc)
+ return -EINVAL;
+
+ dev->tc_to_txq[tc].count = count;
+ dev->tc_to_txq[tc].offset = offset;
+ return 0;
+}
+
+static inline
+int netdev_set_num_tc(struct net_device *dev, u8 num_tc)
+{
+ if (num_tc > 16)
+ return -EINVAL;
+
+ dev->num_tc = num_tc;
+ return 0;
+}
+
+static inline
+u8 netdev_get_num_tc(const struct net_device *dev)
+{
+ return dev->num_tc;
+}
+
+static inline
struct netdev_queue *netdev_get_tx_queue(const struct net_device *dev,
unsigned int index)
{
diff --git a/net/core/dev.c b/net/core/dev.c
index 92d414a..2ca323a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2152,6 +2152,8 @@ u16 __skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb,
unsigned int num_tx_queues)
{
u32 hash;
+ u16 qoffset = 0;
+ u16 qcount = num_tx_queues;
if (skb_rx_queue_recorded(skb)) {
hash = skb_get_rx_queue(skb);
@@ -2160,13 +2162,19 @@ u16 __skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb,
return hash;
}
+ if (dev->num_tc) {
+ u8 tc = netdev_get_prio_tc_map(dev, skb->priority);
+ qoffset = dev->tc_to_txq[tc].offset;
+ qcount = dev->tc_to_txq[tc].count;
+ }
+
if (skb->sk && skb->sk->sk_hash)
hash = skb->sk->sk_hash;
else
hash = (__force u16) skb->protocol ^ skb->rxhash;
hash = jhash_1word(hash, hashrnd);
- return (u16) (((u64) hash * num_tx_queues) >> 32);
+ return (u16) (((u64) hash * qcount) >> 32) + qoffset;
}
EXPORT_SYMBOL(__skb_tx_hash);
^ permalink raw reply related
* Driver issue on Linux 2.6.35-22-generic-pae #33-Ubuntu SMP
From: Colum.Flannigan @ 2010-12-17 16:41 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 707 bytes --]
Hi,
I am writing to this address as it is the address listed in the mod info. I purchased 3 of your cards for my infrastucture and am haven an issue with my linux system i am testing this on. Currently i have 1 card installed on this system and it is working fine, however the gigabit card i got from Netgear does not show in the system
I have checked my dmesg's and have no errors relatting to this and the lsmod command does not show it in the list..
I also tried to install the drive listed on the site, but that failed to work as i got errors when running.. Errors were stating the MakeFile did not exist.
Have you any ideas what this could be?? or steps to resolve??
Thanks
Colum
^ permalink raw reply
* Re: [PATCH net-next] ipv6: remove duplicate neigh_ifdown
From: David Miller @ 2010-12-17 17:49 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20101216194254.0f7c7e7b@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 16 Dec 2010 19:42:54 -0800
> When device is being set to down, neigh_ifdown was being called
> twice. Once from addrconf notifier and once from ndisc notifier.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
I'll give you a cookie if you can show me how this patch applies to
net-next-2.6 cleanly. :-)
It only matches the code in net-2.6 which has changed non-trivially in
net-next-2.6
If you worked against net-2.6 and thought "I can't think of anything
that could have changed in net-next-2.6 in this area", that's not
an appropriate way to operate and submit patches.
Please don't be so careless, thanks.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: remove duplicate neigh_ifdown
From: Stephen Hemminger @ 2010-12-17 18:01 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20101217.094916.193712831.davem@davemloft.net>
On Fri, 17 Dec 2010 09:49:16 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 16 Dec 2010 19:42:54 -0800
>
> > When device is being set to down, neigh_ifdown was being called
> > twice. Once from addrconf notifier and once from ndisc notifier.
> >
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> I'll give you a cookie if you can show me how this patch applies to
> net-next-2.6 cleanly. :-)
>
> It only matches the code in net-2.6 which has changed non-trivially in
> net-next-2.6
>
> If you worked against net-2.6 and thought "I can't think of anything
> that could have changed in net-next-2.6 in this area", that's not
> an appropriate way to operate and submit patches.
>
> Please don't be so careless, thanks.
Ok. It followed my last patch that did the rt6_ifdown only
if not loopback. Since you accepted that, I sent the next one.
--
^ permalink raw reply
* Re: [PATCH net-next] ipv6: remove duplicate neigh_ifdown
From: David Miller @ 2010-12-17 18:14 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20101217100101.5e6e6708@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 17 Dec 2010 10:01:01 -0800
> Ok. It followed my last patch that did the rt6_ifdown only
> if not loopback. Since you accepted that, I sent the next one.
I applied it to net-2.6, which is how we handle bug fixes. The
fix naturally propagates to net-next-2.6 the next tiem I do a merge.
This is why I found it quite strange when you were talking about
net-next-2.6 wrt. that bug fix. We never apply clear important fixes
to net-next-2.6 first, then backport it to net-2.6
If such a net-2.6 dependency exists on a net-next-2.6 patch, you just
need to let me know about it so I can do the merge before applying it.
That's all.
^ permalink raw reply
* Re: [PATCH net-2.6] tehuti: Firmware filename is tehuti/bdx.bin
From: David Miller @ 2010-12-17 18:16 UTC (permalink / raw)
To: ben
Cc: joe.jin, baum, andy, netdev, linux-kernel, guru.anbalagane,
greg.marsden, zhenzhong.duan, jaswinder
In-Reply-To: <1292595184.3136.843.camel@localhost>
From: Ben Hutchings <ben@decadent.org.uk>
Date: Fri, 17 Dec 2010 14:13:03 +0000
> My conversion of tehuti to use request_firmware() was confused about
> the filename of the firmware blob. Change the driver to match the
> blob.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] via-velocity: fix the WOL bug on 1000M full duplex forced mode
From: Jan Ceuleers @ 2010-12-17 18:50 UTC (permalink / raw)
To: David Lv; +Cc: netdev, romieu
In-Reply-To: <AANLkTimVVV76MRn1H8U6nA4V-A7a6WpR2eWgc+mCHjRv@mail.gmail.com>
On 17/12/10 09:25, David Lv wrote:
> I am very Sorry to cause you some trouble.
David, I'm not complaining; just trying to help.
> There are some differences with this version and previous version.
In that case you will need to resubmit it (without the whitespace
damage) for David Miller to be able to apply it.
HTH, Jan
^ permalink raw reply
* Re: [PATCH NEXT 0/3]qlcnic: bug fixes
From: David Miller @ 2010-12-17 19:39 UTC (permalink / raw)
To: amit.salecha; +Cc: netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <1292576342-1359-1-git-send-email-amit.salecha@qlogic.com>
From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Fri, 17 Dec 2010 00:58:59 -0800
> Series of 3 bug fixes, apply them in net-next branch.
All applied, thanks.
^ permalink raw reply
* "x86: allocate space within a region top-down" causes bar0 access issue
From: Jon Mason @ 2010-12-17 19:44 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-kernel, netdev, Ramkrishna Vepa
The following patch is causing problem with the vxge driver/adapter on
HP x86-64 systems. Reads to bar0 to return 0xffffffffffffffff instead
of their intended value. This prevents the vxge module from loading
by failing sanity checks in the driver for certain values in bar0. We
are not seeing any issues with this patch on non-HP systems in our
lab.
Can this patch be removed from 2.6.37 until a better solution can be
found?
Thanks,
Jon
commit 1af3c2e45e7a641e774bbb84fa428f2f0bf2d9c9
Author: Bjorn Helgaas <bjorn.helgaas@hp.com>
Date: Tue Oct 26 15:41:54 2010 -0600
x86: allocate space within a region top-down
Request that allocate_resource() use available space from high addresses
first, rather than the default of using low addresses first.
The most common place this makes a difference is when we move or assign
new PCI device resources. Low addresses are generally scarce, so it's
better to use high addresses when possible. This follows Windows practice
for PCI allocation.
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=16228#c42
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 922b5a1..0fe76df 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -788,6 +788,7 @@ void __init setup_arch(char **cmdline_p)
x86_init.oem.arch_setup();
+ resource_alloc_from_bottom = 0;
iomem_resource.end = (1ULL << boot_cpu_data.x86_phys_bits) - 1;
setup_memory_map();
parse_setup_data();
^ permalink raw reply related
* Re: "x86: allocate space within a region top-down" causes bar0 access issue
From: Jesse Barnes @ 2010-12-17 19:47 UTC (permalink / raw)
To: Jon Mason; +Cc: Bjorn Helgaas, linux-kernel, netdev, Ramkrishna Vepa
In-Reply-To: <20101217194457.GA4470@exar.com>
On Fri, 17 Dec 2010 13:44:58 -0600
Jon Mason <jon.mason@exar.com> wrote:
> The following patch is causing problem with the vxge driver/adapter on
> HP x86-64 systems. Reads to bar0 to return 0xffffffffffffffff instead
> of their intended value. This prevents the vxge module from loading
> by failing sanity checks in the driver for certain values in bar0. We
> are not seeing any issues with this patch on non-HP systems in our
> lab.
>
> Can this patch be removed from 2.6.37 until a better solution can be
> found?
Can you try my for-linus branch and see if the problem still exists
there? It's at
git://git.kernel.org/pub/scm/linux/kernel/git/jbarnes/pci-2.6.git.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply
* Re: [PATCH] asix: add USB ID for Logitec LAN-GTJ U2A
From: David Miller @ 2010-12-17 19:51 UTC (permalink / raw)
To: arno; +Cc: dhollis, netdev, stable
In-Reply-To: <87sjxy4w3o.fsf@totally-fudged-out-message-id>
From: Arnaud Ebalard <arno@natisbad.org>
Date: Wed, 15 Dec 2010 23:16:30 +0100
> Logitec LAN-GTJ U2A (http://www.pro.logitec.co.jp/pro/g/gLAN-GTJU2A/)
> USB 2.0 10/10/1000 Ethernet adapter is based on ASIX AX88178 chipset.
>
> This patch adds missing USB ID for the device.
>
> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] via-velocity: fix the WOL bug on 1000M full duplex forced mode
From: David Miller @ 2010-12-17 19:57 UTC (permalink / raw)
To: davidlv.linux; +Cc: jan.ceuleers, netdev, romieu
In-Reply-To: <AANLkTimVVV76MRn1H8U6nA4V-A7a6WpR2eWgc+mCHjRv@mail.gmail.com>
From: David Lv <davidlv.linux@gmail.com>
Date: Fri, 17 Dec 2010 16:25:55 +0800
> I am very Sorry to cause you some trouble.
> There are some differences with this version and previous version.
>
> } else if (SPD_DPX_1000_FULL != pInfo->hw.sOpts.spd_dpx) {
> + if (SPD_DPX_AUTO == pInfo->hw.sOpts.spd_dpx) {
>
> changed to
>
> } else if (SPD_DPX_1000_FULL != vptr->options.spd_dpx) {
> + if (SPD_DPX_AUTO == vptr->options.spd_dpx) {
Can you please also fix up the indentation of the code you
are adding? The indentation of MII_REG_BITS_ON() when you split
it up into multiple lines looks terrible.
I know you want to prevent long lines, but the result is worse
than a long line.
The indentation is much deeper now because you're putting already
deeply indented code under a new conditional, the 1000_FULL test.
So to keep this from looking ugly you have two choices:
1) Use goto:
if (SPD_DPX_1000_FULL == pInfo->hw.sOpts.spd_dpx)
goto skip;
existing code...
skip:
2) Put the inner logic into a helper function, and call that when
the new conditional passes.
Thanks.
^ permalink raw reply
* Re: [PATCH] ip: use checksum options of first fragment also for subsequent fragments
From: David Miller @ 2010-12-17 19:58 UTC (permalink / raw)
To: timo.lindfors; +Cc: netdev
In-Reply-To: <1292313093-11973-1-git-send-email-timo.lindfors@iki.fi>
From: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Date: Tue, 14 Dec 2010 09:51:33 +0200
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=24832
> Signed-off-by: Timo Juhani Lindfors <timo.lindfors@iki.fi>
You need to provide a commit log message that actually describes,
in detail, why you are making this change and what the bug is.
Referencing a bugzilla entry is insufficient.
^ permalink raw reply
* Re: [PATCH] netlink: fix gcc -Wconversion compilation warning
From: David Miller @ 2010-12-17 20:02 UTC (permalink / raw)
To: ebiederm; +Cc: kirill, netdev, pablo, ldv, linux-kernel
In-Reply-To: <m1sjy1uzj6.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 13 Dec 2010 13:35:25 -0800
> "Kirill A. Shutsemov" <kirill@shutemov.name> writes:
>
>> From: Dmitry V. Levin <ldv@altlinux.org>
>>
>> $ cat << EOF | gcc -Wconversion -xc -S -o/dev/null -
>> unsigned f(void) {return NLMSG_HDRLEN;}
>> EOF
>> <stdin>: In function 'f':
>> <stdin>:3:26: warning: negative integer implicitly converted to unsigned type
>>
> This doesn't look like a bad fix, but I believe things will fail if
> we give NLMSG_ALIGN an unsigned long like size_t. Say like sizeof.
What are you talking about? That's exactly his test case,
look at what NLMSG_HDRLEN is defined to, it's exactly the
case you're worried "will fail", it passes sizeof() to
NLMSG_ALIGN.
I think I'll apply Kirill's original patch, it's good enough
and simpler.
^ permalink raw reply
* Re: "x86: allocate space within a region top-down" causes bar0 access issue
From: Bjorn Helgaas @ 2010-12-17 20:16 UTC (permalink / raw)
To: Jon Mason; +Cc: linux-kernel, netdev, Ramkrishna Vepa
In-Reply-To: <20101217194457.GA4470@exar.com>
On Friday, December 17, 2010 12:44:58 pm Jon Mason wrote:
> The following patch is causing problem with the vxge driver/adapter on
> HP x86-64 systems. Reads to bar0 to return 0xffffffffffffffff instead
> of their intended value. This prevents the vxge module from loading
> by failing sanity checks in the driver for certain values in bar0. We
> are not seeing any issues with this patch on non-HP systems in our
> lab.
>
> Can this patch be removed from 2.6.37 until a better solution can be
> found?
There were several issues related to that patch, and it's about to
be reverted. I am curious about the failure you're seeing, though,
and I'd like to understand the cause and make sure it's one of the
issues I've already investigated.
Can you send me the complete dmesg log of a failing boot?
Thanks,
Bjorn
^ permalink raw reply
* Re: [PATCH V7 1/8] ntp: add ADJ_SETOFFSET mode bit
From: Kuwahara,T. @ 2010-12-17 20:16 UTC (permalink / raw)
To: Richard Cochran
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Alan Cox, Arnd Bergmann, Christoph Lameter, David Miller,
John Stultz, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
Thomas Gleixner
In-Reply-To: <880d82bb8120f73973db27e0c48e949014b1a106.1292512461.git.richard.cochran-3mrvs1K0uXizZXS1Dc/lvw@public.gmane.org>
On 12/17/10, Richard Cochran <richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This patch adds a new mode bit into the timex structure. When set, the bit
> instructs the kernel to add the given time value to the current time.
>
The proposed new control mode, ADJ_SETOFFSET, is logically the same as
ADJ_OFFSET with timex.constant == -INFINITY. So it is possible to do
the same thing without risking forward compatibility. (I mean by "risking
forward compatibility" that the mode bit 0x0040 may be defined differently
by the upstream maintainer anytime in the future.)
^ permalink raw reply
* Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine
From: Neil Horman @ 2010-12-17 20:52 UTC (permalink / raw)
To: Mike Waychison
Cc: simon.kagstrom, davem, Matt Mackall, adurbin, linux-kernel,
chavey, Greg KH, netdev, Américo Wang, akpm, linux-api
In-Reply-To: <20101214212909.17022.96801.stgit@mike.mtv.corp.google.com>
On Tue, Dec 14, 2010 at 01:29:10PM -0800, Mike Waychison wrote:
> Representing the internal state within netconsole isn't really a boolean
> value, but rather a state machine with transitions.
>
> This patch introduces 4 states that the netconsole multi-target can
> handle. These states are:
> - NETPOLL_DISABLED:
> The netpoll structure hasn't been setup.
> - NETPOLL_SETTINGUP:
> The netpoll structure is being setup, and only whoever set this
> state can transition out of it. Must come from the NETPOLL_DISABLED
> state.
> - NETPOLL_ENABLED:
> The netpoll structure has been setup and we are good to emit
> packets.
> - NETPOLL_CLEANING:
> Somebody is queued to call netpoll_clean. Whoever does so must
> transition out of this state. Must come from the NETPOLL_CLEANING
> state.
>
> The SETTINGUP state specifically targets the window where
> netpoll_setup() can take a while (for example, waiting on RTNL).
> During this window, we want to exclude console messages from being
> emitted to this netpoll target. We also want to exclude any subsequent
> user thread that tries to simultaneously enable or disable the target.
>
> The CLEANING state will be used in a subsequent patch to safely defer
> netpoll_cleanup to scheduled work in process context (to fix the
> deadlock that will occur whenever NETDEV_UNREGISTER is seen).
>
> When introducing these new transition states, it no longer makes sense
> to 'disable' the target if the interface is going down, only when it is
> being removed completely (or deslaved). In fact, prior to this change,
> we would leak a reference to the network device if an interface was
> brought down, the target removed, and netconsole unloaded.
>
> Signed-off-by: Mike Waychison <mikew@google.com>
> Acked-by: Matt Mackall <mpm@selenic.com>
> ---
> drivers/net/netconsole.c | 62 +++++++++++++++++++++++++++++-----------------
> 1 files changed, 39 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 6e16888..288a025 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -71,16 +71,24 @@ static LIST_HEAD(target_list);
> /* This needs to be a spinlock because write_msg() cannot sleep */
> static DEFINE_SPINLOCK(target_list_lock);
>
> +#define NETPOLL_DISABLED 0
> +#define NETPOLL_SETTINGUP 1
> +#define NETPOLL_ENABLED 2
> +#define NETPOLL_CLEANING 3
> +
> /**
> * struct netconsole_target - Represents a configured netconsole target.
> * @list: Links this target into the target_list.
> * @item: Links us into the configfs subsystem hierarchy.
> - * @enabled: On / off knob to enable / disable target.
> - * Visible from userspace (read-write).
> - * We maintain a strict 1:1 correspondence between this and
> - * whether the corresponding netpoll is active or inactive.
> + * @np_state: Enabled / Disabled / SettingUp / Cleaning
> + * Visible from userspace (read-write) as "enabled".
> + * We maintain a state machine here of the valid states. Either a
> + * target is enabled or disabled, but it may also be in a
> + * transitional state whereby nobody is allowed to act on the
> + * target other than whoever owns the transition.
> + *
> * Also, other parameters of a target may be modified at
> - * runtime only when it is disabled (enabled == 0).
> + * runtime only when it is disabled (np_state == NETPOLL_ENABLED).
> * @np: The netpoll structure for this target.
> * Contains the other userspace visible parameters:
> * dev_name (read-write)
> @@ -96,7 +104,7 @@ struct netconsole_target {
> #ifdef CONFIG_NETCONSOLE_DYNAMIC
> struct config_item item;
> #endif
> - int enabled;
> + int np_state;
> struct netpoll np;
> };
>
> @@ -189,7 +197,7 @@ static struct netconsole_target *alloc_param_target(char *target_config)
> if (err)
> goto fail;
>
> - nt->enabled = 1;
> + nt->np_state = NETPOLL_ENABLED;
>
> return nt;
>
> @@ -275,7 +283,8 @@ static long strtol10_check_range(const char *cp, long min, long max)
>
> static ssize_t show_enabled(struct netconsole_target *nt, char *buf)
> {
> - return snprintf(buf, PAGE_SIZE, "%d\n", nt->enabled);
> + return snprintf(buf, PAGE_SIZE, "%d\n",
> + nt->np_state == NETPOLL_ENABLED);
> }
>
> static ssize_t show_dev_name(struct netconsole_target *nt, char *buf)
> @@ -337,9 +346,12 @@ static ssize_t store_enabled(struct netconsole_target *nt,
>
> if (enabled) { /* 1 */
> spin_lock_irqsave(&target_list_lock, flags);
> - if (nt->enabled)
> + if (nt->np_state != NETPOLL_DISABLED)
> goto busy;
> - spin_unlock_irqrestore(&target_list_lock, flags);
> + else {
> + nt->np_state = NETPOLL_SETTINGUP;
> + spin_unlock_irqrestore(&target_list_lock, flags);
> + }
>
> /*
> * Skip netpoll_parse_options() -- all the attributes are
> @@ -350,9 +362,9 @@ static ssize_t store_enabled(struct netconsole_target *nt,
> err = netpoll_setup(&nt->np);
> spin_lock_irqsave(&target_list_lock, flags);
> if (err)
> - nt->enabled = 0;
> + nt->np_state = NETPOLL_DISABLED;
> else
> - nt->enabled = 1;
> + nt->np_state = NETPOLL_ENABLED;
> spin_unlock_irqrestore(&target_list_lock, flags);
> if (err)
> return err;
> @@ -360,10 +372,17 @@ static ssize_t store_enabled(struct netconsole_target *nt,
> printk(KERN_INFO "netconsole: network logging started\n");
> } else { /* 0 */
> spin_lock_irqsave(&target_list_lock, flags);
> - nt->enabled = 0;
> + if (nt->np_state == NETPOLL_ENABLED)
> + nt->np_state = NETPOLL_CLEANING;
> + else if (nt->np_state != NETPOLL_DISABLED)
> + goto busy;
> spin_unlock_irqrestore(&target_list_lock, flags);
>
> netpoll_cleanup(&nt->np);
> +
> + spin_lock_irqsave(&target_list_lock, flags);
> + nt->np_state = NETPOLL_DISABLED;
> + spin_unlock_irqrestore(&target_list_lock, flags);
> }
>
> return strnlen(buf, count);
> @@ -486,7 +505,7 @@ static ssize_t store_locked_##_name(struct netconsole_target *nt, \
> unsigned long flags; \
> ssize_t ret; \
> spin_lock_irqsave(&target_list_lock, flags); \
> - if (nt->enabled) { \
> + if (nt->np_state != NETPOLL_DISABLED) { \
> printk(KERN_ERR "netconsole: target (%s) is enabled, " \
> "disable to update parameters\n", \
> config_item_name(&nt->item)); \
> @@ -642,7 +661,7 @@ static void drop_netconsole_target(struct config_group *group,
> * The target may have never been enabled, or was manually disabled
> * before being removed so netpoll may have already been cleaned up.
> */
> - if (nt->enabled)
> + if (nt->np_state == NETPOLL_ENABLED)
> netpoll_cleanup(&nt->np);
>
> config_item_put(&nt->item);
> @@ -680,16 +699,17 @@ static int netconsole_netdev_event(struct notifier_block *this,
> struct net_device *dev = ptr;
>
> if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
> - event == NETDEV_BONDING_DESLAVE || event == NETDEV_GOING_DOWN))
> + event == NETDEV_BONDING_DESLAVE))
> goto done;
>
> spin_lock_irqsave(&target_list_lock, flags);
> list_for_each_entry(nt, &target_list, list) {
> - if (nt->np.dev == dev) {
> + if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
> switch (event) {
> case NETDEV_CHANGENAME:
> strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
> break;
> + case NETDEV_BONDING_DESLAVE:
> case NETDEV_UNREGISTER:
I don't follow what you're doing here. Previously NETDEV_BONDING_DESLAVE events
simply caused the netconsole interface to get deactivated, and now we're doing a
dev_put on the bonded interface bacause of the above move. Given that
NETDEV_BONDING_DESLAVE events are generated when a slave leaves the bond, this
doesn't seem right, or am I missing something
Regards
Neil
^ permalink raw reply
* net-2.6 merged into net-next-2.6
From: David Miller @ 2010-12-17 21:03 UTC (permalink / raw)
To: netdev; +Cc: linville
I just did a merge, it was mostly clean except for some
wireless bits (some -next work conflicted with the recent
"use_new_eeprom_foo" changes in iwlwifi).
John please take a quick look and see if I made any mistakes.
Thanks.
^ permalink raw reply
* Re: Driver issue on Linux 2.6.35-22-generic-pae #33-Ubuntu SMP
From: Francois Romieu @ 2010-12-17 21:14 UTC (permalink / raw)
To: Colum.Flannigan; +Cc: netdev
In-Reply-To: <CB723940FBFB4D13BBE4C9AF2700F00A.MAI@omegasrvshn.com>
Colum.Flannigan@omegasrvshn.com <Colum.Flannigan@omegasrvshn.com> :
[...]
> I am writing to this address as it is the address listed in the mod info.
Please be nice with me and cut your lines below 80 characters.
> I purchased 3 of your cards for my infrastucture and am haven an issue with
> my linux system i am testing this on. Currently i have 1 card installed on
> this system and it is working fine, however the gigabit card i got from
> Netgear does not show in the system.
You forgot to specify the kind of card you are using.
(3x) Netgear GA311 ?
> I have checked my dmesg's and have no errors relatting to this and
> the lsmod command does not show it in the list...
The device driver can not help much as long as lsmod does not list
the card. If it is a PCI device detection problem (i.e. card correctly
inserted, slot and card ok), you may take it on linux-kernel but you
will probably be required to try a more recent kernel and provide more
info (lspci -H1 and -H2 output, dmesg, etc.).
--
Ueimor
^ permalink raw reply
* Re: [PATCH v3 03/22] netconsole: Introduce 'enabled' state-machine
From: Mike Waychison @ 2010-12-17 21:20 UTC (permalink / raw)
To: Neil Horman
Cc: simon.kagstrom, davem, Matt Mackall, adurbin, linux-kernel,
chavey, Greg KH, netdev, Américo Wang, akpm, linux-api
In-Reply-To: <20101217205238.GA8642@hmsreliant.think-freely.org>
On Fri, Dec 17, 2010 at 12:52 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Tue, Dec 14, 2010 at 01:29:10PM -0800, Mike Waychison wrote:
>> @@ -680,16 +699,17 @@ static int netconsole_netdev_event(struct notifier_block *this,
>> struct net_device *dev = ptr;
>>
>> if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
>> - event == NETDEV_BONDING_DESLAVE || event == NETDEV_GOING_DOWN))
>> + event == NETDEV_BONDING_DESLAVE))
>> goto done;
>>
>> spin_lock_irqsave(&target_list_lock, flags);
>> list_for_each_entry(nt, &target_list, list) {
>> - if (nt->np.dev == dev) {
>> + if (nt->np_state == NETPOLL_ENABLED && nt->np.dev == dev) {
>> switch (event) {
>> case NETDEV_CHANGENAME:
>> strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
>> break;
>> + case NETDEV_BONDING_DESLAVE:
>> case NETDEV_UNREGISTER:
> I don't follow what you're doing here. Previously NETDEV_BONDING_DESLAVE events
> simply caused the netconsole interface to get deactivated, and now we're doing a
> dev_put on the bonded interface bacause of the above move. Given that
> NETDEV_BONDING_DESLAVE events are generated when a slave leaves the bond, this
> doesn't seem right, or am I missing something
Ya, I did a poor job of explaining this bit. Let me try now:
- NETDEV_GOING_DOWN is unneccesary because we will always see a
NETDEV_UNREGISTER event, which is why it is removed.
- I wasn't sure how to handle the NETDEV_BONDING_DESLAVE event in a
consistent way. Originally we would disable the target, but this
actually leaks nt->np as it will never get cleaned up properly. To
the user, it looks as if the target gets disabled.
I don't think you have any contention with the first comment above
(though I need to document it).
You are right in that with this patch, the target is left enabled,
when we should probably mark it as disabled. I'd be happy to add the
transition from NETPOLL_ENABLED -> NETPOLL_DISABLED here if you'd
like, but it'd move again in the next patch anyway (which already
re-introduces the state transition in defer_netpoll_cleanup()). Sorry
this is confusing :( I'll make it clearer in the next series
version.
^ permalink raw reply
* Re: [PATCH net-next-2.6 v2 1/1] can: c_can: Added support for Bosch C_CAN controller
From: Wolfgang Grandegger @ 2010-12-17 21:21 UTC (permalink / raw)
To: Bhupesh Sharma
Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1292407130-19791-1-git-send-email-bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
Hi Bhupesh,
here comes my first quick preview.
On 12/15/2010 10:58 AM, Bhupesh Sharma wrote:
> Bosch C_CAN controller is a full-CAN implementation which is compliant
> to CAN protocol version 2.0 part A and B. Bosch C_CAN user manual can be
> obtained from:
> http://www.semiconductors.bosch.de/pdf/Users_Manual_C_CAN.pdf
>
> This patch adds the support for this controller.
> The following are the design choices made while writing the controller driver:
> 1. Interface Register set IF1 has be used only in the current design.
> 2. Out of the 32 Message objects available, 16 are kept aside for RX purposes
> and the rest for TX purposes.
> 3. NAPI implementation is such that both the TX and RX paths function in
> polling mode.
>
> Changes since V1:
> 1. Implemented C_CAN as a platform driver with means of providing the
> platform details and register offsets which may vary for different SoCs
> through platform data struct.
> 2. Implemented NAPI.
> 3. Removed memcpy calls globally.
> 4. Implemented CAN_CTRLMODE_*
> 5. Implemented and used priv->can.do_get_berr_counter.
> 6. Implemented c_can registers as a struct instead of enum.
> 7. Improved the TX path by implementing routines to get next Tx and echo msg
> objects.
>
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> ---
> drivers/net/can/Kconfig | 7 +
> drivers/net/can/Makefile | 1 +
> drivers/net/can/c_can.c | 1217 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1225 insertions(+), 0 deletions(-)
> create mode 100644 drivers/net/can/c_can.c
>
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 9d9e453..25d9d2e 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -41,6 +41,13 @@ config CAN_AT91
> ---help---
> This is a driver for the SoC CAN controller in Atmel's AT91SAM9263.
>
> +config CAN_C_CAN
> + tristate "Bosch C_CAN controller"
> + depends on CAN_DEV
> + ---help---
> + If you say yes to this option, support will be included for the
> + Bosch C_CAN controller.
> +
> config CAN_TI_HECC
> depends on CAN_DEV && ARCH_OMAP3
> tristate "TI High End CAN Controller"
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index 0057537..b6cbe74 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -12,6 +12,7 @@ obj-y += usb/
> obj-$(CONFIG_CAN_SJA1000) += sja1000/
> obj-$(CONFIG_CAN_MSCAN) += mscan/
> obj-$(CONFIG_CAN_AT91) += at91_can.o
> +obj-$(CONFIG_CAN_C_CAN) += c_can.o
> obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
> obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
> obj-$(CONFIG_CAN_BFIN) += bfin_can.o
> diff --git a/drivers/net/can/c_can.c b/drivers/net/can/c_can.c
> new file mode 100644
> index 0000000..c281c17
> --- /dev/null
> +++ b/drivers/net/can/c_can.c
> @@ -0,0 +1,1217 @@
> +/*
> + * CAN bus driver for Bosch C_CAN controller
> + *
> + * Copyright (C) 2010 ST Microelectronics
> + * Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>
> + *
> + * Borrowed heavily from the C_CAN driver originally written by:
> + * Copyright (C) 2007
> + * - Sascha Hauer, Marc Kleine-Budde, Pengutronix <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + * - Simon Kallweit, intefo AG <simon.kallweit-+G9qxTFKJT/tRgLqZ5aouw@public.gmane.org>
> + *
> + * Bosch C_CAN controller is compliant to CAN protocol version 2.0 part A and B.
> + * Bosch C_CAN user manual can be obtained from:
> + * http://www.semiconductors.bosch.de/pdf/Users_Manual_C_CAN.pdf
> + *
> + * 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/version.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/netdevice.h>
> +#include <linux/if_arp.h>
> +#include <linux/if_ether.h>
> +#include <linux/list.h>
> +#include <linux/delay.h>
> +#include <linux/workqueue.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +
> +#define DRV_NAME "c_can"
> +
> +/* control register */
> +#define CONTROL_TEST (1 << 7)
> +#define CONTROL_CCE (1 << 6)
> +#define CONTROL_DISABLE_AR (1 << 5)
> +#define CONTROL_ENABLE_AR (0 << 5)
> +#define CONTROL_EIE (1 << 3)
> +#define CONTROL_SIE (1 << 2)
> +#define CONTROL_IE (1 << 1)
> +#define CONTROL_INIT (1 << 0)
> +
> +/* test register */
> +#define TEST_RX (1 << 7)
> +#define TEST_TX1 (1 << 6)
> +#define TEST_TX2 (1 << 5)
> +#define TEST_LBACK (1 << 4)
> +#define TEST_SILENT (1 << 3)
> +#define TEST_BASIC (1 << 2)
> +
> +/* status register */
> +#define STATUS_BOFF (1 << 7)
> +#define STATUS_EWARN (1 << 6)
> +#define STATUS_EPASS (1 << 5)
> +#define STATUS_RXOK (1 << 4)
> +#define STATUS_TXOK (1 << 3)
> +#define STATUS_LEC_MASK 0x07
> +#define LEC_STUFF_ERROR 1
> +#define LEC_FORM_ERROR 2
> +#define LEC_ACK_ERROR 3
> +#define LEC_BIT1_ERROR 4
> +#define LEC_BIT0_ERROR 5
> +#define LEC_CRC_ERROR 6
Could be an enum!?
> +/* error counter register */
> +#define ERR_COUNTER_TEC_MASK 0xff
> +#define ERR_COUNTER_TEC_SHIFT 0x0
> +#define ERR_COUNTER_REC_SHIFT 8
> +#define ERR_COUNTER_REC_MASK (0x7f << ERR_COUNTER_REC_SHIFT)
> +#define ERR_COUNTER_RP_SHIFT 15
> +#define ERR_COUNTER_RP_MASK (0x1 << ERR_COUNTER_RP_SHIFT)
> +
> +/* bit-timing register */
> +#define BTR_BRP_MASK 0x3f
> +#define BTR_BRP_SHIFT 0
> +#define BTR_SJW_SHIFT 6
> +#define BTR_SJW_MASK (0x3 << BTR_SJW_SHIFT)
> +#define BTR_TSEG1_SHIFT 8
> +#define BTR_TSEG1_MASK (0xf << BTR_TSEG1_SHIFT)
> +#define BTR_TSEG2_SHIFT 12
> +#define BTR_TSEG2_MASK (0x7 << BTR_TSEG2_SHIFT)
> +
> +/* brp extension register */
> +#define BRP_EXT_BRPE_MASK 0x0f
> +#define BRP_EXT_BRPE_SHIFT 0
> +
> +/* IFx command request */
> +#define IF_COMR_BUSY (1 << 15)
> +
> +/* IFx command mask */
> +#define IF_COMM_WR (1 << 7)
> +#define IF_COMM_MASK (1 << 6)
> +#define IF_COMM_ARB (1 << 5)
> +#define IF_COMM_CONTROL (1 << 4)
> +#define IF_COMM_CLR_INT_PND (1 << 3)
> +#define IF_COMM_TXRQST (1 << 2)
> +#define IF_COMM_DATAA (1 << 1)
> +#define IF_COMM_DATAB (1 << 0)
> +#define IF_COMM_ALL (IF_COMM_MASK | IF_COMM_ARB | \
> + IF_COMM_CONTROL | IF_COMM_TXRQST | \
> + IF_COMM_DATAA | IF_COMM_DATAB)
> +
> +/* IFx arbitration */
> +#define IF_ARB_MSGVAL (1 << 15)
> +#define IF_ARB_MSGXTD (1 << 14)
> +#define IF_ARB_TRANSMIT (1 << 13)
> +
> +/* IFx message control */
> +#define IF_MCONT_NEWDAT (1 << 15)
> +#define IF_MCONT_MSGLST (1 << 14)
> +#define IF_MCONT_INTPND (1 << 13)
> +#define IF_MCONT_UMASK (1 << 12)
> +#define IF_MCONT_TXIE (1 << 11)
> +#define IF_MCONT_RXIE (1 << 10)
> +#define IF_MCONT_RMTEN (1 << 9)
> +#define IF_MCONT_TXRQST (1 << 8)
> +#define IF_MCONT_EOB (1 << 7)
> +
> +/*
> + * IFx register masks:
> + * allow easy operation on 16-bit registers when the
> + * argument is 32-bit instead
> + */
> +#define IFX_WRITE_LOW_16BIT(x) (x & 0xFFFF)
> +#define IFX_WRITE_HIGH_16BIT(x) ((x & 0xFFFF0000) >> 16)
> +
> +/* message object split */
> +#define C_CAN_NO_OF_OBJECTS 31
> +#define C_CAN_MSG_OBJ_RX_NUM 16
> +#define C_CAN_MSG_OBJ_TX_NUM 16
> +
> +#define C_CAN_MSG_OBJ_RX_FIRST 0
> +#define C_CAN_MSG_OBJ_RX_LAST (C_CAN_MSG_OBJ_RX_FIRST + \
> + C_CAN_MSG_OBJ_RX_NUM - 1)
> +
> +#define C_CAN_MSG_OBJ_TX_FIRST (C_CAN_MSG_OBJ_RX_LAST + 1)
> +#define C_CAN_MSG_OBJ_TX_LAST (C_CAN_MSG_OBJ_TX_FIRST + \
> + C_CAN_MSG_OBJ_TX_NUM - 1)
> +#define C_CAN_NEXT_MSG_OBJ_MASK (C_CAN_MSG_OBJ_TX_NUM - 1)
> +#define RECEIVE_OBJECT_BITS 0x0000ffff
> +
> +/* status interrupt */
> +#define STATUS_INTERRUPT 0x8000
> +
> +/* napi related */
> +#define C_CAN_NAPI_WEIGHT C_CAN_MSG_OBJ_RX_NUM
> +
> +/* c_can IF registers */
> +struct c_can_if_regs {
> + u16 com_reg;
> + u16 com_mask;
> + u16 mask1;
> + u16 mask2;
> + u16 arb1;
> + u16 arb2;
> + u16 msg_cntrl;
> + u16 data_a1;
> + u16 data_a2;
> + u16 data_b1;
> + u16 data_b2;
> + u16 _reserved[13];
> +};
> +
> +/* c_can hardware registers */
> +struct c_can_regs {
> + u16 control;
> + u16 status;
> + u16 error_counter;
> + u16 btr;
> + u16 ir;
> + u16 test;
> + u16 brp_ext;
> + u16 _reserved1;
> + struct c_can_if_regs ifreg[2]; /* [0] = IF1 and [1] = IF2 */
> + u16 _reserved2[8];
> + u16 txrqst1;
> + u16 txrqst2;
> + u16 _reserved3[6];
> + u16 newdat1;
> + u16 newdat2;
> + u16 _reserved4[6];
> + u16 intpnd1;
> + u16 intpnd2;
> + u16 _reserved5[6];
> + u16 msgval1;
> + u16 msgval2;
> + u16 _reserved6[6];
> +};
> +
> +/*
> + * c_can error types:
> + * Bus errors (BUS_OFF, ERROR_WARNING, ERROR_PASSIVE) are supported
> + */
> +enum c_can_bus_error_types {
> + C_CAN_NO_ERROR = 0,
> + C_CAN_BUS_OFF,
> + C_CAN_ERROR_WARNING,
> + C_CAN_ERROR_PASSIVE
> +};
> +enum c_can_interrupt_mode {
> + ENABLE_MODULE_INTERRUPT = 0,
> + DISABLE_MODULE_INTERRUPT,
> + ENABLE_ALL_INTERRUPTS,
> + DISABLE_ALL_INTERRUPTS
> +};
> +
> +/* c_can private data structure */
> +struct c_can_priv {
> + struct can_priv can; /* must be the first member */
> + struct napi_struct napi;
> + struct net_device *dev;
> + int tx_object;
> + int current_status;
> + int last_status;
> + u16 (*read_reg) (struct c_can_priv *priv, void *reg);
> + void (*write_reg) (struct c_can_priv *priv, void *reg, u16 val);
> + struct c_can_regs __iomem *reg_base;
> + unsigned long irq_flags; /* for request_irq() */
> + unsigned int tx_next;
> + unsigned int tx_echo;
> + struct clk *clk;
> +};
> +
> +static struct can_bittiming_const c_can_bittiming_const = {
> + .name = DRV_NAME,
> + .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
> + .tseg1_max = 16,
> + .tseg2_min = 1, /* Time segment 2 = phase_seg2 */
> + .tseg2_max = 8,
> + .sjw_max = 4,
> + .brp_min = 1,
> + .brp_max = 1024, /* 6-bit BRP field + 4-bit BRPE field*/
> + .brp_inc = 1,
> +};
> +
> +static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
> +{
> + return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
> + C_CAN_MSG_OBJ_TX_FIRST;
> +}
> +
> +static inline int get_tx_echo_msg_obj(const struct c_can_priv *priv)
> +{
> + return (priv->tx_echo & C_CAN_NEXT_MSG_OBJ_MASK) +
> + C_CAN_MSG_OBJ_TX_FIRST;
> +}
> +
> +/* 16-bit c_can registers can be arranged differently in the memory
> + * architecture of different implementations. For example: 16-bit
> + * registers can be aligned to a 16-bit boundary or 32-bit boundary etc.
> + * Handle the same by providing a common read/write interface.
> + */
Nitpicking: please use here and in other places the recommended style
for multi-line comments:
/*
* Comment ...
*/
> +static u16 c_can_read_reg_aligned_to_16bit(void *reg)
> +{
> + return readw(reg);
> +}
> +
> +static void c_can_write_reg_aligned_to_16bit(void *reg, u16 val)
> +{
> + writew(val, reg);
> +}
To profit from type checking, you should use "u16 __iomem *reg" instead
of "void *reg". Also, I think iowrite16 is preferred nowadays.
> +static u16 c_can_read_reg_aligned_to_32bit(struct c_can_priv *priv, void *reg)
> +{
> + return readw(reg + (u32)reg - (u32)priv->reg_base);
> +}
> +
> +static void c_can_write_reg_aligned_to_32bit(struct c_can_priv *priv,
> + void *reg, u16 val)
> +{
> + writew(val, reg + (u32)reg - (u32)priv->reg_base);
> +}
> +
This will not work properly on 64-bit systems. "(long)" should be used,
at least. Any better ideas?
> +static u32 c_can_read_reg32(struct c_can_priv *priv, void *reg)
> +{
> + u32 val = priv->read_reg(priv, reg);
> + val |= ((u32) priv->read_reg(priv, reg + 2)) << 16;
> + return val;
> +}
> +
> +static inline int c_can_configure_interrupts(struct c_can_priv *priv,
> + enum c_can_interrupt_mode intr_mode)
> +{
> + unsigned int cntrl_save = priv->read_reg(priv,
> + &priv->reg_base->control);
> +
> + switch (intr_mode) {
> + case ENABLE_MODULE_INTERRUPT:
> + cntrl_save |= CONTROL_IE;
> + break;
> + case DISABLE_MODULE_INTERRUPT:
> + cntrl_save &= ~CONTROL_IE;
> + break;
> + case ENABLE_ALL_INTERRUPTS:
> + cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
> + break;
> + case DISABLE_ALL_INTERRUPTS:
> + cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + priv->write_reg(priv, &priv->reg_base->control, cntrl_save);
> +
> + return 0;
> +}
Do you really need this function using a switch case. The first two
cases are not used anywhere. I think
void c_can_enable_all_interrupts(struct c_can_priv *priv, int enable);
would be fine.
> +static inline int c_can_object_get(struct net_device *dev,
> + int iface, int objno, int mask)
> +{
> + struct c_can_priv *priv = netdev_priv(dev);
> + int timeout = (6 / priv->can.clock.freq);
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_mask,
> + IFX_WRITE_LOW_16BIT(mask));
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_reg,
> + IFX_WRITE_LOW_16BIT(objno + 1));
> +
> + /* as per specs, after writting the message object number in the
> + * IF command request register the transfer b/w interface
> + * register and message RAM must be complete in 6 CAN-CLK
> + * period. The delay accounts for the same
> + */
> + udelay(timeout);
> + if ((priv->read_reg(priv, &priv->reg_base->ifreg[iface].com_reg)) &
I don't think you need the inner brackets.
> + IF_COMR_BUSY) {
> + dev_info(dev->dev.parent, "timed out in object get\n");
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +static inline int c_can_object_put(struct net_device *dev,
> + int iface, int objno, int mask)
> +{
> + struct c_can_priv *priv = netdev_priv(dev);
> + int timeout = (6 / priv->can.clock.freq);
Hm, "timeout = 0" does not look resonable.
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_mask,
> + (IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].com_reg,
> + IFX_WRITE_LOW_16BIT(objno + 1));
> +
> + /* as per specs, after writting the message object number in the
> + * IF command request register the transfer b/w interface
> + * register and message RAM must be complete in 6 CAN-CLK
> + * period. The delay accounts for the same
> + */
> + udelay(timeout);
> + if ((priv->read_reg(priv, &priv->reg_base->ifreg[iface].com_reg)) &
> + IF_COMR_BUSY) {
> + dev_info(dev->dev.parent, "timed out in object put\n");
dev_err() seems more appropriate.
> + return -ETIMEDOUT;
> + }
Is the timeout really needed? If yes, re-trying various times would more
more safe.
> + return 0;
> +}
> +
> +int c_can_write_msg_object(struct net_device *dev,
> + int iface, struct can_frame *frame, int objno)
> +{
> + u16 flags = 0;
> + unsigned int id;
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + if (frame->can_id & CAN_EFF_FLAG) {
> + id = frame->can_id & CAN_EFF_MASK;
> + flags |= IF_ARB_MSGXTD;
> + } else
> + id = ((frame->can_id & CAN_SFF_MASK) << 18);
> +
> + if (!(frame->can_id & CAN_RTR_FLAG))
> + flags |= IF_ARB_TRANSMIT;
> +
> + flags |= IF_ARB_MSGVAL;
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1,
> + IFX_WRITE_LOW_16BIT(id));
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2, flags |
> + IFX_WRITE_HIGH_16BIT(id));
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].data_a1,
> + (*(u16 *)(frame->data)));
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].data_a2,
> + (*(u32 *)(frame->data)) >> 16);
> +
> + if (frame->can_dlc > 4) {
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].data_b1,
> + (*(u16 *)(frame->data + 4)));
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].data_b2,
> + (*(u32 *)(frame->data + 4)) >> 16);
> + } else
> + *(u32 *)(frame->data + 4) = 0;
Is this code endianess safe?
> +
> + return frame->can_dlc;
> +}
> +
> +static int c_can_read_msg_object(struct net_device *dev, int iface, int objno)
> +{
> + u16 flags;
> + int ctrl;
> + unsigned int val, data;
> + struct c_can_priv *priv = netdev_priv(dev);
> + struct net_device_stats *stats = &dev->stats;
> + struct sk_buff *skb;
> + struct can_frame *frame;
> +
> + skb = alloc_can_skb(dev, &frame);
> + if (!skb) {
> + stats->rx_dropped++;
> + return -ENOMEM;
> + }
> +
> + val = c_can_object_get(dev, iface, objno, IF_COMM_ALL &
> + ~IF_COMM_TXRQST);
> + if (val < 0)
> + return val;
> +
> + ctrl = priv->read_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl);
> + if (ctrl & IF_MCONT_MSGLST) {
> + stats->rx_errors++;
> + dev_info(dev->dev.parent, "msg lost in buffer %d\n", objno);
> + }
You should create an error message for that error as well.
> + frame->can_dlc = get_can_dlc(ctrl & 0x0F);
> + data = priv->read_reg(priv, &priv->reg_base->ifreg[iface].data_a1) |
> + (priv->read_reg(priv, &priv->reg_base->ifreg[iface].data_a2) <<
> + 16);
> + *(u32 *)(frame->data) = data;
> + if (frame->can_dlc > 4) {
> + data = priv->read_reg(priv,
> + &priv->reg_base->ifreg[iface].data_b1) |
> + (priv->read_reg(priv,
> + &priv->reg_base->ifreg[iface].data_b2) <<
> + 16);
> + *(u32 *)(frame->data + 4) = data;
> + } else
> + *(u32 *)(frame->data + 4) = 0;
Ditto.
> + flags = priv->read_reg(priv, &priv->reg_base->ifreg[iface].arb2);
> + val = priv->read_reg(priv, &priv->reg_base->ifreg[iface].arb1) |
> + (flags << 16);
> +
> + if (flags & IF_ARB_MSGXTD)
> + frame->can_id = (val & CAN_EFF_MASK) | CAN_EFF_FLAG;
> + else
> + frame->can_id = (val >> 18) & CAN_SFF_MASK;
> +
> + if (flags & IF_ARB_TRANSMIT)
> + frame->can_id |= CAN_RTR_FLAG;
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl, ctrl &
> + ~(IF_MCONT_MSGLST | IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> +
> + val = c_can_object_put(dev, iface, objno, IF_COMM_CONTROL);
> + if (val < 0)
> + return val;
> +
> + netif_receive_skb(skb);
> +
> + stats->rx_packets++;
> + stats->rx_bytes += frame->can_dlc;
> +
> + return 0;
The return values are not handled anywhere!
> +}
> +
> +static int c_can_setup_receive_object(struct net_device *dev, int iface,
> + int objno, unsigned int mask,
> + unsigned int id, unsigned int mcont)
> +{
> + int ret;
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].mask1,
> + IFX_WRITE_LOW_16BIT(mask));
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].mask2,
> + IFX_WRITE_HIGH_16BIT(mask));
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1,
> + IFX_WRITE_LOW_16BIT(id));
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2,
> + (IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl, mcont);
> + ret = c_can_object_put(dev, iface, objno, IF_COMM_ALL &
> + ~IF_COMM_TXRQST);
> + if (ret < 0)
> + return ret;
Ditto.
> +
> + dev_dbg(dev->dev.parent, "obj no:%d, msgval:0x%08x\n", objno,
> + c_can_read_reg32(priv, &priv->reg_base->msgval1));
> +
> + return 0;
> +}
> +
> +static int c_can_inval_msg_object(struct net_device *dev, int iface, int objno)
> +{
> + int ret;
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb1, 0);
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].arb2, 0);
> + priv->write_reg(priv, &priv->reg_base->ifreg[iface].msg_cntrl, 0);
> +
> + ret = c_can_object_put(dev, iface, objno,
> + IF_COMM_ARB | IF_COMM_CONTROL);
> + if (ret < 0)
> + return ret;
> +
> + dev_dbg(dev->dev.parent, "obj no:%d, msgval:0x%08x\n", objno,
> + c_can_read_reg32(priv, &priv->reg_base->msgval1));
> +
> + return 0;
Ditto.
> +}
> +
> +static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + u32 val;
> + u32 msg_obj_no;
> + struct c_can_priv *priv = netdev_priv(dev);
> + struct can_frame *frame = (struct can_frame *)skb->data;
> +
> + if (can_dropped_invalid_skb(dev, skb))
> + return NETDEV_TX_OK;
> +
> + msg_obj_no = get_tx_next_msg_obj(priv);
> +
> + /* prepare message object for transmission */
> + val = c_can_write_msg_object(dev, 0, frame, msg_obj_no);
> +
> + /* enable interrupt for this message object */
> + priv->write_reg(priv, &priv->reg_base->ifreg[0].msg_cntrl,
> + IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
> + (val & 0xf));
> + val = c_can_object_put(dev, 0, msg_obj_no, IF_COMM_ALL);
> + if (val < 0)
> + return val;
> +
> + can_put_echo_skb(skb, dev, msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> +
> + priv->tx_next++;
> + if ((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) == 0)
> + netif_stop_queue(dev);
> +
> + return NETDEV_TX_OK;
> +}
> +
> +static int c_can_set_bittiming(struct net_device *dev)
> +{
> + unsigned int reg_btr, reg_brpe, ctrl_save;
> + u8 brp, brpe, sjw, tseg1, tseg2;
> + u32 ten_bit_brp;
> + struct c_can_priv *priv = netdev_priv(dev);
> + const struct can_bittiming *bt = &priv->can.bittiming;
> +
> + /* c_can provides a 6-bit brp and 4-bit brpe fields */
> + ten_bit_brp = bt->brp - 1;
> + brp = ten_bit_brp & BTR_BRP_MASK;
> + brpe = ten_bit_brp >> 6;
> +
> + sjw = bt->sjw - 1;
> + tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
> + tseg2 = bt->phase_seg2 - 1;
> +
> + reg_btr = ((brp) | (sjw << BTR_SJW_SHIFT) | (tseg1 << BTR_TSEG1_SHIFT) |
> + (tseg2 << BTR_TSEG2_SHIFT));
> +
> + reg_brpe = brpe & BRP_EXT_BRPE_MASK;
> +
> + dev_dbg(dev->dev.parent,
> + "brp = %d, brpe = %d, sjw = %d, seg1 = %d, seg2 = %d\n",
> + brp, brpe, sjw, tseg1, tseg2);
> + dev_dbg(dev->dev.parent, "setting BTR to %04x\n", reg_btr);
> + dev_dbg(dev->dev.parent, "setting BRPE to %04x\n", reg_brpe);
Like for the other drivers, could you please use one dev_info() here:
dev_dbg(dev->dev.parent, "setting BTR=%04x BRPE=%04x\n", ...);
> + ctrl_save = priv->read_reg(priv, &priv->reg_base->control);
> + priv->write_reg(priv, &priv->reg_base->control,
> + ctrl_save | CONTROL_CCE | CONTROL_INIT);
> + priv->write_reg(priv, &priv->reg_base->btr, reg_btr);
> + priv->write_reg(priv, &priv->reg_base->brp_ext, reg_brpe);
> + priv->write_reg(priv, &priv->reg_base->control, ctrl_save);
> +
> + return 0;
> +}
> +
> +/*
> + * Configure C_CAN message objects for Tx and Rx purposes:
> + * C_CAN provides a total of 32 message objects that can be configured
> + * either for Tx or Rx purposes. Here the first 16 message objects are used as
> + * a reception FIFO. The end of reception FIFO is signified by the EoB bit
> + * being SET. The remaining 16 message objects are kept aside for Tx purposes.
> + * See user guide document for further details on configuring message
> + * objects.
> + */
Did you verify *in-order* transmisson and reception? You could use the
canfdtest program from the can-utils.
> +static int c_can_configure_msg_objects(struct net_device *dev)
> +{
> + int i;
> +
> + /* first invalidate all message objects */
> + for (i = 0; i <= C_CAN_NO_OF_OBJECTS; i++)
> + c_can_inval_msg_object(dev, 0, i);
> +
> + /* setup receive message objects */
> + for (i = C_CAN_MSG_OBJ_RX_FIRST + 1 ; i < C_CAN_MSG_OBJ_RX_LAST; i++)
> + c_can_setup_receive_object(dev, 0, i, 0, 0,
> + ((IF_MCONT_RXIE | IF_MCONT_UMASK) & ~IF_MCONT_EOB));
> +
> + c_can_setup_receive_object(dev, 0, C_CAN_MSG_OBJ_RX_LAST, 0, 0,
> + IF_MCONT_EOB | IF_MCONT_RXIE | IF_MCONT_UMASK);
> + return 0;
> +}
> +
> +/*
> + * Configure C_CAN chip:
> + * - enable/disable auto-retransmission
> + * - set operating mode
> + * - configure message objects
> + */
> +static int c_can_chip_config(struct net_device *dev)
> +{
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> + /* disable automatic retransmission */
> + priv->write_reg(priv, &priv->reg_base->control,
> + CONTROL_DISABLE_AR);
> + else
> + /* enable automatic retransmission */
> + priv->write_reg(priv, &priv->reg_base->control,
> + CONTROL_ENABLE_AR);
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> + /* loopback mode : useful for self-test function */
> + priv->write_reg(priv, &priv->reg_base->control, (CONTROL_EIE |
> + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> + priv->write_reg(priv, &priv->reg_base->test, TEST_LBACK);
> + } else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> + /* silent mode : bus-monitoring mode */
> + priv->write_reg(priv, &priv->reg_base->control, (CONTROL_EIE |
> + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> + priv->write_reg(priv, &priv->reg_base->test, TEST_SILENT);
> + } else if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
> + CAN_CTRLMODE_LOOPBACK)) {
As I see it, this case is never entered.
> + /* loopback + silent mode : useful for hot self-test */
> + priv->write_reg(priv, &priv->reg_base->control, (CONTROL_EIE |
> + CONTROL_SIE | CONTROL_IE | CONTROL_TEST));
> + priv->write_reg(priv, &priv->reg_base->test,
> + (TEST_LBACK | TEST_SILENT));
> + } else
> + /* normal mode*/
> + priv->write_reg(priv, &priv->reg_base->control,
> + (CONTROL_EIE | CONTROL_SIE | CONTROL_IE));
> +
> + /* configure message objects */
> + c_can_configure_msg_objects(dev);
> +
> + return 0;
> +}
> +
> +static int c_can_start(struct net_device *dev)
> +{
> + int err;
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + /* enable status change, error and module interrupts */
> + c_can_configure_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> +
> + /* basic c_can configuration */
> + err = c_can_chip_config(dev);
> + if (err)
> + return err;
> +
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + /* reset tx helper pointers */
> + priv->tx_next = priv->tx_echo = 0;
> +
> + return 0;
> +}
> +
> +static int c_can_stop(struct net_device *dev)
> +{
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + /* disable all interrupts */
> + c_can_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> +
> + /* set the state as STOPPED */
> + priv->can.state = CAN_STATE_STOPPED;
> +
> + return 0;
> +}
> +
> +static int c_can_set_mode(struct net_device *dev, enum can_mode mode)
> +{
> + switch (mode) {
> + case CAN_MODE_START:
> + c_can_start(dev);
> + netif_wake_queue(dev);
> + dev_info(dev->dev.parent,
> + "c_can CAN_MODE_START requested\n");
Please remove.
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int c_can_get_state(const struct net_device *dev,
> + enum can_state *state)
> +{
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + *state = priv->can.state;
> +
> + return 0;
> +}
Please remove. This callback is only required if state changes cannot be
mantained in the interrupt context.
> +static int c_can_get_berr_counter(const struct net_device *dev,
> + struct can_berr_counter *bec)
> +{
> + unsigned int reg_err_counter;
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + reg_err_counter = priv->read_reg(priv, &priv->reg_base->error_counter);
> + bec->rxerr = ((reg_err_counter & ERR_COUNTER_REC_MASK) >>
> + ERR_COUNTER_REC_SHIFT);
> + bec->txerr = (reg_err_counter & ERR_COUNTER_TEC_MASK);
> +
> + return 0;
> +}
> +
> +/*
> + * theory of operation:
> + *
> + * priv->tx_echo holds the number of the oldest can_frame put for
> + * transmission into the hardware, but not yet ACKed by the CAN tx
> + * complete IRQ.
> + *
> + * We iterate from priv->tx_echo to priv->tx_next and check if the
> + * packet has been transmitted, echo it back to the CAN framework. If
> + * we discover a not yet transmitted package, stop looking for more.
> + */
> +static void c_can_do_tx(struct net_device *dev)
> +{
> + u32 val;
> + u32 msg_obj_no;
> + struct c_can_priv *priv = netdev_priv(dev);
> + struct net_device_stats *stats = &dev->stats;
> +
> + for (/* nix */; (priv->tx_next - priv->tx_echo) > 0; priv->tx_echo++) {
> + msg_obj_no = get_tx_echo_msg_obj(priv);
> + c_can_inval_msg_object(dev, 0, msg_obj_no);
> + val = c_can_read_reg32(priv, &priv->reg_base->txrqst1);
> + if (!(val & (1 << msg_obj_no))) {
> + can_get_echo_skb(dev,
> + msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> + stats->tx_bytes += priv->read_reg(priv,
> + &priv->reg_base->ifreg[0].msg_cntrl)
> + & 0xF;
Please use a #define for 0xf.
> + stats->tx_packets++;
> + }
> + }
> +
> + /* restart queue if wrap-up or if queue stalled on last pkt */
> + if (((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) != 0) ||
> + ((priv->tx_echo & C_CAN_NEXT_MSG_OBJ_MASK) == 0))
> + netif_wake_queue(dev);
> +}
> +
> +/*
> + * c_can_do_rx_poll - read multiple CAN messages from message objects
> + */
> +static int c_can_do_rx_poll(struct net_device *dev, int quota)
> +{
> + u32 num_rx_pkts = 0;
> + unsigned int msg_obj;
> + struct c_can_priv *priv = netdev_priv(dev);
> + u32 val = c_can_read_reg32(priv, &priv->reg_base->newdat1);
> +
> + while (val & RECEIVE_OBJECT_BITS) {
> + for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
> + msg_obj <= C_CAN_MSG_OBJ_RX_LAST; msg_obj++) {
> + if (val & (1 << msg_obj)) {
> + c_can_read_msg_object(dev, 0, msg_obj);
> + num_rx_pkts++;
> + quota--;
Where do you handle quota?
> + }
> + }
> +
> + val = c_can_read_reg32(priv, &priv->reg_base->newdat1);
> + }
> +
> + return num_rx_pkts;
> +}
> +
> +static int c_can_err(struct net_device *dev,
> + enum c_can_bus_error_types error_type,
> + int lec_type)
> +{
> + unsigned int reg_err_counter;
> + unsigned int rx_err_passive;
> + struct c_can_priv *priv = netdev_priv(dev);
> + struct net_device_stats *stats = &dev->stats;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct can_berr_counter bec;
> +
> + /* propogate the error condition to the CAN stack */
> + skb = alloc_can_err_skb(dev, &cf);
> + if (unlikely(!skb))
> + return 0;
> +
> + c_can_get_berr_counter(dev, &bec);
> + reg_err_counter = priv->read_reg(priv, &priv->reg_base->error_counter);
> + rx_err_passive = ((reg_err_counter & ERR_COUNTER_RP_MASK) >>
> + ERR_COUNTER_RP_SHIFT);
> +
> + if (error_type & C_CAN_ERROR_WARNING) {
> + /* error warning state */
> + priv->can.can_stats.error_warning++;
> + priv->can.state = CAN_STATE_ERROR_WARNING;
> + cf->can_id |= CAN_ERR_CRTL;
> + if (bec.rxerr > 96)
> + cf->data[1] = CAN_ERR_CRTL_RX_WARNING;
> + if (bec.txerr > 96)
> + cf->data[1] = CAN_ERR_CRTL_TX_WARNING;
> + }
> + if (error_type & C_CAN_ERROR_PASSIVE) {
> + /* error passive state */
> + priv->can.can_stats.error_passive++;
> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
> + cf->can_id |= CAN_ERR_CRTL;
> + if (rx_err_passive)
> + cf->data[1] = CAN_ERR_CRTL_RX_PASSIVE;
> + if (bec.txerr > 127)
> + cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE;
> + }
> + if (error_type & C_CAN_BUS_OFF) {
> + /* bus-off state */
> + priv->can.state = CAN_STATE_BUS_OFF;
> + cf->can_id |= CAN_ERR_BUSOFF;
> + /* disable all interrupts in bus-off mode to ensure that
> + * the CPU is not hogged down
> + */
> + c_can_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> + can_bus_off(dev);
> + }
> +
> + /* check for 'last error code' which tells us the
> + * type of the last error to occur on the CAN bus
> + */
> + if (lec_type) {
> + /* common for all type of bus errors */
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> +
> + if (lec_type & LEC_STUFF_ERROR) {
> + dev_info(dev->dev.parent, "stuff error\n");
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + }
> + if (lec_type & LEC_FORM_ERROR) {
> + dev_info(dev->dev.parent, "form error\n");
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + }
> + if (lec_type & LEC_ACK_ERROR) {
> + dev_info(dev->dev.parent, "ack error\n");
> + cf->data[2] |= (CAN_ERR_PROT_LOC_ACK |
> + CAN_ERR_PROT_LOC_ACK_DEL);
> + }
> + if (lec_type & LEC_BIT1_ERROR) {
> + dev_info(dev->dev.parent, "bit1 error\n");
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + }
> + if (lec_type & LEC_BIT0_ERROR) {
> + dev_info(dev->dev.parent, "bit0 error\n");
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + }
> + if (lec_type & LEC_CRC_ERROR) {
> + dev_info(dev->dev.parent, "CRC error\n");
Please use dev_dbg() here and above
> + cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> + CAN_ERR_PROT_LOC_CRC_DEL);
> + }
> + }
The lec should be handled by a switch statement. Also, please use
dev_dbg in favor of dev_info.
> + netif_receive_skb(skb);
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +
> + return 1;
> +}
> +
> +static int c_can_poll(struct napi_struct *napi, int quota)
> +{
> + u16 irqstatus;
> + int lec_type = 0;
> + int work_done = 0;
> + struct net_device *dev = napi->dev;
> + struct c_can_priv *priv = netdev_priv(dev);
> + enum c_can_bus_error_types error_type = C_CAN_NO_ERROR;
> +
> + irqstatus = priv->read_reg(priv, &priv->reg_base->ir);
> +
> + /* status events have the highest priority */
> + if (irqstatus == STATUS_INTERRUPT) {
> + priv->current_status = priv->read_reg(priv,
> + &priv->reg_base->status);
> +
> + /* handle Tx/Rx events */
> + if (priv->current_status & STATUS_TXOK)
> + priv->write_reg(priv, &priv->reg_base->status,
> + (priv->current_status & ~STATUS_TXOK));
> +
> + if (priv->current_status & STATUS_RXOK)
> + priv->write_reg(priv, &priv->reg_base->status,
> + (priv->current_status & ~STATUS_RXOK));
> +
> + /* handle bus error events */
> + if (priv->current_status & STATUS_EWARN) {
> + dev_info(dev->dev.parent,
> + "entered error warning state\n");
> + error_type = C_CAN_ERROR_WARNING;
> + }
> + if ((priv->current_status & STATUS_EPASS) &&
> + (!(priv->last_status & STATUS_EPASS))) {
> + dev_info(dev->dev.parent,
> + "entered error passive state\n");
> + error_type = C_CAN_ERROR_PASSIVE;
> + }
> + if ((priv->current_status & STATUS_BOFF) &&
> + (!(priv->last_status & STATUS_BOFF))) {
> + dev_info(dev->dev.parent,
> + "entered bus off state\n");
> + error_type = C_CAN_BUS_OFF;
> + }
> + if (priv->current_status & STATUS_LEC_MASK)
> + lec_type = (priv->current_status & STATUS_LEC_MASK);
> +
> + /* handle bus recovery events */
> + if ((!(priv->current_status & STATUS_EPASS)) &&
> + (priv->last_status & STATUS_EPASS)) {
> + dev_info(dev->dev.parent,
> + "left error passive state\n");
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> + }
> + if ((!(priv->current_status & STATUS_BOFF)) &&
> + (priv->last_status & STATUS_BOFF)) {
> + dev_info(dev->dev.parent,
> + "left bus off state\n");
Please use dev_dbg here and above.
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> + }
> +
> + priv->last_status = priv->current_status;
> +
> + /* handle error on the bus */
> + if (error_type != C_CAN_NO_ERROR)
> + work_done += c_can_err(dev, error_type, lec_type);
> + } else if ((irqstatus > C_CAN_MSG_OBJ_RX_FIRST) &&
> + (irqstatus <= C_CAN_MSG_OBJ_RX_LAST)) {
> + /* handle events corresponding to receive message objects */
> + work_done += c_can_do_rx_poll(dev, (quota - work_done));
> + quota--;
Why do you decrement quota here?
> + } else if ((irqstatus > C_CAN_MSG_OBJ_TX_FIRST) &&
> + (irqstatus <= C_CAN_MSG_OBJ_TX_LAST)) {
> + /* handle events corresponding to transmit message objects */
> + c_can_do_tx(dev);
> + }
> +
> + if (work_done < quota) {
> + napi_complete(napi);
> + /* enable all IRQs */
> + c_can_configure_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> + }
> +
> + return work_done;
> +}
> +
> +static irqreturn_t c_can_isr(int irq, void *dev_id)
> +{
> + struct net_device *dev = (struct net_device *)dev_id;
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + /* disable all interrupts and schedule the NAPI */
> + c_can_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> + napi_schedule(&priv->napi);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int c_can_open(struct net_device *dev)
> +{
> + int err;
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + /* open the can device */
> + err = open_candev(dev);
> + if (err) {
> + dev_err(dev->dev.parent, "failed to open can device\n");
> + return err;
> + }
> +
> + /* register interrupt handler */
> + err = request_irq(dev->irq, &c_can_isr, priv->irq_flags, dev->name,
> + (void *)dev);
I don't think you need the (void *) cast.
> + if (err < 0) {
> + dev_err(dev->dev.parent, "failed to attach interrupt\n");
> + goto exit_irq_fail;
> + }
> +
> + /* start the c_can controller */
> + err = c_can_start(dev);
> + if (err)
> + goto exit_start_fail;
> + napi_enable(&priv->napi);
> +
> + netif_start_queue(dev);
> +
> + return 0;
> +
> +exit_start_fail:
> + free_irq(dev->irq, dev);
> +exit_irq_fail:
> + close_candev(dev);
> + return err;
> +}
> +
> +static int c_can_close(struct net_device *dev)
> +{
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + netif_stop_queue(dev);
> + napi_disable(&priv->napi);
> + c_can_stop(dev);
> + free_irq(dev->irq, dev);
> + close_candev(dev);
> +
> + return 0;
> +}
> +
> +static const struct net_device_ops c_can_netdev_ops = {
> + .ndo_open = c_can_open,
> + .ndo_stop = c_can_close,
> + .ndo_start_xmit = c_can_start_xmit,
> +};
> +
> +static int c_can_probe(struct platform_device *pdev)
Please use __devinit ...
> +{
> + int ret;
> + void __iomem *addr;
> + struct net_device *dev;
> + struct c_can_priv *priv;
> + struct resource *mem, *irq;
> + struct clk *clk;
> +
> + /* get the appropriate clk */
> + clk = clk_get(&pdev->dev, NULL);
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "no clock defined\n");
> + ret = -ENODEV;
> + goto exit;
> + }
> +
> + /* get the platform data */
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (!mem || (irq <= 0)) {
> + ret = -ENODEV;
> + goto exit_free_clk;
> + }
> +
> + if (!request_mem_region(mem->start, resource_size(mem), DRV_NAME)) {
> + dev_err(&pdev->dev, "resource unavailable\n");
> + ret = -ENODEV;
> + goto exit_free_clk;
> + }
> +
> + addr = ioremap(mem->start, resource_size(mem));
> + if (!addr) {
> + dev_err(&pdev->dev, "failed to map can port\n");
> + ret = -ENOMEM;
> + goto exit_release_mem;
> + }
> +
> + /* allocate the c_can device */
> + dev = alloc_candev(sizeof(struct c_can_priv), C_CAN_MSG_OBJ_TX_NUM);
> + if (!dev) {
> + ret = -ENOMEM;
> + goto exit_iounmap;
> + }
> +
> + priv = netdev_priv(dev);
> +
> + priv->irq_flags = irq->flags;
> + priv->reg_base = addr;
> + priv->can.clock.freq = clk_get_rate(clk);
> + priv->clk = clk;
> +
> + switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
> + case IORESOURCE_MEM_32BIT:
> + priv->read_reg = c_can_read_reg_aligned_to_32bit;
> + priv->write_reg = c_can_write_reg_aligned_to_32bit;
> + break;
> + case IORESOURCE_MEM_16BIT:
> + default:
> + priv->read_reg = c_can_read_reg_aligned_to_16bit;
> + priv->write_reg = c_can_write_reg_aligned_to_16bit;
> + break;
> + }
> +
> + priv->dev = dev;
> + priv->can.bittiming_const = &c_can_bittiming_const;
> + priv->can.do_set_bittiming = c_can_set_bittiming;
> + priv->can.do_get_state = c_can_get_state;
> + priv->can.do_set_mode = c_can_set_mode;
> + priv->can.do_get_berr_counter = c_can_get_berr_counter;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_ONE_SHOT |
> + CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY |
> + CAN_CTRLMODE_BERR_REPORTING;
Where is CAN_CTRLMODE_BERR_REPORTING implemented? Note that it has
nothing to do with do_get_berr_counter. Please check the SJA1000 driver:
http://lxr.linux.no/#linux+v2.6.36/drivers/net/can/sja1000/sja1000.c#L146
Bus error handling can be requested by the user via netlink interface.
# ip link set canX type can ... berr-reporting on
The driver then usually enables the bus error interrupts. I just realize
that Documentation/networking/can.txt is not up-to-date. I will provide
a patch a.s.a.p.
> + netif_napi_add(dev, &priv->napi, c_can_poll, C_CAN_NAPI_WEIGHT);
> +
> + dev->irq = irq->start;
> + dev->flags |= IFF_ECHO; /* we support local echo */
> + dev->netdev_ops = &c_can_netdev_ops;
> + platform_set_drvdata(pdev, dev);
> + SET_NETDEV_DEV(dev, &pdev->dev);
> +
> + ret = register_candev(dev);
> + if (ret) {
> + dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> + DRV_NAME, ret);
> + goto exit_free_device;
> + }
> +
> + dev_info(&pdev->dev, "%s device registered (reg_base=%p, irq=%d)\n",
> + DRV_NAME, priv->reg_base, dev->irq);
> + return 0;
> +
> +exit_free_device:
> + platform_set_drvdata(pdev, NULL);
> + free_candev(dev);
> +exit_iounmap:
> + iounmap(addr);
> +exit_release_mem:
> + release_mem_region(mem->start, resource_size(mem));
> +exit_free_clk:
> + clk_put(clk);
> +exit:
> + dev_err(&pdev->dev, "probe failed\n");
> +
> + return ret;
> +}
> +
> +static int c_can_remove(struct platform_device *pdev)
... and __devexit.
> +{
> + struct net_device *dev = platform_get_drvdata(pdev);
> + struct c_can_priv *priv = netdev_priv(dev);
> + struct resource *mem;
> +
> + /* disable all interrupts */
> + c_can_configure_interrupts(priv, DISABLE_ALL_INTERRUPTS);
> +
> + unregister_candev(dev);
> + platform_set_drvdata(pdev, NULL);
> +
> + free_candev(dev);
> + iounmap(priv->reg_base);
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + release_mem_region(mem->start, resource_size(mem));
> +
> + clk_put(priv->clk);
> +
> + return 0;
> +}
> +
> +static struct platform_driver c_can_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + },
> + .probe = c_can_probe,
> + .remove = c_can_remove,
Please use __devexit_p.
> +};
No "=" alignment please.
> +static int __init c_can_init(void)
> +{
> + return platform_driver_register(&c_can_driver);
> +}
> +module_init(c_can_init);
> +
> +static void __exit c_can_exit(void)
> +{
> + platform_driver_unregister(&c_can_driver);
> +}
> +module_exit(c_can_exit);
> +
> +MODULE_AUTHOR("Bhupesh Sharma <bhupesh.sharma-qxv4g6HH51o@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("CAN bus driver for Bosch C_CAN controller");
You could also use the new netdev_dbg and friends instead of
dev_dbg(dev->dev.parent, ...).
Thanks for you contribution.
Wolfgang.
^ permalink raw reply
* [PATCH] hostap: remove netif_stop_queue from init
From: Meelis Roos @ 2010-12-17 21:27 UTC (permalink / raw)
To: David Miller; +Cc: j, netdev, linux-wireless
In-Reply-To: <20101210.094747.193727608.davem@davemloft.net>
Fix runtime warning with backtrace from hostap by removing
netif_stop_queue() call before register_netdev. Tested to work fine on
hostap_pci Prism 2.5.
Signed-off-by: Meelis Roos <mroos@linux.ee>
diff --git a/drivers/net/wireless/hostap/hostap_main.c b/drivers/net/wireless/hostap/hostap_main.c
index 25a2722..1d9aed6 100644
--- a/drivers/net/wireless/hostap/hostap_main.c
+++ b/drivers/net/wireless/hostap/hostap_main.c
@@ -891,7 +891,6 @@ void hostap_setup_dev(struct net_device *dev, local_info_t *local,
SET_ETHTOOL_OPS(dev, &prism2_ethtool_ops);
- netif_stop_queue(dev);
}
static int hostap_enable_hostapd(local_info_t *local, int rtnl_locked)
--
Meelis Roos (mroos@linux.ee)
^ 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