* Re: [PATCH] net: dev_getfirstbyhwtype() optimization
From: Paul E. McKenney @ 2010-03-19 11:54 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1268975642.2894.218.camel@edumazet-laptop>
On Fri, Mar 19, 2010 at 06:14:02AM +0100, Eric Dumazet wrote:
> Le jeudi 18 mars 2010 à 19:32 -0700, Paul E. McKenney a écrit :
> > On Thu, Mar 18, 2010 at 10:27:25PM +0100, Eric Dumazet wrote:
> > > Use RCU to avoid RTNL use in dev_getfirstbyhwtype()
> > >
> > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > > ---
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 17b1686..0f2e9fc 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -772,14 +772,17 @@ EXPORT_SYMBOL(__dev_getfirstbyhwtype);
> > >
> > > struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type)
> > > {
> > > - struct net_device *dev;
> > > + struct net_device *dev, *ret = NULL;
> > >
> > > - rtnl_lock();
> > > - dev = __dev_getfirstbyhwtype(net, type);
> > > - if (dev)
> > > - dev_hold(dev);
> > > - rtnl_unlock();
> > > - return dev;
> > > + rcu_read_lock();
> > > + for_each_netdev_rcu(net, dev)
> > > + if (dev->type == type) {
> > > + dev_hold(dev);
> > > + ret = dev;
> > > + break;
> > > + }
> > > + rcu_read_unlock();
> > > + return ret;
> >
> > Looks good, but I don't understand how it helps to introduce the
> > local variable "ret".
> >
>
> Thanks for reviewing Paul !
>
> Not only it helps, its necessary :)
>
> for_each_netdev_rcu(net, dev) {
> if (cond) {
> dev_hold(dev);
> break;
> }
> }
> makes no guarantee dev is NULL if we hit the list end :
>
>
> /**
> * list_for_each_entry_rcu - iterate over rcu list of given type
> * @pos: the type * to use as a loop cursor.
> * @head: the head for your list.
> * @member: the name of the list_struct within the struct.
> *
> * This list-traversal primitive may safely run concurrently with
> * the _rcu list-mutation primitives such as list_add_rcu()
> * as long as the traversal is guarded by rcu_read_lock().
> */
> #define list_for_each_entry_rcu(pos, head, member) \
> for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
> prefetch(pos->member.next), &pos->member != (head); \
> pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
Right! Got it, thank you!
Thanx, Paul
^ permalink raw reply
* RE: [PATCH] netxen: The driver doesn't work on NX_P3_B1 so cause probe to fail.
From: Amit Salecha @ 2010-03-19 12:03 UTC (permalink / raw)
To: David Miller, ebiederm@xmission.com; +Cc: netdev@vger.kernel.org, Ameen Rahman
In-Reply-To: <20100318.222126.27823381.davem@davemloft.net>
David,
Eric's initial problem got resolved by using newer firmware (link problem).
He is facing another problem, mac address are all ff:ff:ff.
Though this problem goes away with driver reload.
We had asked for fw dump to analyze this problem in detail.
-Amit Salecha
-----Original Message-----
From: Amit Salecha
Sent: Friday, March 19, 2010 11:07 AM
To: 'David Miller'; ebiederm@xmission.com
Cc: netdev@vger.kernel.org; Ameen Rahman
Subject: RE: [PATCH] netxen: The driver doesn't work on NX_P3_B1 so cause probe to fail.
We are working on this problem, will let you know our decision by end of day.
-Thanks
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Friday, March 19, 2010 10:51 AM
To: ebiederm@xmission.com
Cc: Amit Salecha; netdev@vger.kernel.org; Ameen Rahman
Subject: Re: [PATCH] netxen: The driver doesn't work on NX_P3_B1 so cause probe to fail.
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 18 Mar 2010 02:43:39 -0700
> Amit Salecha <amit.salecha@qlogic.com> writes:
>
>> Sorry for all the problem you faced.
>>
>> But you shouldn't add support of device which is not supported.
>> Netxen is now owned by Qlogic. You should first contact Qlogic to solve your problem.
>> Qlogic will take needed action based on problem.
>
> I'm not adding support. I am sending a patch removing support for cards
> that do not work with the current driver and have not worked since 2.6.31.
You qlogic folks better resolve this _FAST_ or else I'll
make an executive decision about how to handle this and
I guarentee I'll make a decision that you will not like.
Thanks. :-)
^ permalink raw reply
* [net-2.6 PATCH 1/3] ixgbevf: Fix VF Stats accounting after reset
From: Jeff Kirsher @ 2010-03-19 12:59 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Greg Rose, Jeff Kirsher
From: Greg Rose <gregory.v.rose@intel.com>
The counters in the 82599 Virtual Function are not clear on read. They
accumulate to the maximum value and then roll over. They are also not
cleared when the VF executes a soft reset, so it is possible they are
non-zero when the driver loads and starts. This has all been accounted
for in the code that keeps the stats up to date but there is one case
that is not. When the PF driver is reset the counters in the VF are
all reset to zero. This adds an additional accounting overhead into
the VF driver when the PF is reset under its feet. This patch adds
additional counters that are used by the VF driver to accumulate and
save stats after a PF reset has been detected. Prior to this patch
displaying the stats in the VF after the PF has reset would show
bogus data.
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbevf/ethtool.c | 42 +++++++++++++++-------
drivers/net/ixgbevf/ixgbevf_main.c | 68 +++++++++++++++++++++++-------------
drivers/net/ixgbevf/vf.h | 6 +++
3 files changed, 78 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ixgbevf/ethtool.c b/drivers/net/ixgbevf/ethtool.c
index 399be0c..6fdd651 100644
--- a/drivers/net/ixgbevf/ethtool.c
+++ b/drivers/net/ixgbevf/ethtool.c
@@ -46,22 +46,32 @@ struct ixgbe_stats {
int sizeof_stat;
int stat_offset;
int base_stat_offset;
+ int saved_reset_offset;
};
-#define IXGBEVF_STAT(m, b) sizeof(((struct ixgbevf_adapter *)0)->m), \
- offsetof(struct ixgbevf_adapter, m), \
- offsetof(struct ixgbevf_adapter, b)
+#define IXGBEVF_STAT(m, b, r) sizeof(((struct ixgbevf_adapter *)0)->m), \
+ offsetof(struct ixgbevf_adapter, m), \
+ offsetof(struct ixgbevf_adapter, b), \
+ offsetof(struct ixgbevf_adapter, r)
static struct ixgbe_stats ixgbe_gstrings_stats[] = {
- {"rx_packets", IXGBEVF_STAT(stats.vfgprc, stats.base_vfgprc)},
- {"tx_packets", IXGBEVF_STAT(stats.vfgptc, stats.base_vfgptc)},
- {"rx_bytes", IXGBEVF_STAT(stats.vfgorc, stats.base_vfgorc)},
- {"tx_bytes", IXGBEVF_STAT(stats.vfgotc, stats.base_vfgotc)},
- {"tx_busy", IXGBEVF_STAT(tx_busy, zero_base)},
- {"multicast", IXGBEVF_STAT(stats.vfmprc, stats.base_vfmprc)},
- {"rx_csum_offload_good", IXGBEVF_STAT(hw_csum_rx_good, zero_base)},
- {"rx_csum_offload_errors", IXGBEVF_STAT(hw_csum_rx_error, zero_base)},
- {"tx_csum_offload_ctxt", IXGBEVF_STAT(hw_csum_tx_good, zero_base)},
- {"rx_header_split", IXGBEVF_STAT(rx_hdr_split, zero_base)},
+ {"rx_packets", IXGBEVF_STAT(stats.vfgprc, stats.base_vfgprc,
+ stats.saved_reset_vfgprc)},
+ {"tx_packets", IXGBEVF_STAT(stats.vfgptc, stats.base_vfgptc,
+ stats.saved_reset_vfgptc)},
+ {"rx_bytes", IXGBEVF_STAT(stats.vfgorc, stats.base_vfgorc,
+ stats.saved_reset_vfgorc)},
+ {"tx_bytes", IXGBEVF_STAT(stats.vfgotc, stats.base_vfgotc,
+ stats.saved_reset_vfgotc)},
+ {"tx_busy", IXGBEVF_STAT(tx_busy, zero_base, zero_base)},
+ {"multicast", IXGBEVF_STAT(stats.vfmprc, stats.base_vfmprc,
+ stats.saved_reset_vfmprc)},
+ {"rx_csum_offload_good", IXGBEVF_STAT(hw_csum_rx_good, zero_base,
+ zero_base)},
+ {"rx_csum_offload_errors", IXGBEVF_STAT(hw_csum_rx_error, zero_base,
+ zero_base)},
+ {"tx_csum_offload_ctxt", IXGBEVF_STAT(hw_csum_tx_good, zero_base,
+ zero_base)},
+ {"rx_header_split", IXGBEVF_STAT(rx_hdr_split, zero_base, zero_base)},
};
#define IXGBE_QUEUE_STATS_LEN 0
@@ -455,10 +465,14 @@ static void ixgbevf_get_ethtool_stats(struct net_device *netdev,
ixgbe_gstrings_stats[i].stat_offset;
char *b = (char *)adapter +
ixgbe_gstrings_stats[i].base_stat_offset;
+ char *r = (char *)adapter +
+ ixgbe_gstrings_stats[i].saved_reset_offset;
data[i] = ((ixgbe_gstrings_stats[i].sizeof_stat ==
sizeof(u64)) ? *(u64 *)p : *(u32 *)p) -
((ixgbe_gstrings_stats[i].sizeof_stat ==
- sizeof(u64)) ? *(u64 *)b : *(u32 *)b);
+ sizeof(u64)) ? *(u64 *)b : *(u32 *)b) +
+ ((ixgbe_gstrings_stats[i].sizeof_stat ==
+ sizeof(u64)) ? *(u64 *)r : *(u32 *)r);
}
}
diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index ca653c4..43927e1 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -1610,6 +1610,44 @@ static inline void ixgbevf_rx_desc_queue_enable(struct ixgbevf_adapter *adapter,
(adapter->rx_ring[rxr].count - 1));
}
+static void ixgbevf_save_reset_stats(struct ixgbevf_adapter *adapter)
+{
+ /* Only save pre-reset stats if there are some */
+ if (adapter->stats.vfgprc || adapter->stats.vfgptc) {
+ adapter->stats.saved_reset_vfgprc += adapter->stats.vfgprc -
+ adapter->stats.base_vfgprc;
+ adapter->stats.saved_reset_vfgptc += adapter->stats.vfgptc -
+ adapter->stats.base_vfgptc;
+ adapter->stats.saved_reset_vfgorc += adapter->stats.vfgorc -
+ adapter->stats.base_vfgorc;
+ adapter->stats.saved_reset_vfgotc += adapter->stats.vfgotc -
+ adapter->stats.base_vfgotc;
+ adapter->stats.saved_reset_vfmprc += adapter->stats.vfmprc -
+ adapter->stats.base_vfmprc;
+ }
+}
+
+static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+
+ adapter->stats.last_vfgprc = IXGBE_READ_REG(hw, IXGBE_VFGPRC);
+ adapter->stats.last_vfgorc = IXGBE_READ_REG(hw, IXGBE_VFGORC_LSB);
+ adapter->stats.last_vfgorc |=
+ (((u64)(IXGBE_READ_REG(hw, IXGBE_VFGORC_MSB))) << 32);
+ adapter->stats.last_vfgptc = IXGBE_READ_REG(hw, IXGBE_VFGPTC);
+ adapter->stats.last_vfgotc = IXGBE_READ_REG(hw, IXGBE_VFGOTC_LSB);
+ adapter->stats.last_vfgotc |=
+ (((u64)(IXGBE_READ_REG(hw, IXGBE_VFGOTC_MSB))) << 32);
+ adapter->stats.last_vfmprc = IXGBE_READ_REG(hw, IXGBE_VFMPRC);
+
+ adapter->stats.base_vfgprc = adapter->stats.last_vfgprc;
+ adapter->stats.base_vfgorc = adapter->stats.last_vfgorc;
+ adapter->stats.base_vfgptc = adapter->stats.last_vfgptc;
+ adapter->stats.base_vfgotc = adapter->stats.last_vfgotc;
+ adapter->stats.base_vfmprc = adapter->stats.last_vfmprc;
+}
+
static int ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
{
struct net_device *netdev = adapter->netdev;
@@ -1656,6 +1694,9 @@ static int ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
/* enable transmits */
netif_tx_start_all_queues(netdev);
+ ixgbevf_save_reset_stats(adapter);
+ ixgbevf_init_last_counter_stats(adapter);
+
/* bring the link up in the watchdog, this could race with our first
* link up interrupt but shouldn't be a problem */
adapter->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
@@ -2228,27 +2269,6 @@ out:
return err;
}
-static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
-{
- struct ixgbe_hw *hw = &adapter->hw;
-
- adapter->stats.last_vfgprc = IXGBE_READ_REG(hw, IXGBE_VFGPRC);
- adapter->stats.last_vfgorc = IXGBE_READ_REG(hw, IXGBE_VFGORC_LSB);
- adapter->stats.last_vfgorc |=
- (((u64)(IXGBE_READ_REG(hw, IXGBE_VFGORC_MSB))) << 32);
- adapter->stats.last_vfgptc = IXGBE_READ_REG(hw, IXGBE_VFGPTC);
- adapter->stats.last_vfgotc = IXGBE_READ_REG(hw, IXGBE_VFGOTC_LSB);
- adapter->stats.last_vfgotc |=
- (((u64)(IXGBE_READ_REG(hw, IXGBE_VFGOTC_MSB))) << 32);
- adapter->stats.last_vfmprc = IXGBE_READ_REG(hw, IXGBE_VFMPRC);
-
- adapter->stats.base_vfgprc = adapter->stats.last_vfgprc;
- adapter->stats.base_vfgorc = adapter->stats.last_vfgorc;
- adapter->stats.base_vfgptc = adapter->stats.last_vfgptc;
- adapter->stats.base_vfgotc = adapter->stats.last_vfgotc;
- adapter->stats.base_vfmprc = adapter->stats.last_vfmprc;
-}
-
#define UPDATE_VF_COUNTER_32bit(reg, last_counter, counter) \
{ \
u32 current_counter = IXGBE_READ_REG(hw, reg); \
@@ -2416,9 +2436,9 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
}
}
-pf_has_reset:
ixgbevf_update_stats(adapter);
+pf_has_reset:
/* Force detection of hung controller every watchdog period */
adapter->detect_tx_hung = true;
@@ -3390,8 +3410,6 @@ static int __devinit ixgbevf_probe(struct pci_dev *pdev,
/* setup the private structure */
err = ixgbevf_sw_init(adapter);
- ixgbevf_init_last_counter_stats(adapter);
-
#ifdef MAX_SKB_FRAGS
netdev->features = NETIF_F_SG |
NETIF_F_IP_CSUM |
@@ -3449,6 +3467,8 @@ static int __devinit ixgbevf_probe(struct pci_dev *pdev,
adapter->netdev_registered = true;
+ ixgbevf_init_last_counter_stats(adapter);
+
/* print the MAC address */
hw_dbg(hw, "%2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x\n",
netdev->dev_addr[0],
diff --git a/drivers/net/ixgbevf/vf.h b/drivers/net/ixgbevf/vf.h
index 799600e..1f31b05 100644
--- a/drivers/net/ixgbevf/vf.h
+++ b/drivers/net/ixgbevf/vf.h
@@ -157,6 +157,12 @@ struct ixgbevf_hw_stats {
u64 vfgorc;
u64 vfgotc;
u64 vfmprc;
+
+ u64 saved_reset_vfgprc;
+ u64 saved_reset_vfgptc;
+ u64 saved_reset_vfgorc;
+ u64 saved_reset_vfgotc;
+ u64 saved_reset_vfmprc;
};
struct ixgbevf_info {
^ permalink raw reply related
* [net-2.6 PATCH 2/3] ixgbevf: Shorten up delay timer for watchdog task
From: Jeff Kirsher @ 2010-03-19 13:00 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Greg Rose, Jeff Kirsher
In-Reply-To: <20100319125950.9799.70157.stgit@localhost.localdomain>
From: Greg Rose <gregory.v.rose@intel.com>
The recovery from PF reset works better when you shorten up the delay
until the watchdog task executes.
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbevf/ixgbevf_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index 43927e1..3de93ae 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -965,7 +965,7 @@ static irqreturn_t ixgbevf_msix_mbx(int irq, void *data)
if ((msg & IXGBE_MBVFICR_VFREQ_MASK) == IXGBE_PF_CONTROL_MSG)
mod_timer(&adapter->watchdog_timer,
- round_jiffies(jiffies + 10));
+ round_jiffies(jiffies + 1));
return IRQ_HANDLED;
}
^ permalink raw reply related
* [net-2.6 PATCH 3/3] ixgbevf: Message formatting cleanups
From: Jeff Kirsher @ 2010-03-19 13:00 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Greg Rose, Jeff Kirsher
In-Reply-To: <20100319125950.9799.70157.stgit@localhost.localdomain>
From: Greg Rose <gregory.v.rose@intel.com>
Clean up some text output formatting.
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbevf/ixgbevf_main.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index 3de93ae..d6cbd94 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -2419,7 +2419,7 @@ static void ixgbevf_watchdog_task(struct work_struct *work)
if (!netif_carrier_ok(netdev)) {
hw_dbg(&adapter->hw, "NIC Link is Up %s, ",
((link_speed == IXGBE_LINK_SPEED_10GB_FULL) ?
- "10 Gbps" : "1 Gbps"));
+ "10 Gbps\n" : "1 Gbps\n"));
netif_carrier_on(netdev);
netif_tx_wake_all_queues(netdev);
} else {
@@ -2695,7 +2695,7 @@ static int ixgbevf_open(struct net_device *netdev)
if (hw->adapter_stopped) {
err = IXGBE_ERR_MBX;
printk(KERN_ERR "Unable to start - perhaps the PF"
- "Driver isn't up yet\n");
+ " Driver isn't up yet\n");
goto err_setup_reset;
}
}
^ permalink raw reply related
* Re: [PATCH net-next-2.6 1/2] can: sja1000: allow shared interrupt definition
From: Wolfgang Grandegger @ 2010-03-19 13:13 UTC (permalink / raw)
To: yegor_sub1-ZJVcf1zZPRSebONBosFW4Q
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4BA356A4.5040205-ZJVcf1zZPRSebONBosFW4Q@public.gmane.org>
Yegor Yefremov wrote:
> SJA1000: allow shared interrupt definition
>
> extend the AND mask, so that IRQF_SHARED flag remains
>
> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Thanks,
Wolfgang.
^ permalink raw reply
* Re: [PATCH net-next-2.6 2/2] can: sja1000: add read/write routines for 8, 16 and 32-bit register access
From: Wolfgang Grandegger @ 2010-03-19 13:14 UTC (permalink / raw)
To: yegor_sub1-ZJVcf1zZPRSebONBosFW4Q
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4BA35704.2060909-ZJVcf1zZPRSebONBosFW4Q@public.gmane.org>
Yegor Yefremov wrote:
> SJA1000: add read/write routines for 8, 16 and 32-bit register access
>
> add routines for 8, 16 and 32-bit access like in
> drivers/i2c/busses/i2c-pca-platform.c
>
> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Thanks,
Wolfgang.
^ permalink raw reply
* Re: [PATCH] pktgen node allocation
From: robert @ 2010-03-19 13:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Robert Olsson, David Miller, netdev
In-Reply-To: <1268990933.3048.15.camel@edumazet-laptop>
Eric Dumazet writes:
> Le vendredi 19 mars 2010 à 09:44 +0100, Robert Olsson a écrit :
>
> I cannot understand how this can help.
>
> __netdev_alloc_skb() is supposed to already take into account NUMA
> properties :
>
> int node = dev->dev.parent ? dev_to_node(dev->dev.parent) : -1;
>
> If this doesnt work, we should correct core stack, not only pktgen :)
>
> Are you allocating memory in the node where pktgen CPU is running or the
> node close to the NIC ?
I didn't say it should help the idea was to give some hooks to
experiment and see effects with different node memory allocations.
There are many degrees of freedom wrt buses(device)/CPU/menory.
Cheers
--ro
^ permalink raw reply
* Re: [Bugme-new] [Bug 15507] New: kernel misses 3rd part of tcp handshake (ACK), stays in SYN_RECV state
From: Eric Dumazet @ 2010-03-19 13:42 UTC (permalink / raw)
To: Joshua Roys; +Cc: Andrew Morton, netdev, bugzilla-daemon, bugme-daemon
In-Reply-To: <4BA365BA.4040309@gmail.com>
Le vendredi 19 mars 2010 à 07:53 -0400, Joshua Roys a écrit :
> On 03/18/2010 07:12 PM, Eric Dumazet wrote:
> >
> > I would say this is expected if httpd server set DEFER_ACCEPT socket
> > option.
> >
> >
> >
>
> Gah! You've got to be kidding :) If that's what it is, close the
> bug... and I'll go read some man-pages!
>
So you _confirm_ DEFER_ACCEPT is not used, its _important_ for us.
I dont believe my comment was trivial at all :)
I gave a hint to myself and other network guys, because we did some
changes in this area lately (commit b103cf34 tcp: fix TCP_DEFER_ACCEPT
retrans calculation) from Julian Anastasov.
git describe b103cf34
v2.6.31-9056-gb103cf3
Please boot a fresh vm, then try exactly 10 'bad connects', please make
each attempt last at least 2 minutes. (you can start all these in
parallel)
And report :
netstat -s
(My FC12 copy doesnt exhibit this problem)
Thanks
^ permalink raw reply
* Re: [PATCH] pktgen node allocation
From: Eric Dumazet @ 2010-03-19 13:47 UTC (permalink / raw)
To: robert; +Cc: David Miller, netdev
In-Reply-To: <19363.32154.39665.185451@gargle.gargle.HOWL>
Le vendredi 19 mars 2010 à 14:35 +0100, robert@herjulf.net a écrit :
> Eric Dumazet writes:
> > Le vendredi 19 mars 2010 à 09:44 +0100, Robert Olsson a écrit :
> >
> > I cannot understand how this can help.
> >
> > __netdev_alloc_skb() is supposed to already take into account NUMA
> > properties :
> >
> > int node = dev->dev.parent ? dev_to_node(dev->dev.parent) : -1;
> >
> > If this doesnt work, we should correct core stack, not only pktgen :)
> >
> > Are you allocating memory in the node where pktgen CPU is running or the
> > node close to the NIC ?
>
> I didn't say it should help the idea was to give some hooks to
> experiment and see effects with different node memory allocations.
> There are many degrees of freedom wrt buses(device)/CPU/menory.
>
Well, you said "Tested this with 10 Intel 82599 ports w. TYAN S7025
E5520 CPU's. Was able to TX/DMA ~80 Gbit/s to Ethernet wires."
I am interested to know what particular setup you did to maximize
throughput then, or are you saing you managed to reduce it ? :)
^ permalink raw reply
* [net-next-2.6 PATCH] bonding: flush unicast and multicast lists when changing type
From: Jiri Pirko @ 2010-03-19 14:00 UTC (permalink / raw)
To: netdev; +Cc: fubar, bonding-devel, davem
After the type change, addresses in unicast and multicast lists wouldn't make
sense, not to mention possible different lenghts. So flush both lists here.
Note "dev_addr_discard" will be very soon replaced by "dev_mc_flush" (once
mc_list conversion will be done).
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/bonding/bond_main.c | 4 ++++
include/linux/netdevice.h | 2 ++
net/core/dev.c | 6 ++++--
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index cbe9e35..c2aceaa 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1490,6 +1490,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
goto err_undo_flags;
}
+ /* Flush unicast and multicast addresses */
+ dev_unicast_flush(bond_dev);
+ dev_addr_discard(bond_dev);
+
if (slave_dev->type != ARPHRD_ETHER)
bond_setup_by_slave(bond_dev, slave_dev);
else
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 813bed7..3cff4ba 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1994,10 +1994,12 @@ extern int dev_unicast_delete(struct net_device *dev, void *addr);
extern int dev_unicast_add(struct net_device *dev, void *addr);
extern int dev_unicast_sync(struct net_device *to, struct net_device *from);
extern void dev_unicast_unsync(struct net_device *to, struct net_device *from);
+extern void dev_unicast_flush(struct net_device *dev);
extern int dev_mc_delete(struct net_device *dev, void *addr, int alen, int all);
extern int dev_mc_add(struct net_device *dev, void *addr, int alen, int newonly);
extern int dev_mc_sync(struct net_device *to, struct net_device *from);
extern void dev_mc_unsync(struct net_device *to, struct net_device *from);
+extern void dev_addr_discard(struct net_device *dev);
extern int __dev_addr_delete(struct dev_addr_list **list, int *count, void *addr, int alen, int all);
extern int __dev_addr_add(struct dev_addr_list **list, int *count, void *addr, int alen, int newonly);
extern int __dev_addr_sync(struct dev_addr_list **to, int *to_count, struct dev_addr_list **from, int *from_count);
diff --git a/net/core/dev.c b/net/core/dev.c
index d1f027c..80a0608 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4455,12 +4455,13 @@ void dev_unicast_unsync(struct net_device *to, struct net_device *from)
}
EXPORT_SYMBOL(dev_unicast_unsync);
-static void dev_unicast_flush(struct net_device *dev)
+void dev_unicast_flush(struct net_device *dev)
{
netif_addr_lock_bh(dev);
__hw_addr_flush(&dev->uc);
netif_addr_unlock_bh(dev);
}
+EXPORT_SYMBOL(dev_unicast_flush);
static void dev_unicast_init(struct net_device *dev)
{
@@ -4482,7 +4483,7 @@ static void __dev_addr_discard(struct dev_addr_list **list)
}
}
-static void dev_addr_discard(struct net_device *dev)
+void dev_addr_discard(struct net_device *dev)
{
netif_addr_lock_bh(dev);
@@ -4491,6 +4492,7 @@ static void dev_addr_discard(struct net_device *dev)
netif_addr_unlock_bh(dev);
}
+EXPORT_SYMBOL(dev_addr_discard);
/**
* dev_get_flags - get flags reported to userspace
--
1.6.6.1
^ permalink raw reply related
* [net-2.6 PATCH] ixgbe: fix for real_num_tx_queues update issue
From: Jeff Kirsher @ 2010-03-19 14:33 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Vasu Dev, Jeff Kirsher
From: Vasu Dev <vasu.dev@intel.com>
Currently netdev_features_change is called before fcoe tx queues
setup is done, so this patch moves calling of netdev_features_change
after tx queues setup is done in ixgbe_init_interrupt_scheme, so
that real_num_tx_queues is updated correctly on each fcoe enable
or disable.
This allows additional fcoe queues updated correctly in vlan driver
for their correct queue selection.
Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_fcoe.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_fcoe.c b/drivers/net/ixgbe/ixgbe_fcoe.c
index 4123dec..700cfc0 100644
--- a/drivers/net/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ixgbe/ixgbe_fcoe.c
@@ -614,9 +614,9 @@ int ixgbe_fcoe_enable(struct net_device *netdev)
netdev->vlan_features |= NETIF_F_FSO;
netdev->vlan_features |= NETIF_F_FCOE_MTU;
netdev->fcoe_ddp_xid = IXGBE_FCOE_DDP_MAX - 1;
- netdev_features_change(netdev);
ixgbe_init_interrupt_scheme(adapter);
+ netdev_features_change(netdev);
if (netif_running(netdev))
netdev->netdev_ops->ndo_open(netdev);
@@ -660,11 +660,11 @@ int ixgbe_fcoe_disable(struct net_device *netdev)
netdev->vlan_features &= ~NETIF_F_FSO;
netdev->vlan_features &= ~NETIF_F_FCOE_MTU;
netdev->fcoe_ddp_xid = 0;
- netdev_features_change(netdev);
ixgbe_cleanup_fcoe(adapter);
-
ixgbe_init_interrupt_scheme(adapter);
+ netdev_features_change(netdev);
+
if (netif_running(netdev))
netdev->netdev_ops->ndo_open(netdev);
rc = 0;
^ permalink raw reply related
* [PATCH] net: Don't drop route cache entry in ipv4_negative_advice unless PTMU expired
From: Guenter Roeck @ 2010-03-19 14:41 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Al Viro, Denis V. Lunev, Guenter Roeck
TCP sessions over IPv4 can get stuck if routers between endpoints
do not fragment packets but implement PMTU instead.
Setup is as follows
MTU1 MTU2 MTU1
A--------B------C------D
with MTU1 > MTU2. A and D are endpoints, B and C are routers. B and C
implement PMTU and drop packets larger than MTU2 (for example because
DF is set on all packets). TCP sessions are initiated between A and D.
There is packet loss between A and D, causing frequent TCP retransmits.
After the number of retransmits on a TCP session reaches tcp_retries1,
tcp calls dst_negative_advice() prior to each retransmit. This results
in route cache entries for the peer to be deleted in ipv4_negative_advice()
if the Path MTU is set.
If the outstanding data on an affected TCP session is larger than MTU2, packets
sent from the endpoints will be dropped by B or C, and ICMP NEEDFRAG will be
returned. A and D receive NEEDFRAG messages and update PMTU.
Before the next retransmit, tcp will again call dst_negative_advice(), causing
the route cache entry (with correct PMTU) to be deleted. The retransmitted
packet will be larger than MTU2, causing it to be dropped again.
This sequence repeats until the TCP session aborts or is terminated.
Problem is fixed by removing route cache entries in ipv4_negative_advice()
only if the PMTU is expired.
Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
net/ipv4/route.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d9b4024..7e1c9e4 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1512,7 +1512,8 @@ static struct dst_entry *ipv4_negative_advice(struct dst_entry *dst)
ip_rt_put(rt);
ret = NULL;
} else if ((rt->rt_flags & RTCF_REDIRECTED) ||
- rt->u.dst.expires) {
+ (rt->u.dst.expires &&
+ time_after_eq(jiffies, rt->u.dst.expires))) {
unsigned hash = rt_hash(rt->fl.fl4_dst, rt->fl.fl4_src,
rt->fl.oif,
rt_genid(dev_net(dst->dev)));
--
1.6.0.4
^ permalink raw reply related
* [net-2.6 PATCH] ixgbe: Set IXGBE_RSC_CB(skb)->DMA field to zero after unmapping the address
From: Jeff Kirsher @ 2010-03-19 14:41 UTC (permalink / raw)
To: davem
Cc: netdev, gospo, Mallikarjuna R Chilakala, Peter P Waskiewicz Jr,
Jeff Kirsher
From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
As per Simon Horman's feedback set IXGBE_RSC_CB(skb)->dma to zero
after unmapping HWRSC DMA address to avoid double freeing.
Signed-off-by: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_main.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 18b5b21..d75c46f 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -935,10 +935,12 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
if (skb->prev)
skb = ixgbe_transform_rsc_queue(skb, &(rx_ring->rsc_count));
if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) {
- if (IXGBE_RSC_CB(skb)->dma)
+ if (IXGBE_RSC_CB(skb)->dma) {
pci_unmap_single(pdev, IXGBE_RSC_CB(skb)->dma,
rx_ring->rx_buf_len,
PCI_DMA_FROMDEVICE);
+ IXGBE_RSC_CB(skb)->dma = 0;
+ }
if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED)
rx_ring->rsc_count += skb_shinfo(skb)->nr_frags;
else
@@ -3126,10 +3128,12 @@ static void ixgbe_clean_rx_ring(struct ixgbe_adapter *adapter,
rx_buffer_info->skb = NULL;
do {
struct sk_buff *this = skb;
- if (IXGBE_RSC_CB(this)->dma)
+ if (IXGBE_RSC_CB(this)->dma) {
pci_unmap_single(pdev, IXGBE_RSC_CB(this)->dma,
rx_ring->rx_buf_len,
PCI_DMA_FROMDEVICE);
+ IXGBE_RSC_CB(this)->dma = 0;
+ }
skb = skb->prev;
dev_kfree_skb(this);
} while (skb);
^ permalink raw reply related
* [PATCH] net: rtnetlink: ignore NETDEV_PRE_TYPE_CHANGE in rtnetlink_event()
From: Patrick McHardy @ 2010-03-19 14:42 UTC (permalink / raw)
To: David Miller; +Cc: jpirko, Linux Netdev List
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 01.diff --]
[-- Type: text/x-diff, Size: 1560 bytes --]
Ignore the new NETDEV_PRE_TYPE_CHANGE event in rtnetlink_event() since
there have been no changes userspace needs to be notified of.
Also add a comment to the netdev notifier event definitions to remind
people to update the exclusion list when adding new event types.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
include/linux/notifier.h | 5 ++++-
net/core/rtnetlink.c | 1 +
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index f3635fc..9c5d3fa 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -182,7 +182,10 @@ static inline int notifier_to_errno(int ret)
* VC switch chains (for loadable kernel svgalib VC switch helpers) etc...
*/
-/* netdevice notifier chain */
+/* netdevice notifier chain. Please remember to update the rtnetlink
+ * notification exclusion list in rtnetlink_event() when adding new
+ * types.
+ */
#define NETDEV_UP 0x0001 /* For now you can't veto a device up/down */
#define NETDEV_DOWN 0x0002
#define NETDEV_REBOOT 0x0003 /* Tell a protocol stack a network interface
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e1121f0..ffc6cf3 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1513,6 +1513,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_POST_INIT:
case NETDEV_REGISTER:
case NETDEV_CHANGE:
+ case NETDEV_PRE_TYPE_CHANGE:
case NETDEV_GOING_DOWN:
case NETDEV_UNREGISTER:
case NETDEV_UNREGISTER_BATCH:
--
1.6.5.7
^ permalink raw reply related
* Re: [Bugme-new] [Bug 15507] New: kernel misses 3rd part of tcp handshake (ACK), stays in SYN_RECV state
From: Eric Dumazet @ 2010-03-19 14:46 UTC (permalink / raw)
To: Joshua Roys; +Cc: Andrew Morton, netdev, bugzilla-daemon, bugme-daemon
In-Reply-To: <4BA365BA.4040309@gmail.com>
Le vendredi 19 mars 2010 à 07:53 -0400, Joshua Roys a écrit :
>
> Gah! You've got to be kidding :) If that's what it is, close the
> bug... and I'll go read some man-pages!
So I checked FC12 httpd and yes, it does use DEFER_ACCEPT (setting val
to 1 second).
So you hit a problem that was corrected by following commit.
Time to bug RedHat I suppose...
commit d1b99ba41d6c5aa1ed2fc634323449dd656899e9
Author: Julian Anastasov <ja@ssi.bg>
Date: Mon Oct 19 10:01:56 2009 +0000
tcp: accept socket after TCP_DEFER_ACCEPT period
Willy Tarreau and many other folks in recent years
were concerned what happens when the TCP_DEFER_ACCEPT period
expires for clients which sent ACK packet. They prefer clients
that actively resend ACK on our SYN-ACK retransmissions to be
converted from open requests to sockets and queued to the
listener for accepting after the deferring period is finished.
Then application server can decide to wait longer for data
or to properly terminate the connection with FIN if read()
returns EAGAIN which is an indication for accepting after
the deferring period. This change still can have side effects
for applications that expect always to see data on the accepted
socket. Others can be prepared to work in both modes (with or
without TCP_DEFER_ACCEPT period) and their data processing can
ignore the read=EAGAIN notification and to allocate resources for
clients which proved to have no data to send during the deferring
period. OTOH, servers that use TCP_DEFER_ACCEPT=1 as flag (not
as a timeout) to wait for data will notice clients that didn't
send data for 3 seconds but that still resend ACKs.
Thanks to Willy Tarreau for the initial idea and to
Eric Dumazet for the review and testing the change.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 624c3c9..4c03598 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -641,8 +641,8 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
if (!(flg & TCP_FLAG_ACK))
return NULL;
- /* If TCP_DEFER_ACCEPT is set, drop bare ACK. */
- if (inet_csk(sk)->icsk_accept_queue.rskq_defer_accept &&
+ /* While TCP_DEFER_ACCEPT is active, drop bare ACK. */
+ if (req->retrans < inet_csk(sk)->icsk_accept_queue.rskq_defer_accept &&
TCP_SKB_CB(skb)->end_seq == tcp_rsk(req)->rcv_isn + 1) {
inet_rsk(req)->acked = 1;
return NULL;
^ permalink raw reply related
* Re: [PATCH] net: rtnetlink: ignore NETDEV_PRE_TYPE_CHANGE in rtnetlink_event()
From: Jiri Pirko @ 2010-03-19 14:47 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, Linux Netdev List
In-Reply-To: <4BA38D50.4090909@trash.net>
Fri, Mar 19, 2010 at 03:42:24PM CET, kaber@trash.net wrote:
>
>Ignore the new NETDEV_PRE_TYPE_CHANGE event in rtnetlink_event() since
>there have been no changes userspace needs to be notified of.
>
>Also add a comment to the netdev notifier event definitions to remind
>people to update the exclusion list when adding new event types.
Hmm, didn't have a clue this is needed. Anyway, original
"NETDEV_BONDING_OLDTYPE" and "NETDEV_BONDING_NEWTYPE" weren't here either.
Also you might want to add "NETDEV_POST_TYPE_CHANGE".
Jirka
>
>Signed-off-by: Patrick McHardy <kaber@trash.net>
>---
> include/linux/notifier.h | 5 ++++-
> net/core/rtnetlink.c | 1 +
> 2 files changed, 5 insertions(+), 1 deletions(-)
>
>diff --git a/include/linux/notifier.h b/include/linux/notifier.h
>index f3635fc..9c5d3fa 100644
>--- a/include/linux/notifier.h
>+++ b/include/linux/notifier.h
>@@ -182,7 +182,10 @@ static inline int notifier_to_errno(int ret)
> * VC switch chains (for loadable kernel svgalib VC switch helpers) etc...
> */
>
>-/* netdevice notifier chain */
>+/* netdevice notifier chain. Please remember to update the rtnetlink
>+ * notification exclusion list in rtnetlink_event() when adding new
>+ * types.
>+ */
> #define NETDEV_UP 0x0001 /* For now you can't veto a device up/down */
> #define NETDEV_DOWN 0x0002
> #define NETDEV_REBOOT 0x0003 /* Tell a protocol stack a network interface
>diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>index e1121f0..ffc6cf3 100644
>--- a/net/core/rtnetlink.c
>+++ b/net/core/rtnetlink.c
>@@ -1513,6 +1513,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
> case NETDEV_POST_INIT:
> case NETDEV_REGISTER:
> case NETDEV_CHANGE:
>+ case NETDEV_PRE_TYPE_CHANGE:
> case NETDEV_GOING_DOWN:
> case NETDEV_UNREGISTER:
> case NETDEV_UNREGISTER_BATCH:
>--
>1.6.5.7
>
^ permalink raw reply
* Re: [PATCH] net: rtnetlink: ignore NETDEV_PRE_TYPE_CHANGE in rtnetlink_event()
From: Patrick McHardy @ 2010-03-19 14:56 UTC (permalink / raw)
To: Jiri Pirko; +Cc: David Miller, Linux Netdev List
In-Reply-To: <20100319144707.GD2811@psychotron.redhat.com>
Jiri Pirko wrote:
> Fri, Mar 19, 2010 at 03:42:24PM CET, kaber@trash.net wrote:
>
>
>
>> Ignore the new NETDEV_PRE_TYPE_CHANGE event in rtnetlink_event() since
>> there have been no changes userspace needs to be notified of.
>>
>> Also add a comment to the netdev notifier event definitions to remind
>> people to update the exclusion list when adding new event types.
>
> Hmm, didn't have a clue this is needed. Anyway, original
> "NETDEV_BONDING_OLDTYPE" and "NETDEV_BONDING_NEWTYPE" weren't here either.
Its not strictly needed, it just avoids sending events without new
information.
> Also you might want to add "NETDEV_POST_TYPE_CHANGE".
I left this out on purpose since userspace might be interested in the
type change.
^ permalink raw reply
* [PATCH] if_tunnel.h: add missing ams/byteorder.h include
From: Paulius Zaleckas @ 2010-03-19 15:04 UTC (permalink / raw)
To: davem; +Cc: contact, eric.dumazet, Fred.L.Templin, netdev, linux-kernel
From: None <None>
When compiling userspace application which includes
if_tunnel.h and uses GRE_* defines you will get undefined
reference to __cpu_to_be16.
Fix this by adding missing #include <asm/byteorder.h>
Cc: stable@kernel.org
Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>
---
include/linux/if_tunnel.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/if_tunnel.h b/include/linux/if_tunnel.h
index 1822d63..16b92d0 100644
--- a/include/linux/if_tunnel.h
+++ b/include/linux/if_tunnel.h
@@ -2,6 +2,7 @@
#define _IF_TUNNEL_H_
#include <linux/types.h>
+#include <asm/byteorder.h>
#ifdef __KERNEL__
#include <linux/ip.h>
^ permalink raw reply related
* [PATCH v2] if_tunnel.h: add missing ams/byteorder.h include
From: Paulius Zaleckas @ 2010-03-19 15:09 UTC (permalink / raw)
To: davem; +Cc: contact, eric.dumazet, Fred.L.Templin, netdev, linux-kernel
When compiling userspace application which includes
if_tunnel.h and uses GRE_* defines you will get undefined
reference to __cpu_to_be16.
Fix this by adding missing #include <asm/byteorder.h>
Cc: stable@kernel.org
Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>
---
include/linux/if_tunnel.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/if_tunnel.h b/include/linux/if_tunnel.h
index 1822d63..16b92d0 100644
--- a/include/linux/if_tunnel.h
+++ b/include/linux/if_tunnel.h
@@ -2,6 +2,7 @@
#define _IF_TUNNEL_H_
#include <linux/types.h>
+#include <asm/byteorder.h>
#ifdef __KERNEL__
#include <linux/ip.h>
^ permalink raw reply related
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
From: Ira W. Snyder @ 2010-03-19 15:13 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: linux-kernel, socketcan-core, netdev, sameo
In-Reply-To: <4BA34026.8010806@grandegger.com>
On Fri, Mar 19, 2010 at 10:13:10AM +0100, Wolfgang Grandegger wrote:
> Ira W. Snyder wrote:
> > The Janz CMOD-IO PCI MODULbus carrier board is a PCI to MODULbus bridge,
> > which may host many different types of MODULbus daughterboards, including
> > CAN and GPIO controllers.
> >
> > Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
>
> You can add my "Reviewed-by: Wolfgang Grandegger <wg@grandegger.com>".
>
> Just one concern:
>
> > ---
> > drivers/mfd/Kconfig | 8 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/janz-cmodio.c | 339 +++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/janz.h | 54 +++++++
> > 4 files changed, 402 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/mfd/janz-cmodio.c
> > create mode 100644 include/linux/mfd/janz.h
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 8782978..f1858d7 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -27,6 +27,14 @@ config MFD_SM501_GPIO
> > lines on the SM501. The platform data is used to supply the
> > base number for the first GPIO line to register.
> >
> > +config MFD_JANZ_CMODIO
> > + tristate "Support for Janz CMOD-IO PCI MODULbus Carrier Board"
> > + ---help---
> > + This is the core driver for the Janz CMOD-IO PCI MODULbus
> > + carrier board. This device is a PCI to MODULbus bridge which may
> > + host many different types of MODULbus daughterboards, including
> > + CAN and GPIO controllers.
> > +
> > config MFD_ASIC3
> > bool "Support for Compaq ASIC3"
> > depends on GENERIC_HARDIRQS && GPIOLIB && ARM
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index e09eb48..e8fa905 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -3,6 +3,7 @@
> > #
> >
> > obj-$(CONFIG_MFD_SM501) += sm501.o
> > +obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o
> > obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
> > obj-$(CONFIG_MFD_SH_MOBILE_SDHI) += sh_mobile_sdhi.o
> >
> > diff --git a/drivers/mfd/janz-cmodio.c b/drivers/mfd/janz-cmodio.c
> > new file mode 100644
> > index 0000000..914280e
> > --- /dev/null
> > +++ b/drivers/mfd/janz-cmodio.c
> > @@ -0,0 +1,339 @@
> > +/*
> > + * Janz CMOD-IO MODULbus Carrier Board PCI Driver
> > + *
> > + * Copyright (c) 2010 Ira W. Snyder <iws@ovro.caltech.edu>
> > + *
> > + * Lots of inspiration and code was copied from drivers/mfd/sm501.c
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the
> > + * Free Software Foundation; either version 2 of the License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/pci.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/delay.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include <linux/mfd/janz.h>
> > +
> > +#define DRV_NAME "janz-cmodio"
> > +
> > +/* Size of each MODULbus module in PCI BAR4 */
> > +#define CMODIO_MODULBUS_SIZE 0x200
> > +
> > +/* Maximum number of MODULbus modules on a CMOD-IO carrier board */
> > +#define CMODIO_MAX_MODULES 4
> > +
> > +/* Module Parameters */
> > +static unsigned int num_modules = CMODIO_MAX_MODULES;
> > +static unsigned char *modules[CMODIO_MAX_MODULES] = {
> > + "janz-ican3",
> > + "janz-ican3",
> > + "",
> > + "janz-ttl",
> > +};
>
> This is probably not a good default but just your private configuration.
>
Yep, that's the configuration I have. Do you have any suggestions for a
better default? I could make them all blank strings, which means "no
daughtercard in this slot". That is my only idea for a different
default.
Thanks for the review!
Ira
^ permalink raw reply
* Re: [PATCH 2/3] can: add support for Janz VMOD-ICAN3 Intelligent CAN module
From: Ira W. Snyder @ 2010-03-19 15:19 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, sameo-VuQAYsv1563Yd54FQh9/CA
In-Reply-To: <4BA33D5A.8070000-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
On Fri, Mar 19, 2010 at 10:01:14AM +0100, Wolfgang Grandegger wrote:
> Hi Ira,
>
> we already discussed this patch on the SocketCAN mailing list and there
> are just a few minor issues and the request to add support for the new
> "berr-reporting" option, if feasible. See:
>
> commit 52c793f24054f5dc30d228e37e0e19cc8313f086
> Author: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Date: Mon Feb 22 22:21:17 2010 +0000
>
> can: netlink support for bus-error reporting and counters
>
> This patch makes the bus-error reporting configurable and allows to
> retrieve the CAN TX and RX bus error counters via netlink interface.
> I have added support for the SJA1000. The TX and RX bus error counters
> are also copied to the data fields 6..7 of error messages when state
> changes are reported.
>
> Should not be a big deal.
>
I think this patch came along since my last post of the driver. I must
have missed it. I'll try and add support.
> More inline...
>
> Ira W. Snyder wrote:
> > The Janz VMOD-ICAN3 is a MODULbus daughterboard which fits onto any
> > MODULbus carrier board. It is an intelligent CAN controller with a
> > microcontroller and associated firmware.
> >
> > Signed-off-by: Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
> > Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> > Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > ---
> > drivers/net/can/Kconfig | 10 +
> > drivers/net/can/Makefile | 1 +
> > drivers/net/can/janz-ican3.c | 1659 ++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 1670 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/net/can/janz-ican3.c
> >
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> > index 05b7517..2c5227c 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -63,6 +63,16 @@ config CAN_BFIN
> > To compile this driver as a module, choose M here: the
> > module will be called bfin_can.
> >
> > +config CAN_JANZ_ICAN3
> > + tristate "Janz VMOD-ICAN3 Intelligent CAN controller"
> > + depends on CAN_DEV && MFD_JANZ_CMODIO
> > + ---help---
> > + Driver for Janz VMOD-ICAN3 Intelligent CAN controller module, which
> > + connects to a MODULbus carrier board.
> > +
> > + This driver can also be built as a module. If so, the module will be
> > + called janz-ican3.ko.
> > +
> > source "drivers/net/can/mscan/Kconfig"
> >
> > source "drivers/net/can/sja1000/Kconfig"
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> > index 7a702f2..9047cd0 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -15,5 +15,6 @@ obj-$(CONFIG_CAN_AT91) += at91_can.o
> > obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
> > obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
> > obj-$(CONFIG_CAN_BFIN) += bfin_can.o
> > +obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-ican3.o
> >
> > ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> > diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
> > new file mode 100644
> > index 0000000..94d4995
> > --- /dev/null
> > +++ b/drivers/net/can/janz-ican3.c
> [snip]
> +struct ican3_dev {
> > +
> > + /* must be the first member */
> > + struct can_priv can;
> > +
> > + /* CAN network device */
> > + struct net_device *ndev;
> > + struct napi_struct napi;
> > +
> > + /* Device for printing */
> > + struct device *dev;
> > +
> > + /* module number */
> > + unsigned int num;
> > +
> > + /* base address of registers and IRQ */
> > + struct janz_cmodio_onboard_regs __iomem *ctrl;
> > + struct ican3_dpm_control *dpmctrl;
> > + void __iomem *dpm;
> > + int irq;
> > +
> > + /* old and new style host interface */
> > + unsigned int iftype;
> > + spinlock_t lock;
>
> Please describe what the lock is used for.
>
> > + /* new host interface */
> > + unsigned int rx_int;
> > + unsigned int rx_num;
> > + unsigned int tx_num;
> > +
> > + /* fast host interface */
> > + unsigned int fastrx_start;
> > + unsigned int fastrx_int;
> > + unsigned int fastrx_num;
> > + unsigned int fasttx_start;
> > + unsigned int fasttx_num;
> > +
> > + /* first free DPM page */
> > + unsigned int free_page;
> > +};
>
> [snip]
> > +static void ican3_to_can_frame(struct ican3_dev *mod,
> > + struct ican3_fast_desc *desc,
> > + struct can_frame *cf)
> > +{
> > + if ((desc->command & ICAN3_CAN_TYPE_MASK) == ICAN3_CAN_TYPE_SFF) {
> > + dev_dbg(mod->dev, "%s: old frame format\n", __func__);
>
> This prints a debug message for every message which is not really
> useful for the user. Please remove.
>
> > + if (desc->data[1] & ICAN3_SFF_RTR)
> > + cf->can_id |= CAN_RTR_FLAG;
> > +
> > + cf->can_id |= desc->data[0] << 3;
> > + cf->can_id |= (desc->data[1] & 0xe0) >> 5;
> > + cf->can_dlc = desc->data[1] & ICAN3_CAN_DLC_MASK;
> > + memcpy(cf->data, &desc->data[2], sizeof(cf->data));
> > + } else {
> > + dev_dbg(mod->dev, "%s: new frame format\n", __func__);
>
> Ditto.
>
> > + cf->can_dlc = desc->data[0] & ICAN3_CAN_DLC_MASK;
> > + if (desc->data[0] & ICAN3_EFF_RTR)
> > + cf->can_id |= CAN_RTR_FLAG;
> > +
> > + if (desc->data[0] & ICAN3_EFF) {
> > + cf->can_id |= CAN_EFF_FLAG;
> > + cf->can_id |= desc->data[2] << 21; /* 28-21 */
> > + cf->can_id |= desc->data[3] << 13; /* 20-13 */
> > + cf->can_id |= desc->data[4] << 5; /* 12-5 */
> > + cf->can_id |= (desc->data[5] & 0xf8) >> 3;
> > + } else {
> > + cf->can_id |= desc->data[2] << 3; /* 10-3 */
> > + cf->can_id |= desc->data[3] >> 5; /* 2-0 */
> > + }
> > +
> > + memcpy(cf->data, &desc->data[6], sizeof(cf->data));
> > + }
> > +}
> > +
> > +static void can_frame_to_ican3(struct ican3_dev *mod,
> > + struct can_frame *cf,
> > + struct ican3_fast_desc *desc)
> > +{
> > + /* clear out any stale data in the descriptor */
> > + memset(desc->data, 0, sizeof(desc->data));
> > +
> > + /* we always use the extended format, with the ECHO flag set */
> > + desc->command = ICAN3_CAN_TYPE_EFF;
> > + desc->data[0] |= cf->can_dlc;
> > + desc->data[1] |= ICAN3_ECHO;
> > +
> > + if (cf->can_id & CAN_RTR_FLAG)
> > + desc->data[0] |= ICAN3_EFF_RTR;
> > +
> > + /* pack the id into the correct places */
> > + if (cf->can_id & CAN_EFF_FLAG) {
> > + dev_dbg(mod->dev, "%s: extended frame\n", __func__);
>
> Ditto.
>
> > + desc->data[0] |= ICAN3_EFF;
> > + desc->data[2] = (cf->can_id & 0x1fe00000) >> 21; /* 28-21 */
> > + desc->data[3] = (cf->can_id & 0x001fe000) >> 13; /* 20-13 */
> > + desc->data[4] = (cf->can_id & 0x00001fe0) >> 5; /* 12-5 */
> > + desc->data[5] = (cf->can_id & 0x0000001f) << 3; /* 4-0 */
> > + } else {
> > + dev_dbg(mod->dev, "%s: standard frame\n", __func__);
>
> Ditto.
>
> > + desc->data[2] = (cf->can_id & 0x7F8) >> 3; /* bits 10-3 */
> > + desc->data[3] = (cf->can_id & 0x007) << 5; /* bits 2-0 */
> > + }
> > +
> > + /* copy the data bits into the descriptor */
> > + memcpy(&desc->data[6], cf->data, sizeof(cf->data));
> > +}
>
> [snip]
> > +/*
> > + * Handle CAN Event Indication Messages from the firmware
> > + *
> > + * The ICAN3 firmware provides the values of some SJA1000 registers when it
> > + * generates this message. The code below is largely copied from the
> > + * drivers/net/can/sja1000/sja1000.c file, and adapted as necessary
> > + */
> > +static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg)
> > +{
> > + struct net_device *dev = mod->ndev;
> > + struct net_device_stats *stats = &dev->stats;
> > + enum can_state state = mod->can.state;
> > + struct can_frame *cf;
> > + struct sk_buff *skb;
> > + u8 status, isrc;
> > +
> > + /* we can only handle the SJA1000 part */
> > + if (msg->data[1] != CEVTIND_CHIP_SJA1000) {
> > + dev_err(mod->dev, "unable to handle errors on non-SJA1000\n");
> > + return -ENODEV;
> > + }
> > +
> > + /* check the message length for sanity */
> > + if (msg->len < 6) {
> > + dev_err(mod->dev, "error message too short\n");
> > + return -EINVAL;
> > + }
> > +
> > + skb = alloc_can_err_skb(dev, &cf);
> > + if (skb == NULL)
> > + return -ENOMEM;
> > +
> > + isrc = msg->data[0];
> > + status = msg->data[3];
> > +
> > + /* data overrun interrupt */
> > + if (isrc == CEVTIND_DOI || isrc == CEVTIND_LOST) {
>
> Here and for the other errors below a dev_dbg() would be useful. Please
> check sja1000.c.
>
> > + cf->can_id |= CAN_ERR_CRTL;
> > + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> > + stats->rx_over_errors++;
> > + stats->rx_errors++;
> > + dev_info(mod->dev, "%s: overflow frame generated\n", __func__);
>
> s/dev_info/dev_dbg/ ?
>
Whoops, a leftover from debugging. Will change to dev_dbg().
> > + }
> > +
> > + /* error warning interrupt */
> > + if (isrc == CEVTIND_EI) {
> > + if (status & SR_BS) {
> > + state = CAN_STATE_BUS_OFF;
> > + cf->can_id |= CAN_ERR_BUSOFF;
> > + can_bus_off(dev);
> > + } else if (status & SR_ES) {
> > + state = CAN_STATE_ERROR_WARNING;
> > + } else {
> > + state = CAN_STATE_ERROR_ACTIVE;
> > + }
> > + }
> > +
> > + /* bus error interrupt */
> > + if (isrc == CEVTIND_BEI) {
> > + u8 ecc = msg->data[2];
>
> Add an empty line, please.
>
> > + mod->can.can_stats.bus_error++;
> > + stats->rx_errors++;
> > + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> > +
> > + switch (ecc & ECC_MASK) {
> > + case ECC_BIT:
> > + cf->data[2] |= CAN_ERR_PROT_BIT;
> > + break;
> > + case ECC_FORM:
> > + cf->data[2] |= CAN_ERR_PROT_FORM;
> > + break;
> > + case ECC_STUFF:
> > + cf->data[2] |= CAN_ERR_PROT_STUFF;
> > + break;
> > + default:
> > + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> > + cf->data[3] = ecc & ECC_SEG;
> > + break;
> > + }
> > +
> > + if ((ecc & ECC_DIR) == 0)
> > + cf->data[2] |= CAN_ERR_PROT_TX;
> > + }
> > +
> > + if (state != mod->can.state && (state == CAN_STATE_ERROR_WARNING ||
> > + state == CAN_STATE_ERROR_PASSIVE)) {
> > + u8 rxerr = msg->data[4];
> > + u8 txerr = msg->data[5];
>
> Ditto.
>
> > + cf->can_id |= CAN_ERR_CRTL;
> > + if (state == CAN_STATE_ERROR_WARNING) {
> > + mod->can.can_stats.error_warning++;
> > + cf->data[1] = (txerr > rxerr) ?
> > + CAN_ERR_CRTL_TX_WARNING :
> > + CAN_ERR_CRTL_RX_WARNING;
> > + } else {
> > + mod->can.can_stats.error_passive++;
> > + cf->data[1] = (txerr > rxerr) ?
> > + CAN_ERR_CRTL_TX_PASSIVE :
> > + CAN_ERR_CRTL_RX_PASSIVE;
> > + }
> > + }
> > +
> > + mod->can.state = state;
> > + stats->rx_errors++;
> > + stats->rx_bytes += cf->can_dlc;
> > + netif_rx(skb);
> > + return 0;
> > +}
>
> [snip]
> > +static irqreturn_t ican3_irq(int irq, void *dev_id)
> > +{
> > + struct ican3_dev *mod = dev_id;
> > + u8 stat;
> > +
> > + /*
> > + * The interrupt status register on this device reports interrupts
> > + * as zeroes instead of using ones like most other devices
> > + */
> > + stat = ioread8(&mod->ctrl->int_disable) & (1 << mod->num);
> > + if (stat == (1 << mod->num))
> > + return IRQ_NONE;
> > +
> > + dev_dbg(mod->dev, "IRQ: module %d\n", mod->num);
>
> Please remove this dev_dbg() as well.
>
> [snip]
> > +/*
> > + * Startup an ICAN module, bringing it into fast mode
> > + */
> > +static int __devinit ican3_startup_module(struct ican3_dev *mod)
> > +{
> > + int ret;
> > +
> > + ret = ican3_reset_module(mod);
> > + if (ret) {
> > + dev_err(mod->dev, "unable to reset module\n");
> > + return ret;
> > + }
> > +
> > + /* re-enable interrupts so we can send messages */
> > + iowrite8(1 << mod->num, &mod->ctrl->int_enable);
> > +
> > + ret = ican3_msg_connect(mod);
> > + if (ret) {
> > + dev_err(mod->dev, "unable to connect to module\n");
> > + return ret;
> > + }
> > +
> > + ican3_init_new_host_interface(mod);
> > + ret = ican3_msg_newhostif(mod);
> > + if (ret) {
> > + dev_err(mod->dev, "unable to switch to new-style interface\n");
> > + return ret;
> > + }
> > +
> > + ret = ican3_set_termination(mod, true);
> > + if (ret) {
> > + dev_err(mod->dev, "unable to enable termination\n");
> > + return ret;
> > + }
>
>
> Could you please allow the user to disable termination, e.g. via module parameter
> "bus_termination=0" in case he uses a cable with built-in termination.
>
What would you think about a sysfs node instead, so it could be changed
at runtime, on a per-daughtercard basis? Do you think enabling bus
termination is a good default? IMO, it is a pretty safe default for most
users.
> [snip]
> > +static int __devinit ican3_probe(struct platform_device *pdev)
> > +{
> > + struct janz_platform_data *pdata;
> > + struct net_device *ndev;
> > + struct ican3_dev *mod;
> > + struct resource *res;
> > + struct device *dev;
> > + int ret;
> > +
> > + pdata = pdev->dev.platform_data;
> > + if (!pdata)
> > + return -ENXIO;
> > +
> > + dev_dbg(&pdev->dev, "probe: module number %d\n", pdata->modno);
> > +
> > + /* save the struct device for printing */
> > + dev = &pdev->dev;
> > +
> > + /* allocate the CAN device and private data */
> > + ndev = alloc_candev(sizeof(*mod), 0);
> > + if (!ndev) {
> > + dev_err(dev, "unable to allocate CANdev\n");
> > + ret = -ENOMEM;
> > + goto out_return;
> > + }
> > +
> > + platform_set_drvdata(pdev, ndev);
> > + mod = netdev_priv(ndev);
> > + mod->ndev = ndev;
> > + mod->dev = &pdev->dev;
> > + mod->num = pdata->modno;
> > + netif_napi_add(ndev, &mod->napi, ican3_napi, ICAN3_RX_BUFFERS);
> > + spin_lock_init(&mod->lock);
> > +
> > + /* the first unallocated page in the DPM is 9 */
> > + mod->free_page = DPM_FREE_START;
> > +
> > + ndev->netdev_ops = &ican3_netdev_ops;
> > + ndev->flags |= IFF_ECHO;
> > + SET_NETDEV_DEV(ndev, &pdev->dev);
> > +
> > + mod->can.clock.freq = 8000000;
>
> Please use a constant here.
> [snip]
>
> Please fix and resubmit with my:
>
> "Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>"
>
> for the SocketCAN part.
>
Thanks for the review!
Ira
^ permalink raw reply
* Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board
From: Wolfgang Grandegger @ 2010-03-19 15:35 UTC (permalink / raw)
To: Ira W. Snyder
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, sameo-VuQAYsv1563Yd54FQh9/CA
In-Reply-To: <20100319151326.GA13672-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
Ira W. Snyder wrote:
> On Fri, Mar 19, 2010 at 10:13:10AM +0100, Wolfgang Grandegger wrote:
>> Ira W. Snyder wrote:
>>> The Janz CMOD-IO PCI MODULbus carrier board is a PCI to MODULbus bridge,
>>> which may host many different types of MODULbus daughterboards, including
>>> CAN and GPIO controllers.
>>>
>>> Signed-off-by: Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
>>> Cc: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> You can add my "Reviewed-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>".
>>
>> Just one concern:
>>
>>> ---
>>> drivers/mfd/Kconfig | 8 +
>>> drivers/mfd/Makefile | 1 +
>>> drivers/mfd/janz-cmodio.c | 339 +++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/mfd/janz.h | 54 +++++++
>>> 4 files changed, 402 insertions(+), 0 deletions(-)
>>> create mode 100644 drivers/mfd/janz-cmodio.c
>>> create mode 100644 include/linux/mfd/janz.h
>>>
>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>> index 8782978..f1858d7 100644
>>> --- a/drivers/mfd/Kconfig
>>> +++ b/drivers/mfd/Kconfig
>>> @@ -27,6 +27,14 @@ config MFD_SM501_GPIO
>>> lines on the SM501. The platform data is used to supply the
>>> base number for the first GPIO line to register.
>>>
>>> +config MFD_JANZ_CMODIO
>>> + tristate "Support for Janz CMOD-IO PCI MODULbus Carrier Board"
>>> + ---help---
>>> + This is the core driver for the Janz CMOD-IO PCI MODULbus
>>> + carrier board. This device is a PCI to MODULbus bridge which may
>>> + host many different types of MODULbus daughterboards, including
>>> + CAN and GPIO controllers.
>>> +
>>> config MFD_ASIC3
>>> bool "Support for Compaq ASIC3"
>>> depends on GENERIC_HARDIRQS && GPIOLIB && ARM
>>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>>> index e09eb48..e8fa905 100644
>>> --- a/drivers/mfd/Makefile
>>> +++ b/drivers/mfd/Makefile
>>> @@ -3,6 +3,7 @@
>>> #
>>>
>>> obj-$(CONFIG_MFD_SM501) += sm501.o
>>> +obj-$(CONFIG_MFD_JANZ_CMODIO) += janz-cmodio.o
>>> obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
>>> obj-$(CONFIG_MFD_SH_MOBILE_SDHI) += sh_mobile_sdhi.o
>>>
>>> diff --git a/drivers/mfd/janz-cmodio.c b/drivers/mfd/janz-cmodio.c
>>> new file mode 100644
>>> index 0000000..914280e
>>> --- /dev/null
>>> +++ b/drivers/mfd/janz-cmodio.c
>>> @@ -0,0 +1,339 @@
>>> +/*
>>> + * Janz CMOD-IO MODULbus Carrier Board PCI Driver
>>> + *
>>> + * Copyright (c) 2010 Ira W. Snyder <iws-lulEs6mt1IksTUYHLfqkUA@public.gmane.org>
>>> + *
>>> + * Lots of inspiration and code was copied from drivers/mfd/sm501.c
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License as published by the
>>> + * Free Software Foundation; either version 2 of the License, or (at your
>>> + * option) any later version.
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/init.h>
>>> +#include <linux/pci.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#include <linux/mfd/janz.h>
>>> +
>>> +#define DRV_NAME "janz-cmodio"
>>> +
>>> +/* Size of each MODULbus module in PCI BAR4 */
>>> +#define CMODIO_MODULBUS_SIZE 0x200
>>> +
>>> +/* Maximum number of MODULbus modules on a CMOD-IO carrier board */
>>> +#define CMODIO_MAX_MODULES 4
>>> +
>>> +/* Module Parameters */
>>> +static unsigned int num_modules = CMODIO_MAX_MODULES;
>>> +static unsigned char *modules[CMODIO_MAX_MODULES] = {
>>> + "janz-ican3",
>>> + "janz-ican3",
>>> + "",
>>> + "janz-ttl",
>>> +};
>> This is probably not a good default but just your private configuration.
>>
>
> Yep, that's the configuration I have. Do you have any suggestions for a
> better default? I could make them all blank strings, which means "no
> daughtercard in this slot". That is my only idea for a different
> default.
That's probably better. And if no boards are defined via module
parameter, return with an error and print an error message telling the
user what to do. Also maybe s/""/"empty"/ and a better the
MODULE_DESCRIPTION could be useful.
Wolfgang.
^ permalink raw reply
* [PATCH net-next-2.6] tcp: Add SNMP counter for DEFER_ACCEPT
From: Eric Dumazet @ 2010-03-19 15:37 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Its currently hard to diagnose when ACK frames are dropped because an
application set TCP_DEFER_ACCEPT on its listening socket.
See http://bugzilla.kernel.org/show_bug.cgi?id=15507
This patch adds a SNMP value, named TCPDeferAcceptDrop
netstat -s | grep TCPDeferAcceptDrop
TCPDeferAcceptDrop: 0
This counter is incremented every time we drop a pure ACK frame received
by a socket in SYN_RECV state because its SYNACK retrans count is lower
than defer_accept value.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/tcp_minisocks.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/include/linux/snmp.h b/include/linux/snmp.h
index 4435d10..d2a9aa3 100644
--- a/include/linux/snmp.h
+++ b/include/linux/snmp.h
@@ -227,6 +227,7 @@ enum
LINUX_MIB_SACKSHIFTFALLBACK,
LINUX_MIB_TCPBACKLOGDROP,
LINUX_MIB_TCPMINTTLDROP, /* RFC 5082 */
+ LINUX_MIB_TCPDEFERACCEPTDROP,
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 4f1f337..3dc9914 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -251,6 +251,7 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPSackShiftFallback", LINUX_MIB_SACKSHIFTFALLBACK),
SNMP_MIB_ITEM("TCPBacklogDrop", LINUX_MIB_TCPBACKLOGDROP),
SNMP_MIB_ITEM("TCPMinTTLDrop", LINUX_MIB_TCPMINTTLDROP),
+ SNMP_MIB_ITEM("TCPDeferAcceptDrop", LINUX_MIB_TCPDEFERACCEPTDROP),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 4199bc6..32f9627 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -671,6 +671,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
if (req->retrans < inet_csk(sk)->icsk_accept_queue.rskq_defer_accept &&
TCP_SKB_CB(skb)->end_seq == tcp_rsk(req)->rcv_isn + 1) {
inet_rsk(req)->acked = 1;
+ NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPDEFERACCEPTDROP);
return NULL;
}
^ permalink raw reply related
* Re: [PATCH 2/3] can: add support for Janz VMOD-ICAN3 Intelligent CAN module
From: Wolfgang Grandegger @ 2010-03-19 15:45 UTC (permalink / raw)
To: Ira W. Snyder; +Cc: linux-kernel, socketcan-core, netdev, sameo
In-Reply-To: <20100319151914.GB13672@ovro.caltech.edu>
Ira W. Snyder wrote:
> On Fri, Mar 19, 2010 at 10:01:14AM +0100, Wolfgang Grandegger wrote:
>> Hi Ira,
>>
>> we already discussed this patch on the SocketCAN mailing list and there
>> are just a few minor issues and the request to add support for the new
>> "berr-reporting" option, if feasible. See:
>>
>> commit 52c793f24054f5dc30d228e37e0e19cc8313f086
>> Author: Wolfgang Grandegger <wg@grandegger.com>
>> Date: Mon Feb 22 22:21:17 2010 +0000
>>
>> can: netlink support for bus-error reporting and counters
>>
>> This patch makes the bus-error reporting configurable and allows to
>> retrieve the CAN TX and RX bus error counters via netlink interface.
>> I have added support for the SJA1000. The TX and RX bus error counters
>> are also copied to the data fields 6..7 of error messages when state
>> changes are reported.
>>
>> Should not be a big deal.
>>
>
> I think this patch came along since my last post of the driver. I must
> have missed it. I'll try and add support.
No problem, it's really new. Just just need to enable BEI depending on
CAN_CTRLMODE_BERR_REPORTING.
>> More inline...
>>
>> Ira W. Snyder wrote:
>>> The Janz VMOD-ICAN3 is a MODULbus daughterboard which fits onto any
>>> MODULbus carrier board. It is an intelligent CAN controller with a
>>> microcontroller and associated firmware.
>>>
>>> Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
>>> Cc: socketcan-core@lists.berlios.de
>>> Cc: netdev@vger.kernel.org
>>> ---
>>> drivers/net/can/Kconfig | 10 +
>>> drivers/net/can/Makefile | 1 +
>>> drivers/net/can/janz-ican3.c | 1659 ++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 1670 insertions(+), 0 deletions(-)
>>> create mode 100644 drivers/net/can/janz-ican3.c
>>>
>>> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
>>> index 05b7517..2c5227c 100644
>>> --- a/drivers/net/can/Kconfig
>>> +++ b/drivers/net/can/Kconfig
>>> @@ -63,6 +63,16 @@ config CAN_BFIN
>>> To compile this driver as a module, choose M here: the
>>> module will be called bfin_can.
>>>
>>> +config CAN_JANZ_ICAN3
>>> + tristate "Janz VMOD-ICAN3 Intelligent CAN controller"
>>> + depends on CAN_DEV && MFD_JANZ_CMODIO
>>> + ---help---
>>> + Driver for Janz VMOD-ICAN3 Intelligent CAN controller module, which
>>> + connects to a MODULbus carrier board.
>>> +
>>> + This driver can also be built as a module. If so, the module will be
>>> + called janz-ican3.ko.
>>> +
>>> source "drivers/net/can/mscan/Kconfig"
>>>
>>> source "drivers/net/can/sja1000/Kconfig"
>>> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
>>> index 7a702f2..9047cd0 100644
>>> --- a/drivers/net/can/Makefile
>>> +++ b/drivers/net/can/Makefile
>>> @@ -15,5 +15,6 @@ obj-$(CONFIG_CAN_AT91) += at91_can.o
>>> obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
>>> obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
>>> obj-$(CONFIG_CAN_BFIN) += bfin_can.o
>>> +obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-ican3.o
>>>
>>> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
>>> diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
>>> new file mode 100644
>>> index 0000000..94d4995
>>> --- /dev/null
>>> +++ b/drivers/net/can/janz-ican3.c
>> [snip]
>> +struct ican3_dev {
>>> +
>>> + /* must be the first member */
>>> + struct can_priv can;
>>> +
>>> + /* CAN network device */
>>> + struct net_device *ndev;
>>> + struct napi_struct napi;
>>> +
>>> + /* Device for printing */
>>> + struct device *dev;
>>> +
>>> + /* module number */
>>> + unsigned int num;
>>> +
>>> + /* base address of registers and IRQ */
>>> + struct janz_cmodio_onboard_regs __iomem *ctrl;
>>> + struct ican3_dpm_control *dpmctrl;
>>> + void __iomem *dpm;
>>> + int irq;
>>> +
>>> + /* old and new style host interface */
>>> + unsigned int iftype;
>>> + spinlock_t lock;
>> Please describe what the lock is used for.
>>
>>> + /* new host interface */
>>> + unsigned int rx_int;
>>> + unsigned int rx_num;
>>> + unsigned int tx_num;
>>> +
>>> + /* fast host interface */
>>> + unsigned int fastrx_start;
>>> + unsigned int fastrx_int;
>>> + unsigned int fastrx_num;
>>> + unsigned int fasttx_start;
>>> + unsigned int fasttx_num;
>>> +
>>> + /* first free DPM page */
>>> + unsigned int free_page;
>>> +};
>> [snip]
>>> +static void ican3_to_can_frame(struct ican3_dev *mod,
>>> + struct ican3_fast_desc *desc,
>>> + struct can_frame *cf)
>>> +{
>>> + if ((desc->command & ICAN3_CAN_TYPE_MASK) == ICAN3_CAN_TYPE_SFF) {
>>> + dev_dbg(mod->dev, "%s: old frame format\n", __func__);
>> This prints a debug message for every message which is not really
>> useful for the user. Please remove.
>>
>>> + if (desc->data[1] & ICAN3_SFF_RTR)
>>> + cf->can_id |= CAN_RTR_FLAG;
>>> +
>>> + cf->can_id |= desc->data[0] << 3;
>>> + cf->can_id |= (desc->data[1] & 0xe0) >> 5;
>>> + cf->can_dlc = desc->data[1] & ICAN3_CAN_DLC_MASK;
>>> + memcpy(cf->data, &desc->data[2], sizeof(cf->data));
>>> + } else {
>>> + dev_dbg(mod->dev, "%s: new frame format\n", __func__);
>> Ditto.
>>
>>> + cf->can_dlc = desc->data[0] & ICAN3_CAN_DLC_MASK;
>>> + if (desc->data[0] & ICAN3_EFF_RTR)
>>> + cf->can_id |= CAN_RTR_FLAG;
>>> +
>>> + if (desc->data[0] & ICAN3_EFF) {
>>> + cf->can_id |= CAN_EFF_FLAG;
>>> + cf->can_id |= desc->data[2] << 21; /* 28-21 */
>>> + cf->can_id |= desc->data[3] << 13; /* 20-13 */
>>> + cf->can_id |= desc->data[4] << 5; /* 12-5 */
>>> + cf->can_id |= (desc->data[5] & 0xf8) >> 3;
>>> + } else {
>>> + cf->can_id |= desc->data[2] << 3; /* 10-3 */
>>> + cf->can_id |= desc->data[3] >> 5; /* 2-0 */
>>> + }
>>> +
>>> + memcpy(cf->data, &desc->data[6], sizeof(cf->data));
>>> + }
>>> +}
>>> +
>>> +static void can_frame_to_ican3(struct ican3_dev *mod,
>>> + struct can_frame *cf,
>>> + struct ican3_fast_desc *desc)
>>> +{
>>> + /* clear out any stale data in the descriptor */
>>> + memset(desc->data, 0, sizeof(desc->data));
>>> +
>>> + /* we always use the extended format, with the ECHO flag set */
>>> + desc->command = ICAN3_CAN_TYPE_EFF;
>>> + desc->data[0] |= cf->can_dlc;
>>> + desc->data[1] |= ICAN3_ECHO;
>>> +
>>> + if (cf->can_id & CAN_RTR_FLAG)
>>> + desc->data[0] |= ICAN3_EFF_RTR;
>>> +
>>> + /* pack the id into the correct places */
>>> + if (cf->can_id & CAN_EFF_FLAG) {
>>> + dev_dbg(mod->dev, "%s: extended frame\n", __func__);
>> Ditto.
>>
>>> + desc->data[0] |= ICAN3_EFF;
>>> + desc->data[2] = (cf->can_id & 0x1fe00000) >> 21; /* 28-21 */
>>> + desc->data[3] = (cf->can_id & 0x001fe000) >> 13; /* 20-13 */
>>> + desc->data[4] = (cf->can_id & 0x00001fe0) >> 5; /* 12-5 */
>>> + desc->data[5] = (cf->can_id & 0x0000001f) << 3; /* 4-0 */
>>> + } else {
>>> + dev_dbg(mod->dev, "%s: standard frame\n", __func__);
>> Ditto.
>>
>>> + desc->data[2] = (cf->can_id & 0x7F8) >> 3; /* bits 10-3 */
>>> + desc->data[3] = (cf->can_id & 0x007) << 5; /* bits 2-0 */
>>> + }
>>> +
>>> + /* copy the data bits into the descriptor */
>>> + memcpy(&desc->data[6], cf->data, sizeof(cf->data));
>>> +}
>> [snip]
>>> +/*
>>> + * Handle CAN Event Indication Messages from the firmware
>>> + *
>>> + * The ICAN3 firmware provides the values of some SJA1000 registers when it
>>> + * generates this message. The code below is largely copied from the
>>> + * drivers/net/can/sja1000/sja1000.c file, and adapted as necessary
>>> + */
>>> +static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg)
>>> +{
>>> + struct net_device *dev = mod->ndev;
>>> + struct net_device_stats *stats = &dev->stats;
>>> + enum can_state state = mod->can.state;
>>> + struct can_frame *cf;
>>> + struct sk_buff *skb;
>>> + u8 status, isrc;
>>> +
>>> + /* we can only handle the SJA1000 part */
>>> + if (msg->data[1] != CEVTIND_CHIP_SJA1000) {
>>> + dev_err(mod->dev, "unable to handle errors on non-SJA1000\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + /* check the message length for sanity */
>>> + if (msg->len < 6) {
>>> + dev_err(mod->dev, "error message too short\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + skb = alloc_can_err_skb(dev, &cf);
>>> + if (skb == NULL)
>>> + return -ENOMEM;
>>> +
>>> + isrc = msg->data[0];
>>> + status = msg->data[3];
>>> +
>>> + /* data overrun interrupt */
>>> + if (isrc == CEVTIND_DOI || isrc == CEVTIND_LOST) {
>> Here and for the other errors below a dev_dbg() would be useful. Please
>> check sja1000.c.
>>
>>> + cf->can_id |= CAN_ERR_CRTL;
>>> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
>>> + stats->rx_over_errors++;
>>> + stats->rx_errors++;
>>> + dev_info(mod->dev, "%s: overflow frame generated\n", __func__);
>> s/dev_info/dev_dbg/ ?
>>
>
> Whoops, a leftover from debugging. Will change to dev_dbg().
>
>>> + }
>>> +
>>> + /* error warning interrupt */
>>> + if (isrc == CEVTIND_EI) {
>>> + if (status & SR_BS) {
>>> + state = CAN_STATE_BUS_OFF;
>>> + cf->can_id |= CAN_ERR_BUSOFF;
>>> + can_bus_off(dev);
>>> + } else if (status & SR_ES) {
>>> + state = CAN_STATE_ERROR_WARNING;
>>> + } else {
>>> + state = CAN_STATE_ERROR_ACTIVE;
>>> + }
>>> + }
>>> +
>>> + /* bus error interrupt */
>>> + if (isrc == CEVTIND_BEI) {
>>> + u8 ecc = msg->data[2];
>> Add an empty line, please.
>>
>>> + mod->can.can_stats.bus_error++;
>>> + stats->rx_errors++;
>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> +
>>> + switch (ecc & ECC_MASK) {
>>> + case ECC_BIT:
>>> + cf->data[2] |= CAN_ERR_PROT_BIT;
>>> + break;
>>> + case ECC_FORM:
>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>>> + break;
>>> + case ECC_STUFF:
>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>> + break;
>>> + default:
>>> + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
>>> + cf->data[3] = ecc & ECC_SEG;
>>> + break;
>>> + }
>>> +
>>> + if ((ecc & ECC_DIR) == 0)
>>> + cf->data[2] |= CAN_ERR_PROT_TX;
>>> + }
>>> +
>>> + if (state != mod->can.state && (state == CAN_STATE_ERROR_WARNING ||
>>> + state == CAN_STATE_ERROR_PASSIVE)) {
>>> + u8 rxerr = msg->data[4];
>>> + u8 txerr = msg->data[5];
>> Ditto.
>>
>>> + cf->can_id |= CAN_ERR_CRTL;
>>> + if (state == CAN_STATE_ERROR_WARNING) {
>>> + mod->can.can_stats.error_warning++;
>>> + cf->data[1] = (txerr > rxerr) ?
>>> + CAN_ERR_CRTL_TX_WARNING :
>>> + CAN_ERR_CRTL_RX_WARNING;
>>> + } else {
>>> + mod->can.can_stats.error_passive++;
>>> + cf->data[1] = (txerr > rxerr) ?
>>> + CAN_ERR_CRTL_TX_PASSIVE :
>>> + CAN_ERR_CRTL_RX_PASSIVE;
>>> + }
>>> + }
>>> +
>>> + mod->can.state = state;
>>> + stats->rx_errors++;
>>> + stats->rx_bytes += cf->can_dlc;
>>> + netif_rx(skb);
>>> + return 0;
>>> +}
>> [snip]
>>> +static irqreturn_t ican3_irq(int irq, void *dev_id)
>>> +{
>>> + struct ican3_dev *mod = dev_id;
>>> + u8 stat;
>>> +
>>> + /*
>>> + * The interrupt status register on this device reports interrupts
>>> + * as zeroes instead of using ones like most other devices
>>> + */
>>> + stat = ioread8(&mod->ctrl->int_disable) & (1 << mod->num);
>>> + if (stat == (1 << mod->num))
>>> + return IRQ_NONE;
>>> +
>>> + dev_dbg(mod->dev, "IRQ: module %d\n", mod->num);
>> Please remove this dev_dbg() as well.
>>
>> [snip]
>>> +/*
>>> + * Startup an ICAN module, bringing it into fast mode
>>> + */
>>> +static int __devinit ican3_startup_module(struct ican3_dev *mod)
>>> +{
>>> + int ret;
>>> +
>>> + ret = ican3_reset_module(mod);
>>> + if (ret) {
>>> + dev_err(mod->dev, "unable to reset module\n");
>>> + return ret;
>>> + }
>>> +
>>> + /* re-enable interrupts so we can send messages */
>>> + iowrite8(1 << mod->num, &mod->ctrl->int_enable);
>>> +
>>> + ret = ican3_msg_connect(mod);
>>> + if (ret) {
>>> + dev_err(mod->dev, "unable to connect to module\n");
>>> + return ret;
>>> + }
>>> +
>>> + ican3_init_new_host_interface(mod);
>>> + ret = ican3_msg_newhostif(mod);
>>> + if (ret) {
>>> + dev_err(mod->dev, "unable to switch to new-style interface\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = ican3_set_termination(mod, true);
>>> + if (ret) {
>>> + dev_err(mod->dev, "unable to enable termination\n");
>>> + return ret;
>>> + }
>>
>> Could you please allow the user to disable termination, e.g. via module parameter
>> "bus_termination=0" in case he uses a cable with built-in termination.
>>
>
> What would you think about a sysfs node instead, so it could be changed
> at runtime, on a per-daughtercard basis? Do you think enabling bus
> termination is a good default? IMO, it is a pretty safe default for most
> users.
I have no problem with the default. There should just be a way to
disable termination somehow. A dev_info(dev, "CAN bus termination
enabled") would furthermore make clear, that it's active. Setting
termination via SysFS is the better solution, of course, as it's per
device. It might even be useful to handle this in future in a common way
via CAN_CTRLMODE_BUS_TERMINATION.
Wolfgang.
^ permalink raw reply
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