* Re: [PATCH net-next-2.6] net: Introduce for_each_netdev_rcu() iterator
From: Eric Dumazet @ 2009-11-02 15:21 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
In-Reply-To: <4AEEF690.4000208@gmail.com>
Eric Dumazet a écrit :
> Adds RCU management to the list of netdevices.
>
Oops, there is an error in rose_dev_get(), I forgot one read_unlock()
> diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
> index 9478d9b..bd7a544 100644
> --- a/net/rose/rose_route.c
> +++ b/net/rose/rose_route.c
> @@ -618,8 +618,8 @@ struct net_device *rose_dev_get(rose_address *addr)
> {
> struct net_device *dev;
>
> - read_lock(&dev_base_lock);
> - for_each_netdev(&init_net, dev) {
> + rcu_read_lock();
> + for_each_netdev_rcu(&init_net, dev) {
> if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE && rosecmp(addr, (rose_address *)dev->dev_addr) == 0) {
> dev_hold(dev);
> goto out;
Here is an updated patch :
[PATCH net-next-2.6 take2] net: Introduce for_each_netdev_rcu() iterator
Adds RCU management to the list of netdevices.
Convert some for_each_netdev() users to RCU version, if
it can avoid read_lock-ing dev_base_lock
Ie:
read_lock(&dev_base_loack);
for_each_netdev(net, dev)
some_action();
read_unlock(&dev_base_lock);
becomes :
rcu_read_lock();
for_each_netdev_rcu(net, dev)
some_action();
rcu_read_unlock();
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/netdevice.h | 2 ++
net/core/dev.c | 30 ++++++++++++++++--------------
net/decnet/af_decnet.c | 6 +++---
net/decnet/dn_fib.c | 6 +++---
net/decnet/dn_route.c | 6 +++---
net/ipv4/devinet.c | 30 +++++++++++-------------------
net/ipv6/addrconf.c | 20 +++++++-------------
net/ipv6/anycast.c | 6 +++---
net/netrom/nr_route.c | 15 ++++++++-------
net/rose/rose_route.c | 18 +++++++++---------
net/sctp/protocol.c | 6 +++---
11 files changed, 68 insertions(+), 77 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index bcf1083..5077de0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1081,6 +1081,8 @@ extern rwlock_t dev_base_lock; /* Device list lock */
#define for_each_netdev(net, d) \
list_for_each_entry(d, &(net)->dev_base_head, dev_list)
+#define for_each_netdev_rcu(net, d) \
+ list_for_each_entry_rcu(d, &(net)->dev_base_head, dev_list)
#define for_each_netdev_safe(net, d, n) \
list_for_each_entry_safe(d, n, &(net)->dev_base_head, dev_list)
#define for_each_netdev_continue(net, d) \
diff --git a/net/core/dev.c b/net/core/dev.c
index 76a1502..bf629ac 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -175,7 +175,7 @@ static struct list_head ptype_all __read_mostly; /* Taps */
* The @dev_base_head list is protected by @dev_base_lock and the rtnl
* semaphore.
*
- * Pure readers hold dev_base_lock for reading.
+ * Pure readers hold dev_base_lock for reading, or rcu_read_lock()
*
* Writers must hold the rtnl semaphore while they loop through the
* dev_base_head list, and hold dev_base_lock for writing when they do the
@@ -212,7 +212,7 @@ static int list_netdevice(struct net_device *dev)
ASSERT_RTNL();
write_lock_bh(&dev_base_lock);
- list_add_tail(&dev->dev_list, &net->dev_base_head);
+ list_add_tail_rcu(&dev->dev_list, &net->dev_base_head);
hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
hlist_add_head_rcu(&dev->index_hlist,
dev_index_hash(net, dev->ifindex));
@@ -229,7 +229,7 @@ static void unlist_netdevice(struct net_device *dev)
/* Unlink dev from the device chain */
write_lock_bh(&dev_base_lock);
- list_del(&dev->dev_list);
+ list_del_rcu(&dev->dev_list);
hlist_del_rcu(&dev->name_hlist);
hlist_del_rcu(&dev->index_hlist);
write_unlock_bh(&dev_base_lock);
@@ -799,15 +799,15 @@ struct net_device *dev_get_by_flags(struct net *net, unsigned short if_flags,
struct net_device *dev, *ret;
ret = NULL;
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
if (((dev->flags ^ if_flags) & mask) == 0) {
dev_hold(dev);
ret = dev;
break;
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return ret;
}
EXPORT_SYMBOL(dev_get_by_flags);
@@ -3077,18 +3077,18 @@ static int dev_ifconf(struct net *net, char __user *arg)
* in detail.
*/
void *dev_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(RCU)
{
struct net *net = seq_file_net(seq);
loff_t off;
struct net_device *dev;
- read_lock(&dev_base_lock);
+ rcu_read_lock();
if (!*pos)
return SEQ_START_TOKEN;
off = 1;
- for_each_netdev(net, dev)
+ for_each_netdev_rcu(net, dev)
if (off++ == *pos)
return dev;
@@ -3097,16 +3097,18 @@ void *dev_seq_start(struct seq_file *seq, loff_t *pos)
void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
- struct net *net = seq_file_net(seq);
+ struct net_device *dev = (v == SEQ_START_TOKEN) ?
+ first_net_device(seq_file_net(seq)) :
+ next_net_device((struct net_device *)v);
+
++*pos;
- return v == SEQ_START_TOKEN ?
- first_net_device(net) : next_net_device((struct net_device *)v);
+ return rcu_dereference(dev);
}
void dev_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(RCU)
{
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static void dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev)
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 664965c..2e35584 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -749,9 +749,9 @@ static int dn_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
if (!(saddr->sdn_flags & SDF_WILD)) {
if (le16_to_cpu(saddr->sdn_nodeaddrl)) {
- read_lock(&dev_base_lock);
+ rcu_read_lock();
ldev = NULL;
- for_each_netdev(&init_net, dev) {
+ for_each_netdev_rcu(&init_net, dev) {
if (!dev->dn_ptr)
continue;
if (dn_dev_islocal(dev, dn_saddr2dn(saddr))) {
@@ -759,7 +759,7 @@ static int dn_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
break;
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
if (ldev == NULL)
return -EADDRNOTAVAIL;
}
diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c
index 27ea2e9..fd641f6 100644
--- a/net/decnet/dn_fib.c
+++ b/net/decnet/dn_fib.c
@@ -607,8 +607,8 @@ static void dn_fib_del_ifaddr(struct dn_ifaddr *ifa)
ASSERT_RTNL();
/* Scan device list */
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
dn_db = dev->dn_ptr;
if (dn_db == NULL)
continue;
@@ -619,7 +619,7 @@ static void dn_fib_del_ifaddr(struct dn_ifaddr *ifa)
}
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
if (found_it == 0) {
fib_magic(RTM_DELROUTE, RTN_LOCAL, ifa->ifa_local, 16, ifa);
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 57662ca..860286a 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -908,8 +908,8 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowi *old
dev_put(dev_out);
goto out;
}
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if (!dev->dn_ptr)
continue;
if (!dn_dev_islocal(dev, oldflp->fld_src))
@@ -922,7 +922,7 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowi *old
dev_out = dev;
break;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
if (dev_out == NULL)
goto out;
dev_hold(dev_out);
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index ccccaae..8aa7a13 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -876,19 +876,16 @@ __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope)
if (!addr)
addr = ifa->ifa_local;
} endfor_ifa(in_dev);
-no_in_dev:
- rcu_read_unlock();
+no_in_dev:
if (addr)
- goto out;
+ goto out_unlock;
/* Not loopback addresses on loopback should be preferred
in this case. It is importnat that lo is the first interface
in dev_base list.
*/
- read_lock(&dev_base_lock);
- rcu_read_lock();
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
if ((in_dev = __in_dev_get_rcu(dev)) == NULL)
continue;
@@ -896,12 +893,11 @@ no_in_dev:
if (ifa->ifa_scope != RT_SCOPE_LINK &&
ifa->ifa_scope <= scope) {
addr = ifa->ifa_local;
- goto out_unlock_both;
+ goto out_unlock;
}
} endfor_ifa(in_dev);
}
-out_unlock_both:
- read_unlock(&dev_base_lock);
+out_unlock:
rcu_read_unlock();
out:
return addr;
@@ -962,9 +958,8 @@ __be32 inet_confirm_addr(struct in_device *in_dev,
return confirm_addr_indev(in_dev, dst, local, scope);
net = dev_net(in_dev->dev);
- read_lock(&dev_base_lock);
rcu_read_lock();
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
if ((in_dev = __in_dev_get_rcu(dev))) {
addr = confirm_addr_indev(in_dev, dst, local, scope);
if (addr)
@@ -972,7 +967,6 @@ __be32 inet_confirm_addr(struct in_device *in_dev,
}
}
rcu_read_unlock();
- read_unlock(&dev_base_lock);
return addr;
}
@@ -1240,18 +1234,18 @@ static void devinet_copy_dflt_conf(struct net *net, int i)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
struct in_device *in_dev;
- rcu_read_lock();
+
in_dev = __in_dev_get_rcu(dev);
if (in_dev && !test_bit(i, in_dev->cnf.state))
in_dev->cnf.data[i] = net->ipv4.devconf_dflt->data[i];
- rcu_read_unlock();
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
+/* called with RTNL locked */
static void inet_forward_change(struct net *net)
{
struct net_device *dev;
@@ -1260,7 +1254,6 @@ static void inet_forward_change(struct net *net)
IPV4_DEVCONF_ALL(net, ACCEPT_REDIRECTS) = !on;
IPV4_DEVCONF_DFLT(net, FORWARDING) = on;
- read_lock(&dev_base_lock);
for_each_netdev(net, dev) {
struct in_device *in_dev;
if (on)
@@ -1271,7 +1264,6 @@ static void inet_forward_change(struct net *net)
IN_DEV_CONF_SET(in_dev, FORWARDING, on);
rcu_read_unlock();
}
- read_unlock(&dev_base_lock);
}
static int devinet_conf_proc(ctl_table *ctl, int write,
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 9186484..024bba3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -481,9 +481,8 @@ static void addrconf_forward_change(struct net *net, __s32 newf)
struct net_device *dev;
struct inet6_dev *idev;
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev) {
- rcu_read_lock();
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
idev = __in6_dev_get(dev);
if (idev) {
int changed = (!idev->cnf.forwarding) ^ (!newf);
@@ -491,9 +490,8 @@ static void addrconf_forward_change(struct net *net, __s32 newf)
if (changed)
dev_forward_change(idev);
}
- rcu_read_unlock();
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int addrconf_fixup_forwarding(struct ctl_table *table, int *p, int old)
@@ -1137,10 +1135,9 @@ int ipv6_dev_get_saddr(struct net *net, struct net_device *dst_dev,
hiscore->rule = -1;
hiscore->ifa = NULL;
- read_lock(&dev_base_lock);
rcu_read_lock();
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
struct inet6_dev *idev;
/* Candidate Source Address (section 4)
@@ -1235,7 +1232,6 @@ try_nextdev:
read_unlock_bh(&idev->lock);
}
rcu_read_unlock();
- read_unlock(&dev_base_lock);
if (!hiscore->ifa)
return -EADDRNOTAVAIL;
@@ -4052,9 +4048,8 @@ static void addrconf_disable_change(struct net *net, __s32 newf)
struct net_device *dev;
struct inet6_dev *idev;
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev) {
- rcu_read_lock();
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
idev = __in6_dev_get(dev);
if (idev) {
int changed = (!idev->cnf.disable_ipv6) ^ (!newf);
@@ -4062,9 +4057,8 @@ static void addrconf_disable_change(struct net *net, __s32 newf)
if (changed)
dev_disable_change(idev);
}
- rcu_read_unlock();
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int addrconf_disable_ipv6(struct ctl_table *table, int *p, int old)
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 1ae58be..2f00ca8 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -404,13 +404,13 @@ int ipv6_chk_acast_addr(struct net *net, struct net_device *dev,
if (dev)
return ipv6_chk_acast_dev(dev, addr);
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev)
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev)
if (ipv6_chk_acast_dev(dev, addr)) {
found = 1;
break;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return found;
}
diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
index 4eb1ac9..aacba76 100644
--- a/net/netrom/nr_route.c
+++ b/net/netrom/nr_route.c
@@ -597,15 +597,15 @@ struct net_device *nr_dev_first(void)
{
struct net_device *dev, *first = NULL;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if ((dev->flags & IFF_UP) && dev->type == ARPHRD_NETROM)
if (first == NULL || strncmp(dev->name, first->name, 3) < 0)
first = dev;
}
if (first)
dev_hold(first);
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return first;
}
@@ -617,16 +617,17 @@ struct net_device *nr_dev_get(ax25_address *addr)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
- if ((dev->flags & IFF_UP) && dev->type == ARPHRD_NETROM && ax25cmp(addr, (ax25_address *)dev->dev_addr) == 0) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
+ if ((dev->flags & IFF_UP) && dev->type == ARPHRD_NETROM &&
+ ax25cmp(addr, (ax25_address *)dev->dev_addr) == 0) {
dev_hold(dev);
goto out;
}
}
dev = NULL;
out:
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return dev;
}
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 9478d9b..d936226 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -600,13 +600,13 @@ struct net_device *rose_dev_first(void)
{
struct net_device *dev, *first = NULL;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE)
if (first == NULL || strncmp(dev->name, first->name, 3) < 0)
first = dev;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return first;
}
@@ -618,8 +618,8 @@ struct net_device *rose_dev_get(rose_address *addr)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE && rosecmp(addr, (rose_address *)dev->dev_addr) == 0) {
dev_hold(dev);
goto out;
@@ -627,7 +627,7 @@ struct net_device *rose_dev_get(rose_address *addr)
}
dev = NULL;
out:
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return dev;
}
@@ -635,14 +635,14 @@ static int rose_dev_exists(rose_address *addr)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE && rosecmp(addr, (rose_address *)dev->dev_addr) == 0)
goto out;
}
dev = NULL;
out:
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return dev != NULL;
}
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index d9f4cc2..fe44c57 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -205,14 +205,14 @@ static void sctp_get_local_addr_list(void)
struct list_head *pos;
struct sctp_af *af;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
__list_for_each(pos, &sctp_address_families) {
af = list_entry(pos, struct sctp_af, list);
af->copy_addrlist(&sctp_local_addr_list, dev);
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
/* Free the existing local addresses. */
^ permalink raw reply related
* [PATCH v2 net-next-2.6] can: Driver for the Microchip MCP251x SPI CAN controllers
From: Christian Pellegrin @ 2009-11-02 15:16 UTC (permalink / raw)
To: wg-5Yr1BZd7O62+XT7JhA+gdA, socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
netdev-u79uwXL29TY76Z2rM5mHXA, pthomas8589-Re5JQEeQqe8AvxtiuMwx3w
Cc: Christian Pellegrin
In-Reply-To: <4AED5589.3090106-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
---
drivers/net/can/Kconfig | 6 +
drivers/net/can/Makefile | 1 +
drivers/net/can/mcp251x.c | 1164 ++++++++++++++++++++++++++++++++++
include/linux/can/platform/mcp251x.h | 36 +
4 files changed, 1207 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/can/mcp251x.c
create mode 100644 include/linux/can/platform/mcp251x.h
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 26d77cc..b819cc2 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -102,6 +102,12 @@ config CAN_TI_HECC
Driver for TI HECC (High End CAN Controller) module found on many
TI devices. The device specifications are available from www.ti.com
+config CAN_MCP251X
+ tristate "Microchip MCP251x SPI CAN controllers"
+ depends on CAN_DEV && SPI
+ ---help---
+ Driver for the Microchip MCP251x SPI CAN controllers.
+
config CAN_DEBUG_DEVICES
bool "CAN devices debugging messages"
depends on CAN
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 31f4ab5..1489181 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -12,5 +12,6 @@ obj-y += usb/
obj-$(CONFIG_CAN_SJA1000) += sja1000/
obj-$(CONFIG_CAN_AT91) += at91_can.o
obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
+obj-$(CONFIG_CAN_MCP251X) += mcp251x.o
ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
new file mode 100644
index 0000000..9471bb7
--- /dev/null
+++ b/drivers/net/can/mcp251x.c
@@ -0,0 +1,1164 @@
+/*
+ * CAN bus driver for Microchip 251x CAN Controller with SPI Interface
+ *
+ * MCP2510 support and bug fixes by Christian Pellegrin
+ * <chripell-LERDrqjqfvZg9hUCZPvPmw@public.gmane.org>
+ *
+ * Copyright 2009 Christian Pellegrin EVOL S.r.l.
+ *
+ * Copyright 2007 Raymarine UK, Ltd. All Rights Reserved.
+ * Written under contract by:
+ * Chris Elston, Katalix Systems, Ltd.
+ *
+ * Based on Microchip MCP251x CAN controller driver written by
+ * David Vrabel, Copyright 2006 Arcom Control Systems Ltd.
+ *
+ * Based on CAN bus driver for the CCAN controller written by
+ * - Sascha Hauer, Marc Kleine-Budde, Pengutronix
+ * - Simon Kallweit, intefo AG
+ * Copyright 2007
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ *
+ *
+ * Your platform definition file should specify something like:
+ *
+ * static struct mcp251x_platform_data mcp251x_info = {
+ * .oscillator_frequency = 8000000,
+ * .board_specific_setup = &mcp251x_setup,
+ * .model = CAN_MCP251X_MCP2510,
+ * .power_enable = mcp251x_power_enable,
+ * .transceiver_enable = NULL,
+ * };
+ *
+ * static struct spi_board_info spi_board_info[] = {
+ * {
+ * .modalias = "mcp251x",
+ * .platform_data = &mcp251x_info,
+ * .irq = IRQ_EINT13,
+ * .max_speed_hz = 2*1000*1000,
+ * .chip_select = 2,
+ * },
+ * };
+ *
+ * Please see mcp251x.h for a description of the fields in
+ * struct mcp251x_platform_data.
+ *
+ */
+
+#include <linux/can.h>
+#include <linux/can/core.h>
+#include <linux/can/dev.h>
+#include <linux/can/platform/mcp251x.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/freezer.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/uaccess.h>
+
+/* SPI interface instruction set */
+#define INSTRUCTION_WRITE 0x02
+#define INSTRUCTION_READ 0x03
+#define INSTRUCTION_BIT_MODIFY 0x05
+#define INSTRUCTION_LOAD_TXB(n) (0x40 + 2 * (n))
+#define INSTRUCTION_READ_RXB(n) (((n) == 0) ? 0x90 : 0x94)
+#define INSTRUCTION_RESET 0xC0
+
+/* MPC251x registers */
+#define CANSTAT 0x0e
+#define CANCTRL 0x0f
+# define CANCTRL_REQOP_MASK 0xe0
+# define CANCTRL_REQOP_CONF 0x80
+# define CANCTRL_REQOP_LISTEN_ONLY 0x60
+# define CANCTRL_REQOP_LOOPBACK 0x40
+# define CANCTRL_REQOP_SLEEP 0x20
+# define CANCTRL_REQOP_NORMAL 0x00
+# define CANCTRL_OSM 0x08
+# define CANCTRL_ABAT 0x10
+#define TEC 0x1c
+#define REC 0x1d
+#define CNF1 0x2a
+# define CNF1_SJW_SHIFT 6
+#define CNF2 0x29
+# define CNF2_BTLMODE 0x80
+# define CNF2_SAM 0x40
+# define CNF2_PS1_SHIFT 3
+#define CNF3 0x28
+# define CNF3_SOF 0x08
+# define CNF3_WAKFIL 0x04
+# define CNF3_PHSEG2_MASK 0x07
+#define CANINTE 0x2b
+# define CANINTE_MERRE 0x80
+# define CANINTE_WAKIE 0x40
+# define CANINTE_ERRIE 0x20
+# define CANINTE_TX2IE 0x10
+# define CANINTE_TX1IE 0x08
+# define CANINTE_TX0IE 0x04
+# define CANINTE_RX1IE 0x02
+# define CANINTE_RX0IE 0x01
+#define CANINTF 0x2c
+# define CANINTF_MERRF 0x80
+# define CANINTF_WAKIF 0x40
+# define CANINTF_ERRIF 0x20
+# define CANINTF_TX2IF 0x10
+# define CANINTF_TX1IF 0x08
+# define CANINTF_TX0IF 0x04
+# define CANINTF_RX1IF 0x02
+# define CANINTF_RX0IF 0x01
+#define EFLG 0x2d
+# define EFLG_EWARN 0x01
+# define EFLG_RXWAR 0x02
+# define EFLG_TXWAR 0x04
+# define EFLG_RXEP 0x08
+# define EFLG_TXEP 0x10
+# define EFLG_TXBO 0x20
+# define EFLG_RX0OVR 0x40
+# define EFLG_RX1OVR 0x80
+#define TXBCTRL(n) (((n) * 0x10) + 0x30 + TXBCTRL_OFF)
+# define TXBCTRL_ABTF 0x40
+# define TXBCTRL_MLOA 0x20
+# define TXBCTRL_TXERR 0x10
+# define TXBCTRL_TXREQ 0x08
+#define TXBSIDH(n) (((n) * 0x10) + 0x30 + TXBSIDH_OFF)
+# define SIDH_SHIFT 3
+#define TXBSIDL(n) (((n) * 0x10) + 0x30 + TXBSIDL_OFF)
+# define SIDL_SID_MASK 7
+# define SIDL_SID_SHIFT 5
+# define SIDL_EXIDE_SHIFT 3
+# define SIDL_EID_SHIFT 16
+# define SIDL_EID_MASK 3
+#define TXBEID8(n) (((n) * 0x10) + 0x30 + TXBEID8_OFF)
+#define TXBEID0(n) (((n) * 0x10) + 0x30 + TXBEID0_OFF)
+#define TXBDLC(n) (((n) * 0x10) + 0x30 + TXBDLC_OFF)
+# define DLC_RTR_SHIFT 6
+#define TXBCTRL_OFF 0
+#define TXBSIDH_OFF 1
+#define TXBSIDL_OFF 2
+#define TXBEID8_OFF 3
+#define TXBEID0_OFF 4
+#define TXBDLC_OFF 5
+#define TXBDAT_OFF 6
+#define RXBCTRL(n) (((n) * 0x10) + 0x60 + RXBCTRL_OFF)
+# define RXBCTRL_BUKT 0x04
+# define RXBCTRL_RXM0 0x20
+# define RXBCTRL_RXM1 0x40
+#define RXBSIDH(n) (((n) * 0x10) + 0x60 + RXBSIDH_OFF)
+# define RXBSIDH_SHIFT 3
+#define RXBSIDL(n) (((n) * 0x10) + 0x60 + RXBSIDL_OFF)
+# define RXBSIDL_IDE 0x08
+# define RXBSIDL_EID 3
+# define RXBSIDL_SHIFT 5
+#define RXBEID8(n) (((n) * 0x10) + 0x60 + RXBEID8_OFF)
+#define RXBEID0(n) (((n) * 0x10) + 0x60 + RXBEID0_OFF)
+#define RXBDLC(n) (((n) * 0x10) + 0x60 + RXBDLC_OFF)
+# define RXBDLC_LEN_MASK 0x0f
+# define RXBDLC_RTR 0x40
+#define RXBCTRL_OFF 0
+#define RXBSIDH_OFF 1
+#define RXBSIDL_OFF 2
+#define RXBEID8_OFF 3
+#define RXBEID0_OFF 4
+#define RXBDLC_OFF 5
+#define RXBDAT_OFF 6
+
+#define GET_BYTE(val, byte) \
+ (((val) >> ((byte) * 8)) & 0xff)
+#define SET_BYTE(val, byte) \
+ (((val) & 0xff) << ((byte) * 8))
+
+/*
+ * Buffer size required for the largest SPI transfer (i.e., reading a
+ * frame)
+ */
+#define CAN_FRAME_MAX_DATA_LEN 8
+#define SPI_TRANSFER_BUF_LEN (6 + CAN_FRAME_MAX_DATA_LEN)
+#define CAN_FRAME_MAX_BITS 128
+
+#define TX_ECHO_SKB_MAX 1
+
+#define DEVICE_NAME "mcp251x"
+
+static int mcp251x_enable_dma; /* Enable SPI DMA. Default: 0 (Off) */
+module_param(mcp251x_enable_dma, int, S_IRUGO);
+MODULE_PARM_DESC(mcp251x_enable_dma, "Enable SPI DMA. Default: 0 (Off)");
+
+static struct can_bittiming_const mcp251x_bittiming_const = {
+ .name = DEVICE_NAME,
+ .tseg1_min = 3,
+ .tseg1_max = 16,
+ .tseg2_min = 2,
+ .tseg2_max = 8,
+ .sjw_max = 4,
+ .brp_min = 1,
+ .brp_max = 64,
+ .brp_inc = 1,
+};
+
+struct mcp251x_priv {
+ struct can_priv can;
+ struct net_device *net;
+ struct spi_device *spi;
+
+ struct mutex spi_lock; /* SPI buffer lock */
+ u8 *spi_tx_buf;
+ u8 *spi_rx_buf;
+ dma_addr_t spi_tx_dma;
+ dma_addr_t spi_rx_dma;
+
+ struct sk_buff *tx_skb;
+ int tx_len;
+ struct workqueue_struct *wq;
+ struct work_struct tx_work;
+ struct work_struct irq_work;
+ struct completion awake;
+ int wake;
+ int force_quit;
+ int after_suspend;
+#define AFTER_SUSPEND_UP 1
+#define AFTER_SUSPEND_DOWN 2
+#define AFTER_SUSPEND_POWER 4
+#define AFTER_SUSPEND_RESTART 8
+ int restart_tx;
+};
+
+static void mcp251x_clean(struct net_device *net)
+{
+ struct mcp251x_priv *priv = netdev_priv(net);
+
+ net->stats.tx_errors++;
+ if (priv->tx_skb)
+ dev_kfree_skb(priv->tx_skb);
+ if (priv->tx_len)
+ can_free_echo_skb(priv->net, 0);
+ priv->tx_skb = NULL;
+ priv->tx_len = 0;
+}
+
+/*
+ * Note about handling of error return of mcp251x_spi_trans: accessing
+ * registers via SPI is not really different conceptually than using
+ * normal I/O assembler instructions, although it's much more
+ * complicated from a practical POV. So it's not advisable to always
+ * check the return value of this function. Imagine that every
+ * read{b,l}, write{b,l} and friends would be bracketed in "if ( < 0)
+ * error();", it would be a great mess (well there are some situation
+ * when exception handling C++ like could be useful after all). So we
+ * just check that transfers are OK at the beginning of our
+ * conversation with the chip and to avoid doing really nasty things
+ * (like injecting bogus packets in the network stack).
+ */
+static int mcp251x_spi_trans(struct spi_device *spi, int len)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+ struct spi_transfer t = {
+ .tx_buf = priv->spi_tx_buf,
+ .rx_buf = priv->spi_rx_buf,
+ .len = len,
+ .cs_change = 0,
+ };
+ struct spi_message m;
+ int ret;
+
+ spi_message_init(&m);
+
+ if (mcp251x_enable_dma) {
+ t.tx_dma = priv->spi_tx_dma;
+ t.rx_dma = priv->spi_rx_dma;
+ m.is_dma_mapped = 1;
+ }
+
+ spi_message_add_tail(&t, &m);
+
+ ret = spi_sync(spi, &m);
+ if (ret)
+ dev_err(&spi->dev, "spi transfer failed: ret = %d\n", ret);
+ return ret;
+}
+
+static u8 mcp251x_read_reg(struct spi_device *spi, uint8_t reg)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+ u8 val = 0;
+
+ mutex_lock(&priv->spi_lock);
+
+ priv->spi_tx_buf[0] = INSTRUCTION_READ;
+ priv->spi_tx_buf[1] = reg;
+
+ mcp251x_spi_trans(spi, 3);
+ val = priv->spi_rx_buf[2];
+
+ mutex_unlock(&priv->spi_lock);
+
+ return val;
+}
+
+static void mcp251x_write_reg(struct spi_device *spi, u8 reg, uint8_t val)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+
+ mutex_lock(&priv->spi_lock);
+
+ priv->spi_tx_buf[0] = INSTRUCTION_WRITE;
+ priv->spi_tx_buf[1] = reg;
+ priv->spi_tx_buf[2] = val;
+
+ mcp251x_spi_trans(spi, 3);
+
+ mutex_unlock(&priv->spi_lock);
+}
+
+static void mcp251x_write_bits(struct spi_device *spi, u8 reg,
+ u8 mask, uint8_t val)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+
+ mutex_lock(&priv->spi_lock);
+
+ priv->spi_tx_buf[0] = INSTRUCTION_BIT_MODIFY;
+ priv->spi_tx_buf[1] = reg;
+ priv->spi_tx_buf[2] = mask;
+ priv->spi_tx_buf[3] = val;
+
+ mcp251x_spi_trans(spi, 4);
+
+ mutex_unlock(&priv->spi_lock);
+}
+
+static void mcp251x_hw_tx_frame(struct spi_device *spi, u8 *buf,
+ int len, int tx_buf_idx)
+{
+ struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+
+ if (pdata->model == CAN_MCP251X_MCP2510) {
+ int i;
+
+ for (i = 1; i < TXBDAT_OFF + len; i++)
+ mcp251x_write_reg(spi, TXBCTRL(tx_buf_idx) + i,
+ buf[i]);
+ } else {
+ mutex_lock(&priv->spi_lock);
+ memcpy(priv->spi_tx_buf, buf, TXBDAT_OFF + len);
+ mcp251x_spi_trans(spi, TXBDAT_OFF + len);
+ mutex_unlock(&priv->spi_lock);
+ }
+}
+
+static void mcp251x_hw_tx(struct spi_device *spi, struct can_frame *frame,
+ int tx_buf_idx)
+{
+ u32 sid, eid, exide, rtr;
+ u8 buf[SPI_TRANSFER_BUF_LEN];
+
+ exide = (frame->can_id & CAN_EFF_FLAG) ? 1 : 0; /* Extended ID Enable */
+ if (exide)
+ sid = (frame->can_id & CAN_EFF_MASK) >> 18;
+ else
+ sid = frame->can_id & CAN_SFF_MASK; /* Standard ID */
+ eid = frame->can_id & CAN_EFF_MASK; /* Extended ID */
+ rtr = (frame->can_id & CAN_RTR_FLAG) ? 1 : 0; /* Remote transmission */
+
+ buf[TXBCTRL_OFF] = INSTRUCTION_LOAD_TXB(tx_buf_idx);
+ buf[TXBSIDH_OFF] = sid >> SIDH_SHIFT;
+ buf[TXBSIDL_OFF] = ((sid & SIDL_SID_MASK) << SIDL_SID_SHIFT) |
+ (exide << SIDL_EXIDE_SHIFT) |
+ ((eid >> SIDL_EID_SHIFT) & SIDL_EID_MASK);
+ buf[TXBEID8_OFF] = GET_BYTE(eid, 1);
+ buf[TXBEID0_OFF] = GET_BYTE(eid, 0);
+ buf[TXBDLC_OFF] = (rtr << DLC_RTR_SHIFT) | frame->can_dlc;
+ memcpy(buf + TXBDAT_OFF, frame->data, frame->can_dlc);
+ mcp251x_hw_tx_frame(spi, buf, frame->can_dlc, tx_buf_idx);
+ mcp251x_write_reg(spi, TXBCTRL(tx_buf_idx), TXBCTRL_TXREQ);
+}
+
+static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *buf,
+ int buf_idx)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+ struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+
+ if (pdata->model == CAN_MCP251X_MCP2510) {
+ int i, len;
+
+ for (i = 1; i < RXBDAT_OFF; i++)
+ buf[i] = mcp251x_read_reg(spi, RXBCTRL(buf_idx) + i);
+ len = buf[RXBDLC_OFF] & RXBDLC_LEN_MASK;
+ if (len > 8)
+ len = 8;
+ for (; i < (RXBDAT_OFF + len); i++)
+ buf[i] = mcp251x_read_reg(spi, RXBCTRL(buf_idx) + i);
+ } else {
+ mutex_lock(&priv->spi_lock);
+
+ priv->spi_tx_buf[RXBCTRL_OFF] = INSTRUCTION_READ_RXB(buf_idx);
+ mcp251x_spi_trans(spi, SPI_TRANSFER_BUF_LEN);
+ memcpy(buf, priv->spi_rx_buf, SPI_TRANSFER_BUF_LEN);
+
+ mutex_unlock(&priv->spi_lock);
+ }
+}
+
+static void mcp251x_hw_rx(struct spi_device *spi, int buf_idx)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+ struct sk_buff *skb;
+ struct can_frame *frame;
+ u8 buf[SPI_TRANSFER_BUF_LEN];
+
+ skb = alloc_can_skb(priv->net, &frame);
+ if (!skb) {
+ dev_err(&spi->dev, "cannot allocate RX skb\n");
+ priv->net->stats.rx_dropped++;
+ return;
+ }
+
+ mcp251x_hw_rx_frame(spi, buf, buf_idx);
+ if (buf[RXBSIDL_OFF] & RXBSIDL_IDE) {
+ /* Extended ID format */
+ frame->can_id = CAN_EFF_FLAG;
+ frame->can_id |=
+ /* Extended ID part */
+ SET_BYTE(buf[RXBSIDL_OFF] & RXBSIDL_EID, 2) |
+ SET_BYTE(buf[RXBEID8_OFF], 1) |
+ SET_BYTE(buf[RXBEID0_OFF], 0) |
+ /* Standard ID part */
+ (((buf[RXBSIDH_OFF] << RXBSIDH_SHIFT) |
+ (buf[RXBSIDL_OFF] >> RXBSIDL_SHIFT)) << 18);
+ /* Remote transmission request */
+ if (buf[RXBDLC_OFF] & RXBDLC_RTR)
+ frame->can_id |= CAN_RTR_FLAG;
+ } else {
+ /* Standard ID format */
+ frame->can_id =
+ (buf[RXBSIDH_OFF] << RXBSIDH_SHIFT) |
+ (buf[RXBSIDL_OFF] >> RXBSIDL_SHIFT);
+ }
+ /* Data length */
+ frame->can_dlc = buf[RXBDLC_OFF] & RXBDLC_LEN_MASK;
+ if (frame->can_dlc > 8) {
+ dev_warn(&spi->dev, "invalid frame recevied\n");
+ priv->net->stats.rx_errors++;
+ dev_kfree_skb(skb);
+ return;
+ }
+ memcpy(frame->data, buf + RXBDAT_OFF, frame->can_dlc);
+
+ priv->net->stats.rx_packets++;
+ priv->net->stats.rx_bytes += frame->can_dlc;
+ netif_rx(skb);
+}
+
+static void mcp251x_hw_sleep(struct spi_device *spi)
+{
+ mcp251x_write_reg(spi, CANCTRL, CANCTRL_REQOP_SLEEP);
+}
+
+static void mcp251x_hw_wakeup(struct spi_device *spi)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+
+ priv->wake = 1;
+
+ /* Can only wake up by generating a wake-up interrupt. */
+ mcp251x_write_bits(spi, CANINTE, CANINTE_WAKIE, CANINTE_WAKIE);
+ mcp251x_write_bits(spi, CANINTF, CANINTF_WAKIF, CANINTF_WAKIF);
+
+ /* Wait until the device is awake */
+ if (!wait_for_completion_timeout(&priv->awake, HZ))
+ dev_err(&spi->dev, "MCP251x didn't wake-up\n");
+}
+
+static netdev_tx_t mcp251x_hard_start_xmit(struct sk_buff *skb,
+ struct net_device *net)
+{
+ struct mcp251x_priv *priv = netdev_priv(net);
+ struct spi_device *spi = priv->spi;
+
+ if (priv->tx_skb || priv->tx_len) {
+ dev_warn(&spi->dev, "hard_xmit called while tx busy\n");
+ netif_stop_queue(net);
+ return NETDEV_TX_BUSY;
+ }
+
+ if (skb->len != sizeof(struct can_frame)) {
+ dev_err(&spi->dev, "dropping packet - bad length\n");
+ dev_kfree_skb(skb);
+ net->stats.tx_dropped++;
+ return NETDEV_TX_OK;
+ }
+
+ netif_stop_queue(net);
+ priv->tx_skb = skb;
+ net->trans_start = jiffies;
+ queue_work(priv->wq, &priv->tx_work);
+
+ return NETDEV_TX_OK;
+}
+
+static int mcp251x_do_set_mode(struct net_device *net, enum can_mode mode)
+{
+ struct mcp251x_priv *priv = netdev_priv(net);
+
+ switch (mode) {
+ case CAN_MODE_START:
+ /* We have to delay work since SPI I/O may sleep */
+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ priv->restart_tx = 1;
+ if (priv->can.restart_ms == 0)
+ priv->after_suspend = AFTER_SUSPEND_RESTART;
+ queue_work(priv->wq, &priv->irq_work);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static void mcp251x_set_normal_mode(struct spi_device *spi)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+ unsigned long timeout;
+
+ /* Enable interrupts */
+ mcp251x_write_reg(spi, CANINTE,
+ CANINTE_ERRIE | CANINTE_TX2IE | CANINTE_TX1IE |
+ CANINTE_TX0IE | CANINTE_RX1IE | CANINTE_RX0IE |
+ CANINTF_MERRF);
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
+ /* Put device into loopback mode */
+ mcp251x_write_reg(spi, CANCTRL, CANCTRL_REQOP_LOOPBACK);
+ } else {
+ /* Put device into normal mode */
+ mcp251x_write_reg(spi, CANCTRL, CANCTRL_REQOP_NORMAL);
+
+ /* Wait for the device to enter normal mode */
+ timeout = jiffies + HZ;
+ while (mcp251x_read_reg(spi, CANSTAT) & CANCTRL_REQOP_MASK) {
+ schedule();
+ if (time_after(jiffies, timeout)) {
+ dev_err(&spi->dev, "MCP251x didn't"
+ " enter in normal mode\n");
+ return;
+ }
+ }
+ }
+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
+}
+
+static int mcp251x_do_set_bittiming(struct net_device *net)
+{
+ struct mcp251x_priv *priv = netdev_priv(net);
+ struct can_bittiming *bt = &priv->can.bittiming;
+ struct spi_device *spi = priv->spi;
+
+ mcp251x_write_reg(spi, CNF1, ((bt->sjw - 1) << CNF1_SJW_SHIFT) |
+ (bt->brp - 1));
+ mcp251x_write_reg(spi, CNF2, CNF2_BTLMODE |
+ (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES ?
+ CNF2_SAM : 0) |
+ ((bt->phase_seg1 - 1) << CNF2_PS1_SHIFT) |
+ (bt->prop_seg - 1));
+ mcp251x_write_bits(spi, CNF3, CNF3_PHSEG2_MASK,
+ (bt->phase_seg2 - 1));
+ dev_info(&spi->dev, "CNF: 0x%02x 0x%02x 0x%02x\n",
+ mcp251x_read_reg(spi, CNF1),
+ mcp251x_read_reg(spi, CNF2),
+ mcp251x_read_reg(spi, CNF3));
+
+ return 0;
+}
+
+static int mcp251x_setup(struct net_device *net, struct mcp251x_priv *priv,
+ struct spi_device *spi)
+{
+ int ret;
+
+ ret = open_candev(net);
+ if (ret) {
+ dev_err(&spi->dev, "unable to set initial baudrate!\n");
+ return ret;
+ }
+
+ /* Enable RX0->RX1 buffer roll over and disable filters */
+ mcp251x_write_bits(spi, RXBCTRL(0),
+ RXBCTRL_BUKT | RXBCTRL_RXM0 | RXBCTRL_RXM1,
+ RXBCTRL_BUKT | RXBCTRL_RXM0 | RXBCTRL_RXM1);
+ mcp251x_write_bits(spi, RXBCTRL(1),
+ RXBCTRL_RXM0 | RXBCTRL_RXM1,
+ RXBCTRL_RXM0 | RXBCTRL_RXM1);
+ return 0;
+}
+
+static void mcp251x_hw_reset(struct spi_device *spi)
+{
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+ int ret;
+
+ mutex_lock(&priv->spi_lock);
+
+ priv->spi_tx_buf[0] = INSTRUCTION_RESET;
+
+ ret = spi_write(spi, priv->spi_tx_buf, 1);
+
+ mutex_unlock(&priv->spi_lock);
+
+ if (ret)
+ dev_err(&spi->dev, "reset failed: ret = %d\n", ret);
+ /* Wait for reset to finish */
+ mdelay(10);
+}
+
+static int mcp251x_hw_probe(struct spi_device *spi)
+{
+ int st1, st2;
+
+ mcp251x_hw_reset(spi);
+
+ /*
+ * Please note that these are "magic values" based on after
+ * reset defaults taken from data sheet which allows us to see
+ * if we really have a chip on the bus (we avoid common all
+ * zeroes or all ones situations)
+ */
+ st1 = mcp251x_read_reg(spi, CANSTAT) & 0xEE;
+ st2 = mcp251x_read_reg(spi, CANCTRL) & 0x17;
+
+ dev_dbg(&spi->dev, "CANSTAT 0x%02x CANCTRL 0x%02x\n", st1, st2);
+
+ /* Check for power up default values */
+ return (st1 == 0x80 && st2 == 0x07) ? 1 : 0;
+}
+
+static irqreturn_t mcp251x_can_isr(int irq, void *dev_id)
+{
+ struct net_device *net = (struct net_device *)dev_id;
+ struct mcp251x_priv *priv = netdev_priv(net);
+
+ /* Schedule bottom half */
+ if (!work_pending(&priv->irq_work))
+ queue_work(priv->wq, &priv->irq_work);
+
+ return IRQ_HANDLED;
+}
+
+static int mcp251x_open(struct net_device *net)
+{
+ struct mcp251x_priv *priv = netdev_priv(net);
+ struct spi_device *spi = priv->spi;
+ struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+ int ret;
+
+ if (pdata->transceiver_enable)
+ pdata->transceiver_enable(1);
+
+ priv->force_quit = 0;
+ priv->tx_skb = NULL;
+ priv->tx_len = 0;
+
+ ret = request_irq(spi->irq, mcp251x_can_isr,
+ IRQF_TRIGGER_FALLING, DEVICE_NAME, net);
+ if (ret) {
+ dev_err(&spi->dev, "failed to acquire irq %d\n", spi->irq);
+ return ret;
+ }
+
+ mcp251x_hw_wakeup(spi);
+ mcp251x_hw_reset(spi);
+ ret = mcp251x_setup(net, priv, spi);
+ if (ret) {
+ free_irq(spi->irq, net);
+ if (pdata->transceiver_enable)
+ pdata->transceiver_enable(0);
+ return ret;
+ }
+ mcp251x_set_normal_mode(spi);
+ netif_wake_queue(net);
+
+ return 0;
+}
+
+static int mcp251x_stop(struct net_device *net)
+{
+ struct mcp251x_priv *priv = netdev_priv(net);
+ struct spi_device *spi = priv->spi;
+ struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+
+ close_candev(net);
+
+ /* Disable and clear pending interrupts */
+ mcp251x_write_reg(spi, CANINTE, 0x00);
+ mcp251x_write_reg(spi, CANINTF, 0x00);
+
+ priv->force_quit = 1;
+ free_irq(spi->irq, net);
+ flush_workqueue(priv->wq);
+
+ mcp251x_write_reg(spi, TXBCTRL(0), 0);
+ if (priv->tx_skb || priv->tx_len)
+ mcp251x_clean(net);
+
+ mcp251x_hw_sleep(spi);
+
+ if (pdata->transceiver_enable)
+ pdata->transceiver_enable(0);
+
+ priv->can.state = CAN_STATE_STOPPED;
+
+ return 0;
+}
+
+static void mcp251x_tx_work_handler(struct work_struct *ws)
+{
+ struct mcp251x_priv *priv = container_of(ws, struct mcp251x_priv,
+ tx_work);
+ struct spi_device *spi = priv->spi;
+ struct net_device *net = priv->net;
+ struct can_frame *frame;
+
+ if (priv->tx_skb) {
+ frame = (struct can_frame *)priv->tx_skb->data;
+
+ if (priv->can.state == CAN_STATE_BUS_OFF) {
+ mcp251x_clean(net);
+ netif_wake_queue(net);
+ return;
+ }
+ if (frame->can_dlc > CAN_FRAME_MAX_DATA_LEN)
+ frame->can_dlc = CAN_FRAME_MAX_DATA_LEN;
+ mcp251x_hw_tx(spi, frame, 0);
+ priv->tx_len = 1 + frame->can_dlc;
+ can_put_echo_skb(priv->tx_skb, net, 0);
+ priv->tx_skb = NULL;
+ }
+}
+
+static void mcp251x_irq_work_handler(struct work_struct *ws)
+{
+ struct mcp251x_priv *priv = container_of(ws, struct mcp251x_priv,
+ irq_work);
+ struct spi_device *spi = priv->spi;
+ struct net_device *net = priv->net;
+ u8 txbnctrl;
+ u8 intf;
+ enum can_state new_state;
+
+ if (priv->after_suspend) {
+ mdelay(10);
+ mcp251x_hw_reset(spi);
+ mcp251x_setup(net, priv, spi);
+ if (priv->after_suspend & AFTER_SUSPEND_RESTART) {
+ mcp251x_set_normal_mode(spi);
+ } else if (priv->after_suspend & AFTER_SUSPEND_UP) {
+ netif_device_attach(net);
+ /* Clean since we lost tx buffer */
+ if (priv->tx_skb || priv->tx_len) {
+ mcp251x_clean(net);
+ netif_wake_queue(net);
+ }
+ mcp251x_set_normal_mode(spi);
+ } else {
+ mcp251x_hw_sleep(spi);
+ }
+ priv->after_suspend = 0;
+ }
+
+ if (priv->can.restart_ms == 0 && priv->can.state == CAN_STATE_BUS_OFF) {
+ while (!priv->force_quit && !freezing(current) &&
+ (intf = mcp251x_read_reg(spi, CANINTF)))
+ mcp251x_write_bits(spi, CANINTF, intf, 0x00);
+ return;
+ }
+
+ while (!priv->force_quit && !freezing(current)) {
+ u8 eflag = mcp251x_read_reg(spi, EFLG);
+ int can_id = 0, data1 = 0;
+
+ mcp251x_write_reg(spi, EFLG, 0x00);
+
+ if (priv->restart_tx) {
+ priv->restart_tx = 0;
+ mcp251x_write_reg(spi, TXBCTRL(0), 0);
+ if (priv->tx_skb || priv->tx_len)
+ mcp251x_clean(net);
+ netif_wake_queue(net);
+ can_id |= CAN_ERR_RESTARTED;
+ }
+
+ if (priv->wake) {
+ /* Wait whilst the device wakes up */
+ mdelay(10);
+ priv->wake = 0;
+ }
+
+ intf = mcp251x_read_reg(spi, CANINTF);
+ mcp251x_write_bits(spi, CANINTF, intf, 0x00);
+
+ /* Update can state */
+ if (eflag & EFLG_TXBO) {
+ new_state = CAN_STATE_BUS_OFF;
+ can_id |= CAN_ERR_BUSOFF;
+ } else if (eflag & EFLG_TXEP) {
+ new_state = CAN_STATE_ERROR_PASSIVE;
+ can_id |= CAN_ERR_CRTL;
+ data1 |= CAN_ERR_CRTL_TX_PASSIVE;
+ } else if (eflag & EFLG_RXEP) {
+ new_state = CAN_STATE_ERROR_PASSIVE;
+ can_id |= CAN_ERR_CRTL;
+ data1 |= CAN_ERR_CRTL_RX_PASSIVE;
+ } else if (eflag & EFLG_TXWAR) {
+ new_state = CAN_STATE_ERROR_WARNING;
+ can_id |= CAN_ERR_CRTL;
+ data1 |= CAN_ERR_CRTL_TX_WARNING;
+ } else if (eflag & EFLG_RXWAR) {
+ new_state = CAN_STATE_ERROR_WARNING;
+ can_id |= CAN_ERR_CRTL;
+ data1 |= CAN_ERR_CRTL_RX_WARNING;
+ } else {
+ new_state = CAN_STATE_ERROR_ACTIVE;
+ }
+
+ /* Update can state statistics */
+ switch (priv->can.state) {
+ case CAN_STATE_ERROR_ACTIVE:
+ if (new_state >= CAN_STATE_ERROR_WARNING &&
+ new_state <= CAN_STATE_BUS_OFF)
+ priv->can.can_stats.error_warning++;
+ case CAN_STATE_ERROR_WARNING: /* fallthrough */
+ if (new_state >= CAN_STATE_ERROR_PASSIVE &&
+ new_state <= CAN_STATE_BUS_OFF)
+ priv->can.can_stats.error_passive++;
+ break;
+ default:
+ break;
+ }
+ priv->can.state = new_state;
+
+ if ((intf & CANINTF_ERRIF) || (can_id & CAN_ERR_RESTARTED)) {
+ struct sk_buff *skb;
+ struct can_frame *frame;
+
+ /* Create error frame */
+ skb = alloc_can_err_skb(net, &frame);
+ if (skb) {
+ /* Set error frame flags based on bus state */
+ frame->can_id = can_id;
+ frame->data[1] = data1;
+
+ /* Update net stats for overflows */
+ if (eflag & (EFLG_RX0OVR | EFLG_RX1OVR)) {
+ if (eflag & EFLG_RX0OVR)
+ net->stats.rx_over_errors++;
+ if (eflag & EFLG_RX1OVR)
+ net->stats.rx_over_errors++;
+ frame->can_id |= CAN_ERR_CRTL;
+ frame->data[1] |=
+ CAN_ERR_CRTL_RX_OVERFLOW;
+ }
+
+ netif_rx(skb);
+ } else {
+ dev_info(&spi->dev,
+ "cannot allocate error skb\n");
+ }
+ }
+
+ if (priv->can.state == CAN_STATE_BUS_OFF) {
+ if (priv->can.restart_ms == 0) {
+ can_bus_off(net);
+ mcp251x_hw_sleep(spi);
+ return;
+ }
+ }
+
+ if (intf == 0)
+ break;
+
+ if (intf & CANINTF_WAKIF)
+ complete(&priv->awake);
+
+ if (intf & CANINTF_MERRF) {
+ /* If there are pending Tx buffers, restart queue */
+ txbnctrl = mcp251x_read_reg(spi, TXBCTRL(0));
+ if (!(txbnctrl & TXBCTRL_TXREQ)) {
+ if (priv->tx_skb || priv->tx_len)
+ mcp251x_clean(net);
+ netif_wake_queue(net);
+ }
+ }
+
+ if (intf & (CANINTF_TX2IF | CANINTF_TX1IF | CANINTF_TX0IF)) {
+ net->stats.tx_packets++;
+ net->stats.tx_bytes += priv->tx_len - 1;
+ if (priv->tx_len) {
+ can_get_echo_skb(net, 0);
+ priv->tx_len = 0;
+ }
+ netif_wake_queue(net);
+ }
+
+ if (intf & CANINTF_RX0IF)
+ mcp251x_hw_rx(spi, 0);
+
+ if (intf & CANINTF_RX1IF)
+ mcp251x_hw_rx(spi, 1);
+ }
+}
+
+static const struct net_device_ops mcp251x_netdev_ops = {
+ .ndo_open = mcp251x_open,
+ .ndo_stop = mcp251x_stop,
+ .ndo_start_xmit = mcp251x_hard_start_xmit,
+};
+
+static int __devinit mcp251x_can_probe(struct spi_device *spi)
+{
+ struct net_device *net;
+ struct mcp251x_priv *priv;
+ struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+ int ret = -ENODEV;
+
+ if (!pdata)
+ /* Platform data is required for osc freq */
+ goto error_out;
+
+ /* Allocate can/net device */
+ net = alloc_candev(sizeof(struct mcp251x_priv), TX_ECHO_SKB_MAX);
+ if (!net) {
+ ret = -ENOMEM;
+ goto error_alloc;
+ }
+
+ net->netdev_ops = &mcp251x_netdev_ops;
+ net->flags |= IFF_ECHO;
+
+ priv = netdev_priv(net);
+ priv->can.bittiming_const = &mcp251x_bittiming_const;
+ priv->can.do_set_mode = mcp251x_do_set_mode;
+ priv->can.clock.freq = pdata->oscillator_frequency / 2;
+ priv->can.do_set_bittiming = mcp251x_do_set_bittiming;
+ priv->net = net;
+ dev_set_drvdata(&spi->dev, priv);
+
+ priv->spi = spi;
+ mutex_init(&priv->spi_lock);
+
+ /* If requested, allocate DMA buffers */
+ if (mcp251x_enable_dma) {
+ spi->dev.coherent_dma_mask = ~0;
+
+ /*
+ * Minimum coherent DMA allocation is PAGE_SIZE, so allocate
+ * that much and share it between Tx and Rx DMA buffers.
+ */
+ priv->spi_tx_buf = dma_alloc_coherent(&spi->dev,
+ PAGE_SIZE,
+ &priv->spi_tx_dma,
+ GFP_DMA);
+
+ if (priv->spi_tx_buf) {
+ priv->spi_rx_buf = (u8 *)(priv->spi_tx_buf +
+ (PAGE_SIZE / 2));
+ priv->spi_rx_dma = (dma_addr_t)(priv->spi_tx_dma +
+ (PAGE_SIZE / 2));
+ } else {
+ /* Fall back to non-DMA */
+ mcp251x_enable_dma = 0;
+ }
+ }
+
+ /* Allocate non-DMA buffers */
+ if (!mcp251x_enable_dma) {
+ priv->spi_tx_buf = kmalloc(SPI_TRANSFER_BUF_LEN, GFP_KERNEL);
+ if (!priv->spi_tx_buf) {
+ ret = -ENOMEM;
+ goto error_tx_buf;
+ }
+ priv->spi_rx_buf = kmalloc(SPI_TRANSFER_BUF_LEN, GFP_KERNEL);
+ if (!priv->spi_tx_buf) {
+ ret = -ENOMEM;
+ goto error_rx_buf;
+ }
+ }
+
+ if (pdata->power_enable)
+ pdata->power_enable(1);
+
+ /* Call out to platform specific setup */
+ if (pdata->board_specific_setup)
+ pdata->board_specific_setup(spi);
+
+ SET_NETDEV_DEV(net, &spi->dev);
+
+ priv->wq = create_freezeable_workqueue("mcp251x_wq");
+
+ INIT_WORK(&priv->tx_work, mcp251x_tx_work_handler);
+ INIT_WORK(&priv->irq_work, mcp251x_irq_work_handler);
+
+ init_completion(&priv->awake);
+
+ /* Configure the SPI bus */
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ spi_setup(spi);
+
+ if (!mcp251x_hw_probe(spi)) {
+ dev_info(&spi->dev, "Probe failed\n");
+ goto error_probe;
+ }
+ mcp251x_hw_sleep(spi);
+
+ if (pdata->transceiver_enable)
+ pdata->transceiver_enable(0);
+
+ ret = register_candev(net);
+ if (ret >= 0) {
+ dev_info(&spi->dev, "probed\n");
+ return ret;
+ }
+error_probe:
+ if (!mcp251x_enable_dma)
+ kfree(priv->spi_rx_buf);
+error_rx_buf:
+ if (!mcp251x_enable_dma)
+ kfree(priv->spi_tx_buf);
+error_tx_buf:
+ free_candev(net);
+ if (mcp251x_enable_dma)
+ dma_free_coherent(&spi->dev, PAGE_SIZE,
+ priv->spi_tx_buf, priv->spi_tx_dma);
+error_alloc:
+ dev_err(&spi->dev, "probe failed\n");
+error_out:
+ return ret;
+}
+
+static int __devexit mcp251x_can_remove(struct spi_device *spi)
+{
+ struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+ struct net_device *net = priv->net;
+
+ unregister_candev(net);
+ free_candev(net);
+
+ priv->force_quit = 1;
+ flush_workqueue(priv->wq);
+ destroy_workqueue(priv->wq);
+
+ if (mcp251x_enable_dma) {
+ dma_free_coherent(&spi->dev, PAGE_SIZE,
+ priv->spi_tx_buf, priv->spi_tx_dma);
+ } else {
+ kfree(priv->spi_tx_buf);
+ kfree(priv->spi_rx_buf);
+ }
+
+ if (pdata->power_enable)
+ pdata->power_enable(0);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int mcp251x_can_suspend(struct spi_device *spi, pm_message_t state)
+{
+ struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+ struct net_device *net = priv->net;
+
+ if (netif_running(net)) {
+ netif_device_detach(net);
+
+ mcp251x_hw_sleep(spi);
+ if (pdata->transceiver_enable)
+ pdata->transceiver_enable(0);
+ priv->after_suspend = AFTER_SUSPEND_UP;
+ } else {
+ priv->after_suspend = AFTER_SUSPEND_DOWN;
+ }
+
+ if (pdata->power_enable) {
+ pdata->power_enable(0);
+ priv->after_suspend |= AFTER_SUSPEND_POWER;
+ }
+
+ return 0;
+}
+
+static int mcp251x_can_resume(struct spi_device *spi)
+{
+ struct mcp251x_platform_data *pdata = spi->dev.platform_data;
+ struct mcp251x_priv *priv = dev_get_drvdata(&spi->dev);
+
+ if (priv->after_suspend & AFTER_SUSPEND_POWER) {
+ pdata->power_enable(1);
+ queue_work(priv->wq, &priv->irq_work);
+ } else {
+ if (priv->after_suspend & AFTER_SUSPEND_UP) {
+ if (pdata->transceiver_enable)
+ pdata->transceiver_enable(1);
+ queue_work(priv->wq, &priv->irq_work);
+ } else {
+ priv->after_suspend = 0;
+ }
+ }
+ return 0;
+}
+#else
+#define mcp251x_can_suspend NULL
+#define mcp251x_can_resume NULL
+#endif
+
+static struct spi_driver mcp251x_can_driver = {
+ .driver = {
+ .name = DEVICE_NAME,
+ .bus = &spi_bus_type,
+ .owner = THIS_MODULE,
+ },
+
+ .probe = mcp251x_can_probe,
+ .remove = __devexit_p(mcp251x_can_remove),
+ .suspend = mcp251x_can_suspend,
+ .resume = mcp251x_can_resume,
+};
+
+static int __init mcp251x_can_init(void)
+{
+ return spi_register_driver(&mcp251x_can_driver);
+}
+
+static void __exit mcp251x_can_exit(void)
+{
+ spi_unregister_driver(&mcp251x_can_driver);
+}
+
+module_init(mcp251x_can_init);
+module_exit(mcp251x_can_exit);
+
+MODULE_AUTHOR("Chris Elston <celston-Bm0nJX+W7e9BDgjK7y7TUQ@public.gmane.org>, "
+ "Christian Pellegrin <chripell-LERDrqjqfvZg9hUCZPvPmw@public.gmane.org>");
+MODULE_DESCRIPTION("Microchip 251x CAN driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/can/platform/mcp251x.h b/include/linux/can/platform/mcp251x.h
new file mode 100644
index 0000000..1448177
--- /dev/null
+++ b/include/linux/can/platform/mcp251x.h
@@ -0,0 +1,36 @@
+#ifndef __CAN_PLATFORM_MCP251X_H__
+#define __CAN_PLATFORM_MCP251X_H__
+
+/*
+ *
+ * CAN bus driver for Microchip 251x CAN Controller with SPI Interface
+ *
+ */
+
+#include <linux/spi/spi.h>
+
+/**
+ * struct mcp251x_platform_data - MCP251X SPI CAN controller platform data
+ * @oscillator_frequency: - oscillator frequency in Hz
+ * @model: - actual type of chip
+ * @board_specific_setup: - called before probing the chip (power,reset)
+ * @transceiver_enable: - called to power on/off the transceiver
+ * @power_enable: - called to power on/off the mcp *and* the
+ * transceiver
+ *
+ * Please note that you should define power_enable or transceiver_enable or
+ * none of them. Defining both of them is no use.
+ *
+ */
+
+struct mcp251x_platform_data {
+ unsigned long oscillator_frequency;
+ int model;
+#define CAN_MCP251X_MCP2510 0
+#define CAN_MCP251X_MCP2515 1
+ int (*board_specific_setup)(struct spi_device *spi);
+ int (*transceiver_enable)(int enable);
+ int (*power_enable) (int enable);
+};
+
+#endif /* __CAN_PLATFORM_MCP251X_H__ */
--
1.5.6.5
^ permalink raw reply related
* [PATCH net-next-2.6] net: Introduce for_each_netdev_rcu() iterator
From: Eric Dumazet @ 2009-11-02 15:11 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
Adds RCU management to the list of netdevices.
Convert some for_each_netdev() users to RCU version, if
it can avoid read_lock-ing dev_base_lock
Ie:
read_lock(&dev_base_loack);
for_each_netdev(net, dev)
some_action();
read_unlock(&dev_base_lock);
becomes :
rcu_read_lock();
for_each_netdev_rcu(net, dev)
some_action();
rcu_read_unlock();
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/netdevice.h | 2 ++
net/core/dev.c | 30 ++++++++++++++++--------------
net/decnet/af_decnet.c | 6 +++---
net/decnet/dn_fib.c | 6 +++---
net/decnet/dn_route.c | 6 +++---
net/ipv4/devinet.c | 30 +++++++++++-------------------
net/ipv6/addrconf.c | 20 +++++++-------------
net/ipv6/anycast.c | 6 +++---
net/netrom/nr_route.c | 15 ++++++++-------
net/rose/rose_route.c | 16 ++++++++--------
net/sctp/protocol.c | 6 +++---
11 files changed, 67 insertions(+), 76 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index bcf1083..5077de0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1081,6 +1081,8 @@ extern rwlock_t dev_base_lock; /* Device list lock */
#define for_each_netdev(net, d) \
list_for_each_entry(d, &(net)->dev_base_head, dev_list)
+#define for_each_netdev_rcu(net, d) \
+ list_for_each_entry_rcu(d, &(net)->dev_base_head, dev_list)
#define for_each_netdev_safe(net, d, n) \
list_for_each_entry_safe(d, n, &(net)->dev_base_head, dev_list)
#define for_each_netdev_continue(net, d) \
diff --git a/net/core/dev.c b/net/core/dev.c
index 76a1502..bf629ac 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -175,7 +175,7 @@ static struct list_head ptype_all __read_mostly; /* Taps */
* The @dev_base_head list is protected by @dev_base_lock and the rtnl
* semaphore.
*
- * Pure readers hold dev_base_lock for reading.
+ * Pure readers hold dev_base_lock for reading, or rcu_read_lock()
*
* Writers must hold the rtnl semaphore while they loop through the
* dev_base_head list, and hold dev_base_lock for writing when they do the
@@ -212,7 +212,7 @@ static int list_netdevice(struct net_device *dev)
ASSERT_RTNL();
write_lock_bh(&dev_base_lock);
- list_add_tail(&dev->dev_list, &net->dev_base_head);
+ list_add_tail_rcu(&dev->dev_list, &net->dev_base_head);
hlist_add_head_rcu(&dev->name_hlist, dev_name_hash(net, dev->name));
hlist_add_head_rcu(&dev->index_hlist,
dev_index_hash(net, dev->ifindex));
@@ -229,7 +229,7 @@ static void unlist_netdevice(struct net_device *dev)
/* Unlink dev from the device chain */
write_lock_bh(&dev_base_lock);
- list_del(&dev->dev_list);
+ list_del_rcu(&dev->dev_list);
hlist_del_rcu(&dev->name_hlist);
hlist_del_rcu(&dev->index_hlist);
write_unlock_bh(&dev_base_lock);
@@ -799,15 +799,15 @@ struct net_device *dev_get_by_flags(struct net *net, unsigned short if_flags,
struct net_device *dev, *ret;
ret = NULL;
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
if (((dev->flags ^ if_flags) & mask) == 0) {
dev_hold(dev);
ret = dev;
break;
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return ret;
}
EXPORT_SYMBOL(dev_get_by_flags);
@@ -3077,18 +3077,18 @@ static int dev_ifconf(struct net *net, char __user *arg)
* in detail.
*/
void *dev_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(dev_base_lock)
+ __acquires(RCU)
{
struct net *net = seq_file_net(seq);
loff_t off;
struct net_device *dev;
- read_lock(&dev_base_lock);
+ rcu_read_lock();
if (!*pos)
return SEQ_START_TOKEN;
off = 1;
- for_each_netdev(net, dev)
+ for_each_netdev_rcu(net, dev)
if (off++ == *pos)
return dev;
@@ -3097,16 +3097,18 @@ void *dev_seq_start(struct seq_file *seq, loff_t *pos)
void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
- struct net *net = seq_file_net(seq);
+ struct net_device *dev = (v == SEQ_START_TOKEN) ?
+ first_net_device(seq_file_net(seq)) :
+ next_net_device((struct net_device *)v);
+
++*pos;
- return v == SEQ_START_TOKEN ?
- first_net_device(net) : next_net_device((struct net_device *)v);
+ return rcu_dereference(dev);
}
void dev_seq_stop(struct seq_file *seq, void *v)
- __releases(dev_base_lock)
+ __releases(RCU)
{
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static void dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev)
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 664965c..2e35584 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -749,9 +749,9 @@ static int dn_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
if (!(saddr->sdn_flags & SDF_WILD)) {
if (le16_to_cpu(saddr->sdn_nodeaddrl)) {
- read_lock(&dev_base_lock);
+ rcu_read_lock();
ldev = NULL;
- for_each_netdev(&init_net, dev) {
+ for_each_netdev_rcu(&init_net, dev) {
if (!dev->dn_ptr)
continue;
if (dn_dev_islocal(dev, dn_saddr2dn(saddr))) {
@@ -759,7 +759,7 @@ static int dn_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
break;
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
if (ldev == NULL)
return -EADDRNOTAVAIL;
}
diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c
index 27ea2e9..fd641f6 100644
--- a/net/decnet/dn_fib.c
+++ b/net/decnet/dn_fib.c
@@ -607,8 +607,8 @@ static void dn_fib_del_ifaddr(struct dn_ifaddr *ifa)
ASSERT_RTNL();
/* Scan device list */
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
dn_db = dev->dn_ptr;
if (dn_db == NULL)
continue;
@@ -619,7 +619,7 @@ static void dn_fib_del_ifaddr(struct dn_ifaddr *ifa)
}
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
if (found_it == 0) {
fib_magic(RTM_DELROUTE, RTN_LOCAL, ifa->ifa_local, 16, ifa);
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 57662ca..860286a 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -908,8 +908,8 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowi *old
dev_put(dev_out);
goto out;
}
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if (!dev->dn_ptr)
continue;
if (!dn_dev_islocal(dev, oldflp->fld_src))
@@ -922,7 +922,7 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowi *old
dev_out = dev;
break;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
if (dev_out == NULL)
goto out;
dev_hold(dev_out);
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index ccccaae..8aa7a13 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -876,19 +876,16 @@ __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope)
if (!addr)
addr = ifa->ifa_local;
} endfor_ifa(in_dev);
-no_in_dev:
- rcu_read_unlock();
+no_in_dev:
if (addr)
- goto out;
+ goto out_unlock;
/* Not loopback addresses on loopback should be preferred
in this case. It is importnat that lo is the first interface
in dev_base list.
*/
- read_lock(&dev_base_lock);
- rcu_read_lock();
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
if ((in_dev = __in_dev_get_rcu(dev)) == NULL)
continue;
@@ -896,12 +893,11 @@ no_in_dev:
if (ifa->ifa_scope != RT_SCOPE_LINK &&
ifa->ifa_scope <= scope) {
addr = ifa->ifa_local;
- goto out_unlock_both;
+ goto out_unlock;
}
} endfor_ifa(in_dev);
}
-out_unlock_both:
- read_unlock(&dev_base_lock);
+out_unlock:
rcu_read_unlock();
out:
return addr;
@@ -962,9 +958,8 @@ __be32 inet_confirm_addr(struct in_device *in_dev,
return confirm_addr_indev(in_dev, dst, local, scope);
net = dev_net(in_dev->dev);
- read_lock(&dev_base_lock);
rcu_read_lock();
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
if ((in_dev = __in_dev_get_rcu(dev))) {
addr = confirm_addr_indev(in_dev, dst, local, scope);
if (addr)
@@ -972,7 +967,6 @@ __be32 inet_confirm_addr(struct in_device *in_dev,
}
}
rcu_read_unlock();
- read_unlock(&dev_base_lock);
return addr;
}
@@ -1240,18 +1234,18 @@ static void devinet_copy_dflt_conf(struct net *net, int i)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
struct in_device *in_dev;
- rcu_read_lock();
+
in_dev = __in_dev_get_rcu(dev);
if (in_dev && !test_bit(i, in_dev->cnf.state))
in_dev->cnf.data[i] = net->ipv4.devconf_dflt->data[i];
- rcu_read_unlock();
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
+/* called with RTNL locked */
static void inet_forward_change(struct net *net)
{
struct net_device *dev;
@@ -1260,7 +1254,6 @@ static void inet_forward_change(struct net *net)
IPV4_DEVCONF_ALL(net, ACCEPT_REDIRECTS) = !on;
IPV4_DEVCONF_DFLT(net, FORWARDING) = on;
- read_lock(&dev_base_lock);
for_each_netdev(net, dev) {
struct in_device *in_dev;
if (on)
@@ -1271,7 +1264,6 @@ static void inet_forward_change(struct net *net)
IN_DEV_CONF_SET(in_dev, FORWARDING, on);
rcu_read_unlock();
}
- read_unlock(&dev_base_lock);
}
static int devinet_conf_proc(ctl_table *ctl, int write,
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 9186484..024bba3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -481,9 +481,8 @@ static void addrconf_forward_change(struct net *net, __s32 newf)
struct net_device *dev;
struct inet6_dev *idev;
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev) {
- rcu_read_lock();
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
idev = __in6_dev_get(dev);
if (idev) {
int changed = (!idev->cnf.forwarding) ^ (!newf);
@@ -491,9 +490,8 @@ static void addrconf_forward_change(struct net *net, __s32 newf)
if (changed)
dev_forward_change(idev);
}
- rcu_read_unlock();
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int addrconf_fixup_forwarding(struct ctl_table *table, int *p, int old)
@@ -1137,10 +1135,9 @@ int ipv6_dev_get_saddr(struct net *net, struct net_device *dst_dev,
hiscore->rule = -1;
hiscore->ifa = NULL;
- read_lock(&dev_base_lock);
rcu_read_lock();
- for_each_netdev(net, dev) {
+ for_each_netdev_rcu(net, dev) {
struct inet6_dev *idev;
/* Candidate Source Address (section 4)
@@ -1235,7 +1232,6 @@ try_nextdev:
read_unlock_bh(&idev->lock);
}
rcu_read_unlock();
- read_unlock(&dev_base_lock);
if (!hiscore->ifa)
return -EADDRNOTAVAIL;
@@ -4052,9 +4048,8 @@ static void addrconf_disable_change(struct net *net, __s32 newf)
struct net_device *dev;
struct inet6_dev *idev;
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev) {
- rcu_read_lock();
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev) {
idev = __in6_dev_get(dev);
if (idev) {
int changed = (!idev->cnf.disable_ipv6) ^ (!newf);
@@ -4062,9 +4057,8 @@ static void addrconf_disable_change(struct net *net, __s32 newf)
if (changed)
dev_disable_change(idev);
}
- rcu_read_unlock();
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
static int addrconf_disable_ipv6(struct ctl_table *table, int *p, int old)
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 1ae58be..2f00ca8 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -404,13 +404,13 @@ int ipv6_chk_acast_addr(struct net *net, struct net_device *dev,
if (dev)
return ipv6_chk_acast_dev(dev, addr);
- read_lock(&dev_base_lock);
- for_each_netdev(net, dev)
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev)
if (ipv6_chk_acast_dev(dev, addr)) {
found = 1;
break;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return found;
}
diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
index 4eb1ac9..aacba76 100644
--- a/net/netrom/nr_route.c
+++ b/net/netrom/nr_route.c
@@ -597,15 +597,15 @@ struct net_device *nr_dev_first(void)
{
struct net_device *dev, *first = NULL;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if ((dev->flags & IFF_UP) && dev->type == ARPHRD_NETROM)
if (first == NULL || strncmp(dev->name, first->name, 3) < 0)
first = dev;
}
if (first)
dev_hold(first);
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return first;
}
@@ -617,16 +617,17 @@ struct net_device *nr_dev_get(ax25_address *addr)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
- if ((dev->flags & IFF_UP) && dev->type == ARPHRD_NETROM && ax25cmp(addr, (ax25_address *)dev->dev_addr) == 0) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
+ if ((dev->flags & IFF_UP) && dev->type == ARPHRD_NETROM &&
+ ax25cmp(addr, (ax25_address *)dev->dev_addr) == 0) {
dev_hold(dev);
goto out;
}
}
dev = NULL;
out:
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return dev;
}
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 9478d9b..bd7a544 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -600,13 +600,13 @@ struct net_device *rose_dev_first(void)
{
struct net_device *dev, *first = NULL;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE)
if (first == NULL || strncmp(dev->name, first->name, 3) < 0)
first = dev;
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return first;
}
@@ -618,8 +618,8 @@ struct net_device *rose_dev_get(rose_address *addr)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE && rosecmp(addr, (rose_address *)dev->dev_addr) == 0) {
dev_hold(dev);
goto out;
@@ -635,14 +635,14 @@ static int rose_dev_exists(rose_address *addr)
{
struct net_device *dev;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
if ((dev->flags & IFF_UP) && dev->type == ARPHRD_ROSE && rosecmp(addr, (rose_address *)dev->dev_addr) == 0)
goto out;
}
dev = NULL;
out:
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
return dev != NULL;
}
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index d9f4cc2..fe44c57 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -205,14 +205,14 @@ static void sctp_get_local_addr_list(void)
struct list_head *pos;
struct sctp_af *af;
- read_lock(&dev_base_lock);
- for_each_netdev(&init_net, dev) {
+ rcu_read_lock();
+ for_each_netdev_rcu(&init_net, dev) {
__list_for_each(pos, &sctp_address_families) {
af = list_entry(pos, struct sctp_af, list);
af->copy_addrlist(&sctp_local_addr_list, dev);
}
}
- read_unlock(&dev_base_lock);
+ rcu_read_unlock();
}
/* Free the existing local addresses. */
^ permalink raw reply related
* Re: [PATCH net-next-2.6] Driver for the Microchip MCP251x SPI CAN controllers
From: christian pellegrin @ 2009-11-02 15:08 UTC (permalink / raw)
To: Paul Thomas
Cc: Wolfgang Grandegger, socketcan-core, spi-devel-general, netdev
In-Reply-To: <c785bba30911010840m2ad73abawf8434f42337a26d3@mail.gmail.com>
On Sun, Nov 1, 2009 at 5:40 PM, Paul Thomas <pthomas8589@gmail.com> wrote:
> Yes, I will be testing ready to work on this hopefully on Monday or
That will be great: I only have a mcp2510 right now. Thanks in
advance, I'll put you in CC with v2.
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
^ permalink raw reply
* Re: [PATCH net-next-2.6] Driver for the Microchip MCP251x SPI CAN controllers
From: christian pellegrin @ 2009-11-02 15:06 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4AED5589.3090106-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
On Sun, Nov 1, 2009 at 10:31 AM, Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org> wrote:
> Hi Christian,
>
Hi,
> there are a few. In general, please check the usage of {} for if
sorry for missing this: I read your link below: I missed that rule on
first reading! And I tend to trust checkpatch.pl too much ;-)
> statements and check if "if (ret)" should be used instead of "if (ret <
> 0)" if 0 means success and !0 failure. I don't have a MCP251x hardware
ok, I misunderstood this to. Now I think it's ok.
I'm replying to this thread with v2 patch. I'm rebasing the
differences against SVN trunk too, but I'm waiting to send them until
this patch is accepted in net-next-2.6 since their are only of
cosmetic nature.
>
> Please use the subject prefix "can: Driver for the..."
ack
>> + depends on CAN && CAN_DEV && SPI
>
> You can drop the redundant dependency on "CAN".
>
ack, I just copied AT91 CAN which has this dependency
>> + * <chripell-LERDrqjqfvZg9hUCZPvPmw@public.gmane.org>
>
> Please add your "Copyright ...".
>
ack
>> +#include <linux/can/core.h>
>
> I don't think you need "can/core.h"?
>
I tried without but there are some dependencies in "can/dev.h" to some
netdev stuff that are broken if I omit it.
>> +#include <linux/if_arp.h>
>
> And that one either.
>
ack
>> +#define TXBCTRL(n) ((n * 0x10) + 0x30)
>
> Please put brackets around "n": (((n) * 0x10) + 0x30)
> Also the proper offset definition should be used.
>
> #define TXBCTRL(n) (((n) * 0x10) + 0x30 + TXBCTRL_OFF)
>
> Here and in similar cases below.
ack
>> +#define RXBDAT_OFF 6
>
> I was thinking to use structure(s) for the offsets (register layout)
> above, but fiddling with offsetof() does probably not make the code more
> readable.
>
Well I try avoid to unpacking bitfields with structs, I saw too many
problems witch cross-compiling when you have many different
architectures. I guess that using u8, u16 and similar should solve the
problems but I'm a bit like my cat: after she burned her whiskers, she
always stays away from kitchen stoves.
>> + if (ret < 0)
>
> if (ret) ?
>
ack
>> +static void mcp251x_hw_tx_frame(struct spi_device *spi, u8 *data,
>
> s/data/buf/ to avoid confusion with the CAN payload data.
>
ack
>> +static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *data,
>
> s/data/buf/, see above.
>
ack
>> + SET_BYTE(buf[RXBEID0_OFF], 0) |
>
> Please don't align arguments or variables.
>
ack, sorry for the nostalgia of ASCII art times
>> + }
>
> Remove {}, please.
>
ack
>> + (buf[RXBSIDL_OFF] >> RXBSIDL_SHIFT);
>
> Please use {} here as well.
>
ack
>> + return 0;
>
> return NETDEV_TX_OK; ?
>
ack
>> + mcp251x_write_reg(spi, CANCTRL, CANCTRL_REQOP_LOOPBACK);
>
> Please use brackets here as well. See:
>
> http://lxr.linux.no/#linux+v2.6.31/Documentation/CodingStyle#L171
>
ack and thanks for the link
>> + /* Store original mode and set mode to config */
>
> Do you need that. The bit-timing can only be set when the device is
> stopped (down).
>
ack, you are right, no need for it because normal/loopback mode is
always set afterwards in _open
>> + if (ret < 0)
>
> if (ret) ?
>
ack
>> + if (ret < 0) {
>
> "if (ret)" ?
>
ack
>> + if (ret < 0) {
>
> "if (ret)" ?
>
ack
>> + disable_irq(spi->irq);
>
> free_irq? And what about the transeiver? The usual goto cleanup method
> would make sense here.
>
ack
>> + disable_irq(spi->irq);
>
> Why not freeing the irq already here?
>
ack, you are right
>> + flush_workqueue(priv->wq);
>> +
>> + mcp251x_write_reg(spi, TXBCTRL(0), 0);
>
> Hm, but you still need the interrupt!?
no, SPI interface and MCP251x interrupt are different
>> + close_candev(net);
>
> You should call close_candev early to cancel the buf-off recovery timer.
>
ack
>> + mcp251x_set_normal_mode(spi);
>
> Please use {} here as well.
>
ack
>> + mcp251x_hw_sleep(spi);
>
> Please use {} here as well.
>
ack
>> + new_state = CAN_STATE_ERROR_ACTIVE;
>
> Please use {} here as well.
>
ack
>> + "cannot allocate error skb\n");
>
> Please use {} here as well.
>
ack
>> + struct mcp251x_platform_data *pdata)
>
> Add __devinit or, even better, put the code into mcp251x_can_probe?
>
ack, moved altogether
>> + priv->can.do_set_bittiming = mcp251x_do_set_bittiming;
>
> Don't align expressions. Use just *one* space before and after "=".
>
ack
>> + mcp251x_enable_dma = 0;
>
> Please use {} here as well.
>
ack
>> + priv->spi_tx_buf, priv->spi_tx_dma);
>
> Please use {} here as well.
>
ack
>> + priv->after_suspend = AFTER_SUSPEND_DOWN;
>
> Please use {} here as well.
>
ack
>
> Please use {} here as well and check for similar cases. I might not have
> spotted all.
>
ack, I searched for all else and checked
>> + .resume = mcp251x_can_resume,
>
> Use just *one* space before and after "=".
>
ack
Thanks for the review!
--
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."
^ permalink raw reply
* [PATCH 2/2] tc35815: Kill unused code
From: Atsushi Nemoto @ 2009-11-02 14:34 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Ralf Roesch
- TC35815_DMA_SYNC_ONDEMAND is always enabled.
- WORKAROUND_LOSTCAR is always enabled.
- WORKAROUND_100HALF_PROMISC is always enabled.
- GATHER_TXINT is always enabled.
- TC35815_USE_PACKEDBUFFER is always disabled.
- NO_CHECK_CARRIER is always disabled.
---
drivers/net/tc35815.c | 224 +------------------------------------------------
1 files changed, 3 insertions(+), 221 deletions(-)
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index 803eb64..6572e8a 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -50,13 +50,6 @@ static const char *version = "tc35815.c:v" DRV_VERSION "\n";
#include <asm/io.h>
#include <asm/byteorder.h>
-/* First, a few definitions that the brave might change. */
-
-#define GATHER_TXINT /* On-Demand Tx Interrupt */
-#define WORKAROUND_LOSTCAR
-#define WORKAROUND_100HALF_PROMISC
-/* #define TC35815_USE_PACKEDBUFFER */
-
enum tc35815_chiptype {
TC35815CF = 0,
TC35815_NWU,
@@ -326,17 +319,10 @@ struct BDesc {
/* Some useful constants. */
-#undef NO_CHECK_CARRIER /* Does not check No-Carrier with TP */
-#ifdef NO_CHECK_CARRIER
-#define TX_CTL_CMD (Tx_EnComp | Tx_EnTxPar | Tx_EnLateColl | \
- Tx_EnExColl | Tx_EnExDefer | Tx_EnUnder | \
- Tx_En) /* maybe 0x7b01 */
-#else
-#define TX_CTL_CMD (Tx_EnComp | Tx_EnTxPar | Tx_EnLateColl | \
+#define TX_CTL_CMD (Tx_EnTxPar | Tx_EnLateColl | \
Tx_EnExColl | Tx_EnLCarr | Tx_EnExDefer | Tx_EnUnder | \
Tx_En) /* maybe 0x7b01 */
-#endif
/* Do not use Rx_StripCRC -- it causes trouble on BLEx/FDAEx condition */
#define RX_CTL_CMD (Rx_EnGood | Rx_EnRxPar | Rx_EnLongErr | Rx_EnOver \
| Rx_EnCRCErr | Rx_EnAlign | Rx_RxEn) /* maybe 0x6f01 */
@@ -357,13 +343,6 @@ struct BDesc {
#define TX_THRESHOLD_KEEP_LIMIT 10
/* 16 + RX_BUF_NUM * 8 + RX_FD_NUM * 16 + TX_FD_NUM * 32 <= PAGE_SIZE*FD_PAGE_NUM */
-#ifdef TC35815_USE_PACKEDBUFFER
-#define FD_PAGE_NUM 2
-#define RX_BUF_NUM 8 /* >= 2 */
-#define RX_FD_NUM 250 /* >= 32 */
-#define TX_FD_NUM 128
-#define RX_BUF_SIZE PAGE_SIZE
-#else /* TC35815_USE_PACKEDBUFFER */
#define FD_PAGE_NUM 4
#define RX_BUF_NUM 128 /* < 256 */
#define RX_FD_NUM 256 /* >= 32 */
@@ -377,7 +356,6 @@ struct BDesc {
#define RX_BUF_SIZE \
L1_CACHE_ALIGN(ETH_FRAME_LEN + VLAN_HLEN + ETH_FCS_LEN + NET_IP_ALIGN)
#endif
-#endif /* TC35815_USE_PACKEDBUFFER */
#define RX_FD_RESERVE (2 / 2) /* max 2 BD per RxFD */
#define NAPI_WEIGHT 16
@@ -435,11 +413,7 @@ struct tc35815_local {
/*
* Transmitting: Batch Mode.
* 1 BD in 1 TxFD.
- * Receiving: Packing Mode. (TC35815_USE_PACKEDBUFFER)
- * 1 circular FD for Free Buffer List.
- * RX_BUF_NUM BD in Free Buffer FD.
- * One Free Buffer BD has PAGE_SIZE data buffer.
- * Or Non-Packing Mode.
+ * Receiving: Non-Packing Mode.
* 1 circular FD for Free Buffer List.
* RX_BUF_NUM BD in Free Buffer FD.
* One Free Buffer BD has ETH_FRAME_LEN data buffer.
@@ -453,21 +427,11 @@ struct tc35815_local {
struct RxFD *rfd_limit;
struct RxFD *rfd_cur;
struct FrFD *fbl_ptr;
-#ifdef TC35815_USE_PACKEDBUFFER
- unsigned char fbl_curid;
- void *data_buf[RX_BUF_NUM]; /* packing */
- dma_addr_t data_buf_dma[RX_BUF_NUM];
- struct {
- struct sk_buff *skb;
- dma_addr_t skb_dma;
- } tx_skbs[TX_FD_NUM];
-#else
unsigned int fbl_count;
struct {
struct sk_buff *skb;
dma_addr_t skb_dma;
} tx_skbs[TX_FD_NUM], rx_skbs[RX_BUF_NUM];
-#endif
u32 msg_enable;
enum tc35815_chiptype chiptype;
};
@@ -482,51 +446,6 @@ static inline void *fd_bus_to_virt(struct tc35815_local *lp, dma_addr_t bus)
return (void *)((u8 *)lp->fd_buf + (bus - lp->fd_buf_dma));
}
#endif
-#ifdef TC35815_USE_PACKEDBUFFER
-static inline void *rxbuf_bus_to_virt(struct tc35815_local *lp, dma_addr_t bus)
-{
- int i;
- for (i = 0; i < RX_BUF_NUM; i++) {
- if (bus >= lp->data_buf_dma[i] &&
- bus < lp->data_buf_dma[i] + PAGE_SIZE)
- return (void *)((u8 *)lp->data_buf[i] +
- (bus - lp->data_buf_dma[i]));
- }
- return NULL;
-}
-
-#define TC35815_DMA_SYNC_ONDEMAND
-static void *alloc_rxbuf_page(struct pci_dev *hwdev, dma_addr_t *dma_handle)
-{
-#ifdef TC35815_DMA_SYNC_ONDEMAND
- void *buf;
- /* pci_map + pci_dma_sync will be more effective than
- * pci_alloc_consistent on some archs. */
- buf = (void *)__get_free_page(GFP_ATOMIC);
- if (!buf)
- return NULL;
- *dma_handle = pci_map_single(hwdev, buf, PAGE_SIZE,
- PCI_DMA_FROMDEVICE);
- if (pci_dma_mapping_error(hwdev, *dma_handle)) {
- free_page((unsigned long)buf);
- return NULL;
- }
- return buf;
-#else
- return pci_alloc_consistent(hwdev, PAGE_SIZE, dma_handle);
-#endif
-}
-
-static void free_rxbuf_page(struct pci_dev *hwdev, void *buf, dma_addr_t dma_handle)
-{
-#ifdef TC35815_DMA_SYNC_ONDEMAND
- pci_unmap_single(hwdev, dma_handle, PAGE_SIZE, PCI_DMA_FROMDEVICE);
- free_page((unsigned long)buf);
-#else
- pci_free_consistent(hwdev, PAGE_SIZE, buf, dma_handle);
-#endif
-}
-#else /* TC35815_USE_PACKEDBUFFER */
static struct sk_buff *alloc_rxbuf_skb(struct net_device *dev,
struct pci_dev *hwdev,
dma_addr_t *dma_handle)
@@ -551,7 +470,6 @@ static void free_rxbuf_skb(struct pci_dev *hwdev, struct sk_buff *skb, dma_addr_
PCI_DMA_FROMDEVICE);
dev_kfree_skb_any(skb);
}
-#endif /* TC35815_USE_PACKEDBUFFER */
/* Index to functions, as function prototypes. */
@@ -646,8 +564,6 @@ static void tc_handle_link_change(struct net_device *dev)
* TX4939 PCFG.SPEEDn bit will be changed on
* NETDEV_CHANGE event.
*/
-
-#if !defined(NO_CHECK_CARRIER) && defined(WORKAROUND_LOSTCAR)
/*
* WORKAROUND: enable LostCrS only if half duplex
* operation.
@@ -657,7 +573,6 @@ static void tc_handle_link_change(struct net_device *dev)
lp->chiptype != TC35815_TX4939)
tc_writel(tc_readl(&tr->Tx_Ctl) | Tx_EnLCarr,
&tr->Tx_Ctl);
-#endif
lp->speed = phydev->speed;
lp->duplex = phydev->duplex;
@@ -666,11 +581,9 @@ static void tc_handle_link_change(struct net_device *dev)
if (phydev->link != lp->link) {
if (phydev->link) {
-#ifdef WORKAROUND_100HALF_PROMISC
/* delayed promiscuous enabling */
if (dev->flags & IFF_PROMISC)
tc35815_set_multicast_list(dev);
-#endif
} else {
lp->speed = 0;
lp->duplex = -1;
@@ -997,25 +910,6 @@ tc35815_init_queues(struct net_device *dev)
if (!lp->fd_buf)
return -ENOMEM;
for (i = 0; i < RX_BUF_NUM; i++) {
-#ifdef TC35815_USE_PACKEDBUFFER
- lp->data_buf[i] =
- alloc_rxbuf_page(lp->pci_dev,
- &lp->data_buf_dma[i]);
- if (!lp->data_buf[i]) {
- while (--i >= 0) {
- free_rxbuf_page(lp->pci_dev,
- lp->data_buf[i],
- lp->data_buf_dma[i]);
- lp->data_buf[i] = NULL;
- }
- pci_free_consistent(lp->pci_dev,
- PAGE_SIZE * FD_PAGE_NUM,
- lp->fd_buf,
- lp->fd_buf_dma);
- lp->fd_buf = NULL;
- return -ENOMEM;
- }
-#else
lp->rx_skbs[i].skb =
alloc_rxbuf_skb(dev, lp->pci_dev,
&lp->rx_skbs[i].skb_dma);
@@ -1033,15 +927,9 @@ tc35815_init_queues(struct net_device *dev)
lp->fd_buf = NULL;
return -ENOMEM;
}
-#endif
}
printk(KERN_DEBUG "%s: FD buf %p DataBuf",
dev->name, lp->fd_buf);
-#ifdef TC35815_USE_PACKEDBUFFER
- printk(" DataBuf");
- for (i = 0; i < RX_BUF_NUM; i++)
- printk(" %p", lp->data_buf[i]);
-#endif
printk("\n");
} else {
for (i = 0; i < FD_PAGE_NUM; i++)
@@ -1074,7 +962,6 @@ tc35815_init_queues(struct net_device *dev)
lp->fbl_ptr = (struct FrFD *)fd_addr;
lp->fbl_ptr->fd.FDNext = cpu_to_le32(fd_virt_to_bus(lp, lp->fbl_ptr));
lp->fbl_ptr->fd.FDCtl = cpu_to_le32(RX_BUF_NUM | FD_CownsFD);
-#ifndef TC35815_USE_PACKEDBUFFER
/*
* move all allocated skbs to head of rx_skbs[] array.
* fbl_count mighe not be RX_BUF_NUM if alloc_rxbuf_skb() in
@@ -1092,11 +979,7 @@ tc35815_init_queues(struct net_device *dev)
lp->fbl_count++;
}
}
-#endif
for (i = 0; i < RX_BUF_NUM; i++) {
-#ifdef TC35815_USE_PACKEDBUFFER
- lp->fbl_ptr->bd[i].BuffData = cpu_to_le32(lp->data_buf_dma[i]);
-#else
if (i >= lp->fbl_count) {
lp->fbl_ptr->bd[i].BuffData = 0;
lp->fbl_ptr->bd[i].BDCtl = 0;
@@ -1104,15 +987,11 @@ tc35815_init_queues(struct net_device *dev)
}
lp->fbl_ptr->bd[i].BuffData =
cpu_to_le32(lp->rx_skbs[i].skb_dma);
-#endif
/* BDID is index of FrFD.bd[] */
lp->fbl_ptr->bd[i].BDCtl =
cpu_to_le32(BD_CownsBD | (i << BD_RxBDID_SHIFT) |
RX_BUF_SIZE);
}
-#ifdef TC35815_USE_PACKEDBUFFER
- lp->fbl_curid = 0;
-#endif
printk(KERN_DEBUG "%s: TxFD %p RxFD %p FrFD %p\n",
dev->name, lp->tfd_base, lp->rfd_base, lp->fbl_ptr);
@@ -1186,19 +1065,11 @@ tc35815_free_queues(struct net_device *dev)
lp->fbl_ptr = NULL;
for (i = 0; i < RX_BUF_NUM; i++) {
-#ifdef TC35815_USE_PACKEDBUFFER
- if (lp->data_buf[i]) {
- free_rxbuf_page(lp->pci_dev,
- lp->data_buf[i], lp->data_buf_dma[i]);
- lp->data_buf[i] = NULL;
- }
-#else
if (lp->rx_skbs[i].skb) {
free_rxbuf_skb(lp->pci_dev, lp->rx_skbs[i].skb,
lp->rx_skbs[i].skb_dma);
lp->rx_skbs[i].skb = NULL;
}
-#endif
}
if (lp->fd_buf) {
pci_free_consistent(lp->pci_dev, PAGE_SIZE * FD_PAGE_NUM,
@@ -1244,7 +1115,7 @@ dump_rxfd(struct RxFD *fd)
return bd_count;
}
-#if defined(DEBUG) || defined(TC35815_USE_PACKEDBUFFER)
+#ifdef DEBUG
static void
dump_frfd(struct FrFD *fd)
{
@@ -1261,9 +1132,7 @@ dump_frfd(struct FrFD *fd)
le32_to_cpu(fd->bd[i].BDCtl));
printk("\n");
}
-#endif
-#ifdef DEBUG
static void
panic_queues(struct net_device *dev)
{
@@ -1466,9 +1335,7 @@ static int tc35815_send_packet(struct sk_buff *skb, struct net_device *dev)
(struct tc35815_regs __iomem *)dev->base_addr;
/* Start DMA Transmitter. */
txfd->fd.FDNext |= cpu_to_le32(FD_Next_EOL);
-#ifdef GATHER_TXINT
txfd->fd.FDCtl |= cpu_to_le32(FD_FrmOpt_IntTx);
-#endif
if (netif_msg_tx_queued(lp)) {
printk("%s: starting TxFD.\n", dev->name);
dump_txfd(txfd);
@@ -1640,50 +1507,9 @@ tc35815_rx(struct net_device *dev, int limit)
struct sk_buff *skb;
unsigned char *data;
int cur_bd;
-#ifdef TC35815_USE_PACKEDBUFFER
- int offset;
-#endif
if (--limit < 0)
break;
-#ifdef TC35815_USE_PACKEDBUFFER
- BUG_ON(bd_count > 2);
- skb = dev_alloc_skb(pkt_len + NET_IP_ALIGN);
- if (skb == NULL) {
- printk(KERN_NOTICE "%s: Memory squeeze, dropping packet.\n",
- dev->name);
- dev->stats.rx_dropped++;
- break;
- }
- skb_reserve(skb, NET_IP_ALIGN);
-
- data = skb_put(skb, pkt_len);
-
- /* copy from receive buffer */
- cur_bd = 0;
- offset = 0;
- while (offset < pkt_len && cur_bd < bd_count) {
- int len = le32_to_cpu(lp->rfd_cur->bd[cur_bd].BDCtl) &
- BD_BuffLength_MASK;
- dma_addr_t dma = le32_to_cpu(lp->rfd_cur->bd[cur_bd].BuffData);
- void *rxbuf = rxbuf_bus_to_virt(lp, dma);
- if (offset + len > pkt_len)
- len = pkt_len - offset;
-#ifdef TC35815_DMA_SYNC_ONDEMAND
- pci_dma_sync_single_for_cpu(lp->pci_dev,
- dma, len,
- PCI_DMA_FROMDEVICE);
-#endif
- memcpy(data + offset, rxbuf, len);
-#ifdef TC35815_DMA_SYNC_ONDEMAND
- pci_dma_sync_single_for_device(lp->pci_dev,
- dma, len,
- PCI_DMA_FROMDEVICE);
-#endif
- offset += len;
- cur_bd++;
- }
-#else /* TC35815_USE_PACKEDBUFFER */
BUG_ON(bd_count > 1);
cur_bd = (le32_to_cpu(lp->rfd_cur->bd[0].BDCtl)
& BD_RxBDID_MASK) >> BD_RxBDID_SHIFT;
@@ -1711,7 +1537,6 @@ tc35815_rx(struct net_device *dev, int limit)
memmove(skb->data, skb->data - NET_IP_ALIGN,
pkt_len);
data = skb_put(skb, pkt_len);
-#endif /* TC35815_USE_PACKEDBUFFER */
if (netif_msg_pktdata(lp))
print_eth(data);
skb->protocol = eth_type_trans(skb, dev);
@@ -1753,19 +1578,11 @@ tc35815_rx(struct net_device *dev, int limit)
BUG_ON(id >= RX_BUF_NUM);
#endif
/* free old buffers */
-#ifdef TC35815_USE_PACKEDBUFFER
- while (lp->fbl_curid != id)
-#else
lp->fbl_count--;
while (lp->fbl_count < RX_BUF_NUM)
-#endif
{
-#ifdef TC35815_USE_PACKEDBUFFER
- unsigned char curid = lp->fbl_curid;
-#else
unsigned char curid =
(id + 1 + lp->fbl_count) % RX_BUF_NUM;
-#endif
struct BDesc *bd = &lp->fbl_ptr->bd[curid];
#ifdef DEBUG
bdctl = le32_to_cpu(bd->BDCtl);
@@ -1776,7 +1593,6 @@ tc35815_rx(struct net_device *dev, int limit)
}
#endif
/* pass BD to controller */
-#ifndef TC35815_USE_PACKEDBUFFER
if (!lp->rx_skbs[curid].skb) {
lp->rx_skbs[curid].skb =
alloc_rxbuf_skb(dev,
@@ -1786,21 +1602,11 @@ tc35815_rx(struct net_device *dev, int limit)
break; /* try on next reception */
bd->BuffData = cpu_to_le32(lp->rx_skbs[curid].skb_dma);
}
-#endif /* TC35815_USE_PACKEDBUFFER */
/* Note: BDLength was modified by chip. */
bd->BDCtl = cpu_to_le32(BD_CownsBD |
(curid << BD_RxBDID_SHIFT) |
RX_BUF_SIZE);
-#ifdef TC35815_USE_PACKEDBUFFER
- lp->fbl_curid = (curid + 1) % RX_BUF_NUM;
- if (netif_msg_rx_status(lp)) {
- printk("%s: Entering new FBD %d\n",
- dev->name, lp->fbl_curid);
- dump_frfd(lp->fbl_ptr);
- }
-#else
lp->fbl_count++;
-#endif
}
}
@@ -1872,11 +1678,7 @@ static int tc35815_poll(struct napi_struct *napi, int budget)
return received;
}
-#ifdef NO_CHECK_CARRIER
-#define TX_STA_ERR (Tx_ExColl|Tx_Under|Tx_Defer|Tx_LateColl|Tx_TxPar|Tx_SQErr)
-#else
#define TX_STA_ERR (Tx_ExColl|Tx_Under|Tx_Defer|Tx_NCarr|Tx_LateColl|Tx_TxPar|Tx_SQErr)
-#endif
static void
tc35815_check_tx_stat(struct net_device *dev, int status)
@@ -1890,16 +1692,12 @@ tc35815_check_tx_stat(struct net_device *dev, int status)
if (status & Tx_TxColl_MASK)
dev->stats.collisions += status & Tx_TxColl_MASK;
-#ifndef NO_CHECK_CARRIER
/* TX4939 does not have NCarr */
if (lp->chiptype == TC35815_TX4939)
status &= ~Tx_NCarr;
-#ifdef WORKAROUND_LOSTCAR
/* WORKAROUND: ignore LostCrS in full duplex operation */
if (!lp->link || lp->duplex == DUPLEX_FULL)
status &= ~Tx_NCarr;
-#endif
-#endif
if (!(status & TX_STA_ERR)) {
/* no error. */
@@ -1929,12 +1727,10 @@ tc35815_check_tx_stat(struct net_device *dev, int status)
dev->stats.tx_fifo_errors++;
msg = "Excessive Deferral.";
}
-#ifndef NO_CHECK_CARRIER
if (status & Tx_NCarr) {
dev->stats.tx_carrier_errors++;
msg = "Lost Carrier Sense.";
}
-#endif
if (status & Tx_LateColl) {
dev->stats.tx_aborted_errors++;
msg = "Late Collision.";
@@ -2025,9 +1821,7 @@ tc35815_txdone(struct net_device *dev)
/* start DMA Transmitter again */
txhead->fd.FDNext |= cpu_to_le32(FD_Next_EOL);
-#ifdef GATHER_TXINT
txhead->fd.FDCtl |= cpu_to_le32(FD_FrmOpt_IntTx);
-#endif
if (netif_msg_tx_queued(lp)) {
printk("%s: start TxFD on queue.\n",
dev->name);
@@ -2138,14 +1932,12 @@ tc35815_set_multicast_list(struct net_device *dev)
(struct tc35815_regs __iomem *)dev->base_addr;
if (dev->flags & IFF_PROMISC) {
-#ifdef WORKAROUND_100HALF_PROMISC
/* With some (all?) 100MHalf HUB, controller will hang
* if we enabled promiscuous mode before linkup... */
struct tc35815_local *lp = netdev_priv(dev);
if (!lp->link)
return;
-#endif
/* Enable promiscuous mode */
tc_writel(CAM_CompEn | CAM_BroadAcc | CAM_GroupAcc | CAM_StationAcc, &tr->CAM_Ctl);
} else if ((dev->flags & IFF_ALLMULTI) ||
@@ -2332,9 +2124,6 @@ static void tc35815_chip_init(struct net_device *dev)
tc_writel(DMA_BURST_SIZE | DMA_RxAlign_2, &tr->DMA_Ctl);
else
tc_writel(DMA_BURST_SIZE, &tr->DMA_Ctl);
-#ifdef TC35815_USE_PACKEDBUFFER
- tc_writel(RxFrag_EnPack | ETH_ZLEN, &tr->RxFragSize); /* Packing */
-#endif
tc_writel(0, &tr->TxPollCtr); /* Batch mode */
tc_writel(TX_THRESHOLD, &tr->TxThrsh);
tc_writel(INT_EN_CMD, &tr->Int_En);
@@ -2352,19 +2141,12 @@ static void tc35815_chip_init(struct net_device *dev)
tc_writel(RX_CTL_CMD, &tr->Rx_Ctl); /* start MAC receiver */
/* start MAC transmitter */
-#ifndef NO_CHECK_CARRIER
/* TX4939 does not have EnLCarr */
if (lp->chiptype == TC35815_TX4939)
txctl &= ~Tx_EnLCarr;
-#ifdef WORKAROUND_LOSTCAR
/* WORKAROUND: ignore LostCrS in full duplex operation */
if (!lp->phy_dev || !lp->link || lp->duplex == DUPLEX_FULL)
txctl &= ~Tx_EnLCarr;
-#endif
-#endif /* !NO_CHECK_CARRIER */
-#ifdef GATHER_TXINT
- txctl &= ~Tx_EnComp; /* disable global tx completion int. */
-#endif
tc_writel(txctl, &tr->Tx_Ctl);
}
--
1.5.6.5
^ permalink raw reply related
* [PATCH 1/2] tc35815: Kill non-napi code
From: Atsushi Nemoto @ 2009-11-02 14:34 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Ralf Roesch
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
drivers/net/tc35815.c | 68 +------------------------------------------------
1 files changed, 1 insertions(+), 67 deletions(-)
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index 0d621ca..803eb64 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -22,12 +22,7 @@
* All Rights Reserved.
*/
-#define TC35815_NAPI
-#ifdef TC35815_NAPI
-#define DRV_VERSION "1.38-NAPI"
-#else
-#define DRV_VERSION "1.38"
-#endif
+#define DRV_VERSION "1.39"
static const char *version = "tc35815.c:v" DRV_VERSION "\n";
#define MODNAME "tc35815"
@@ -563,12 +558,8 @@ static void free_rxbuf_skb(struct pci_dev *hwdev, struct sk_buff *skb, dma_addr_
static int tc35815_open(struct net_device *dev);
static int tc35815_send_packet(struct sk_buff *skb, struct net_device *dev);
static irqreturn_t tc35815_interrupt(int irq, void *dev_id);
-#ifdef TC35815_NAPI
static int tc35815_rx(struct net_device *dev, int limit);
static int tc35815_poll(struct napi_struct *napi, int budget);
-#else
-static void tc35815_rx(struct net_device *dev);
-#endif
static void tc35815_txdone(struct net_device *dev);
static int tc35815_close(struct net_device *dev);
static struct net_device_stats *tc35815_get_stats(struct net_device *dev);
@@ -924,9 +915,7 @@ static int __devinit tc35815_init_one(struct pci_dev *pdev,
dev->netdev_ops = &tc35815_netdev_ops;
dev->ethtool_ops = &tc35815_ethtool_ops;
dev->watchdog_timeo = TC35815_TX_TIMEOUT;
-#ifdef TC35815_NAPI
netif_napi_add(dev, &lp->napi, tc35815_poll, NAPI_WEIGHT);
-#endif
dev->irq = pdev->irq;
dev->base_addr = (unsigned long)ioaddr;
@@ -1401,9 +1390,7 @@ tc35815_open(struct net_device *dev)
return -EAGAIN;
}
-#ifdef TC35815_NAPI
napi_enable(&lp->napi);
-#endif
/* Reset the hardware here. Don't forget to set the station address. */
spin_lock_irq(&lp->lock);
@@ -1537,11 +1524,7 @@ static void tc35815_fatal_error_interrupt(struct net_device *dev, u32 status)
tc35815_schedule_restart(dev);
}
-#ifdef TC35815_NAPI
static int tc35815_do_interrupt(struct net_device *dev, u32 status, int limit)
-#else
-static int tc35815_do_interrupt(struct net_device *dev, u32 status)
-#endif
{
struct tc35815_local *lp = netdev_priv(dev);
int ret = -1;
@@ -1580,12 +1563,7 @@ static int tc35815_do_interrupt(struct net_device *dev, u32 status)
/* normal notification */
if (status & Int_IntMacRx) {
/* Got a packet(s). */
-#ifdef TC35815_NAPI
ret = tc35815_rx(dev, limit);
-#else
- tc35815_rx(dev);
- ret = 0;
-#endif
lp->lstats.rx_ints++;
}
if (status & Int_IntMacTx) {
@@ -1593,12 +1571,8 @@ static int tc35815_do_interrupt(struct net_device *dev, u32 status)
lp->lstats.tx_ints++;
tc35815_txdone(dev);
netif_wake_queue(dev);
-#ifdef TC35815_NAPI
if (ret < 0)
ret = 0;
-#else
- ret = 0;
-#endif
}
return ret;
}
@@ -1613,7 +1587,6 @@ static irqreturn_t tc35815_interrupt(int irq, void *dev_id)
struct tc35815_local *lp = netdev_priv(dev);
struct tc35815_regs __iomem *tr =
(struct tc35815_regs __iomem *)dev->base_addr;
-#ifdef TC35815_NAPI
u32 dmactl = tc_readl(&tr->DMA_Ctl);
if (!(dmactl & DMA_IntMask)) {
@@ -1630,22 +1603,6 @@ static irqreturn_t tc35815_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
return IRQ_NONE;
-#else
- int handled;
- u32 status;
-
- spin_lock(&lp->lock);
- status = tc_readl(&tr->Int_Src);
- /* BLEx, FDAEx will be cleared later */
- tc_writel(status & ~(Int_BLEx | Int_FDAEx),
- &tr->Int_Src); /* write to clear */
- handled = tc35815_do_interrupt(dev, status);
- if (status & (Int_BLEx | Int_FDAEx))
- tc_writel(status & (Int_BLEx | Int_FDAEx), &tr->Int_Src);
- (void)tc_readl(&tr->Int_Src); /* flush */
- spin_unlock(&lp->lock);
- return IRQ_RETVAL(handled >= 0);
-#endif /* TC35815_NAPI */
}
#ifdef CONFIG_NET_POLL_CONTROLLER
@@ -1658,20 +1615,13 @@ static void tc35815_poll_controller(struct net_device *dev)
#endif
/* We have a good packet(s), get it/them out of the buffers. */
-#ifdef TC35815_NAPI
static int
tc35815_rx(struct net_device *dev, int limit)
-#else
-static void
-tc35815_rx(struct net_device *dev)
-#endif
{
struct tc35815_local *lp = netdev_priv(dev);
unsigned int fdctl;
int i;
-#ifdef TC35815_NAPI
int received = 0;
-#endif
while (!((fdctl = le32_to_cpu(lp->rfd_cur->fd.FDCtl)) & FD_CownsFD)) {
int status = le32_to_cpu(lp->rfd_cur->fd.FDStat);
@@ -1694,10 +1644,8 @@ tc35815_rx(struct net_device *dev)
int offset;
#endif
-#ifdef TC35815_NAPI
if (--limit < 0)
break;
-#endif
#ifdef TC35815_USE_PACKEDBUFFER
BUG_ON(bd_count > 2);
skb = dev_alloc_skb(pkt_len + NET_IP_ALIGN);
@@ -1767,12 +1715,8 @@ tc35815_rx(struct net_device *dev)
if (netif_msg_pktdata(lp))
print_eth(data);
skb->protocol = eth_type_trans(skb, dev);
-#ifdef TC35815_NAPI
netif_receive_skb(skb);
received++;
-#else
- netif_rx(skb);
-#endif
dev->stats.rx_packets++;
dev->stats.rx_bytes += pkt_len;
} else {
@@ -1888,12 +1832,9 @@ tc35815_rx(struct net_device *dev)
#endif
}
-#ifdef TC35815_NAPI
return received;
-#endif
}
-#ifdef TC35815_NAPI
static int tc35815_poll(struct napi_struct *napi, int budget)
{
struct tc35815_local *lp = container_of(napi, struct tc35815_local, napi);
@@ -1930,7 +1871,6 @@ static int tc35815_poll(struct napi_struct *napi, int budget)
}
return received;
}
-#endif
#ifdef NO_CHECK_CARRIER
#define TX_STA_ERR (Tx_ExColl|Tx_Under|Tx_Defer|Tx_LateColl|Tx_TxPar|Tx_SQErr)
@@ -2050,11 +1990,7 @@ tc35815_txdone(struct net_device *dev)
pci_unmap_single(lp->pci_dev, lp->tx_skbs[lp->tfd_end].skb_dma, skb->len, PCI_DMA_TODEVICE);
lp->tx_skbs[lp->tfd_end].skb = NULL;
lp->tx_skbs[lp->tfd_end].skb_dma = 0;
-#ifdef TC35815_NAPI
dev_kfree_skb_any(skb);
-#else
- dev_kfree_skb_irq(skb);
-#endif
}
txfd->fd.FDSystem = cpu_to_le32(0xffffffff);
@@ -2118,9 +2054,7 @@ tc35815_close(struct net_device *dev)
struct tc35815_local *lp = netdev_priv(dev);
netif_stop_queue(dev);
-#ifdef TC35815_NAPI
napi_disable(&lp->napi);
-#endif
if (lp->phy_dev)
phy_stop(lp->phy_dev);
cancel_work_sync(&lp->restart_work);
--
1.5.6.5
^ permalink raw reply related
* Re: [net-next-2.6 PATCH v4 3/3] TCPCT part 1c: initial SYN exchange with SYNACK data
From: Eric Dumazet @ 2009-11-02 13:31 UTC (permalink / raw)
To: William Allen Simpson; +Cc: Linux Kernel Network Developers
In-Reply-To: <4AEECFA8.1080306@gmail.com>
William Allen Simpson a écrit :
> Eric Dumazet wrote:
>> This part is really hard to review, and might be splitted ?
>>
>> cleanups could be done in a cleanup patch only
>>
>> Examples:
>>
>> - tmp_opt.mss_clamp = 536;
>> - tmp_opt.user_mss = tcp_sk(sk)->rx_opt.user_mss;
>> + tmp_opt.mss_clamp = TCP_MIN_RCVMSS;
>> + tmp_opt.user_mss = tp->rx_opt.user_mss;
>>
>>
>> - tp->mss_cache = 536;
>> + tp->mss_cache = TCP_MIN_RCVMSS;
>>
> Often hard to decide what's "cleanup" and what's essential. I'll look at
> that again for the next round, but I've already split the original single
> patch into multiple parts.
cleanups are trivial, and should be separated from functionnal changes.
>
>
>> Also your tests are reversed, if you look at the existing coding style.
>>
> I checked Documentation/CodingStyle, and that's not specified. I've seen
> plenty of examples of modern security coding style around here.
>
> As a long-time (25+ years) consultant and 30 years C programmer, I'm
> heedful of the project coding style, and had to endure many variants.
>
> Where I'm working with others' code, you'll note that I keep the same
> style, no matter how ugly, as that makes patches easier to read.
>
>
>> Example :
>>
>> + /* TCP Cookie Transactions */
>> + if (0 < sysctl_tcp_cookie_size) {
>> + /* Default, cookies without s_data. */
>> + tp->cookie_values =
>> + kzalloc(sizeof(*tp->cookie_values),
>> + sk->sk_allocation);
>> + if (NULL != tp->cookie_values)
>> + kref_init(&tp->cookie_values->kref);
>> + }
>>
>> should be ->
>>
>> + /* TCP Cookie Transactions */
>> + if (sysctl_tcp_cookie_size > 0) {
>> + /* Default, cookies without s_data. */
>> + tp->cookie_values =
>> + kzalloc(sizeof(*tp->cookie_values),
>> + sk->sk_allocation);
>> + if (tp->cookie_values != NULL)
>> + kref_init(&tp->cookie_values->kref);
>> + }
>>
> And "tp->cookie_values != NULL" is egregiously poor C practice. It's very
> hard for code review to ensure that didn't get truncated to "= NULL". The
> important visual element is the NULL, not the variable name.
Maybe, but check in linux source code and you'll see this poor pratice is the facto.
Dont try to change our minds, because it wont happen.
>
> Also, avoid "!tp->cookie_values", as this is *not* a boolean.
Oh, good to learn that ! operator only applies to boolean. I didnt know that.
>
> When I'm adding new code, I use constant-to-the-left security coding style,
> as they teach in modern universities (lately also for PHP). And this is a
> security extension, so a security style is particularly appropriate.
>
> As in switch statements, constant-to-the-left makes the value obvious,
> especially in a series (and assists transforming if series into a switch).
>
> For complex tests, this makes the code much more readable and easier to
> visually verify on code walk-through:
>
> + if (0 < tmp_opt.cookie_plus
> + && tmp_opt.saw_tstamp
> + && !tp->cookie_out_never
> + && (0 < sysctl_tcp_cookie_size
> + || (NULL != tp->cookie_values
> + && 0 < tp->cookie_values->cookie_desired))) {
>
> Consistent use of security style would have obviated a lot of foolish >= 0
> tests that seem to be constantly in need of fixing. It's a bad idea to
> depend on the compiler to catch non-executable code.
You can _talk_, I can stop reviewing your patches, and wait another gentle guy do the job,
because I am 30 years experimented (and tired ?) programmer, and dont want to
lose my time to discuss Coding-Style with you ?
Cooking patches to linux is not only matter of good ideas and programming (and Dropping
patches for the masses).
Its also a matter of convincing _people_ that your additions will be maintainable
when you leave kernel programming and let people like us correct bugs.
For the moment, I am not convinced at all. I prefer to talk now.
Note: I did read your TCPCT 25 pages documentation and very am interested by this
improvement, but its _also_ important to implement it in the normal way.
(I wish this document could be public in a RFC form)
^ permalink raw reply
* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: Eric Dumazet @ 2009-11-02 13:16 UTC (permalink / raw)
To: William Allen Simpson
Cc: Linux Kernel Developers, Linux Kernel Network Developers
In-Reply-To: <4AEED23A.7070009@gmail.com>
William Allen Simpson a écrit :
> Eric Dumazet wrote:
>> cookie_hash() runs in a non preemptable context. CPU cannot change
>> under us.
>>
>> (or else, we would not use __get_cpu_var(ipv4_cookie_scratch); )
>>
>> And of course, each cpu gets its own scratch area, thanks to
>> __get_cpu_var()
>>
> Interesting. I'm not sure that running CPU intensive functions like
> SHA1 in
> a non-preemptable context is a good idea. I'd assumed it wasn't!
>
> Perhaps you could point at the documentation in the code that explains
> this?
I suggest you read Documentations/ files about softirq
http://docs.blackfin.uclinux.org/kernel/generated/kernel-hacking.xml
Large part of network code is run by softirq handler, and a softirq handler
is not preemptable with another softirq (including itself).
> Perhaps a function header comment that mentions it?
So we are going to add a header to thousand of functions repeating this prereq ?
>
> All I know is (from testing) that the tcp_minisockets.c caller is sometimes
> called in a fashion that requires atomic allocation, and other times
> does not!
Maybe callers have different contexts (running from softirq handler or
from process context). Atomic ops are expensive and we try to avoid them
if/when possible.
>
> See my "Subject: query: tcpdump versus atomic?" thread from Oct 14th.
You probably add a bug in your kernel, leaving a function with unpaired lock/unlock
of notallow_something/allow_something
There are books about linux internals that you could read if you want some extra
documentation. Dont ask me details, I never read them :)
^ permalink raw reply
* Re: [RFC] multiqueue changes
From: Jarek Poplawski @ 2009-11-02 13:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, mchan, kaber, netdev
In-Reply-To: <4AEED899.9040106@gmail.com>
On Mon, Nov 02, 2009 at 02:03:21PM +0100, Eric Dumazet wrote:
...
> Sure, but I prefer a patch from you ;)
>
Wrong code! (RFP? ;-)
Jarek P.
^ permalink raw reply
* Re: [RFC] multiqueue changes
From: Eric Dumazet @ 2009-11-02 13:03 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, mchan, kaber, netdev
In-Reply-To: <20091102130223.GB7790@ff.dom.local>
Jarek Poplawski a écrit :
> On Mon, Nov 02, 2009 at 04:39:07AM -0800, David Miller wrote:
>> From: Jarek Poplawski <jarkao2@gmail.com>
>> Date: Mon, 2 Nov 2009 12:30:29 +0000
>>
>>> Right, but it's not a 50% chance, I guess? A user most of the time
>>> gets consistently multiqueue or non-multiqueue behavior after open,
>>> unless I miss something. Then such an exceptional state could be
>>> handled by real_num_tx_queues (just like in case of powered of cpus).
>>> The main difference is to hold in num_tx_queues something that is
>>> really available vs max possible value for all configs.
>> I see your point, yes this would seem to be a reasonable way
>> to start handling num_tx_queues and real_num_tx_queues.
>
> Very nice! So, I hope Eric should be satisfied with these requested
> comments already :-)
>
Sure, but I prefer a patch from you ;)
^ permalink raw reply
* Re: [RFC] multiqueue changes
From: Jarek Poplawski @ 2009-11-02 13:02 UTC (permalink / raw)
To: David Miller; +Cc: mchan, kaber, eric.dumazet, netdev
In-Reply-To: <20091102.043907.236634594.davem@davemloft.net>
On Mon, Nov 02, 2009 at 04:39:07AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Mon, 2 Nov 2009 12:30:29 +0000
>
> > Right, but it's not a 50% chance, I guess? A user most of the time
> > gets consistently multiqueue or non-multiqueue behavior after open,
> > unless I miss something. Then such an exceptional state could be
> > handled by real_num_tx_queues (just like in case of powered of cpus).
> > The main difference is to hold in num_tx_queues something that is
> > really available vs max possible value for all configs.
>
> I see your point, yes this would seem to be a reasonable way
> to start handling num_tx_queues and real_num_tx_queues.
Very nice! So, I hope Eric should be satisfied with these requested
comments already :-)
Thanks everybody,
Jarek P.
^ permalink raw reply
* Re: [net-next-2.6 PATCH v4 3/3] TCPCT part 1c: initial SYN exchange with SYNACK data
From: Ilpo Järvinen @ 2009-11-02 12:57 UTC (permalink / raw)
To: William Allen Simpson; +Cc: Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <4AEECFA8.1080306@gmail.com>
On Mon, 2 Nov 2009, William Allen Simpson wrote:
> Eric Dumazet wrote:
> > This part is really hard to review, and might be splitted ?
> >
> > cleanups could be done in a cleanup patch only
> >
> > Examples:
> >
> > - tmp_opt.mss_clamp = 536;
> > - tmp_opt.user_mss = tcp_sk(sk)->rx_opt.user_mss;
> > + tmp_opt.mss_clamp = TCP_MIN_RCVMSS;
> > + tmp_opt.user_mss = tp->rx_opt.user_mss;
> >
> >
> > - tp->mss_cache = 536;
> > + tp->mss_cache = TCP_MIN_RCVMSS;
> >
> Often hard to decide what's "cleanup" and what's essential. I'll look at
> that again for the next round, but I've already split the original single
> patch into multiple parts.
Are you talking about particular case?!? ...You can safely split into even
more parts if there are cleanups which is essential. ...We'll not stop you
from doing that nor be angry if do that.
> > Also your tests are reversed, if you look at the existing coding style.
> >
> I checked Documentation/CodingStyle, and that's not specified. I've seen
> plenty of examples of modern security coding style around here.
>
> As a long-time (25+ years) consultant and 30 years C programmer, I'm
> heedful of the project coding style, and had to endure many variants.
>
> Where I'm working with others' code, you'll note that I keep the same
> style, no matter how ugly, as that makes patches easier to read.
>
>
> > Example :
> >
> > + /* TCP Cookie Transactions */
> > + if (0 < sysctl_tcp_cookie_size) {
> > + /* Default, cookies without s_data. */
> > + tp->cookie_values =
> > + kzalloc(sizeof(*tp->cookie_values),
> > + sk->sk_allocation);
> > + if (NULL != tp->cookie_values)
> > + kref_init(&tp->cookie_values->kref);
> > + }
> >
> > should be ->
> >
> > + /* TCP Cookie Transactions */
> > + if (sysctl_tcp_cookie_size > 0) {
> > + /* Default, cookies without s_data. */
> > + tp->cookie_values =
> > + kzalloc(sizeof(*tp->cookie_values),
> > + sk->sk_allocation);
> > + if (tp->cookie_values != NULL)
> > + kref_init(&tp->cookie_values->kref);
> > + }
> >
> And "tp->cookie_values != NULL" is egregiously poor C practice. It's very
> hard for code review to ensure that didn't get truncated to "= NULL". The
> important visual element is the NULL, not the variable name.
>
> Also, avoid "!tp->cookie_values", as this is *not* a boolean.
>
> When I'm adding new code, I use constant-to-the-left security coding style,
> as they teach in modern universities (lately also for PHP). And this is a
> security extension, so a security style is particularly appropriate.
>
> As in switch statements, constant-to-the-left makes the value obvious,
> especially in a series (and assists transforming if series into a switch).
>
> For complex tests, this makes the code much more readable and easier to
> visually verify on code walk-through:
>
> + if (0 < tmp_opt.cookie_plus
> + && tmp_opt.saw_tstamp
> + && !tp->cookie_out_never
> + && (0 < sysctl_tcp_cookie_size
> + || (NULL != tp->cookie_values
> + && 0 < tp->cookie_values->cookie_desired))) {
>
> Consistent use of security style would have obviated a lot of foolish >= 0
> tests that seem to be constantly in need of fixing. It's a bad idea to
> depend on the compiler to catch non-executable code.
That kind of response certainly won't help you any. ...First, you said you
adapt the current style but for some reason immediately start to say why
you would careless about that principle. ...Also, telling that you have
lots of experience here and there will not get you there either ;-).
--
i.
^ permalink raw reply
* Re: [RFC] multiqueue changes
From: David Miller @ 2009-11-02 12:39 UTC (permalink / raw)
To: jarkao2; +Cc: mchan, kaber, eric.dumazet, netdev
In-Reply-To: <20091102123029.GA7790@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Mon, 2 Nov 2009 12:30:29 +0000
> Right, but it's not a 50% chance, I guess? A user most of the time
> gets consistently multiqueue or non-multiqueue behavior after open,
> unless I miss something. Then such an exceptional state could be
> handled by real_num_tx_queues (just like in case of powered of cpus).
> The main difference is to hold in num_tx_queues something that is
> really available vs max possible value for all configs.
I see your point, yes this would seem to be a reasonable way
to start handling num_tx_queues and real_num_tx_queues.
^ permalink raw reply
* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: William Allen Simpson @ 2009-11-02 12:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Developers, Linux Kernel Network Developers
In-Reply-To: <4AEEBAC6.7020308@gmail.com>
Eric Dumazet wrote:
> cookie_hash() runs in a non preemptable context. CPU cannot change under us.
>
> (or else, we would not use __get_cpu_var(ipv4_cookie_scratch); )
>
> And of course, each cpu gets its own scratch area, thanks to __get_cpu_var()
>
Interesting. I'm not sure that running CPU intensive functions like SHA1 in
a non-preemptable context is a good idea. I'd assumed it wasn't!
Perhaps you could point at the documentation in the code that explains this?
Perhaps a function header comment that mentions it?
All I know is (from testing) that the tcp_minisockets.c caller is sometimes
called in a fashion that requires atomic allocation, and other times does not!
See my "Subject: query: tcpdump versus atomic?" thread from Oct 14th.
^ permalink raw reply
* Re: [RFC] multiqueue changes
From: Jarek Poplawski @ 2009-11-02 12:30 UTC (permalink / raw)
To: David Miller; +Cc: mchan, kaber, eric.dumazet, netdev
In-Reply-To: <20091102.033533.08766686.davem@davemloft.net>
On Mon, Nov 02, 2009 at 03:35:33AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Sun, 1 Nov 2009 14:20:17 +0100
>
> > There is a question if we can predict in ->probe() MSI-X should be
> > successfully enabled in ->ndo_open() for probed hardware. If so,
> > then it could go e.g. like this:
>
> We never can know this.
>
> Another device driver can eat up all the MSI-X vectors in the PCI
> domain before we make the request_irq() calls in ->open().
Right, but it's not a 50% chance, I guess? A user most of the time
gets consistently multiqueue or non-multiqueue behavior after open,
unless I miss something. Then such an exceptional state could be
handled by real_num_tx_queues (just like in case of powered of cpus).
The main difference is to hold in num_tx_queues something that is
really available vs max possible value for all configs.
Jarek P.
^ permalink raw reply
* Re: [net-next PATCH 0/4] qlge: Add ethtool self-test.
From: David Miller @ 2009-11-02 12:26 UTC (permalink / raw)
To: ron.mercer; +Cc: netdev
In-Reply-To: <1256940816-27540-1-git-send-email-ron.mercer@qlogic.com>
From: Ron Mercer <ron.mercer@qlogic.com>
Date: Fri, 30 Oct 2009 15:13:32 -0700
> Changes for QLGE.
>
> 1) Add ethtool selftest.
> 2) Reduce debug print output.
> 3) Generic cleanup.
I've applied patches #1 and #2
Patch #3 needs to be looked into based upon the feedback we
gave you.
Patch #4 needs to be respun based upon whatever happens to
patch #3.
Thanks Ron.
^ permalink raw reply
* Re: [net-next-2.6 PATCH v4 3/3] TCPCT part 1c: initial SYN exchange with SYNACK data
From: William Allen Simpson @ 2009-11-02 12:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers
In-Reply-To: <4AEDDF33.9030205@gmail.com>
Eric Dumazet wrote:
> This part is really hard to review, and might be splitted ?
>
> cleanups could be done in a cleanup patch only
>
> Examples:
>
> - tmp_opt.mss_clamp = 536;
> - tmp_opt.user_mss = tcp_sk(sk)->rx_opt.user_mss;
> + tmp_opt.mss_clamp = TCP_MIN_RCVMSS;
> + tmp_opt.user_mss = tp->rx_opt.user_mss;
>
>
> - tp->mss_cache = 536;
> + tp->mss_cache = TCP_MIN_RCVMSS;
>
Often hard to decide what's "cleanup" and what's essential. I'll look at
that again for the next round, but I've already split the original single
patch into multiple parts.
> Also your tests are reversed, if you look at the existing coding style.
>
I checked Documentation/CodingStyle, and that's not specified. I've seen
plenty of examples of modern security coding style around here.
As a long-time (25+ years) consultant and 30 years C programmer, I'm
heedful of the project coding style, and had to endure many variants.
Where I'm working with others' code, you'll note that I keep the same
style, no matter how ugly, as that makes patches easier to read.
> Example :
>
> + /* TCP Cookie Transactions */
> + if (0 < sysctl_tcp_cookie_size) {
> + /* Default, cookies without s_data. */
> + tp->cookie_values =
> + kzalloc(sizeof(*tp->cookie_values),
> + sk->sk_allocation);
> + if (NULL != tp->cookie_values)
> + kref_init(&tp->cookie_values->kref);
> + }
>
> should be ->
>
> + /* TCP Cookie Transactions */
> + if (sysctl_tcp_cookie_size > 0) {
> + /* Default, cookies without s_data. */
> + tp->cookie_values =
> + kzalloc(sizeof(*tp->cookie_values),
> + sk->sk_allocation);
> + if (tp->cookie_values != NULL)
> + kref_init(&tp->cookie_values->kref);
> + }
>
And "tp->cookie_values != NULL" is egregiously poor C practice. It's very
hard for code review to ensure that didn't get truncated to "= NULL". The
important visual element is the NULL, not the variable name.
Also, avoid "!tp->cookie_values", as this is *not* a boolean.
When I'm adding new code, I use constant-to-the-left security coding style,
as they teach in modern universities (lately also for PHP). And this is a
security extension, so a security style is particularly appropriate.
As in switch statements, constant-to-the-left makes the value obvious,
especially in a series (and assists transforming if series into a switch).
For complex tests, this makes the code much more readable and easier to
visually verify on code walk-through:
+ if (0 < tmp_opt.cookie_plus
+ && tmp_opt.saw_tstamp
+ && !tp->cookie_out_never
+ && (0 < sysctl_tcp_cookie_size
+ || (NULL != tp->cookie_values
+ && 0 < tp->cookie_values->cookie_desired))) {
Consistent use of security style would have obviated a lot of foolish >= 0
tests that seem to be constantly in need of fixing. It's a bad idea to
depend on the compiler to catch non-executable code.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ipv6: no more dev_put() in datagram_send_ctl()
From: David Miller @ 2009-11-02 11:43 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEEC0A2.1070502@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 12:21:06 +0100
> Avoids touching device refcount in datagram_send_ctl(), thanks to RCU
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ipv6: no more dev_put() in inet6_bind()
From: David Miller @ 2009-11-02 11:43 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEEBE2F.4020005@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 12:10:39 +0100
> Avoids touching device refcount in inet6_bind(), thanks to RCU
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ip6tnl: less dev_put() calls
From: David Miller @ 2009-11-02 11:43 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEEB2B1.9070302@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 11:21:37 +0100
> Using dev_get_by_index_rcu() in ip6_tnl_rcv_ctl() & ip6_tnl_xmit_ctl()
> avoids touching device refcount.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] packet: less dev_put() calls
From: David Miller @ 2009-11-02 11:42 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEEA9C4.6040305@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2009 10:43:32 +0100
> - packet_sendmsg_spkt() can use dev_get_by_name_rcu() to avoid touching device refcount.
>
> - packet_getname_spkt() & packet_getname() can use dev_get_by_index_rcu() to
> avoid touching device refcount too.
>
> tpacket_snd() & packet_snd() can not use RCU yet because they can sleep when
> allocating skb.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
You're unstoppable :-)
Applied.
^ permalink raw reply
* Re: [RFC] multiqueue changes
From: David Miller @ 2009-11-02 11:35 UTC (permalink / raw)
To: jarkao2; +Cc: mchan, kaber, eric.dumazet, netdev
In-Reply-To: <20091101132017.GA2598@ami.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Sun, 1 Nov 2009 14:20:17 +0100
> There is a question if we can predict in ->probe() MSI-X should be
> successfully enabled in ->ndo_open() for probed hardware. If so,
> then it could go e.g. like this:
We never can know this.
Another device driver can eat up all the MSI-X vectors in the PCI
domain before we make the request_irq() calls in ->open().
^ permalink raw reply
* Re: Patch: Fix userland build failure in linux/if_tunnel.h
From: David Miller @ 2009-11-02 11:30 UTC (permalink / raw)
To: sposelenov; +Cc: sledz, sledz, wd, linux-kernel, netdev
In-Reply-To: <20091102135750.436a7b54@emcraft.com>
From: Sergei Poselenov <sposelenov@emcraft.com>
Date: Mon, 2 Nov 2009 13:57:50 +0300
> Hm, I received a confirmation, that my fix was applied
> http://www.spinics.net/lists/netdev/msg103860.html
>
> However, it indeed didn't find their way into the mainline.
> I have no idea why, received no objections.
That posting doesn't say that your patch got applied, it says
that Joe Perches's MAINTAINERS file patch got applied.
Look at the context of what I'm replying to, rather than the
fact that you see "Applied." and it happens to be in a thread
you started with another patch.
Your patch is not necessary, current iproute2 sources compile
just fine without your change. iproute2 got changed to include
a local copy of linux/ip_tunnel.h which is modified so that it
compiles properly in userspace.
^ permalink raw reply
* [PATCH net-next-2.6] ipv6: no more dev_put() in datagram_send_ctl()
From: Eric Dumazet @ 2009-11-02 11:21 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
Avoids touching device refcount in datagram_send_ctl(), thanks to RCU
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv6/datagram.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 9f70452..e6f9cdf 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -537,12 +537,17 @@ int datagram_send_ctl(struct net *net,
addr_type = __ipv6_addr_type(&src_info->ipi6_addr);
+ rcu_read_lock();
if (fl->oif) {
- dev = dev_get_by_index(net, fl->oif);
- if (!dev)
+ dev = dev_get_by_index_rcu(net, fl->oif);
+ if (!dev) {
+ rcu_read_unlock();
return -ENODEV;
- } else if (addr_type & IPV6_ADDR_LINKLOCAL)
+ }
+ } else if (addr_type & IPV6_ADDR_LINKLOCAL) {
+ rcu_read_unlock();
return -EINVAL;
+ }
if (addr_type != IPV6_ADDR_ANY) {
int strict = __ipv6_addr_src_scope(addr_type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
@@ -553,8 +558,7 @@ int datagram_send_ctl(struct net *net,
ipv6_addr_copy(&fl->fl6_src, &src_info->ipi6_addr);
}
- if (dev)
- dev_put(dev);
+ rcu_read_unlock();
if (err)
goto exit_f;
^ 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