Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/1] Reviewing batman-adv for net/
From: Sven Eckelmann @ 2010-07-16 14:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n

Hi,

batman-adv is a layer2 meshing protocol currently in
drivers/staging/batman-adv. GregKH recently [1][2] gave us his blessing
to leave the staging tree and move to a more suitable location. To the
best of our knowledge the net tree should be it ?!

Short functionality overview:

The batman-adv module is an implementation of the B.A.T.M.A.N. routing
protocol operating on layer2 optimized for (but not limited to) wireless
networks. Next to finding the best possible path through the network it
encapsulates the data traffic and optimizes the transport as well. There
also is a user space tool "batctl" which offers various functionalities
next to a kernel module configuration interface. Detailed documentation
can be found at the project homepage[3].

I will send a patch referencing this mail. I hope you find time to send
us your opinion and ideas.

Everything is addressed in the last round of comments.

thanks,
	Sven

[1] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2010-June/002881.html
[2] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2010-June/002968.html
[3] http://www.open-mesh.org

^ permalink raw reply

* Re: [PATCH 1/2] Remove REDWOOD_[456] config options and conditional code
From: Josh Boyer @ 2010-07-16 14:20 UTC (permalink / raw)
  To: Christian Dietrich
  Cc: Randy Dunlap, linuxppc-dev, Alexander Kurz, Paul Mackerras,
	John Linn, David Brown, Ladislav Michl, Solomon Peachy, vamos-dev,
	Mike Frysinger, Florian Fainelli, Artem Bityutskiy, Nicolas Pitre,
	netdev, linux-kernel, Milton Miller, Jiri Kosina, Joe Perches,
	linux-mtd, David Woodhouse, David S. Miller
In-Reply-To: <ca1bb25d203618c3548748f5efb6f125a96c89e0.1279282865.git.qy03fugy@stud.informatik.uni-erlangen.de>

On Fri, Jul 16, 2010 at 02:29:02PM +0200, Christian Dietrich wrote:
>The config options for REDWOOD_[456] were commented out in the powerpc
>Kconfig. The ifdefs referencing this options therefore are dead and all
>references to this can be removed (Also dependencies in other KConfig
>files).
>
>Signed-off-by: Christian Dietrich <qy03fugy@stud.informatik.uni-erlangen.de>
>Signed-off-by: Christoph Egger <siccegge@cs.fau.de>

This seems fine with me.

The only question is which tree it coms through.  I'm happy to take it
in via mine if the netdev and MTD people are fine with that.  Otherwise,
my ack is below.

Acked-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

josh

>---
> arch/powerpc/platforms/40x/Kconfig |   16 -------------
> drivers/mtd/maps/Kconfig           |    2 +-
> drivers/mtd/maps/redwood.c         |   43 ------------------------------------
> drivers/net/Kconfig                |    2 +-
> drivers/net/smc91x.h               |   37 -------------------------------
> 5 files changed, 2 insertions(+), 98 deletions(-)
>
>diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
>index ec64264..b721764 100644
>--- a/arch/powerpc/platforms/40x/Kconfig
>+++ b/arch/powerpc/platforms/40x/Kconfig
>@@ -71,22 +71,6 @@ config MAKALU
> 	help
> 	  This option enables support for the AMCC PPC405EX board.
>
>-#config REDWOOD_5
>-#	bool "Redwood-5"
>-#	depends on 40x
>-#	default n
>-#	select STB03xxx
>-#	help
>-#	  This option enables support for the IBM STB04 evaluation board.
>-
>-#config REDWOOD_6
>-#	bool "Redwood-6"
>-#	depends on 40x
>-#	default n
>-#	select STB03xxx
>-#	help
>-#	  This option enables support for the IBM STBx25xx evaluation board.
>-
> #config SYCAMORE
> #	bool "Sycamore"
> #	depends on 40x
>diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
>index f22bc9f..6629d09 100644
>--- a/drivers/mtd/maps/Kconfig
>+++ b/drivers/mtd/maps/Kconfig
>@@ -321,7 +321,7 @@ config MTD_CFI_FLAGADM
>
> config MTD_REDWOOD
> 	tristate "CFI Flash devices mapped on IBM Redwood"
>-	depends on MTD_CFI && ( REDWOOD_4 || REDWOOD_5 || REDWOOD_6 )
>+	depends on MTD_CFI
> 	help
> 	  This enables access routines for the flash chips on the IBM
> 	  Redwood board. If you have one of these boards and would like to
>diff --git a/drivers/mtd/maps/redwood.c b/drivers/mtd/maps/redwood.c
>index 933c0b6..d2c9db0 100644
>--- a/drivers/mtd/maps/redwood.c
>+++ b/drivers/mtd/maps/redwood.c
>@@ -22,8 +22,6 @@
>
> #include <asm/io.h>
>
>-#if !defined (CONFIG_REDWOOD_6)
>-
> #define WINDOW_ADDR 0xffc00000
> #define WINDOW_SIZE 0x00400000
>
>@@ -69,47 +67,6 @@ static struct mtd_partition redwood_flash_partitions[] = {
> 	}
> };
>
>-#else /* CONFIG_REDWOOD_6 */
>-/* FIXME: the window is bigger - armin */
>-#define WINDOW_ADDR 0xff800000
>-#define WINDOW_SIZE 0x00800000
>-
>-#define RW_PART0_OF	0
>-#define RW_PART0_SZ	0x400000	/* 4 MiB data */
>-#define RW_PART1_OF	RW_PART0_OF + RW_PART0_SZ
>-#define RW_PART1_SZ	0x10000		/* 64K VPD */
>-#define RW_PART2_OF	RW_PART1_OF + RW_PART1_SZ
>-#define RW_PART2_SZ	0x400000 - (0x10000 + 0x20000)
>-#define RW_PART3_OF	RW_PART2_OF + RW_PART2_SZ
>-#define RW_PART3_SZ	0x20000
>-
>-static struct mtd_partition redwood_flash_partitions[] = {
>-	{
>-		.name = "Redwood filesystem",
>-		.offset = RW_PART0_OF,
>-		.size = RW_PART0_SZ
>-	},
>-	{
>-		.name = "Redwood OpenBIOS Vital Product Data",
>-		.offset = RW_PART1_OF,
>-		.size = RW_PART1_SZ,
>-		.mask_flags = MTD_WRITEABLE	/* force read-only */
>-	},
>-	{
>-		.name = "Redwood kernel",
>-		.offset = RW_PART2_OF,
>-		.size = RW_PART2_SZ
>-	},
>-	{
>-		.name = "Redwood OpenBIOS",
>-		.offset = RW_PART3_OF,
>-		.size = RW_PART3_SZ,
>-		.mask_flags = MTD_WRITEABLE	/* force read-only */
>-	}
>-};
>-
>-#endif /* CONFIG_REDWOOD_6 */
>-
> struct map_info redwood_flash_map = {
> 	.name = "IBM Redwood",
> 	.size = WINDOW_SIZE,
>diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>index ce2fcdd..313d306 100644
>--- a/drivers/net/Kconfig
>+++ b/drivers/net/Kconfig
>@@ -913,7 +913,7 @@ config SMC91X
> 	tristate "SMC 91C9x/91C1xxx support"
> 	select CRC32
> 	select MII
>-	depends on ARM || REDWOOD_5 || REDWOOD_6 || M32R || SUPERH || \
>+	depends on ARM || M32R || SUPERH || \
> 		MIPS || BLACKFIN || MN10300 || COLDFIRE
> 	help
> 	  This is a driver for SMC's 91x series of Ethernet chipsets,
>diff --git a/drivers/net/smc91x.h b/drivers/net/smc91x.h
>index 8d2772c..ee74791 100644
>--- a/drivers/net/smc91x.h
>+++ b/drivers/net/smc91x.h
>@@ -83,43 +83,6 @@ static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg)
> 	}
> }
>
>-#elif defined(CONFIG_REDWOOD_5) || defined(CONFIG_REDWOOD_6)
>-
>-/* We can only do 16-bit reads and writes in the static memory space. */
>-#define SMC_CAN_USE_8BIT	0
>-#define SMC_CAN_USE_16BIT	1
>-#define SMC_CAN_USE_32BIT	0
>-#define SMC_NOWAIT		1
>-
>-#define SMC_IO_SHIFT		0
>-
>-#define SMC_inw(a, r)		in_be16((volatile u16 *)((a) + (r)))
>-#define SMC_outw(v, a, r)	out_be16((volatile u16 *)((a) + (r)), v)
>-#define SMC_insw(a, r, p, l) 						\
>-	do {								\
>-		unsigned long __port = (a) + (r);			\
>-		u16 *__p = (u16 *)(p);					\
>-		int __l = (l);						\
>-		insw(__port, __p, __l);					\
>-		while (__l > 0) {					\
>-			*__p = swab16(*__p);				\
>-			__p++;						\
>-			__l--;						\
>-		}							\
>-	} while (0)
>-#define SMC_outsw(a, r, p, l) 						\
>-	do {								\
>-		unsigned long __port = (a) + (r);			\
>-		u16 *__p = (u16 *)(p);					\
>-		int __l = (l);						\
>-		while (__l > 0) {					\
>-			/* Believe it or not, the swab isn't needed. */	\
>-			outw( /* swab16 */ (*__p++), __port);		\
>-			__l--;						\
>-		}							\
>-	} while (0)
>-#define SMC_IRQ_FLAGS		(0)
>-
> #elif defined(CONFIG_SA1100_PLEB)
> /* We can only do 16-bit reads and writes in the static memory space. */
> #define SMC_CAN_USE_8BIT	1
>-- 
>1.7.0.4
>

^ permalink raw reply

* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: Marc Kleine-Budde @ 2010-07-16 13:21 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C3F55A9.8030307-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 35390 bytes --]

Wolfgang Grandegger wrote:
> I realized a few issues. You can add my "acked-by" when they are fixed.

thanks for the review.

>>  drivers/net/can/Kconfig              |    6 +
>>  drivers/net/can/Makefile             |    1 +
>>  drivers/net/can/flexcan.c            | 1005 ++++++++++++++++++++++++++++++++++
>>  include/linux/can/platform/flexcan.h |   20 +
>>  4 files changed, 1032 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/net/can/flexcan.c
>>  create mode 100644 include/linux/can/platform/flexcan.h
>>
>> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
>> index 2c5227c..3f13299 100644
>> --- a/drivers/net/can/Kconfig
>> +++ b/drivers/net/can/Kconfig
>> @@ -73,6 +73,12 @@ config CAN_JANZ_ICAN3
>>  	  This driver can also be built as a module. If so, the module will be
>>  	  called janz-ican3.ko.
>>  
>> +config CAN_FLEXCAN
>> +	tristate "Support for Freescale FLEXCAN based chips"
>> +	depends on CAN_DEV
> 
> Some more arch specific dependencies would be nice.

it's tested on MX25 and MX35, I'll add these.

>> +	---help---
>> +	  Say Y here if you want to support for Freescale FlexCAN.
>> +
>>  source "drivers/net/can/mscan/Kconfig"
>>  
>>  source "drivers/net/can/sja1000/Kconfig"
>> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
>> index 9047cd0..0057537 100644
>> --- a/drivers/net/can/Makefile
>> +++ b/drivers/net/can/Makefile
>> @@ -16,5 +16,6 @@ obj-$(CONFIG_CAN_TI_HECC)	+= ti_hecc.o
>>  obj-$(CONFIG_CAN_MCP251X)	+= mcp251x.o
>>  obj-$(CONFIG_CAN_BFIN)		+= bfin_can.o
>>  obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
>> +obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>>  
>>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>> new file mode 100644
>> index 0000000..a3180ba
>> --- /dev/null
>> +++ b/drivers/net/can/flexcan.c
>> @@ -0,0 +1,1005 @@
>> +/*
>> + * flexcan.c - FLEXCAN CAN controller driver
>> + *
>> + * Copyright (c) 2005-2006 Varma Electronics Oy
>> + * Copyright (c) 2009 Sascha Hauer, Pengutronix
>> + * Copyright (c) 2010 Marc Kleine-Budde, Pengutronix
>> + *
>> + * Based on code originally by Andrey Volkov <avolkov-ppI4tVfbJvJWk0Htik3J/w@public.gmane.org>
>> + *
>> + * LICENCE:
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * 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.
>> + *
>> + */
>> +
>> +#include <linux/netdevice.h>
>> +#include <linux/can.h>
>> +#include <linux/can/dev.h>
>> +#include <linux/can/error.h>
>> +#include <linux/can/platform/flexcan.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/if_arp.h>
>> +#include <linux/if_ether.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +
>> +
>> +#include <mach/clock.h>
>> +
>> +#define DRV_NAME		"flexcan"
>> +#define FLEXCAN_NAPI_WEIGHT	8
>> +
>> +
>> +/* FLEXCAN module configuration register (CANMCR) bits */
>> +#define FLEXCAN_MCR_MDIS		BIT(31)
>> +#define FLEXCAN_MCR_FRZ			BIT(30)
>> +#define FLEXCAN_MCR_FEN			BIT(29)
>> +#define FLEXCAN_MCR_HALT		BIT(28)
>> +#define FLEXCAN_MCR_NOT_RDY		BIT(27)
>> +#define FLEXCAN_MCR_WAK_MSK		BIT(26)
>> +#define FLEXCAN_MCR_SOFTRST		BIT(25)
>> +#define FLEXCAN_MCR_FRZ_ACK		BIT(24)
>> +#define FLEXCAN_MCR_SUPV		BIT(23)
>> +#define FLEXCAN_MCR_SLF_WAK		BIT(22)
>> +#define FLEXCAN_MCR_WRN_EN		BIT(21)
>> +#define FLEXCAN_MCR_LPM_ACK		BIT(20)
>> +#define FLEXCAN_MCR_WAK_SRC		BIT(19)
>> +#define FLEXCAN_MCR_DOZE		BIT(18)
>> +#define FLEXCAN_MCR_SRX_DIS		BIT(17)
>> +#define FLEXCAN_MCR_BCC			BIT(16)
>> +#define FLEXCAN_MCR_LPRIO_EN		BIT(13)
>> +#define FLEXCAN_MCR_AEN			BIT(12)
>> +#define FLEXCAN_MCR_MAXMB(x)		((x) & 0xf)
>> +#define FLEXCAN_MCR_IDAM_A		(0 << 8)
>> +#define FLEXCAN_MCR_IDAM_B		(1 << 8)
>> +#define FLEXCAN_MCR_IDAM_C		(2 << 8)
>> +#define FLEXCAN_MCR_IDAM_D		(3 << 8)
>> +
>> +/* FLEXCAN control register (CANCTRL) bits */
>> +#define FLEXCAN_CTRL_PRESDIV(x)		(((x) & 0xff) << 24)
>> +#define FLEXCAN_CTRL_RJW(x)		(((x) & 0x03) << 22)
>> +#define FLEXCAN_CTRL_PSEG1(x)		(((x) & 0x07) << 19)
>> +#define FLEXCAN_CTRL_PSEG2(x)		(((x) & 0x07) << 16)
>> +#define FLEXCAN_CTRL_BOFF_MSK		BIT(15)
>> +#define FLEXCAN_CTRL_ERR_MSK		BIT(14)
>> +#define FLEXCAN_CTRL_CLK_SRC		BIT(13)
>> +#define FLEXCAN_CTRL_LPB		BIT(12)
>> +#define FLEXCAN_CTRL_TWRN_MSK		BIT(11)
>> +#define FLEXCAN_CTRL_RWRN_MSK		BIT(10)
>> +#define FLEXCAN_CTRL_SMP		BIT(7)
>> +#define FLEXCAN_CTRL_BOFF_REC		BIT(6)
>> +#define FLEXCAN_CTRL_TSYNC		BIT(5)
>> +#define FLEXCAN_CTRL_LBUF		BIT(4)
>> +#define FLEXCAN_CTRL_LOM		BIT(3)
>> +#define FLEXCAN_CTRL_PROPSEG(x)		((x) & 0x07)
>> +
>> +/* FLEXCAN error and status register (ESR) bits */
>> +#define FLEXCAN_ESR_TWRN_INT		BIT(17)
>> +#define FLEXCAN_ESR_RWRN_INT		BIT(16)
>> +#define FLEXCAN_ESR_BIT1_ERR		BIT(15)
>> +#define FLEXCAN_ESR_BIT0_ERR		BIT(14)
>> +#define FLEXCAN_ESR_ACK_ERR		BIT(13)
>> +#define FLEXCAN_ESR_CRC_ERR		BIT(12)
>> +#define FLEXCAN_ESR_FRM_ERR		BIT(11)
>> +#define FLEXCAN_ESR_STF_ERR		BIT(10)
>> +#define FLEXCAN_ESR_TX_WRN		BIT(9)
>> +#define FLEXCAN_ESR_RX_WRN		BIT(8)
>> +#define FLEXCAN_ESR_IDLE		BIT(7)
>> +#define FLEXCAN_ESR_TXRX		BIT(6)
>> +#define FLEXCAN_EST_FLT_CONF_SHIFT	(4)
>> +#define FLEXCAN_ESR_FLT_CONF_MASK	(0x2 << FLEXCAN_EST_FLT_CONF_SHIFT)
>> +#define FLEXCAN_ESR_FLT_CONF_ACTIVE	(0x0 << FLEXCAN_EST_FLT_CONF_SHIFT)
>> +#define FLEXCAN_ESR_FLT_CONF_PASSIVE	(0x1 << FLEXCAN_EST_FLT_CONF_SHIFT)
>> +#define FLEXCAN_ESR_BOFF_INT		BIT(2)
>> +#define FLEXCAN_ESR_ERR_INT		BIT(1)
>> +#define FLEXCAN_ESR_WAK_INT		BIT(0)
>> +#define FLEXCAN_ESR_ERR_FRAME \
>> +	(FLEXCAN_ESR_BIT1_ERR | FLEXCAN_ESR_BIT0_ERR | \
>> +	 FLEXCAN_ESR_ACK_ERR | FLEXCAN_ESR_CRC_ERR | \
>> +	 FLEXCAN_ESR_FRM_ERR | FLEXCAN_ESR_STF_ERR)
>> +#define FLEXCAN_ESR_ERR_LINE \
>> +	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | FLEXCAN_ESR_BOFF_INT)
>> +
>> +/* FLEXCAN interrupt flag register (IFLAG) bits */
>> +#define FLEXCAN_TX_BUF_ID		8
>> +#define FLEXCAN_IFLAG_BUF(x)		BIT(x)
>> +#define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW	BIT(7)
>> +#define FLEXCAN_IFLAG_RX_FIFO_WARN	BIT(6)
>> +#define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE	BIT(5)
>> +#define FLEXCAN_IFLAG_DEFAULT \
>> +	(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \
>> +	 FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
>> +
>> +/* FLEXCAN message buffers */
>> +#define FLEXCAN_MB_CNT_CODE(x)		(((x) & 0xf) << 24)
>> +#define FLEXCAN_MB_CNT_SRR		BIT(22)
>> +#define FLEXCAN_MB_CNT_IDE		BIT(21)
>> +#define FLEXCAN_MB_CNT_RTR		BIT(20)
>> +#define FLEXCAN_MB_CNT_LENGTH(x)	(((x) & 0xf) << 16)
>> +#define FLEXCAN_MB_CNT_TIMESTAMP(x)	((x) & 0xffff)
>> +
>> +#define FLEXCAN_MB_CODE_MASK		(0xf0ffffff)
>> +
>> +/* Structure of the message buffer */
>> +struct flexcan_mb {
>> +	u32 can_ctrl;
>> +	u32 can_id;
>> +	u32 data[2];
>> +};
>> +
>> +/* Structure of the hardware registers */
>> +struct flexcan_regs {
>> +	u32 mcr;		/* 0x00 */
>> +	u32 ctrl;		/* 0x04 */
>> +	u32 timer;		/* 0x08 */
>> +	u32 _reserved1;		/* 0x0c */
>> +	u32 rxgmask;		/* 0x10 */
>> +	u32 rx14mask;		/* 0x14 */
>> +	u32 rx15mask;		/* 0x18 */
>> +	u32 ecr;		/* 0x1c */
>> +	u32 esr;		/* 0x20 */
>> +	u32 imask2;		/* 0x24 */
>> +	u32 imask1;		/* 0x28 */
>> +	u32 iflag2;		/* 0x2c */
>> +	u32 iflag1;		/* 0x30 */
>> +	u32 _reserved2[19];
>> +	struct flexcan_mb cantxfg[64];
>> +};
>> +
>> +struct flexcan_priv {
>> +	struct can_priv can;
>> +	struct net_device *dev;
>> +	struct napi_struct napi;
>> +
>> +	void __iomem *base;
>> +	u32 reg_esr;
>> +	u32 reg_ctrl_default;
>> +
>> +	struct clk *clk;
>> +	struct flexcan_platform_data *pdata;
>> +};
>> +
>> +static struct can_bittiming_const flexcan_bittiming_const = {
>> +	.name = DRV_NAME,
>> +	.tseg1_min = 4,
>> +	.tseg1_max = 16,
>> +	.tseg2_min = 2,
>> +	.tseg2_max = 8,
>> +	.sjw_max = 4,
>> +	.brp_min = 1,
>> +	.brp_max = 256,
>> +	.brp_inc = 1,
>> +};
>> +
>> +/*
>> + * Swtich transceiver on or off
>> + */
>> +static void flexcan_transceiver_switch(const struct flexcan_priv *priv, int on)
>> +{
>> +	if (priv->pdata && priv->pdata->transceiver_switch)
>> +		priv->pdata->transceiver_switch(on);
>> +}
>> +
>> +static inline void flexcan_chip_enable(struct flexcan_priv *priv)
>> +{
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	u32 reg;
>> +
>> +	reg = readl(&regs->mcr);
>> +	reg &= ~FLEXCAN_MCR_MDIS;
>> +	writel(reg, &regs->mcr);
>> +
>> +	udelay(10);
>> +}
>> +
>> +static inline void flexcan_chip_disable(struct flexcan_priv *priv)
>> +{
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	u32 reg;
>> +
>> +	reg = readl(&regs->mcr);
>> +	reg |= FLEXCAN_MCR_MDIS;
>> +	writel(reg, &regs->mcr);
>> +}
>> +
>> +static int flexcan_get_berr_counter(const struct net_device *dev,
>> +				    struct can_berr_counter *bec)
>> +{
>> +	const struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	u32 reg = readl(&regs->ecr);
>> +
>> +	bec->txerr = (reg >> 0) & 0xff;
>> +	bec->rxerr = (reg >> 8) & 0xff;
>> +
>> +	return 0;
>> +}
>> +
>> +
>> +static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>> +{
>> +	const struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct net_device_stats *stats = &dev->stats;
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	struct can_frame *frame = (struct can_frame *)skb->data;
>> +	u32 can_id;
>> +	u32 ctrl = FLEXCAN_MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
>> +
>> +	if (can_dropped_invalid_skb(dev, skb))
>> +		return NETDEV_TX_OK;
>> +
>> +	netif_stop_queue(dev);
>> +
>> +	if (frame->can_id & CAN_EFF_FLAG) {
>> +		can_id = frame->can_id & CAN_EFF_MASK;
>> +		ctrl |= FLEXCAN_MB_CNT_IDE | FLEXCAN_MB_CNT_SRR;
>> +	} else {
>> +		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
>> +	}
>> +
>> +	if (frame->can_id & CAN_RTR_FLAG)
>> +		ctrl |= FLEXCAN_MB_CNT_RTR;
>> +
>> +	if (frame->can_dlc > 0) {
>> +		u32 data;
>> +		data = frame->data[0] << 24;
>> +		data |= frame->data[1] << 16;
>> +		data |= frame->data[2] << 8;
>> +		data |= frame->data[3];
>> +		writel(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
>> +	}
> 
> IIRC, in your review of Jürgens patch you suggested to use be32_to_cpu here.

ACK, will change these

>> +	if (frame->can_dlc > 3) {
>> +		u32 data;
>> +		data = frame->data[4] << 24;
>> +		data |= frame->data[5] << 16;
>> +		data |= frame->data[6] << 8;
>> +		data |= frame->data[7];
>> +		writel(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
>> +	}
> 
> Ditto.

ACK

>> +	writel(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
>> +	writel(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
>> +
>> +	kfree_skb(skb);
>> +
>> +	stats->tx_bytes += frame->can_dlc;
>> +
>> +	return NETDEV_TX_OK;
>> +}
>> +
>> +
>> +static void flexcan_poll_err_frame(struct net_device *dev,
>> +				   struct can_frame *cf, u32 reg_esr)
>> +{
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	int error_warning = 0, rx_errors = 0, tx_errors = 0;
>> +
>> +	if (reg_esr & FLEXCAN_ESR_BIT1_ERR) {
>> +		rx_errors = 1;
>> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>> +		cf->data[2] |= CAN_ERR_PROT_BIT1;
>> +	}
>> +
>> +	if (reg_esr & FLEXCAN_ESR_BIT0_ERR) {
>> +		rx_errors = 1;
>> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>> +		cf->data[2] |= CAN_ERR_PROT_BIT0;
>> +	}
>> +
>> +	if (reg_esr & FLEXCAN_ESR_FRM_ERR) {
>> +		rx_errors = 1;
>> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>> +		cf->data[2] |= CAN_ERR_PROT_FORM;
>> +	}
>> +
>> +	if (reg_esr & FLEXCAN_ESR_STF_ERR) {
>> +		rx_errors = 1;
>> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>> +		cf->data[2] |= CAN_ERR_PROT_STUFF;
>> +	}
>> +
>> +
>> +	if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>> +		tx_errors = 1;
>> +		cf->can_id |= CAN_ERR_ACK;
> 
> This is a bus-error as well. Therefore I think it should be:
> 
> 	if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
> 		tx_errors = 1;
> 		cf->can_id |= CAN_ERR_ACK;
> 		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> 		cf->data[3] |= CAN_ERR_PROT_LOC_ACK; /* ACK slot */
> 	}
> 
> I need to check what CAN_ERR_ACK is intended for. Then, cf->can_id could
> be preset with "CAN_ERR_PROT | CAN_ERR_BUSERROR" at the beginning.
> 
>> +	}
>> +
>> +	if (error_warning)
>> +		priv->can.can_stats.error_warning++;
> 
> Hm, error_warning is always 0 !?

this must go into the state handling function, will fix.
> 
>> +	if (rx_errors)
>> +		dev->stats.rx_errors++;
>> +	if (tx_errors)
>> +		dev->stats.tx_errors++;
>> +
>> +}
>> +
>> +static void flexcan_poll_err(struct net_device *dev, u32 reg_esr)
>> +{
>> +	struct sk_buff *skb;
>> +	struct can_frame *cf;
>> +
>> +	skb = alloc_can_err_skb(dev, &cf);
>> +	if (unlikely(!skb))
>> +		return;
>> +
>> +	flexcan_poll_err_frame(dev, cf, reg_esr);
>> +	netif_receive_skb(skb);
>> +
>> +	dev->stats.rx_packets++;
>> +	dev->stats.rx_bytes += cf->can_dlc;
>> +}
>> +
>> +static void flexcan_read_fifo(const struct net_device *dev, struct can_frame *cf)
>> +{
>> +	const struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
>> +	u32 reg_ctrl, reg_id;
>> +
>> +	reg_ctrl = readl(&mb->can_ctrl);
>> +	reg_id = readl(&mb->can_id);
>> +	if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
>> +		cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
>> +	else
>> +		cf->can_id = (reg_id >> 18) & CAN_SFF_MASK;
>> +
>> +	if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
>> +		cf->can_id |= CAN_RTR_FLAG;
>> +	cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
>> +
>> +	*(__be32 *)(cf->data + 0) = cpu_to_be32(readl(&mb->data[0]));
>> +	*(__be32 *)(cf->data + 4) = cpu_to_be32(readl(&mb->data[1]));
>> +
>> +	/* mark as read */
>> +	writel(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
>> +	readl(&regs->timer);
>> +}
>> +
>> +static void flexcan_read_frame(struct net_device *dev)
>> +{
>> +	struct net_device_stats *stats = &dev->stats;
>> +	struct can_frame *cf;
>> +	struct sk_buff *skb;
>> +
>> +	skb = alloc_can_skb(dev, &cf);
>> +	if (unlikely(!skb)) {
>> +		stats->rx_dropped++;
>> +		return;
>> +	}
>> +
>> +	flexcan_read_fifo(dev, cf);
>> +	netif_receive_skb(skb);
>> +
>> +	stats->rx_packets++;
>> +	stats->rx_bytes += cf->can_dlc;
>> +}
>> +
>> +static int flexcan_poll(struct napi_struct *napi, int quota)
>> +{
>> +	struct net_device *dev = napi->dev;
>> +	const struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	u32 reg_iflag1, reg_esr;
>> +	int work_done = 0;
>> +
>> +	reg_iflag1 = readl(&regs->iflag1);
>> +
>> +	/* first handle RX-FIFO */
>> +	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
>> +	       work_done < quota) {
>> +		flexcan_read_frame(dev);
>> +
>> +		work_done++;
>> +		reg_iflag1 = readl(&regs->iflag1);
>> +	}
>> +
>> +	/*
>> +	 * The error bits are clear on read,
>> +	 * so use saved value from irq handler.
>> +	 */
>> +	reg_esr = readl(&regs->esr) | priv->reg_esr;
> 
> Re-reading reg_esr may cause lost of state changes.

To my understanding of the datasheet and my observation, only the error
bits are cleared on read. The bit defining the status
(FLEXCAN_ESR_FLT_CONF_MASK) == error active, error passive and bus off
are not cleared on read.

However there are two bits defining RX and TX warning level, I'll check
these.

>> +	if (work_done < quota) {
>> +		flexcan_poll_err(dev, reg_esr);
> 
> An error frame is created here for each call of flexcan_poll(), not only
> in case of errors.

Doh, will fix this.

> 
>> +		work_done++;
>> +	}
>> +
>> +	if (work_done < quota) {
>> +		napi_complete(napi);
>> +		/* enable IRQs */
>> +		writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
>> +		writel(priv->reg_ctrl_default, &regs->ctrl);
>> +	}
>> +
>> +	return work_done;
>> +}
>> +
>> +static void flexcan_irq_err_state(struct net_device *dev,
>> +				  struct can_frame *cf, enum can_state new_state)
>> +{
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct can_berr_counter bec;
>> +
>> +	flexcan_get_berr_counter(dev, &bec);
>> +
>> +	switch (priv->can.state) {
>> +	case CAN_STATE_ERROR_ACTIVE:
>> +		/*
>> +		 * from: ERROR_ACTIVE
>> +		 * to  : ERROR_WARNING, ERROR_PASSIVE, BUS_OFF
>> +		 * =>  : there was a warning int
>> +		 */
>> +		if (new_state >= CAN_STATE_ERROR_WARNING &&
>> +		    new_state <= CAN_STATE_BUS_OFF) {
>> +			dev_dbg(dev->dev.parent, "Error Warning IRQ\n");
>> +			priv->can.can_stats.error_warning++;
>> +
>> +			cf->can_id |= CAN_ERR_CRTL;
>> +			cf->data[1] = (bec.txerr > bec.rxerr) ?
>> +				CAN_ERR_CRTL_TX_WARNING :
>> +				CAN_ERR_CRTL_RX_WARNING;
>> +		}
>> +	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>> +		/*
>> +		 * from: ERROR_ACTIVE, ERROR_WARNING
>> +		 * to  : ERROR_PASSIVE, BUS_OFF
>> +		 * =>  : error passive int
>> +		 */
>> +		if (new_state >= CAN_STATE_ERROR_PASSIVE &&
>> +		    new_state <= CAN_STATE_BUS_OFF) {
>> +			dev_dbg(dev->dev.parent, "Error Passive IRQ\n");
>> +			priv->can.can_stats.error_passive++;
>> +
>> +			cf->can_id |= CAN_ERR_CRTL;
>> +			cf->data[1] = (bec.txerr > bec.rxerr) ?
>> +				CAN_ERR_CRTL_TX_PASSIVE :
>> +				CAN_ERR_CRTL_RX_PASSIVE;
>> +		}
>> +		break;
>> +	case CAN_STATE_BUS_OFF:
>> +		dev_err(dev->dev.parent,
>> +			"BUG! hardware recovered automatically from BUS_OFF\n");
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	/* process state changes depending on the new state */
>> +	switch (new_state) {
>> +	case CAN_STATE_BUS_OFF:
>> +		cf->can_id |= CAN_ERR_BUSOFF;
>> +		can_bus_off(dev);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +}
>> +
>> +static void flexcan_irq_err(struct net_device *dev)
>> +{
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	struct sk_buff *skb;
>> +	struct can_frame *cf;
>> +	enum can_state new_state;
>> +	u32 reg_esr;
>> +	int flt;
>> +
>> +	reg_esr = readl(&regs->esr);
> 
> As discussed, reg_esr is re-read here. Should probably be passed via
> function argument.

ACK

> 
>> +	writel(reg_esr, &regs->esr);
>> +
>> +	flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
>> +	if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
>> +		if (likely(!(reg_esr & (FLEXCAN_ESR_TX_WRN |
>> +					FLEXCAN_ESR_RX_WRN))))
>> +			new_state = CAN_STATE_ERROR_ACTIVE;
>> +		else
>> +			new_state = CAN_STATE_ERROR_WARNING;
>> +	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE))
>> +		new_state = CAN_STATE_ERROR_PASSIVE;
>> +	else
>> +		new_state = CAN_STATE_BUS_OFF;
>> +
>> +	/* state hasn't changed */
>> +	if (likely(new_state == priv->can.state))
>> +		return;
>> +
>> +	skb = alloc_can_err_skb(dev, &cf);
>> +	if (unlikely(!skb))
>> +		return;
>> +
>> +	flexcan_irq_err_state(dev, cf, new_state);
>> +	netif_rx(skb);
>> +
>> +	dev->stats.rx_packets++;
>> +	dev->stats.rx_bytes += cf->can_dlc;
>> +
>> +	priv->can.state = new_state;
>> +}
>> +
>> +static irqreturn_t flexcan_irq(int irq, void *dev_id)
>> +{
>> +	struct net_device *dev = dev_id;
>> +	struct net_device_stats *stats = &dev->stats;
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	u32 reg_iflag1, reg_esr;
>> +
>> +	reg_iflag1 = readl(&regs->iflag1);
>> +	reg_esr = readl(&regs->esr);
>> +
>> +	/* receive or error interrupt -> napi */
>> +	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
>> +	    (reg_esr & FLEXCAN_ESR_ERR_FRAME)) {
>> +		/*
>> +		 * The error bits are cleared on read,
>> +		 * save for later use.
>> +		 */
>> +		priv->reg_esr = reg_esr;
>> +		writel(FLEXCAN_IFLAG_DEFAULT & ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE,
>> +		       &regs->imask1);
>> +		writel(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_MSK,
>> +		       &regs->ctrl);
>> +		napi_schedule(&priv->napi);
>> +	}
>> +
>> +	/* FIFO overflow */
>> +	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
>> +		writel(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
>> +		dev->stats.rx_over_errors++;
>> +		dev->stats.rx_errors++;
>> +	}
>> +
>> +	/* transmission complete interrupt */
>> +	if (reg_iflag1 & (1 << FLEXCAN_TX_BUF_ID)) {
>> +		stats->tx_packets++;
> 
> Where is stats->tx_bytes incremented?

At the end of flexcan_start_xmit(), here the skb is already freed.

>> +		writel((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
>> +		netif_wake_queue(dev);
>> +	}
>> +
>> +	/* handle state changes */
>> +	flexcan_irq_err(dev);
> 
> This does create and send an error message for *each* interrupt, even
> for non-error interrupts (RX and TX).

No, have a look at flexcan_irq_err(), first it will determine the
current state of the can controller and only if the state changes it
will send an error message.

> Also, state changes are handled in the hard-irq context, while the
> errors are handle in the soft-irq context. This looks tricky and error
> prune to me as both are derived from the esr register. State changes and
> errors might not be realized in the correct order, etc. It might be
> saver to handle both in the poll routine by a common function.

okay, will do.

>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static void flexcan_set_bittiming(struct net_device *dev)
>> +{
>> +	const struct flexcan_priv *priv = netdev_priv(dev);
>> +	const struct can_bittiming *bt = &priv->can.bittiming;
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	u32 reg;
>> +
>> +	reg = readl(&regs->ctrl);
>> +	reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
>> +		 FLEXCAN_CTRL_RJW(0x3) |
>> +		 FLEXCAN_CTRL_PSEG1(0x7) |
>> +		 FLEXCAN_CTRL_PSEG2(0x7) |
>> +		 FLEXCAN_CTRL_PROPSEG(0x7) |
>> +		 FLEXCAN_CTRL_LPB |
>> +		 FLEXCAN_CTRL_SMP |
>> +		 FLEXCAN_CTRL_LOM);
>> +
>> +	reg |= FLEXCAN_CTRL_PRESDIV(bt->brp - 1) |
>> +		FLEXCAN_CTRL_PSEG1(bt->phase_seg1 - 1) |
>> +		FLEXCAN_CTRL_PSEG2(bt->phase_seg2 - 1) |
>> +		FLEXCAN_CTRL_RJW(bt->sjw - 1) |
>> +		FLEXCAN_CTRL_PROPSEG(bt->prop_seg - 1);
>> +
>> +	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
>> +		reg |= FLEXCAN_CTRL_LPB;
>> +	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
>> +		reg |= FLEXCAN_CTRL_LOM;
>> +	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
>> +		reg |= FLEXCAN_CTRL_SMP;
>> +
>> +	dev_info(dev->dev.parent, "writing ctrl=0x%08x\n", reg);
>> +	writel(reg, &regs->ctrl);
>> +
>> +	/* print chip status */
>> +	dev_dbg(dev->dev.parent, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
>> +		readl(&regs->mcr), readl(&regs->ctrl));
> 
> This seems especially useful for development. Please check the other
> dev_dbg's as well.

They are quite good for debugging, too. Hence the "dev_dbg" :)
The stati are just printed during initialisation.

> 
>> +}
>> +
>> +/*
>> + * flexcan_chip_start
>> + *
>> + * this functions is entered with clocks enabled
>> + *
>> + */
>> +static int flexcan_chip_start(struct net_device *dev)
>> +{
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	unsigned int i;
>> +	int err;
>> +	u32 reg_mcr, reg_ctrl;
>> +
>> +	/* enable module */
>> +	flexcan_chip_enable(priv);
>> +
>> +	/* soft reset */
>> +	writel(FLEXCAN_MCR_SOFTRST, &regs->mcr);
>> +	udelay(10);
>> +
>> +	reg_mcr = readl(&regs->mcr);
>> +	if (reg_mcr & FLEXCAN_MCR_SOFTRST) {
>> +		dev_err(dev->dev.parent,
>> +			"Failed to softreset can module (mcr=0x%08x)\n", reg_mcr);
>> +		err = -ENODEV;
>> +		goto out;
>> +	}
>> +
>> +	flexcan_set_bittiming(dev);
>> +
>> +	/*
>> +	 * MCR
>> +	 *
>> +	 * enable freeze
>> +	 * enable fifo
>> +	 * halt now
>> +	 * only supervisor access
>> +	 * enable warning int
>> +	 * choose format C
>> +	 *
>> +	 */
>> +	reg_mcr = readl(&regs->mcr);
>> +	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
>> +		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
>> +		FLEXCAN_MCR_IDAM_C;
>> +	dev_dbg(dev->dev.parent, "%s: writing mcr=0x%08x", __func__, reg_mcr);
>> +	writel(reg_mcr, &regs->mcr);
>> +
>> +	/*
>> +	 * CTRL
>> +	 *
>> +	 * enable bus off interrupt
>> +	 * disable auto busoff recovery
>> +	 * enable tx and rx warning interrupt
>> +	 * transmit lowest buffer first
>> +	 */
>> +	reg_ctrl = readl(&regs->ctrl);
>> +	reg_ctrl |= FLEXCAN_CTRL_BOFF_MSK |FLEXCAN_CTRL_BOFF_REC |
>> +		FLEXCAN_CTRL_TWRN_MSK | FLEXCAN_CTRL_RWRN_MSK |
>> +		FLEXCAN_CTRL_LBUF;
>> +	/*
>> +	 * TODO: for now turn on the error interrupt, otherwise we
>> +	 * don't get any warning or bus passive interrupts.
>> +	 */
>> +	reg_ctrl |= FLEXCAN_CTRL_ERR_MSK;
>> +
>> +	/* save for later use */
>> +	priv->reg_ctrl_default = reg_ctrl;
>> +	dev_dbg(dev->dev.parent, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
>> +	writel(reg_ctrl, &regs->ctrl);
>> +
>> +	for (i = 0; i < ARRAY_SIZE(regs->cantxfg); i++) {
>> +		writel(0, &regs->cantxfg[i].can_ctrl);
>> +		writel(0, &regs->cantxfg[i].can_id);
>> +		writel(0, &regs->cantxfg[i].data[0]);
>> +		writel(0, &regs->cantxfg[i].data[1]);
>> +
>> +		/* put MB into rx queue */
>> +		writel(FLEXCAN_MB_CNT_CODE(0x4), &regs->cantxfg[i].can_ctrl);
>> +	}
>> +
>> +	/* acceptance mask/acceptance code (accept everything) */
>> +	writel(0x0, &regs->rxgmask);
>> +	writel(0x0, &regs->rx14mask);
>> +	writel(0x0, &regs->rx15mask);
>> +
>> +	flexcan_transceiver_switch(priv, 1);
>> +
>> +	/* synchronize with the can bus */
>> +	reg_mcr = readl(&regs->mcr);
>> +	reg_mcr &= ~FLEXCAN_MCR_HALT;
>> +	writel(reg_mcr, &regs->mcr);
>> +
>> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
>> +
>> +	/* enable FIFO interrupts */
>> +	writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
>> +
>> +	/* print chip status */
>> +	dev_dbg(dev->dev.parent, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
>> +		readl(&regs->mcr), readl(&regs->ctrl));
>> +
>> +	return 0;
>> +
>> + out:
>> +	flexcan_chip_disable(priv);
>> +	return err;
>> +}
>> +
>> +/*
>> + * flexcan_chip_stop
>> + *
>> + * this functions is entered with clocks enabled
>> + *
>> + */
>> +static void flexcan_chip_stop(struct net_device *dev)
>> +{
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	u32 reg;
>> +
>> +	/* Disable all interrupts */
>> +	writel(0, &regs->imask1);
>> +
>> +	/* Disable + halt module */
>> +	reg = readl(&regs->mcr);
>> +	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
>> +	writel(reg, &regs->mcr);
>> +
>> +	flexcan_transceiver_switch(priv, 0);
>> +	priv->can.state = CAN_STATE_STOPPED;
>> +
>> +	return;
>> +}
>> +
>> +static int flexcan_open(struct net_device *dev)
>> +{
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	int err;
>> +
>> +	clk_enable(priv->clk);
>> +
>> +	err = open_candev(dev);
>> +	if (err)
>> +		goto out;
>> +
>> +	err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev);
>> +	if (err)
>> +		goto out_close;
>> +
>> +	/* start chip and queuing */
>> +	err = flexcan_chip_start(dev);
>> +	if (err)
>> +		goto out_close;
>> +	napi_enable(&priv->napi);
>> +	netif_start_queue(dev);
>> +
>> +	return 0;
>> +
>> + out_close:
>> +	close_candev(dev);
>> + out:
>> +	clk_disable(priv->clk);
>> +
>> +	return err;
>> +}
>> +
>> +static int flexcan_close(struct net_device *dev)
>> +{
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +
>> +	netif_stop_queue(dev);
>> +	napi_disable(&priv->napi);
>> +	flexcan_chip_stop(dev);
>> +
>> +	free_irq(dev->irq, dev);
>> +	clk_disable(priv->clk);
>> +
>> +	close_candev(dev);
>> +
>> +	return 0;
>> +}
>> +
>> +static int flexcan_set_mode(struct net_device *dev, enum can_mode mode)
>> +{
>> +	int err;
>> +
>> +	switch (mode) {
>> +	case CAN_MODE_START:
>> +		err = flexcan_chip_start(dev);
>> +		if (err)
>> +			return err;
>> +
>> +		netif_wake_queue(dev);
>> +		break;
>> +
>> +	default:
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct net_device_ops flexcan_netdev_ops = {
>> +	.ndo_open	= flexcan_open,
>> +	.ndo_stop	= flexcan_close,
>> +	.ndo_start_xmit	= flexcan_start_xmit,
>> +};
>> +
>> +static int __devinit register_flexcandev(struct net_device *dev)
>> +{
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct flexcan_regs __iomem *regs = priv->base;
>> +	u32 reg, err;
>> +
>> +	clk_enable(priv->clk);
>> +
>> +	/* select "bus clock", chip must be disabled */
>> +	flexcan_chip_disable(priv);
>> +	reg = readl(&regs->ctrl);
>> +	reg |= FLEXCAN_CTRL_CLK_SRC;
>> +	writel(reg, &regs->ctrl);
>> +
>> +	flexcan_chip_enable(priv);
>> +
>> +	/* set freeze, halt and activate FIFO, restrict register access */
>> +	reg = readl(&regs->mcr);
>> +	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
>> +		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
>> +	writel(reg, &regs->mcr);
>> +
>> +	/*
>> +	 * Currently we only support newer versions of this core
>> +	 * featuring a RX FIFO. Older cores found on some Coldfire
>> +	 * derivates are not yet supported.
>> +	 */
>> +	reg = readl(&regs->mcr);
>> +	if (!(reg & FLEXCAN_MCR_FEN)) {
>> +		dev_err(dev->dev.parent,
>> +			"Could not enable RX FIFO, unsupported core\n");
>> +		err = -ENODEV;
>> +		goto out;
>> +	}
>> +
>> +	err = register_candev(dev);
>> +
>> + out:
>> +	/* disable core and turn off clocks */
>> +	flexcan_chip_disable(priv);
>> +	clk_disable(priv->clk);
>> +
>> +	return err;
>> +}
>> +
>> +static void __devexit unregister_flexcandev(struct net_device *dev)
>> +{
>> +	unregister_candev(dev);
>> +}
>> +
>> +static int __devinit flexcan_probe(struct platform_device *pdev)
>> +{
>> +	struct net_device *dev;
>> +	struct flexcan_priv *priv;
>> +	struct resource *mem;
>> +	struct clk *clk;
>> +	void __iomem *base;
>> +	resource_size_t mem_size;
>> +	int err, irq;
>> +
>> +	clk = clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(clk)) {
>> +		dev_err(&pdev->dev, "no clock defined\n");
>> +		err = PTR_ERR(clk);
>> +		goto failed_clock;
>> +	}
>> +
>> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	irq = platform_get_irq(pdev, 0);
>> +	if (!mem || irq <= 0) {
>> +		err = -ENODEV;
>> +		goto failed_get;
>> +	}
>> +
>> +	mem_size = resource_size(mem);
>> +	if (!request_mem_region(mem->start, mem_size, pdev->name)) {
>> +		err = -EBUSY;
>> +		goto failed_req;
>> +	}
>> +
>> +	base = ioremap(mem->start, mem_size);
>> +	if (!base) {
>> +		err = -ENOMEM;
>> +		goto failed_map;
>> +	}
>> +
>> +	dev = alloc_candev(sizeof(struct flexcan_priv), 0);
>> +	if (!dev) {
>> +		err = -ENOMEM;
>> +		goto failed_alloc;
>> +	}
>> +
>> +	dev->netdev_ops = &flexcan_netdev_ops;
>> +	dev->irq = irq;
>> +	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
>> +
>> +	priv = netdev_priv(dev);
>> +	priv->can.clock.freq = clk_get_rate(clk);
>> +	priv->can.bittiming_const = &flexcan_bittiming_const;
>> +	priv->can.do_set_mode = flexcan_set_mode;
>> +	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
>> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
>> +		CAN_CTRLMODE_LISTENONLY	| CAN_CTRLMODE_3_SAMPLES;
>> +	priv->base = base;
>> +	priv->dev = dev;
>> +	priv->clk = clk;
>> +	priv->pdata = pdev->dev.platform_data;
>> +
>> +	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
>> +
>> +	dev_set_drvdata(&pdev->dev, dev);
>> +	SET_NETDEV_DEV(dev, &pdev->dev);
>> +
>> +	err = register_flexcandev(dev);
>> +	if (err) {
>> +		dev_err(&pdev->dev, "registering netdev failed\n");
>> +		goto failed_register;
>> +	}
>> +
>> +	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
>> +		 priv->base, dev->irq);
>> +
>> +	return 0;
>> +
>> + failed_register:
>> +	free_candev(dev);
>> + failed_alloc:
>> +	iounmap(base);
>> + failed_map:
>> +	release_mem_region(mem->start, mem_size);
>> + failed_req:
>> +	clk_put(clk);
>> + failed_get:
>> + failed_clock:
>> +	return err;
>> +}
>> +
>> +static int __devexit flexcan_remove(struct platform_device *pdev)
>> +{
>> +	struct net_device *dev = platform_get_drvdata(pdev);
>> +	struct flexcan_priv *priv = netdev_priv(dev);
>> +	struct resource *mem;
>> +
>> +	unregister_flexcandev(dev);
>> +	platform_set_drvdata(pdev, NULL);
>> +	free_candev(dev);
>> +	iounmap(priv->base);
>> +
>> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	release_mem_region(mem->start, resource_size(mem));
>> +
>> +	clk_put(priv->clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver flexcan_driver = {
>> +	.driver.name = DRV_NAME,
>> +	.probe = flexcan_probe,
>> +	.remove = __devexit_p(flexcan_remove),
>> +};
>> +
>> +static int __init flexcan_init(void)
>> +{
>> +	pr_info("%s netdevice driver\n", DRV_NAME);
>> +	return platform_driver_register(&flexcan_driver);
>> +}
>> +
>> +static void __exit flexcan_exit(void)
>> +{
>> +	platform_driver_unregister(&flexcan_driver);
>> +	pr_info("%s: driver removed\n", DRV_NAME);
>> +}
>> +
>> +module_init(flexcan_init);
>> +module_exit(flexcan_exit);
>> +
>> +MODULE_AUTHOR("Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>, "
>> +	      "Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("CAN port driver for flexcan based chip");
>> diff --git a/include/linux/can/platform/flexcan.h b/include/linux/can/platform/flexcan.h
>> new file mode 100644
>> index 0000000..72b713a
>> --- /dev/null
>> +++ b/include/linux/can/platform/flexcan.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + * Copyright (C) 2010 Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> + *
>> + * This file is released under the GPLv2
>> + *
>> + */
>> +
>> +#ifndef __CAN_PLATFORM_FLEXCAN_H
>> +#define __CAN_PLATFORM_FLEXCAN_H
>> +
>> +/**
>> + * struct flexcan_platform_data - flex CAN controller platform data
>> + * @transceiver_enable:         - called to power on/off the transceiver
>> + *
>> + */
>> +struct flexcan_platform_data {
>> +	void (*transceiver_switch)(int enable);
>> +};
>> +
>> +#endif /* __CAN_PLATFORM_FLEXCAN_H */
> 
> Wolfgang.

cheers, Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* [PATCH] Drivers: net: 8139cp: Improved conformance to the Linux coding style guidelines.
From: Joseph Kogut @ 2010-07-16 13:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, Joseph Kogut

Fixed several issues that made the 8139C+ driver nonconformant to the Linux coding style guidelines.

Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com>
---
 drivers/net/8139cp.c |  304 +++++++++++++++++++++++++-------------------------
 1 files changed, 153 insertions(+), 151 deletions(-)

diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 284a5f4..b91a508 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -2,12 +2,12 @@
 /*
 	Copyright 2001-2004 Jeff Garzik <jgarzik@pobox.com>
 
-	Copyright (C) 2001, 2002 David S. Miller (davem@redhat.com) [tg3.c]
-	Copyright (C) 2000, 2001 David S. Miller (davem@redhat.com) [sungem.c]
-	Copyright 2001 Manfred Spraul				    [natsemi.c]
-	Copyright 1999-2001 by Donald Becker.			    [natsemi.c]
-       	Written 1997-2001 by Donald Becker.			    [8139too.c]
-	Copyright 1998-2001 by Jes Sorensen, <jes@trained-monkey.org>. [acenic.c]
+	Copyright (C) 2001, 2002 David S. Miller (davem@redhat.com)	[tg3.c]
+	Copyright (C) 2000, 2001 David S. Miller (davem@redhat.com)	[sungem.c]
+	Copyright 2001 Manfred Spraul					[natsemi.c]
+	Copyright 1999-2001 by Donald Becker.				[natsemi.c]
+	Written 1997-2001 by Donald Becker.				[8139too.c]
+	Copyright 1998-2001 by Jes Sorensen, <jes@trained-monkey.org>.	[acenic.c]
 
 	This software may be used and distributed according to the terms of
 	the GNU General Public License (GPL), incorporated herein by reference.
@@ -73,18 +73,18 @@
 #include <linux/tcp.h>
 #include <linux/udp.h>
 #include <linux/cache.h>
-#include <asm/io.h>
-#include <asm/irq.h>
-#include <asm/uaccess.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/uaccess.h>
 
 /* VLAN tagging feature enable/disable */
 #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
 #define CP_VLAN_TAG_USED 1
-#define CP_VLAN_TX_TAG(tx_desc,vlan_tag_value) \
+#define CP_VLAN_TX_TAG(tx_desc, vlan_tag_value) \
 	do { (tx_desc)->opts2 = cpu_to_le32(vlan_tag_value); } while (0)
 #else
 #define CP_VLAN_TAG_USED 0
-#define CP_VLAN_TX_TAG(tx_desc,vlan_tag_value) \
+#define CP_VLAN_TX_TAG(tx_desc, vlan_tag_value) \
 	do { (tx_desc)->opts2 = 0; } while (0)
 #endif
 
@@ -99,16 +99,16 @@ MODULE_LICENSE("GPL");
 
 static int debug = -1;
 module_param(debug, int, 0);
-MODULE_PARM_DESC (debug, "8139cp: bitmapped message enable number");
+MODULE_PARM_DESC(debug, "8139cp: bitmapped message enable number");
 
 /* Maximum number of multicast addresses to filter (vs. Rx-all-multicast).
    The RTL chips use a 64 element hash table based on the Ethernet CRC.  */
 static int multicast_filter_limit = 32;
 module_param(multicast_filter_limit, int, 0);
-MODULE_PARM_DESC (multicast_filter_limit, "8139cp: maximum number of filtered multicast addresses");
+MODULE_PARM_DESC(multicast_filter_limit, "8139cp: maximum number of filtered multicast addresses");
 
 #define CP_DEF_MSG_ENABLE	(NETIF_MSG_DRV		| \
-				 NETIF_MSG_PROBE 	| \
+				 NETIF_MSG_PROBE	| \
 				 NETIF_MSG_LINK)
 #define CP_NUM_STATS		14	/* struct cp_dma_stats, plus one */
 #define CP_STATS_SIZE		64	/* size in bytes of DMA stats block */
@@ -130,8 +130,8 @@ MODULE_PARM_DESC (multicast_filter_limit, "8139cp: maximum number of filtered mu
 #define PKT_BUF_SZ		1536	/* Size of each temporary Rx buffer.*/
 #define CP_INTERNAL_PHY		32
 
-/* The following settings are log_2(bytes)-4:  0 == 16 bytes .. 6==1024, 7==end of packet. */
-#define RX_FIFO_THRESH		5	/* Rx buffer level before first PCI xfer.  */
+/* The following settings are log_2(bytes)-4:  0 == 16 bytes .. 6 == 1024, 7 == end of packet. */
+#define RX_FIFO_THRESH		5	/* Rx buffer level before first PCI xfer. */
 #define RX_DMA_BURST		4	/* Maximum PCI burst, '4' is 256 */
 #define TX_DMA_BURST		6	/* Maximum PCI burst, '6' is 1024 */
 #define TX_EARLY_THRESH		256	/* Early Tx threshold, in bytes */
@@ -366,26 +366,26 @@ struct cp_private {
 #define cpr8(reg)	readb(cp->regs + (reg))
 #define cpr16(reg)	readw(cp->regs + (reg))
 #define cpr32(reg)	readl(cp->regs + (reg))
-#define cpw8(reg,val)	writeb((val), cp->regs + (reg))
-#define cpw16(reg,val)	writew((val), cp->regs + (reg))
-#define cpw32(reg,val)	writel((val), cp->regs + (reg))
-#define cpw8_f(reg,val) do {			\
+#define cpw8(reg, val)	writeb((val), cp->regs + (reg))
+#define cpw16(reg, val)	writew((val), cp->regs + (reg))
+#define cpw32(reg, val)	writel((val), cp->regs + (reg))
+#define cpw8_f(reg, val) do {			\
 	writeb((val), cp->regs + (reg));	\
 	readb(cp->regs + (reg));		\
 	} while (0)
-#define cpw16_f(reg,val) do {			\
+#define cpw16_f(reg, val) do {			\
 	writew((val), cp->regs + (reg));	\
 	readw(cp->regs + (reg));		\
 	} while (0)
-#define cpw32_f(reg,val) do {			\
+#define cpw32_f(reg, val) do {			\
 	writel((val), cp->regs + (reg));	\
 	readl(cp->regs + (reg));		\
 	} while (0)
 
 
-static void __cp_set_rx_mode (struct net_device *dev);
-static void cp_tx (struct cp_private *cp);
-static void cp_clean_rings (struct cp_private *cp);
+static void __cp_set_rx_mode(struct net_device *dev);
+static void cp_tx(struct cp_private *cp);
+static void cp_clean_rings(struct cp_private *cp);
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void cp_poll_controller(struct net_device *dev);
 #endif
@@ -440,7 +440,7 @@ static void cp_vlan_rx_register(struct net_device *dev, struct vlan_group *grp)
 }
 #endif /* CP_VLAN_TAG_USED */
 
-static inline void cp_set_rxbufsize (struct cp_private *cp)
+static inline void cp_set_rxbufsize(struct cp_private *cp)
 {
 	unsigned int mtu = cp->dev->mtu;
 
@@ -451,10 +451,10 @@ static inline void cp_set_rxbufsize (struct cp_private *cp)
 		cp->rx_buf_sz = PKT_BUF_SZ;
 }
 
-static inline void cp_rx_skb (struct cp_private *cp, struct sk_buff *skb,
+static inline void cp_rx_skb(struct cp_private *cp, struct sk_buff *skb,
 			      struct cp_desc *desc)
 {
-	skb->protocol = eth_type_trans (skb, cp->dev);
+	skb->protocol = eth_type_trans(skb, cp->dev);
 
 	cp->dev->stats.rx_packets++;
 	cp->dev->stats.rx_bytes += skb->len;
@@ -468,7 +468,7 @@ static inline void cp_rx_skb (struct cp_private *cp, struct sk_buff *skb,
 		netif_receive_skb(skb);
 }
 
-static void cp_rx_err_acct (struct cp_private *cp, unsigned rx_tail,
+static void cp_rx_err_acct(struct cp_private *cp, unsigned rx_tail,
 			    u32 status, u32 len)
 {
 	netif_dbg(cp, rx_err, cp->dev, "rx err, slot %d status 0x%x len %d\n",
@@ -486,7 +486,7 @@ static void cp_rx_err_acct (struct cp_private *cp, unsigned rx_tail,
 		cp->dev->stats.rx_fifo_errors++;
 }
 
-static inline unsigned int cp_rx_csum_ok (u32 status)
+static inline unsigned int cp_rx_csum_ok(u32 status)
 {
 	unsigned int protocol = (status >> 16) & 0x3;
 
@@ -606,7 +606,7 @@ rx_next:
 	return rx;
 }
 
-static irqreturn_t cp_interrupt (int irq, void *dev_instance)
+static irqreturn_t cp_interrupt(int irq, void *dev_instance)
 {
 	struct net_device *dev = dev_instance;
 	struct cp_private *cp;
@@ -674,7 +674,7 @@ static void cp_poll_controller(struct net_device *dev)
 }
 #endif
 
-static void cp_tx (struct cp_private *cp)
+static void cp_tx(struct cp_private *cp)
 {
 	unsigned tx_head = cp->tx_head;
 	unsigned tx_tail = cp->tx_tail;
@@ -731,7 +731,7 @@ static void cp_tx (struct cp_private *cp)
 		netif_wake_queue(cp->dev);
 }
 
-static netdev_tx_t cp_start_xmit (struct sk_buff *skb,
+static netdev_tx_t cp_start_xmit(struct sk_buff *skb,
 					struct net_device *dev)
 {
 	struct cp_private *cp = netdev_priv(dev);
@@ -889,7 +889,7 @@ static netdev_tx_t cp_start_xmit (struct sk_buff *skb,
 /* Set or clear the multicast filter for this adaptor.
    This routine is not state sensitive and need not be SMP locked. */
 
-static void __cp_set_rx_mode (struct net_device *dev)
+static void __cp_set_rx_mode(struct net_device *dev)
 {
 	struct cp_private *cp = netdev_priv(dev);
 	u32 mc_filter[2];	/* Multicast hash filter */
@@ -923,28 +923,28 @@ static void __cp_set_rx_mode (struct net_device *dev)
 	/* We can safely update without stopping the chip. */
 	tmp = cp_rx_config | rx_mode;
 	if (cp->rx_config != tmp) {
-		cpw32_f (RxConfig, tmp);
+		cpw32_f(RxConfig, tmp);
 		cp->rx_config = tmp;
 	}
-	cpw32_f (MAR0 + 0, mc_filter[0]);
-	cpw32_f (MAR0 + 4, mc_filter[1]);
+	cpw32_f(MAR0 + 0, mc_filter[0]);
+	cpw32_f(MAR0 + 4, mc_filter[1]);
 }
 
-static void cp_set_rx_mode (struct net_device *dev)
+static void cp_set_rx_mode(struct net_device *dev)
 {
 	unsigned long flags;
 	struct cp_private *cp = netdev_priv(dev);
 
-	spin_lock_irqsave (&cp->lock, flags);
+	spin_lock_irqsave(&cp->lock, flags);
 	__cp_set_rx_mode(dev);
-	spin_unlock_irqrestore (&cp->lock, flags);
+	spin_unlock_irqrestore(&cp->lock, flags);
 }
 
 static void __cp_get_stats(struct cp_private *cp)
 {
 	/* only lower 24 bits valid; write any value to clear */
-	cp->dev->stats.rx_missed_errors += (cpr32 (RxMissed) & 0xffffff);
-	cpw32 (RxMissed, 0);
+	cp->dev->stats.rx_missed_errors += (cpr32(RxMissed) & 0xffffff);
+	cpw32(RxMissed, 0);
 }
 
 static struct net_device_stats *cp_get_stats(struct net_device *dev)
@@ -954,14 +954,14 @@ static struct net_device_stats *cp_get_stats(struct net_device *dev)
 
 	/* The chip only need report frame silently dropped. */
 	spin_lock_irqsave(&cp->lock, flags);
- 	if (netif_running(dev) && netif_device_present(dev))
- 		__cp_get_stats(cp);
+	if (netif_running(dev) && netif_device_present(dev))
+		__cp_get_stats(cp);
 	spin_unlock_irqrestore(&cp->lock, flags);
 
 	return &dev->stats;
 }
 
-static void cp_stop_hw (struct cp_private *cp)
+static void cp_stop_hw(struct cp_private *cp)
 {
 	cpw16(IntrStatus, ~(cpr16(IntrStatus)));
 	cpw16_f(IntrMask, 0);
@@ -973,7 +973,7 @@ static void cp_stop_hw (struct cp_private *cp)
 	cp->tx_head = cp->tx_tail = 0;
 }
 
-static void cp_reset_hw (struct cp_private *cp)
+static void cp_reset_hw(struct cp_private *cp)
 {
 	unsigned work = 1000;
 
@@ -989,30 +989,30 @@ static void cp_reset_hw (struct cp_private *cp)
 	netdev_err(cp->dev, "hardware reset timeout\n");
 }
 
-static inline void cp_start_hw (struct cp_private *cp)
+static inline void cp_start_hw(struct cp_private *cp)
 {
 	cpw16(CpCmd, cp->cpcmd);
 	cpw8(Cmd, RxOn | TxOn);
 }
 
-static void cp_init_hw (struct cp_private *cp)
+static void cp_init_hw(struct cp_private *cp)
 {
 	struct net_device *dev = cp->dev;
 	dma_addr_t ring_dma;
 
 	cp_reset_hw(cp);
 
-	cpw8_f (Cfg9346, Cfg9346_Unlock);
+	cpw8_f(Cfg9346, Cfg9346_Unlock);
 
 	/* Restore our idea of the MAC address. */
-	cpw32_f (MAC0 + 0, le32_to_cpu (*(__le32 *) (dev->dev_addr + 0)));
-	cpw32_f (MAC0 + 4, le32_to_cpu (*(__le32 *) (dev->dev_addr + 4)));
+	cpw32_f(MAC0 + 0, le32_to_cpu(*(__le32 *) (dev->dev_addr + 0)));
+	cpw32_f(MAC0 + 4, le32_to_cpu(*(__le32 *) (dev->dev_addr + 4)));
 
 	cp_start_hw(cp);
 	cpw8(TxThresh, 0x06); /* XXX convert magic num to a constant */
 
 	__cp_set_rx_mode(dev);
-	cpw32_f (TxConfig, IFG | (TX_DMA_BURST << TxDMAShift));
+	cpw32_f(TxConfig, IFG | (TX_DMA_BURST << TxDMAShift));
 
 	cpw8(Config1, cpr8(Config1) | DriverLoaded | PMEnable);
 	/* Disable Wake-on-LAN. Can be turned on with ETHTOOL_SWOL */
@@ -1073,23 +1073,23 @@ err_out:
 	return -ENOMEM;
 }
 
-static void cp_init_rings_index (struct cp_private *cp)
+static void cp_init_rings_index(struct cp_private *cp)
 {
 	cp->rx_tail = 0;
 	cp->tx_head = cp->tx_tail = 0;
 }
 
-static int cp_init_rings (struct cp_private *cp)
+static int cp_init_rings(struct cp_private *cp)
 {
 	memset(cp->tx_ring, 0, sizeof(struct cp_desc) * CP_TX_RING_SIZE);
 	cp->tx_ring[CP_TX_RING_SIZE - 1].opts1 = cpu_to_le32(RingEnd);
 
 	cp_init_rings_index(cp);
 
-	return cp_refill_rx (cp);
+	return cp_refill_rx(cp);
 }
 
-static int cp_alloc_rings (struct cp_private *cp)
+static int cp_alloc_rings(struct cp_private *cp)
 {
 	void *mem;
 
@@ -1104,7 +1104,7 @@ static int cp_alloc_rings (struct cp_private *cp)
 	return cp_init_rings(cp);
 }
 
-static void cp_clean_rings (struct cp_private *cp)
+static void cp_clean_rings(struct cp_private *cp)
 {
 	struct cp_desc *desc;
 	unsigned i;
@@ -1112,7 +1112,7 @@ static void cp_clean_rings (struct cp_private *cp)
 	for (i = 0; i < CP_RX_RING_SIZE; i++) {
 		if (cp->rx_skb[i]) {
 			desc = cp->rx_ring + i;
-			dma_unmap_single(&cp->pdev->dev,le64_to_cpu(desc->addr),
+			dma_unmap_single(&cp->pdev->dev, le64_to_cpu(desc->addr),
 					 cp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 			dev_kfree_skb(cp->rx_skb[i]);
 		}
@@ -1123,7 +1123,7 @@ static void cp_clean_rings (struct cp_private *cp)
 			struct sk_buff *skb = cp->tx_skb[i];
 
 			desc = cp->tx_ring + i;
-			dma_unmap_single(&cp->pdev->dev,le64_to_cpu(desc->addr),
+			dma_unmap_single(&cp->pdev->dev, le64_to_cpu(desc->addr),
 					 le32_to_cpu(desc->opts1) & 0xffff,
 					 PCI_DMA_TODEVICE);
 			if (le32_to_cpu(desc->opts1) & LastFrag)
@@ -1139,7 +1139,7 @@ static void cp_clean_rings (struct cp_private *cp)
 	memset(cp->tx_skb, 0, sizeof(struct sk_buff *) * CP_TX_RING_SIZE);
 }
 
-static void cp_free_rings (struct cp_private *cp)
+static void cp_free_rings(struct cp_private *cp)
 {
 	cp_clean_rings(cp);
 	dma_free_coherent(&cp->pdev->dev, CP_RING_BYTES, cp->rx_ring,
@@ -1148,7 +1148,7 @@ static void cp_free_rings (struct cp_private *cp)
 	cp->tx_ring = NULL;
 }
 
-static int cp_open (struct net_device *dev)
+static int cp_open(struct net_device *dev)
 {
 	struct cp_private *cp = netdev_priv(dev);
 	int rc;
@@ -1180,7 +1180,7 @@ err_out_hw:
 	return rc;
 }
 
-static int cp_close (struct net_device *dev)
+static int cp_close(struct net_device *dev)
 {
 	struct cp_private *cp = netdev_priv(dev);
 	unsigned long flags;
@@ -1295,32 +1295,32 @@ static void mdio_write(struct net_device *dev, int phy_id, int location,
 }
 
 /* Set the ethtool Wake-on-LAN settings */
-static int netdev_set_wol (struct cp_private *cp,
+static int netdev_set_wol(struct cp_private *cp,
 			   const struct ethtool_wolinfo *wol)
 {
 	u8 options;
 
-	options = cpr8 (Config3) & ~(LinkUp | MagicPacket);
+	options = cpr8(Config3) & ~(LinkUp | MagicPacket);
 	/* If WOL is being disabled, no need for complexity */
 	if (wol->wolopts) {
-		if (wol->wolopts & WAKE_PHY)	options |= LinkUp;
-		if (wol->wolopts & WAKE_MAGIC)	options |= MagicPacket;
+		if (wol->wolopts & WAKE_PHY) options |= LinkUp;
+		if (wol->wolopts & WAKE_MAGIC) options |= MagicPacket;
 	}
 
-	cpw8 (Cfg9346, Cfg9346_Unlock);
-	cpw8 (Config3, options);
-	cpw8 (Cfg9346, Cfg9346_Lock);
+	cpw8(Cfg9346, Cfg9346_Unlock);
+	cpw8(Config3, options);
+	cpw8(Cfg9346, Cfg9346_Lock);
 
 	options = 0; /* Paranoia setting */
-	options = cpr8 (Config5) & ~(UWF | MWF | BWF);
+	options = cpr8(Config5) & ~(UWF | MWF | BWF);
 	/* If WOL is being disabled, no need for complexity */
 	if (wol->wolopts) {
-		if (wol->wolopts & WAKE_UCAST)  options |= UWF;
-		if (wol->wolopts & WAKE_BCAST)	options |= BWF;
-		if (wol->wolopts & WAKE_MCAST)	options |= MWF;
+		if (wol->wolopts & WAKE_UCAST) options |= UWF;
+		if (wol->wolopts & WAKE_BCAST) options |= BWF;
+		if (wol->wolopts & WAKE_MCAST) options |= MWF;
 	}
 
-	cpw8 (Config5, options);
+	cpw8(Config5, options);
 
 	cp->wol_enabled = (wol->wolopts) ? 1 : 0;
 
@@ -1328,35 +1328,36 @@ static int netdev_set_wol (struct cp_private *cp,
 }
 
 /* Get the ethtool Wake-on-LAN settings */
-static void netdev_get_wol (struct cp_private *cp,
-	             struct ethtool_wolinfo *wol)
+static void netdev_get_wol(struct cp_private *cp,
+			   struct ethtool_wolinfo *wol)
 {
 	u8 options;
 
 	wol->wolopts   = 0; /* Start from scratch */
-	wol->supported = WAKE_PHY   | WAKE_BCAST | WAKE_MAGIC |
-		         WAKE_MCAST | WAKE_UCAST;
+	wol->supported = WAKE_PHY   | WAKE_BCAST | WAKE_MAGIC | WAKE_MCAST | WAKE_UCAST;
+
 	/* We don't need to go on if WOL is disabled */
-	if (!cp->wol_enabled) return;
+	if (!cp->wol_enabled)
+		return;
 
-	options        = cpr8 (Config3);
-	if (options & LinkUp)        wol->wolopts |= WAKE_PHY;
-	if (options & MagicPacket)   wol->wolopts |= WAKE_MAGIC;
+	options        = cpr8(Config3);
+	if (options & LinkUp) wol->wolopts |= WAKE_PHY;
+	if (options & MagicPacket) wol->wolopts |= WAKE_MAGIC;
 
 	options        = 0; /* Paranoia setting */
-	options        = cpr8 (Config5);
-	if (options & UWF)           wol->wolopts |= WAKE_UCAST;
-	if (options & BWF)           wol->wolopts |= WAKE_BCAST;
-	if (options & MWF)           wol->wolopts |= WAKE_MCAST;
+	options        = cpr8(Config5);
+	if (options & UWF) wol->wolopts |= WAKE_UCAST;
+	if (options & BWF) wol->wolopts |= WAKE_BCAST;
+	if (options & MWF) wol->wolopts |= WAKE_MCAST;
 }
 
-static void cp_get_drvinfo (struct net_device *dev, struct ethtool_drvinfo *info)
+static void cp_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info)
 {
 	struct cp_private *cp = netdev_priv(dev);
 
-	strcpy (info->driver, DRV_NAME);
-	strcpy (info->version, DRV_VERSION);
-	strcpy (info->bus_info, pci_name(cp->pdev));
+	strcpy(info->driver, DRV_NAME);
+	strcpy(info->version, DRV_VERSION);
+	strcpy(info->bus_info, pci_name(cp->pdev));
 }
 
 static int cp_get_regs_len(struct net_device *dev)
@@ -1364,7 +1365,7 @@ static int cp_get_regs_len(struct net_device *dev)
 	return CP_REGS_SIZE;
 }
 
-static int cp_get_sset_count (struct net_device *dev, int sset)
+static int cp_get_sset_count(struct net_device *dev, int sset)
 {
 	switch (sset) {
 	case ETH_SS_STATS:
@@ -1449,7 +1450,7 @@ static int cp_set_rx_csum(struct net_device *dev, u32 data)
 }
 
 static void cp_get_regs(struct net_device *dev, struct ethtool_regs *regs,
-		        void *p)
+			void *p)
 {
 	struct cp_private *cp = netdev_priv(dev);
 	unsigned long flags;
@@ -1464,30 +1465,30 @@ static void cp_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 	spin_unlock_irqrestore(&cp->lock, flags);
 }
 
-static void cp_get_wol (struct net_device *dev, struct ethtool_wolinfo *wol)
+static void cp_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct cp_private *cp = netdev_priv(dev);
 	unsigned long flags;
 
-	spin_lock_irqsave (&cp->lock, flags);
-	netdev_get_wol (cp, wol);
-	spin_unlock_irqrestore (&cp->lock, flags);
+	spin_lock_irqsave(&cp->lock, flags);
+	netdev_get_wol(cp, wol);
+	spin_unlock_irqrestore(&cp->lock, flags);
 }
 
-static int cp_set_wol (struct net_device *dev, struct ethtool_wolinfo *wol)
+static int cp_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct cp_private *cp = netdev_priv(dev);
 	unsigned long flags;
 	int rc;
 
-	spin_lock_irqsave (&cp->lock, flags);
-	rc = netdev_set_wol (cp, wol);
-	spin_unlock_irqrestore (&cp->lock, flags);
+	spin_lock_irqsave(&cp->lock, flags);
+	rc = netdev_set_wol(cp, wol);
+	spin_unlock_irqrestore(&cp->lock, flags);
 
 	return rc;
 }
 
-static void cp_get_strings (struct net_device *dev, u32 stringset, u8 *buf)
+static void cp_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 {
 	switch (stringset) {
 	case ETH_SS_STATS:
@@ -1499,7 +1500,7 @@ static void cp_get_strings (struct net_device *dev, u32 stringset, u8 *buf)
 	}
 }
 
-static void cp_get_ethtool_stats (struct net_device *dev,
+static void cp_get_ethtool_stats(struct net_device *dev,
 				  struct ethtool_stats *estats, u64 *tmp_stats)
 {
 	struct cp_private *cp = netdev_priv(dev);
@@ -1571,7 +1572,7 @@ static const struct ethtool_ops cp_ethtool_ops = {
 	.set_eeprom		= cp_set_eeprom,
 };
 
-static int cp_ioctl (struct net_device *dev, struct ifreq *rq, int cmd)
+static int cp_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct cp_private *cp = netdev_priv(dev);
 	int rc;
@@ -1599,8 +1600,8 @@ static int cp_set_mac_address(struct net_device *dev, void *p)
 	spin_lock_irq(&cp->lock);
 
 	cpw8_f(Cfg9346, Cfg9346_Unlock);
-	cpw32_f(MAC0 + 0, le32_to_cpu (*(__le32 *) (dev->dev_addr + 0)));
-	cpw32_f(MAC0 + 4, le32_to_cpu (*(__le32 *) (dev->dev_addr + 4)));
+	cpw32_f(MAC0 + 0, le32_to_cpu(*(__le32 *) (dev->dev_addr + 0)));
+	cpw32_f(MAC0 + 4, le32_to_cpu(*(__le32 *) (dev->dev_addr + 4)));
 	cpw8_f(Cfg9346, Cfg9346_Lock);
 
 	spin_unlock_irq(&cp->lock);
@@ -1640,9 +1641,9 @@ static int cp_set_mac_address(struct net_device *dev, void *p)
 
 static void eeprom_cmd_start(void __iomem *ee_addr)
 {
-	writeb (EE_ENB & ~EE_CS, ee_addr);
-	writeb (EE_ENB, ee_addr);
-	eeprom_delay ();
+	writeb(EE_ENB & ~EE_CS, ee_addr);
+	writeb(EE_ENB, ee_addr);
+	eeprom_delay();
 }
 
 static void eeprom_cmd(void __iomem *ee_addr, int cmd, int cmd_len)
@@ -1652,19 +1653,20 @@ static void eeprom_cmd(void __iomem *ee_addr, int cmd, int cmd_len)
 	/* Shift the command bits out. */
 	for (i = cmd_len - 1; i >= 0; i--) {
 		int dataval = (cmd & (1 << i)) ? EE_DATA_WRITE : 0;
-		writeb (EE_ENB | dataval, ee_addr);
-		eeprom_delay ();
-		writeb (EE_ENB | dataval | EE_SHIFT_CLK, ee_addr);
-		eeprom_delay ();
+		writeb(EE_ENB | dataval, ee_addr);
+		eeprom_delay();
+		writeb(EE_ENB | dataval | EE_SHIFT_CLK, ee_addr);
+		eeprom_delay();
 	}
-	writeb (EE_ENB, ee_addr);
-	eeprom_delay ();
+
+	writeb(EE_ENB, ee_addr);
+	eeprom_delay();
 }
 
 static void eeprom_cmd_end(void __iomem *ee_addr)
 {
-	writeb (~EE_CS, ee_addr);
-	eeprom_delay ();
+	writeb(~EE_CS, ee_addr);
+	eeprom_delay();
 }
 
 static void eeprom_extend_cmd(void __iomem *ee_addr, int extend_cmd,
@@ -1677,7 +1679,7 @@ static void eeprom_extend_cmd(void __iomem *ee_addr, int extend_cmd,
 	eeprom_cmd_end(ee_addr);
 }
 
-static u16 read_eeprom (void __iomem *ioaddr, int location, int addr_len)
+static u16 read_eeprom(void __iomem *ioaddr, int location, int addr_len)
 {
 	int i;
 	u16 retval = 0;
@@ -1688,13 +1690,13 @@ static u16 read_eeprom (void __iomem *ioaddr, int location, int addr_len)
 	eeprom_cmd(ee_addr, read_cmd, 3 + addr_len);
 
 	for (i = 16; i > 0; i--) {
-		writeb (EE_ENB | EE_SHIFT_CLK, ee_addr);
-		eeprom_delay ();
+		writeb(EE_ENB | EE_SHIFT_CLK, ee_addr);
+		eeprom_delay();
 		retval =
-		    (retval << 1) | ((readb (ee_addr) & EE_DATA_READ) ? 1 :
+		    (retval << 1) | ((readb(ee_addr) & EE_DATA_READ) ? 1 :
 				     0);
-		writeb (EE_ENB, ee_addr);
-		eeprom_delay ();
+		writeb(EE_ENB, ee_addr);
+		eeprom_delay();
 	}
 
 	eeprom_cmd_end(ee_addr);
@@ -1817,17 +1819,17 @@ static int cp_set_eeprom(struct net_device *dev,
 }
 
 /* Put the board into D3cold state and wait for WakeUp signal */
-static void cp_set_d3_state (struct cp_private *cp)
+static void cp_set_d3_state(struct cp_private *cp)
 {
-	pci_enable_wake (cp->pdev, 0, 1); /* Enable PME# generation */
-	pci_set_power_state (cp->pdev, PCI_D3hot);
+	pci_enable_wake(cp->pdev, 0, 1); /* Enable PME# generation */
+	pci_set_power_state(cp->pdev, PCI_D3hot);
 }
 
 static const struct net_device_ops cp_netdev_ops = {
 	.ndo_open		= cp_open,
 	.ndo_stop		= cp_close,
 	.ndo_validate_addr	= eth_validate_addr,
-	.ndo_set_mac_address 	= cp_set_mac_address,
+	.ndo_set_mac_address	= cp_set_mac_address,
 	.ndo_set_multicast_list	= cp_set_rx_mode,
 	.ndo_get_stats		= cp_get_stats,
 	.ndo_do_ioctl		= cp_ioctl,
@@ -1845,7 +1847,7 @@ static const struct net_device_ops cp_netdev_ops = {
 #endif
 };
 
-static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
+static int cp_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct net_device *dev;
 	struct cp_private *cp;
@@ -1877,7 +1879,7 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 	cp->pdev = pdev;
 	cp->dev = dev;
 	cp->msg_enable = (debug < 0 ? CP_DEF_MSG_ENABLE : debug);
-	spin_lock_init (&cp->lock);
+	spin_lock_init(&cp->lock);
 	cp->mii_if.dev = dev;
 	cp->mii_if.mdio_read = mdio_read;
 	cp->mii_if.mdio_write = mdio_write;
@@ -1950,10 +1952,10 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 	cp_stop_hw(cp);
 
 	/* read MAC address from EEPROM */
-	addr_len = read_eeprom (regs, 0, 8) == 0x8129 ? 8 : 6;
+	addr_len = read_eeprom(regs, 0, 8) == 0x8129 ? 8 : 6;
 	for (i = 0; i < 3; i++)
 		((__le16 *) (dev->dev_addr))[i] =
-		    cpu_to_le16(read_eeprom (regs, i + 7, addr_len));
+		    cpu_to_le16(read_eeprom(regs, i + 7, addr_len));
 	memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
 
 	dev->netdev_ops = &cp_netdev_ops;
@@ -1987,7 +1989,7 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 	pci_set_master(pdev);
 
 	if (cp->wol_enabled)
-		cp_set_d3_state (cp);
+		cp_set_d3_state(cp);
 
 	return 0;
 
@@ -2004,7 +2006,7 @@ err_out_free:
 	return rc;
 }
 
-static void cp_remove_one (struct pci_dev *pdev)
+static void cp_remove_one(struct pci_dev *pdev)
 {
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct cp_private *cp = netdev_priv(dev);
@@ -2012,7 +2014,7 @@ static void cp_remove_one (struct pci_dev *pdev)
 	unregister_netdev(dev);
 	iounmap(cp->regs);
 	if (cp->wol_enabled)
-		pci_set_power_state (pdev, PCI_D0);
+		pci_set_power_state(pdev, PCI_D0);
 	pci_release_regions(pdev);
 	pci_clear_mwi(pdev);
 	pci_disable_device(pdev);
@@ -2021,7 +2023,7 @@ static void cp_remove_one (struct pci_dev *pdev)
 }
 
 #ifdef CONFIG_PM
-static int cp_suspend (struct pci_dev *pdev, pm_message_t state)
+static int cp_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct cp_private *cp = netdev_priv(dev);
@@ -2030,16 +2032,16 @@ static int cp_suspend (struct pci_dev *pdev, pm_message_t state)
 	if (!netif_running(dev))
 		return 0;
 
-	netif_device_detach (dev);
-	netif_stop_queue (dev);
+	netif_device_detach(dev);
+	netif_stop_queue(dev);
 
-	spin_lock_irqsave (&cp->lock, flags);
+	spin_lock_irqsave(&cp->lock, flags);
 
 	/* Disable Rx and Tx */
-	cpw16 (IntrMask, 0);
-	cpw8  (Cmd, cpr8 (Cmd) & (~RxOn | ~TxOn));
+	cpw16(IntrMask, 0);
+	cpw8(Cmd, cpr8(Cmd) & (~RxOn | ~TxOn));
 
-	spin_unlock_irqrestore (&cp->lock, flags);
+	spin_unlock_irqrestore(&cp->lock, flags);
 
 	pci_save_state(pdev);
 	pci_enable_wake(pdev, pci_choose_state(pdev, state), cp->wol_enabled);
@@ -2048,31 +2050,31 @@ static int cp_suspend (struct pci_dev *pdev, pm_message_t state)
 	return 0;
 }
 
-static int cp_resume (struct pci_dev *pdev)
+static int cp_resume(struct pci_dev *pdev)
 {
-	struct net_device *dev = pci_get_drvdata (pdev);
+	struct net_device *dev = pci_get_drvdata(pdev);
 	struct cp_private *cp = netdev_priv(dev);
 	unsigned long flags;
 
 	if (!netif_running(dev))
 		return 0;
 
-	netif_device_attach (dev);
+	netif_device_attach(dev);
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
 	pci_enable_wake(pdev, PCI_D0, 0);
 
 	/* FIXME: sh*t may happen if the Rx ring buffer is depleted */
-	cp_init_rings_index (cp);
-	cp_init_hw (cp);
-	netif_start_queue (dev);
+	cp_init_rings_index(cp);
+	cp_init_hw(cp);
+	netif_start_queue(dev);
 
-	spin_lock_irqsave (&cp->lock, flags);
+	spin_lock_irqsave(&cp->lock, flags);
 
 	mii_check_media(&cp->mii_if, netif_msg_link(cp), false);
 
-	spin_unlock_irqrestore (&cp->lock, flags);
+	spin_unlock_irqrestore(&cp->lock, flags);
 
 	return 0;
 }
@@ -2089,7 +2091,7 @@ static struct pci_driver cp_driver = {
 #endif
 };
 
-static int __init cp_init (void)
+static int __init cp_init(void)
 {
 #ifdef MODULE
 	pr_info("%s", version);
@@ -2097,9 +2099,9 @@ static int __init cp_init (void)
 	return pci_register_driver(&cp_driver);
 }
 
-static void __exit cp_exit (void)
+static void __exit cp_exit(void)
 {
-	pci_unregister_driver (&cp_driver);
+	pci_unregister_driver(&cp_driver);
 }
 
 module_init(cp_init);
-- 
1.7.1

^ permalink raw reply related

* Re: oops in tcp_xmit_retransmit_queue() w/ v2.6.32.15
From: Ilpo Järvinen @ 2010-07-16 13:19 UTC (permalink / raw)
  To: Lennart Schulte, David S. Miller
  Cc: Eric Dumazet, Tejun Heo, lkml, netdev@vger.kernel.org,
	Fehrmann, Henning, Carsten Aulbert
In-Reply-To: <4C404FC5.6040107@nets.rwth-aachen.de>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2300 bytes --]

On Fri, 16 Jul 2010, Lennart Schulte wrote:

> On 16.07.2010 14:02, Ilpo Järvinen wrote:
> > 
> > > > > [ 2754.413150] NULL head, pkts 0
> > > > > [ 2754.413156] Errors caught so far 1
> > > > >          
> > Thanks for reporting the results.
> > 
> > Could you post the oops too or double check do the timestamps really match
> > (and there wasn't more "Errors caught" prints in between)? Since this
> > condition doesn't seem to crash the kernel as also send_head should be
> > NULL, which saves the day here exiting the loop (unless send head would
> > too be corrupt).

Doh, I think we'll deref skb already to get the sacked (wouldn't be 
absolutely necessary but better to not trust side-effects) so it 
certainly is bad even with the send_head exit.

> I can try to do some more testing, perhaps then I will get other results. But
> until now I've always gotten something like above.

It might then be useful to remove if (!caught_it) which was to prevent 
infinite printout if the problem is such that it would have persisted 
forever (now w/o the crash), but since there's no evidence of that.

> With the debug patch the kernel doesn't crash, but I have an oops from a run
> before the patch:

Right, no crash of course, stupid me :-).

Lets start with this (I'm not sure if this helps Tejun's case but 
much doubt it does):

--
[PATCH] tcp: fix crash in tcp_xmit_retransmit_queue

It can happen that there are no packets in queue while calling
tcp_xmit_retransmit_queue(). tcp_write_queue_head() then returns
NULL and that gets deref'ed to get sacked into a local var.

There is no work to do if no packets are outstanding so we just
exit early.

There may still be another bug affecting this same function.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
Reported-by: Lennart Schulte <lennart.schulte@nets.rwth-aachen.de>
---
 net/ipv4/tcp_output.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b4ed957..7ed9dc1 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2208,6 +2208,9 @@ void tcp_xmit_retransmit_queue(struct sock *sk)
 	int mib_idx;
 	int fwd_rexmitting = 0;
 
+	if (!tp->packets_out)
+		return;
+
 	if (!tp->lost_out)
 		tp->retransmit_high = tp->snd_una;
 
-- 
1.5.6.5

^ permalink raw reply related

* [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Chetan Loke @ 2010-07-16 13:18 UTC (permalink / raw)
  To: netdev; +Cc: chetanloke, Loke, Chetan

Hello All,

I meant to 'CC' netdev earlier(http://lkml.org/lkml/2010/7/15/334).
Please 'CC' me.

LKML Post:
http://kerneltrap.org/mailarchive/linux-kernel/2010/7/12/4592938


This proposal will provide the ability to shoot an
(early?/prep-time?)'ioctl' via an 'ethX'
agnostic naming scheme.

Requirement:
R1)Ability to address NICs/interfaces using a mac-addr in ioctls. This
is required because we don't have a consistent naming scheme for
Ethernet devices.Asking customers and/or field-engineers to change
udev rules and
other config files is not feasible.

Existing pain-points:
P1) ioctl needs either i) if-name or ii) if-index before we can invoke
bind() etc.This works fine if you know your configuration and it is not going
to change.However,if we hot-add a NIC and if you have adapters from multiple
vendors(think:driver load order) then upon a reboot,the 'eth'
interfaces can be re-mapped.

Existing work-around(s):
W1) user-apps scan /sys/class/net/ethX/address nodes, grep the hw-addrs
till they find a hwaddr-match and then internally create a hwaddr-ethX
mapping table.
W2) change udev-70..persistent rules file and 'rename' the interfaces
according to your needs.
  W2.1) If renaming were to even succeed then none of the existing
drivers re-register their msix-vectors.
  NETDEV_RENAME(or _CHANGE ) handler in the driver does not tear down
the interrupts etc.Some of the sample msix-vectors are as follows : ethX-rx-0,
ethX-rx-1 ... ethX-rx-N
  So if the interface is renamed then how do we measure/correlate the
interrupt-count?

But there is no programmatic way of deriving the 'ethX' name. I got a
few offline replies to the above post, asking me to continue using W1)
from above.Sorry but that was an ugly hack. Also why not replace the
get-ioctls to a 'sys' read everywhere?? ;).

Solution/Proposal:
S1)   Introduce a new ioctl(SIOCGHWADDR_TO_IFNAMEINDEX_MAP[or pick your
name])
S1.1) Enhance dev_ioctl to handle this new case.
S1.2  Re-use for_each_netdev_rcu::is_etherdev_addr(this will iterate
through dev_addrs). By using the above for_each loop we don't need to
re-invent the
wheel.

Input(ifr->hw_addr) : output -> if_name and if_index if ifr->hw_addr is
found.

This way an app can first shoot down an ioctl(sock_fd,
SIOCGHWADDR_TO_IFNAMEINDEX_MAP,ifr), where ifr.ifr_hwaddr is populated
w/ the mac_addr whose mapping you would like.
Then once the if_name and if_index is known, using other ioctls is easy.


Please review the proposal and the sample code below. If this is not a
good approach and if there is a simple workaround then please let me
know.


Regards
Chetan Loke


----------------------------------------------------------

Sample code(PS- I used a quick and dirty driver to demonstrate the
concept rather than modifying the kernel)


Copyright NetScout Systems
Chetan Loke <loke.c@alumni.neu.edu>

struct foo {
      char name[IFNAMSIZ];
      int  index;
};

/* shamelessly copied from compare_etherdev */
/* eventually is_etherdev_equal will be called by dev_ioctl */
int ntct_is_etherdev_equal(u16 *a,u16 *b) {
      return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) == 0;
}

/* eventually enhance dev_ioctl */
int _do_ioctl(unsigned long arg) {

      struct foo my_foo;
      struct net_device *dev;
      int ret=0;
      int found=0;
      int i=0;

      /* eventually sent via
ioctl(sock_fd)->SIOCG_HWADDR_TO_NAMEIDX_MAP and ifr->hw_addr */
      unsigned char mac_addr[]={0x00,0x50,0x56,0xBB,0x52,0xF7};

      /* eventually use rcu_read_lock(); */
      read_lock(&dev_base_lock);

      /* 2.6.31 doesn't have this defined. eventually use
for_each_netdev_rcu. */
      for_each_netdev(&init_net, dev) {
              dev_hold(dev);

              /* eventually use is_etherdev_addr(addr1,addr2) */
              ret = ntct_is_etherdev_equal((u16 *)dev->dev_addr,(u16*)mac_addr);
              if (ret) {
                      printk("<%s> Found
eth-if:%sifindex:%d\n",__func__,dev->name,dev->ifindex);
                      printk("Mac:");
                      for (i=0;i<ETH_ALEN;i++)
                              printk("%02x%c",(unsigned
char)dev->dev_addr[i],((i < 5)? ':':' '));
                      printk("\n");
                      strcpy(my_foo.name,dev->name);
                      my_foo.index=dev->ifindex;
                      dev_put(dev);
                      found=1;
                      break;
              }
              dev_put(dev);
      }

      /* eventually use rcu_read_unlock(); */
      read_unlock(&dev_base_lock);

      if (!found) {
              printk("<%s> hwaddr<->name mapping not found\n",__func__);
              return -EINVAL;
      }

      return copy_to_user((char *)arg,&my_foo,sizeof(struct foo)) ? -EFAULT : 0;
}

^ permalink raw reply

* Re: [PATCH RFC] act_cpu: packet distributing
From: jamal @ 2010-07-16 13:16 UTC (permalink / raw)
  To: Changli Gao
  Cc: Eric Dumazet, David S. Miller, Patrick McHardy, Tom Herbert,
	netdev
In-Reply-To: <AANLkTiljdBpQkZAGybwFQtej-_j7sFtD72CTErT9TaOX@mail.gmail.com>

On Fri, 2010-07-16 at 07:30 +0800, Changli Gao wrote:

> The tcf_lock is per-instance, so I should not an issue here if the
> corresponding NIC isn't an multiqueue NIC or the instance is
> per-rx-queue. 

It has more to do with config. Actions can be explicitly pointed to 
using the index parameter. Example for configuring two flows using an
aggregate rate with a policer instance 1:
tc filter add dev eth0 ... u32 flow1 action police blah index 1
tc filter add dev eth2 .... u32 flow2 action police blah index 1

If you dont configure it such that multiple tc flows share the same
action as above, then likely the same cpu will always grab the lock. It
is still costly as Eric points out - but i will be very curious to find
out the results.

> I agree
> the performance data is necessary and I'll publish it in the formal
> patch.

If you do this, please compare against rps and non-rps and then i feel
motivated to volunteer to create a setup (not in July unfortunately) to
do a similar test on your code and compare against rps on a
nehalem-with-shitty-sky2 that i can get time on.

cheers,
jamal


^ permalink raw reply

* [GIT PULL] vhost-net fixes
From: Michael S. Tsirkin @ 2010-07-16 12:25 UTC (permalink / raw)
  To: David Miller; +Cc: kvm, virtualization, netdev, linux-kernel

David, please pull the following fixes for 2.6.35.
Thanks!

The following changes since commit 91a72a70594e5212c97705ca6a694bd307f7a26b:

  net/core: neighbour update Oops (2010-07-14 18:02:16 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net

Michael S. Tsirkin (2):
      vhost-net: avoid flush under lock
      vhost: avoid pr_err on condition guest can trigger

 drivers/vhost/net.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

^ permalink raw reply

* [PATCH 1/2] Remove REDWOOD_[456] config options and conditional code
From: Christian Dietrich @ 2010-07-16 12:29 UTC (permalink / raw)
  To: Milton Miller, Josh Boyer, Matt Porter, Benjamin Herrenschmidt,
	Paul 
  Cc: vamos-dev
In-Reply-To: <cover.1279282865.git.qy03fugy@stud.informatik.uni-erlangen.de>

The config options for REDWOOD_[456] were commented out in the powerpc
Kconfig. The ifdefs referencing this options therefore are dead and all
references to this can be removed (Also dependencies in other KConfig
files).

Signed-off-by: Christian Dietrich <qy03fugy@stud.informatik.uni-erlangen.de>
Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
---
 arch/powerpc/platforms/40x/Kconfig |   16 -------------
 drivers/mtd/maps/Kconfig           |    2 +-
 drivers/mtd/maps/redwood.c         |   43 ------------------------------------
 drivers/net/Kconfig                |    2 +-
 drivers/net/smc91x.h               |   37 -------------------------------
 5 files changed, 2 insertions(+), 98 deletions(-)

diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index ec64264..b721764 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -71,22 +71,6 @@ config MAKALU
 	help
 	  This option enables support for the AMCC PPC405EX board.
 
-#config REDWOOD_5
-#	bool "Redwood-5"
-#	depends on 40x
-#	default n
-#	select STB03xxx
-#	help
-#	  This option enables support for the IBM STB04 evaluation board.
-
-#config REDWOOD_6
-#	bool "Redwood-6"
-#	depends on 40x
-#	default n
-#	select STB03xxx
-#	help
-#	  This option enables support for the IBM STBx25xx evaluation board.
-
 #config SYCAMORE
 #	bool "Sycamore"
 #	depends on 40x
diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
index f22bc9f..6629d09 100644
--- a/drivers/mtd/maps/Kconfig
+++ b/drivers/mtd/maps/Kconfig
@@ -321,7 +321,7 @@ config MTD_CFI_FLAGADM
 
 config MTD_REDWOOD
 	tristate "CFI Flash devices mapped on IBM Redwood"
-	depends on MTD_CFI && ( REDWOOD_4 || REDWOOD_5 || REDWOOD_6 )
+	depends on MTD_CFI
 	help
 	  This enables access routines for the flash chips on the IBM
 	  Redwood board. If you have one of these boards and would like to
diff --git a/drivers/mtd/maps/redwood.c b/drivers/mtd/maps/redwood.c
index 933c0b6..d2c9db0 100644
--- a/drivers/mtd/maps/redwood.c
+++ b/drivers/mtd/maps/redwood.c
@@ -22,8 +22,6 @@
 
 #include <asm/io.h>
 
-#if !defined (CONFIG_REDWOOD_6)
-
 #define WINDOW_ADDR 0xffc00000
 #define WINDOW_SIZE 0x00400000
 
@@ -69,47 +67,6 @@ static struct mtd_partition redwood_flash_partitions[] = {
 	}
 };
 
-#else /* CONFIG_REDWOOD_6 */
-/* FIXME: the window is bigger - armin */
-#define WINDOW_ADDR 0xff800000
-#define WINDOW_SIZE 0x00800000
-
-#define RW_PART0_OF	0
-#define RW_PART0_SZ	0x400000	/* 4 MiB data */
-#define RW_PART1_OF	RW_PART0_OF + RW_PART0_SZ
-#define RW_PART1_SZ	0x10000		/* 64K VPD */
-#define RW_PART2_OF	RW_PART1_OF + RW_PART1_SZ
-#define RW_PART2_SZ	0x400000 - (0x10000 + 0x20000)
-#define RW_PART3_OF	RW_PART2_OF + RW_PART2_SZ
-#define RW_PART3_SZ	0x20000
-
-static struct mtd_partition redwood_flash_partitions[] = {
-	{
-		.name = "Redwood filesystem",
-		.offset = RW_PART0_OF,
-		.size = RW_PART0_SZ
-	},
-	{
-		.name = "Redwood OpenBIOS Vital Product Data",
-		.offset = RW_PART1_OF,
-		.size = RW_PART1_SZ,
-		.mask_flags = MTD_WRITEABLE	/* force read-only */
-	},
-	{
-		.name = "Redwood kernel",
-		.offset = RW_PART2_OF,
-		.size = RW_PART2_SZ
-	},
-	{
-		.name = "Redwood OpenBIOS",
-		.offset = RW_PART3_OF,
-		.size = RW_PART3_SZ,
-		.mask_flags = MTD_WRITEABLE	/* force read-only */
-	}
-};
-
-#endif /* CONFIG_REDWOOD_6 */
-
 struct map_info redwood_flash_map = {
 	.name = "IBM Redwood",
 	.size = WINDOW_SIZE,
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index ce2fcdd..313d306 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -913,7 +913,7 @@ config SMC91X
 	tristate "SMC 91C9x/91C1xxx support"
 	select CRC32
 	select MII
-	depends on ARM || REDWOOD_5 || REDWOOD_6 || M32R || SUPERH || \
+	depends on ARM || M32R || SUPERH || \
 		MIPS || BLACKFIN || MN10300 || COLDFIRE
 	help
 	  This is a driver for SMC's 91x series of Ethernet chipsets,
diff --git a/drivers/net/smc91x.h b/drivers/net/smc91x.h
index 8d2772c..ee74791 100644
--- a/drivers/net/smc91x.h
+++ b/drivers/net/smc91x.h
@@ -83,43 +83,6 @@ static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg)
 	}
 }
 
-#elif defined(CONFIG_REDWOOD_5) || defined(CONFIG_REDWOOD_6)
-
-/* We can only do 16-bit reads and writes in the static memory space. */
-#define SMC_CAN_USE_8BIT	0
-#define SMC_CAN_USE_16BIT	1
-#define SMC_CAN_USE_32BIT	0
-#define SMC_NOWAIT		1
-
-#define SMC_IO_SHIFT		0
-
-#define SMC_inw(a, r)		in_be16((volatile u16 *)((a) + (r)))
-#define SMC_outw(v, a, r)	out_be16((volatile u16 *)((a) + (r)), v)
-#define SMC_insw(a, r, p, l) 						\
-	do {								\
-		unsigned long __port = (a) + (r);			\
-		u16 *__p = (u16 *)(p);					\
-		int __l = (l);						\
-		insw(__port, __p, __l);					\
-		while (__l > 0) {					\
-			*__p = swab16(*__p);				\
-			__p++;						\
-			__l--;						\
-		}							\
-	} while (0)
-#define SMC_outsw(a, r, p, l) 						\
-	do {								\
-		unsigned long __port = (a) + (r);			\
-		u16 *__p = (u16 *)(p);					\
-		int __l = (l);						\
-		while (__l > 0) {					\
-			/* Believe it or not, the swab isn't needed. */	\
-			outw( /* swab16 */ (*__p++), __port);		\
-			__l--;						\
-		}							\
-	} while (0)
-#define SMC_IRQ_FLAGS		(0)
-
 #elif defined(CONFIG_SA1100_PLEB)
 /* We can only do 16-bit reads and writes in the static memory space. */
 #define SMC_CAN_USE_8BIT	1
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 0/2] Removing dead code
From: Christian Dietrich @ 2010-07-16 12:28 UTC (permalink / raw)
  To: Milton Miller, Josh Boyer, Matt Porter, Benjamin Herrenschmidt,
	Paul 
  Cc: vamos-dev
In-Reply-To: <redwood56-reply-1-miltonm@bga.com>

Hi all!

I merged the two patches from Christoph Egger[1] to remove the
REDWOOD_[456] config depends. And wrote a second patch, which removes
the redwood/mtd mapping module. I hope this is now acceptable to bring
it into the kernel, if this options are really dead.       

Regards

        Christian Dietrich

[0] http://vamos1.informatik.uni-erlangen.de/
[1] Message-Id: <adba61f63f4439ac17f2e428429f01ae5e65ab15.1279110895.git.siccegge@cs.fau.de>

Christian Dietrich (2):
  Remove REDWOOD_[456] config options and conditional code
  Removed redwood/mtd mapping

 arch/powerpc/platforms/40x/Kconfig |   16 ----
 drivers/mtd/maps/Kconfig           |    8 --
 drivers/mtd/maps/Makefile          |    1 -
 drivers/mtd/maps/redwood.c         |  174 ------------------------------------
 drivers/net/Kconfig                |    2 +-
 drivers/net/smc91x.h               |   37 --------
 6 files changed, 1 insertions(+), 237 deletions(-)
 delete mode 100644 drivers/mtd/maps/redwood.c


^ permalink raw reply

* Re: oops in tcp_xmit_retransmit_queue() w/ v2.6.32.15
From: Lennart Schulte @ 2010-07-16 12:25 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Eric Dumazet, Tejun Heo, David S. Miller, lkml,
	netdev@vger.kernel.org, Fehrmann, Henning, Carsten Aulbert
In-Reply-To: <alpine.DEB.2.00.1007161448330.13946@melkinpaasi.cs.helsinki.fi>

On 16.07.2010 14:02, Ilpo Järvinen wrote:
>
>>>> [ 2754.413150] NULL head, pkts 0
>>>> [ 2754.413156] Errors caught so far 1
>>>>          
> Thanks for reporting the results.
>
> Could you post the oops too or double check do the timestamps really match
> (and there wasn't more "Errors caught" prints in between)? Since this
> condition doesn't seem to crash the kernel as also send_head should be
> NULL, which saves the day here exiting the loop (unless send head would
> too be corrupt).
>    
I can try to do some more testing, perhaps then I will get other 
results. But until now I've always gotten something like above.

With the debug patch the kernel doesn't crash, but I have an oops from a 
run before the patch:

[ 3214.498061] BUG: unable to handle kernel NULL pointer dereference at 
(null)
[ 3214.498085] IP: [<c12657dc>] tcp_xmit_retransmit_queue+0x4c/0x2b0
[ 3214.498121] *pdpt = 00000002cf6fa001
[ 3214.498130] Thread overran stack, or stack corrupted
[ 3214.498138] Oops: 0000 [#1] SMP
[ 3214.498154] last sysfs file: /sys/kernel/uevent_seqnum
[ 3214.498161] Modules linked in: tcp_ancr tcp_ncr
[ 3214.498174]
[ 3214.498180] Pid: 0, comm: swapper Not tainted (2.6.31.9-pae-um-wolff 
#79)
[ 3214.498188] EIP: 0061:[<c12657dc>] EFLAGS: 00010246 CPU: 0
[ 3214.498196] EIP is at tcp_xmit_retransmit_queue+0x4c/0x2b0
[ 3214.498203] EAX: c6da2900 EBX: c6da2880 ECX: 00000000 EDX: e50c512e
[ 3214.498211] ESI: 00000000 EDI: 0000051b EBP: c6da2900 ESP: c13d5cf0
[ 3214.498219]  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0069
[ 3214.498227] Process swapper (pid: 0, ti=c13d4000 task=c13e7a20 
task.ti=c13d4000)
[ 3214.498236] Stack:
[ 3214.498240]  c1005a0b 00000001 00000000 e50c512e c7804300 00000013 
c6da2880 0000051b
[ 3214.498264] <0> e50c512e c1260709 c6cbf840 c6d42000 c1031826 c1288bbd 
c6da2900 c6e09320
[ 3214.498290] <0> c6e09300 00000000 00000000 00000001 e50c512d e521a346 
e50c512e 00000000
[ 3214.498318] Call Trace:
[ 3214.498329]  [<c1005a0b>] ? xen_restore_fl_direct_end+0x0/0x1
[ 3214.498339]  [<c1260709>] ? tcp_ack+0x7f9/0x10d0
[ 3214.498350]  [<c1031826>] ? local_bh_enable+0x56/0x80
[ 3214.498359]  [<c1288bbd>] ? ipt_do_table+0x2dd/0x590
[ 3214.498369]  [<c1261eef>] ? tcp_rcv_state_process+0x41f/0x970
[ 3214.498378]  [<c1267e7f>] ? tcp_v4_do_rcv+0x8f/0x1e0
[ 3214.498387]  [<c1269ccd>] ? tcp_v4_rcv+0x68d/0x7d0
[ 3214.498397]  [<c124d5f0>] ? ip_local_deliver_finish+0x0/0x1e0
[ 3214.498406]  [<c124d687>] ? ip_local_deliver_finish+0x97/0x1e0
[ 3214.498416]  [<c124d5f0>] ? ip_local_deliver_finish+0x0/0x1e0
[ 3214.498425]  [<c124d3eb>] ? ip_rcv_finish+0x13b/0x340
[ 3214.498434]  [<c124d2b0>] ? ip_rcv_finish+0x0/0x340
[ 3214.498442]  [<c124d8c0>] ? ip_rcv+0x0/0x2e0
[ 3214.498452]  [<c1211037>] ? netif_receive_skb+0x2f7/0x4c0
[ 3214.498468]  [<c1213fe0>] ? process_backlog+0x70/0xb0
[ 3214.498476]  [<c1213d98>] ? net_rx_action+0xe8/0x1a0
[ 3214.498486]  [<c103116d>] ? __do_softirq+0x8d/0x120
[ 3214.498494]  [<c100360d>] ? xen_mc_flush+0xed/0x1a0
[ 3214.498504]  [<c1057891>] ? move_native_irq+0x11/0x50
[ 3214.498513]  [<c1031238>] ? do_softirq+0x38/0x40
[ 3214.498523]  [<c11930e2>] ? xen_evtchn_do_upcall+0x142/0x160
[ 3214.498534]  [<c10087d7>] ? xen_do_upcall+0x7/0xc
[ 3214.498543]  [<c10013a7>] ? hypercall_page+0x3a7/0x1010
[ 3214.498552]  [<c100520f>] ? xen_safe_halt+0xf/0x20
[ 3214.498560]  [<c100349c>] ? xen_idle+0x1c/0x30
[ 3214.498569]  [<c1006bba>] ? cpu_idle+0x3a/0x60
[ 3214.498578]  [<c141692a>] ? start_kernel+0x26a/0x300
[ 3214.498616]  [<c1416280>] ? unknown_bootoption+0x0/0x1c0
[ 3214.498630]  [<c141938e>] ? xen_start_kernel+0x3be/0x3e0
[ 3214.498637] Code: 00 00 8b b3 a0 03 00 00 85 f6 0f 84 53 02 00 00 8b 
46 3c 8d ab 80 00 00 00 8b 93 04 04 00 00 39 c2 89 54 24 0c 0f 89 1c 02 
00 00 <8b> 06 0f 18 00 90 39 ee 0f 84 30 01 00 00 39 b3 28 01 00 00 8d
[ 3214.498820] EIP: [<c12657dc>] tcp_xmit_retransmit_queue+0x4c/0x2b0 
SS:ESP 0069:c13d5cf0
[ 3214.498836] CR2: 0000000000000000
[ 3214.498846] ---[ end trace 709a97adf87834a7 ]---
[ 3214.498852] Kernel panic - not syncing: Fatal exception in interrupt
[ 3214.498862] Pid: 0, comm: swapper Tainted: G      D    
2.6.31.9-pae-um-wolff #79
[ 3214.498870] Call Trace:
[ 3214.498878]  [<c102c896>] ? panic+0x46/0x100
[ 3214.498904]  [<c100b428>] ? oops_end+0x98/0xa0
[ 3214.498922]  [<c1019eff>] ? no_context+0x11f/0x1b0
[ 3214.498930]  [<c101a516>] ? do_page_fault+0x66/0x240
[ 3214.498939]  [<c101a4b0>] ? do_page_fault+0x0/0x240
[ 3214.498947]  [<c101a23f>] ? bad_area_nosemaphore+0xf/0x20
[ 3214.498955]  [<c1308b7e>] ? error_code+0x66/0x6c
[ 3214.498963]  [<c101a4b0>] ? do_page_fault+0x0/0x240
[ 3214.498972]  [<c12657dc>] ? tcp_xmit_retransmit_queue+0x4c/0x2b0
[ 3214.498982]  [<c1005a0b>] ? xen_restore_fl_direct_end+0x0/0x1
[ 3214.498991]  [<c1260709>] ? tcp_ack+0x7f9/0x10d0
[ 3214.498999]  [<c1031826>] ? local_bh_enable+0x56/0x80
[ 3214.499009]  [<c1288bbd>] ? ipt_do_table+0x2dd/0x590
[ 3214.499017]  [<c1261eef>] ? tcp_rcv_state_process+0x41f/0x970
[ 3214.499025]  [<c1267e7f>] ? tcp_v4_do_rcv+0x8f/0x1e0
[ 3214.499034]  [<c1269ccd>] ? tcp_v4_rcv+0x68d/0x7d0
[ 3214.499044]  [<c124d5f0>] ? ip_local_deliver_finish+0x0/0x1e0
[ 3214.499053]  [<c124d687>] ? ip_local_deliver_finish+0x97/0x1e0
[ 3214.499063]  [<c124d5f0>] ? ip_local_deliver_finish+0x0/0x1e0
[ 3214.499072]  [<c124d3eb>] ? ip_rcv_finish+0x13b/0x340
[ 3214.499079]  [<c124d2b0>] ? ip_rcv_finish+0x0/0x340
[ 3214.499087]  [<c124d8c0>] ? ip_rcv+0x0/0x2e0
[ 3214.499101]  [<c1211037>] ? netif_receive_skb+0x2f7/0x4c0
[ 3214.499115]  [<c1213fe0>] ? process_backlog+0x70/0xb0
[ 3214.499123]  [<c1213d98>] ? net_rx_action+0xe8/0x1a0
[ 3214.499131]  [<c103116d>] ? __do_softirq+0x8d/0x120
[ 3214.499143]  [<c100360d>] ? xen_mc_flush+0xed/0x1a0
[ 3214.499152]  [<c1057891>] ? move_native_irq+0x11/0x50
[ 3214.499160]  [<c1031238>] ? do_softirq+0x38/0x40
[ 3214.499174]  [<c11930e2>] ? xen_evtchn_do_upcall+0x142/0x160
[ 3214.499188]  [<c10087d7>] ? xen_do_upcall+0x7/0xc
[ 3214.499195]  [<c10013a7>] ? hypercall_page+0x3a7/0x1010
[ 3214.499203]  [<c100520f>] ? xen_safe_halt+0xf/0x20
[ 3214.499214]  [<c100349c>] ? xen_idle+0x1c/0x30
[ 3214.499223]  [<c1006bba>] ? cpu_idle+0x3a/0x60
[ 3214.499231]  [<c141692a>] ? start_kernel+0x26a/0x300
[ 3214.499239]  [<c1416280>] ? unknown_bootoption+0x0/0x1c0
[ 3214.499247]  [<c141938e>] ? xen_start_kernel+0x3be/0x3e0

^ permalink raw reply

* Re: Badness with the kernel version 2.6.35-rc1-git1 running on P6 box
From: Eric Dumazet @ 2010-07-16 12:20 UTC (permalink / raw)
  To: divya
  Cc: LKML, linuxppc-dev, sachinp, benh, netdev, David Miller,
	Jan-Bernd Themann
In-Reply-To: <1279274185.2549.14.camel@edumazet-laptop>

Le vendredi 16 juillet 2010 à 11:56 +0200, Eric Dumazet a écrit :

> [PATCH] ehea: ehea_get_stats() should use GFP_KERNEL
> 
> ehea_get_stats() is called in process context and should use GFP_KERNEL
> allocation instead of GFP_ATOMIC.
> 
> Clearing stats at beginning of ehea_get_stats() is racy in case of
> concurrent stat readers.
> 
> get_stats() can also use netdev net_device_stats, instead of a private
> copy.
> 
> Reported-by: divya <dipraksh@linux.vnet.ibm.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  drivers/net/ehea/ehea.h      |    1 -
>  drivers/net/ehea/ehea_main.c |    6 ++----
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> 

Hmm, net-next-2.6 contains following patch :

commit 3d8009c780ee90fccb5c171caf30aff839f13547
Author: Brian King <brking@linux.vnet.ibm.com>
Date:   Wed Jun 30 11:59:12 2010 +0000

    ehea: Allocate stats buffer with GFP_KERNEL
    
    Since ehea_get_stats calls ehea_h_query_ehea_port, which
    can sleep, we can also sleep when allocating a page in
    this function. This fixes some memory allocation failure
    warnings seen under low memory conditions.
    
    Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index 8b92acb..3beba70 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -335,7 +335,7 @@ static struct net_device_stats
*ehea_get_stats(struct net_device *dev)
 
        memset(stats, 0, sizeof(*stats));
 
-       cb2 = (void *)get_zeroed_page(GFP_ATOMIC);
+       cb2 = (void *)get_zeroed_page(GFP_KERNEL);
        if (!cb2) {
                ehea_error("no mem for cb2");
                goto out;

^ permalink raw reply related

* Re: [PATCH] ipt_REJECT: can't work with bridges
From: Patrick McHardy @ 2010-07-16 12:16 UTC (permalink / raw)
  To: Changli Gao
  Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI, netfilter-devel, netdev
In-Reply-To: <1279251163-2887-1-git-send-email-xiaosuo@gmail.com>

Am 16.07.2010 05:32, schrieb Changli Gao:
> ipt_REJECT: can't work with bridges
> 
> ip_route_me_harder can't create the route cache when the outdev is the same with
> the indev for the skbs whichout a valid protocol set.
> 
> __mkroute_input functions has this check:
> 1998         if (skb->protocol != htons(ETH_P_IP)) {
> 1999                 /* Not IP (i.e. ARP). Do not create route, if it is
> 2000                  * invalid for proxy arp. DNAT routes are always valid.
> 2001                  *
> 2002                  * Proxy arp feature have been extended to allow, ARP
> 2003                  * replies back to the same interface, to support
> 2004                  * Private VLAN switch technologies. See arp.c.
> 2005                  */
> 2006                 if (out_dev == in_dev &&
> 2007                     IN_DEV_PROXY_ARP_PVLAN(in_dev) == 0) {
> 2008                         err = -EINVAL;
> 2009                         goto cleanup;
> 2010                 }
> 2011         }
> 
> This patch gives the new skb a valid protocol to bypass this check. In order to
> make ipt_REJECT work with bridges, you also need to enable ip_forward.

This actually also fixes a regression, back when we used skb_copy_expand
of the old skb, the protocol was properly set, which not only affects
bridges, but also TC classification and packet sockets.

> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
>  net/ipv4/netfilter/ipt_REJECT.c |    1 +
>  1 file changed, 1 insertion(+)
> diff --git a/net/ipv4/netfilter/ipt_REJECT.c b/net/ipv4/netfilter/ipt_REJECT.c
> index bbbd273..e0bca39 100644
> --- a/net/ipv4/netfilter/ipt_REJECT.c
> +++ b/net/ipv4/netfilter/ipt_REJECT.c
> @@ -111,6 +111,7 @@ static void send_reset(struct sk_buff *oldskb, int hook)
>  	/* ip_route_me_harder expects skb->dst to be set */
>  	skb_dst_set_noref(nskb, skb_dst(oldskb));
>  
> +	nskb->protocol = __constant_htons(ETH_P_IP);

You don't need __constant_htons here however, the compiler is smart
enough to this itself. Please resend and add a note to the changelog
stating that this fixes a regression and I'll toss it into
nf-2.6.git.

Thanks!

>  	if (ip_route_me_harder(nskb, addr_type))
>  		goto free_nskb;
>  
> 


^ permalink raw reply

* Re: [PATCHv4] netfilter: add CHECKSUM target
From: Patrick McHardy @ 2010-07-16 12:09 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daniel P. Berrange, Jes Sorensen, David S. Miller, Jan Engelhardt,
	Randy Dunlap, netfilter-devel, netfilter, coreteam, linux-kernel,
	netdev
In-Reply-To: <20100715174216.GA16216@redhat.com>

Am 15.07.2010 19:42, schrieb Michael S. Tsirkin:
> netfilter: correct CHECKSUM header and export it
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Applied, thanks.

^ permalink raw reply

* Re: oops in tcp_xmit_retransmit_queue() w/ v2.6.32.15
From: Ilpo Järvinen @ 2010-07-16 12:02 UTC (permalink / raw)
  To: Lennart Schulte
  Cc: Eric Dumazet, Tejun Heo, David S. Miller, lkml,
	netdev@vger.kernel.org, Fehrmann, Henning, Carsten Aulbert
In-Reply-To: <4C3F053F.7090704@nets.rwth-aachen.de>

On Thu, 15 Jul 2010, Lennart Schulte wrote:

> Since tcp_xmit_retransmit_queue also gets skb == NULL I'm pretty sure it is
> the same bug.
> Up to now I only experienced the problem with ACK loss (without ACK loss the
> test ran about 30min without problems, with ACK loss it had paniced within
> 10min).
> The data sender only has a HTB queue for traffic shaping (set to 20 Mbit/s).
> The ACK loss is done by another router.
> The setup looks like this. This way it seems to be the most realistic.
> 
> o sender with HTB
> |
> |
> o netem queue for forward path delay
> |
> o netem queue for a queue limit
> |
> o netem queue for backward path delay
> |
> o netem queue for ACK loss
> |
> |
> o receiver with HTB
> 
> Perhaps now it is a little big clearer.

> > > [ 2754.413150] NULL head, pkts 0
> > > [ 2754.413156] Errors caught so far 1

Thanks for reporting the results.

Could you post the oops too or double check do the timestamps really match 
(and there wasn't more "Errors caught" prints in between)? Since this 
condition doesn't seem to crash the kernel as also send_head should be 
NULL, which saves the day here exiting the loop (unless send head would 
too be corrupt). ...However, I don't like too much anyway that we can end 
up into tcp_xmit_retransmit_queue loop with packets_out being zero and 
only send_head check side-effect causes proper action.

Besides, Tejun has also found that it's hint->next ptr which is NULL in 
his case so this won't solve his case anyway. Tejun, can you confirm 
whether it was retransmit_skb_hint->next being NULL on _entry time_ to 
tcp_xmit_retransmit_queue() or later on in the loop after the updates done 
by the loop itself to the hint (or that your testing didn't conclude 
either)?

-- 
 i.

^ permalink raw reply

* Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.
From: Richard Cochran @ 2010-07-16 11:49 UTC (permalink / raw)
  To: Andy Fleming; +Cc: netdev
In-Reply-To: <20100716112544.GA2996@riccoc20.at.omicron.at>

On Thu, Jul 08, 2010 at 03:40:41PM -0500, Andy Fleming wrote:
> 
> Wait, is the intent for this mdio read to be done for *every* packet?
> MDIO is spec'ed out to go up to 2.5MHz.  Each transaction takes 64
> cycles.  And I see that reading the timestamp from the PHY you
> submitted support for takes at *least* 2 transactions.  The fastest
> you can process packets would then be under 20,000 packets per second.
> Even on a 100Mb link with full-sized packets, you would be 40% done
> receiving the next packet before you had passed the packet on to the
> stack.  Each MDIO transaction takes 25,600 cycles on a gigahertz
> processor.  It's just too long, IMO, and it looks like this code will
> end up doing up to...10?

Well, it is actually worse than that. From my measurements, 64 cycles
at 2.5 MHz (25.6 usec) is too optimistic. On one Xscale IXP platform,
I get 35 to 40 usec per read. Also, the PHYTER needs six reads for a
Rx and four for a Tx time stamp.

So you are right, it is a very long time to leave interrupts off.

However, not every packet needs a time stamp. The PHY devices that I
know of all are selective. That is, they recognize PTP packets in
hardware and only provide time stamps for those special packets. The
packet rate is not that hight. For example, the "sync" message comes
once every two seconds in PTPv1 and up to ten times per second in
PTPv2.

I have been working on an alternative, which I will post soon. It goes
like this... (comments, please, before I get too far into it!)

* General
   - phy registers itself with net_device { opaque pointer }

   - ts_candidate(skb):
     function to detect likely packets, returns true if
       1. phy_device pointer exists in net_device
       2. data look like PTP, using a BPF

   - work queue:
     1. read out time stamps
     2. match times with queued skbs
     3. deliver skbs with time stamps (or without on timeout)

* Rx path
   - netif_receive_skb:
     if ts_candidate(skb), defer(skb).

* Tx path
   - hook in each MAC driver
   - if ts_candidate(skb), clone and defer(skb).


> I wasn't able to find an example of how you were going to use the
> time-stamp functions you provided.  Could you please go into a little
> more detail about how you intended this to work?

My last PTP series included the PHY stuff, too. There you can see how
it all fits together.

   http://marc.info/?l=linux-netdev&m=127661796627120&w=3


Thanks,
Richard

^ permalink raw reply

* Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.
From: Richard Cochran @ 2010-07-16 11:25 UTC (permalink / raw)
  To: Andy Fleming; +Cc: netdev
In-Reply-To: <20100707071832.GA2816@riccoc20.at.omicron.at>

On Wed, Jul 07, 2010 at 09:18:32AM +0200, Richard Cochran wrote:
> Consider the receive path:
> 
> 1. PTP Packet passed through PHY. PHY recognizes it and stores a time
>    stamp along with some UID from the packet.
> 
> 2. Napi calls the MAC driver's poll function.
> 
> 3. MAC driver acquires packet. At this point, if we want to have a
>    hardware time stamp, we must read it out over the mdio bus, before
>    handing the packet over to the stack via netif_receive_skb().
> 
> If we decide to defer the packet delivery (in step 3), we have to know
> whether the PHY will have a time stamp for this packet. The only way
> to do this is to compare the UIDs, but that requires reading it over
> the mdio bus.

(Forwarding Andy's message to me that was missing a CC to the list)

On Thu, Jul 08, 2010 at 03:40:41PM -0500, Andy Fleming wrote:

Wait, is the intent for this mdio read to be done for *every* packet?
MDIO is spec'ed out to go up to 2.5MHz.  Each transaction takes 64
cycles.  And I see that reading the timestamp from the PHY you
submitted support for takes at *least* 2 transactions.  The fastest
you can process packets would then be under 20,000 packets per second.
Even on a 100Mb link with full-sized packets, you would be 40% done
receiving the next packet before you had passed the packet on to the
stack.  Each MDIO transaction takes 25,600 cycles on a gigahertz
processor.  It's just too long, IMO, and it looks like this code will
end up doing up to...10?

I wasn't able to find an example of how you were going to use the
time-stamp functions you provided.  Could you please go into a little
more detail about how you intended this to work?

^ permalink raw reply

* [PATCH] IPv6: fix CoA check in RH2 input handler (mip6_rthdr_input())
From: Arnaud Ebalard @ 2010-07-16 10:38 UTC (permalink / raw)
  To: David S. Miller, Hideaki YOSHIFUJI; +Cc: netdev

Hi,

The input handler for Type 2 Routing Header (mip6_rthdr_input())
checks if the CoA in the packet matches the CoA in the XFRM state.

Current check is buggy: it compares the adddress in the Type 2
Routing Header, i.e. the HoA, against the expected CoA in the state.
The comparison should be made against the address in the destination
field of the IPv6 header.

The bug remained unnoticed because the main (and possibly only current)
user of the code (UMIP MIPv6 Daemon) initializes the XFRM state with the
unspecified address, i.e. explicitly allows everything.

Yoshifuji-san, can you ack that one?

Cheers,

a+

Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
 net/ipv6/mip6.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/mip6.c b/net/ipv6/mip6.c
index 2794b60..d6e9599 100644
--- a/net/ipv6/mip6.c
+++ b/net/ipv6/mip6.c
@@ -347,11 +347,12 @@ static const struct xfrm_type mip6_destopt_type =
 
 static int mip6_rthdr_input(struct xfrm_state *x, struct sk_buff *skb)
 {
+	struct ipv6hdr *iph = ipv6_hdr(skb);
 	struct rt2_hdr *rt2 = (struct rt2_hdr *)skb->data;
 	int err = rt2->rt_hdr.nexthdr;
 
 	spin_lock(&x->lock);
-	if (!ipv6_addr_equal(&rt2->addr, (struct in6_addr *)x->coaddr) &&
+	if (!ipv6_addr_equal(&iph->daddr, (struct in6_addr *)x->coaddr) &&
 	    !ipv6_addr_any((struct in6_addr *)x->coaddr))
 		err = -ENOENT;
 	spin_unlock(&x->lock);
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 01/11] Removing dead RT2800PCI_SOC
From: Helmut Schaa @ 2010-07-16 10:08 UTC (permalink / raw)
  To: Gertjan van Wingerde
  Cc: Bartlomiej Zolnierkiewicz, Felix Fietkau, John W. Linville,
	Ivo Van Doorn, Christoph Egger, linux-wireless, users, netdev,
	linux-kernel, vamos-dev, Luis Correia
In-Reply-To: <4C4007CD.2070504@gmail.com>

On Fri, Jul 16, 2010 at 9:18 AM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
>
> On 07/16/10 08:57, Helmut Schaa wrote:
> > On Thu, Jul 15, 2010 at 10:41 AM, Bartlomiej Zolnierkiewicz <bzolnier@gmail.com <mailto:bzolnier@gmail.com>> wrote:
> >
> >     On Wednesday 14 July 2010 04:44:44 pm Felix Fietkau wrote:
> >     > On 2010-07-14 3:15 PM, John W. Linville wrote:
> >     > > On Wed, Jul 14, 2010 at 02:52:14PM +0200, Ivo Van Doorn wrote:
> >     > >> On Wed, Jul 14, 2010 at 2:46 PM, Luis Correia <luis.f.correia@gmail.com <mailto:luis.f.correia@gmail.com>> wrote:
> >     > >> > On Wed, Jul 14, 2010 at 13:39, Christoph Egger <siccegge@cs.fau.de <mailto:siccegge@cs.fau.de>> wrote:
> >     > >> >> While RT2800PCI_SOC exists in Kconfig, it depends on either
> >     > >> >> RALINK_RT288X or RALINK_RT305X which are both not available in Kconfig
> >     > >> >> so all Code depending on that can't ever be selected and, if there's
> >     > >> >> no plan to add these options, should be cleaned up
> >     > >> >>
> >     > >> >> Signed-off-by: Christoph Egger <siccegge@cs.fau.de <mailto:siccegge@cs.fau.de>>
> >     > >> >
> >     > >> > NAK,
> >     > >> >
> >     > >> > this is not dead code, it is needed for the Ralink System-on-Chip
> >     > >> > Platform devices.
> >     > >> >
> >     > >> > While I can't fix Kconfig errors and the current KConfig file may be
> >     > >> > wrong, this code cannot and will not be deleted.
> >     > >>
> >     > >> When the config option was introduced, the config options RALINK_RT288X and
> >     > >> RALINK_RT305X were supposed to be merged as well soon after by somebody (Felix?)
> >     > >>
> >     > >> But since testing is done on SoC boards by Helmut and Felix, I assume the code
> >     > >> isn't dead but actually in use.
> >     > >
> >     > > Perhaps Helmut and Felix can send us the missing code?
> >     > The missing code is a MIPS platform port, which is currently being
> >     > maintained in OpenWrt, but is not ready for upstream submission yet.
> >     > I'm not working on this code at the moment, but I think it will be
> >     > submitted once it's ready.
> >
> >     People are using automatic scripts to catch unused config options nowadays
> >     so the issue is quite likely to come back again sooner or later..
> >
> >     Would it be possible to improve situation somehow till the missing parts
> >     get merged?  Maybe by adding a tiny comment documenting RT2800PCI_SOC
> >     situation to Kconfig (if the config option itself really cannot be removed)
> >     until all code is ready etc.?
> >
> >
> > Or we could just remove RT2800PCI_SOC completely and build the soc specific
> > parts always as part of rt2800pci. I mean it's not much code, just the platform
> > driver stuff and the eeprom access.
> >
>
> I'm not sure if that is feasible. Sure, we can reduce the usage of the variable by
> unconditionally compiling in the generic SOC code, but we should not unconditionally
> register the SOC platform device, which is currently also under the scope of this
> Kconfig variable.

Ehm, no, the platform device is not registered in rt2800pci at all,
it's just the platform
driver that gets registered there. The platform device will be
registered in the according
board init code (that only resides in openwrt at the moment).

Helmut

^ permalink raw reply

* Re: Badness with the kernel version 2.6.35-rc1-git1 running on P6 box
From: Eric Dumazet @ 2010-07-16  9:56 UTC (permalink / raw)
  To: divya
  Cc: LKML, linuxppc-dev, sachinp, benh, netdev, David Miller,
	Jan-Bernd Themann
In-Reply-To: <4C401D56.3070108@linux.vnet.ibm.com>

Le vendredi 16 juillet 2010 à 14:20 +0530, divya a écrit :
> Hi ,
> 
> With the latest kernel version 2.6.35-rc5-git1(2f7989efd4398) running on power(p6) box came across the following
> call trace
> 
> Call Trace:
> [c000000006a0e800] [c000000000011c30] .show_stack+0x6c/0x16c (unreliable)
> [c000000006a0e8b0] [c00000000012129c] .__alloc_pages_nodemask+0x6a0/0x75c
> [c000000006a0ea30] [c0000000001527cc] .alloc_pages_current+0xc4/0x104
> [c000000006a0ead0] [c00000000015b1a0] .new_slab+0xe0/0x314
> [c000000006a0eb70] [c00000000015b6fc] .__slab_alloc+0x328/0x644
> [c000000006a0ec50] [c00000000015cc34] .__kmalloc_node_track_caller+0x114/0x194
> [c000000006a0ed00] [c000000000599f6c] .__alloc_skb+0x94/0x180
> [c000000006a0edb0] [c00000000059af5c] .__netdev_alloc_skb+0x3c/0x74
> [c000000006a0ee30] [c0000000004f9480] .ehea_refill_rq_def+0xf8/0x2d0
> [c000000006a0ef30] [c0000000004fab8c] .ehea_up+0x5b8/0x69c
> [c000000006a0f040] [c0000000004facd4] .ehea_open+0x64/0x118
> [c000000006a0f0e0] [c0000000005a6e9c] .__dev_open+0x100/0x168
> [c000000006a0f170] [c0000000005a3ac0] .__dev_change_flags+0x10c/0x1ac
> [c000000006a0f210] [c0000000005a6d44] .dev_change_flags+0x24/0x7c
> [c000000006a0f2a0] [c0000000005b50b4] .do_setlink+0x31c/0x750
> [c000000006a0f3b0] [c0000000005b6724] .rtnl_newlink+0x388/0x618
> [c000000006a0f5f0] [c0000000005b6350] .rtnetlink_rcv_msg+0x268/0x2b4
> [c000000006a0f6a0] [c0000000005cfdc0] .netlink_rcv_skb+0x74/0x108
> [c000000006a0f730] [c0000000005b60c4] .rtnetlink_rcv+0x38/0x5c
> [c000000006a0f7c0] [c0000000005cf8c8] .netlink_unicast+0x318/0x3f4
> [c000000006a0f890] [c0000000005d05b4] .netlink_sendmsg+0x2d0/0x310
> [c000000006a0f970] [c00000000058e1e8] .sock_sendmsg+0xd4/0x110
> [c000000006a0fb50] [c00000000058e514] .SyS_sendmsg+0x1f4/0x288
> [c000000006a0fd70] [c00000000058c2b8] .SyS_socketcall+0x214/0x280
> [c000000006a0fe30] [c0000000000085b4] syscall_exit+0x0/0x40
> Mem-Info:
> Node 0 DMA per-cpu:
> CPU    0: hi:    0, btch:   1 usd:   0
> CPU    1: hi:    0, btch:   1 usd:   0
> CPU    2: hi:    0, btch:   1 usd:   0
> CPU    3: hi:    0, btch:   1 usd:   0
> active_anon:50 inactive_anon:260 isolated_anon:0
>   active_file:159 inactive_file:139 isolated_file:0
>   unevictable:0 dirty:2 writeback:1 unstable:0
>   free:16 slab_reclaimable:66 slab_unreclaimable:502
>   mapped:120 shmem:2 pagetables:37 bounce:0
> Node 0 DMA free:1024kB min:1408kB low:1728kB high:2112kB active_anon:3200kB inactive_anon:16640kB active_file:10176kB inactive_file:8896kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:130944kB mlocked:0kB dirty:128kB writeback:64kB mapped:7680kB shmem:128kB slab_reclaimable:4224kB slab_unreclaimable:32128kB kernel_stack:2528kB pagetables:2368kB unstable:0kB bounce:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
> lowmem_reserve[]: 0 0 0
> Node 0 DMA: 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB 0*8192kB 0*16384kB = 0kB
> 496 total pagecache pages
> 178 pages in swap cache
> Swap cache stats: add 780, delete 602, find 467/551
> Free swap  = 1027904kB
> Total swap = 1044160kB
> 2048 pages RAM
> 683 pages reserved
> 582 pages shared
> 1075 pages non-shared
> SLUB: Unable to allocate memory on node -1 (gfp=0x20)
>    cache: kmalloc-16384, object size: 16384, buffer size: 16384, default order: 2, min order: 0
>    node 0: slabs: 28, objs: 292, free: 0
> ip: page allocation failure. order:0, mode:0x8020
> Call Trace:
> [c000000006a0eb40] [c000000000011c30] .show_stack+0x6c/0x16c (unreliable)
> [c000000006a0ebf0] [c00000000012129c] .__alloc_pages_nodemask+0x6a0/0x75c
> [c000000006a0ed70] [c0000000001527cc] .alloc_pages_current+0xc4/0x104
> [c000000006a0ee10] [c00000000011fca4] .__get_free_pages+0x18/0x90
> [c000000006a0ee90] [c0000000004f7058] .ehea_get_stats+0x4c/0x1bc
> [c000000006a0ef30] [c0000000005a0a04] .dev_get_stats+0x38/0x64
> [c000000006a0efc0] [c0000000005b456c] .rtnl_fill_ifinfo+0x35c/0x85c
> [c000000006a0f150] [c0000000005b5920] .rtmsg_ifinfo+0x164/0x204
> [c000000006a0f210] [c0000000005a6d6c] .dev_change_flags+0x4c/0x7c
> [c000000006a0f2a0] [c0000000005b50b4] .do_setlink+0x31c/0x750
> [c000000006a0f3b0] [c0000000005b6724] .rtnl_newlink+0x388/0x618
> [c000000006a0f5f0] [c0000000005b6350] .rtnetlink_rcv_msg+0x268/0x2b4
> [c000000006a0f6a0] [c0000000005cfdc0] .netlink_rcv_skb+0x74/0x108
> [c000000006a0f730] [c0000000005b60c4] .rtnetlink_rcv+0x38/0x5c
> [c000000006a0f7c0] [c0000000005cf8c8] .netlink_unicast+0x318/0x3f4
> [c000000006a0f890] [c0000000005d05b4] .netlink_sendmsg+0x2d0/0x310
> [c000000006a0f970] [c00000000058e1e8] .sock_sendmsg+0xd4/0x110
> [c000000006a0fb50] [c00000000058e514] .SyS_sendmsg+0x1f4/0x288
> [c000000006a0fd70] [c00000000058c2b8] .SyS_socketcall+0x214/0x280
> [c000000006a0fe30] [c0000000000085b4] syscall_exit+0x0/0x40
> Mem-Info:
> Node 0 DMA per-cpu:
> CPU    0: hi:    0, btch:   1 usd:   0
> CPU    1: hi:    0, btch:   1 usd:   0
> CPU    2: hi:    0, btch:   1 usd:   0
> CPU    3: hi:    0, btch:   1 usd:   0
> 
> The mainline 2.6.35-rc5 worked fine.

Maybe you were lucky with 2.6.35-rc5

Anyway ehea should not use GFP_ATOMIC in its ehea_get_stats() method,
called in process context, but GFP_KERNEL.

Another patch is needed for ehea_refill_rq_def() as well.



[PATCH] ehea: ehea_get_stats() should use GFP_KERNEL

ehea_get_stats() is called in process context and should use GFP_KERNEL
allocation instead of GFP_ATOMIC.

Clearing stats at beginning of ehea_get_stats() is racy in case of
concurrent stat readers.

get_stats() can also use netdev net_device_stats, instead of a private
copy.

Reported-by: divya <dipraksh@linux.vnet.ibm.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/ehea/ehea.h      |    1 -
 drivers/net/ehea/ehea_main.c |    6 ++----
 2 files changed, 2 insertions(+), 5 deletions(-)

^ permalink raw reply

* Re: Raise initial congestion window size / speedup slow  start?
From: Hagen Paul Pfeifer @ 2010-07-16  9:03 UTC (permalink / raw)
  To: Bill Fink
  Cc: David Miller, rick.jones2, lists, davidsen, linux-kernel, netdev
In-Reply-To: <20100714234917.924f420d.billfink@mindspring.com>


On Wed, 14 Jul 2010 23:49:17 -0400, Bill Fink wrote:



> A long, long time ago, I suggested a Path BW Discovery mechanism

> to the IETF, analogous to the Path MTU Discovery mechanism, but

> it didn't get any traction.  Such information could be extremely

> useful to TCP endpoints, to determine a maximum window size to

> use, to effectively rate limit a much stronger sender from

> overpowering a much weaker receiver (for example 10-GigE -> GigE),

> resulting in abominable performance across large RTT paths

> (as low as 12 Mbps), even in the absence of any real network

> contention.



Much weaker middlebox? The windowing mechanism should be sufficient to

avoid endpoints from over-commiting.



Anyway, your proposed draft (I didn't searched for it) sound like a

mechanism similar to RFC 4782: Quick-Start for TCP and IP.





   This document specifies an optional Quick-Start mechanism for

   transport protocols, in cooperation with routers, to determine an

   allowed sending rate at the start and, at times, in the middle of a

   data transfer (e.g., after an idle period).  While Quick-Start is

   designed to be used by a range of transport protocols, in this

   document we only specify its use with TCP.  Quick-Start is designed

   to allow connections to use higher sending rates when there is

   significant unused bandwidth along the path, and the sender and all

   of the routers along the path approve the Quick-Start Request.





Cheers, Hagen

^ permalink raw reply

* [PATCH v2 1/2] bnx2: allocate with GFP_KERNEL flag on RX path init
From: Stanislaw Gruszka @ 2010-07-16  8:55 UTC (permalink / raw)
  To: netdev; +Cc: Michael Chan
In-Reply-To: <C27F8246C663564A84BB7AB3439772421B842BD26F@IRVEXCHCCR01.corp.ad.broadcom.com>

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
v1->v2: use GFP_ATOMIC in bnx2_rx_skb

 drivers/net/bnx2.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index a203f39..a7df539 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -2664,13 +2664,13 @@ bnx2_set_mac_addr(struct bnx2 *bp, u8 *mac_addr, u32 pos)
 }
 
 static inline int
-bnx2_alloc_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
+bnx2_alloc_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gfp_t gfp)
 {
 	dma_addr_t mapping;
 	struct sw_pg *rx_pg = &rxr->rx_pg_ring[index];
 	struct rx_bd *rxbd =
 		&rxr->rx_pg_desc_ring[RX_RING(index)][RX_IDX(index)];
-	struct page *page = alloc_page(GFP_ATOMIC);
+	struct page *page = alloc_page(gfp);
 
 	if (!page)
 		return -ENOMEM;
@@ -2705,7 +2705,7 @@ bnx2_free_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
 }
 
 static inline int
-bnx2_alloc_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
+bnx2_alloc_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gfp_t gfp)
 {
 	struct sk_buff *skb;
 	struct sw_bd *rx_buf = &rxr->rx_buf_ring[index];
@@ -2713,7 +2713,7 @@ bnx2_alloc_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
 	struct rx_bd *rxbd = &rxr->rx_desc_ring[RX_RING(index)][RX_IDX(index)];
 	unsigned long align;
 
-	skb = netdev_alloc_skb(bp->dev, bp->rx_buf_size);
+	skb = __netdev_alloc_skb(bp->dev, bp->rx_buf_size, gfp);
 	if (skb == NULL) {
 		return -ENOMEM;
 	}
@@ -2974,7 +2974,7 @@ bnx2_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, struct sk_buff *skb,
 	int err;
 	u16 prod = ring_idx & 0xffff;
 
-	err = bnx2_alloc_rx_skb(bp, rxr, prod);
+	err = bnx2_alloc_rx_skb(bp, rxr, prod, GFP_ATOMIC);
 	if (unlikely(err)) {
 		bnx2_reuse_rx_skb(bp, rxr, skb, (u16) (ring_idx >> 16), prod);
 		if (hdr_len) {
@@ -3039,7 +3039,8 @@ bnx2_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, struct sk_buff *skb,
 			rx_pg->page = NULL;
 
 			err = bnx2_alloc_rx_page(bp, rxr,
-						 RX_PG_RING_IDX(pg_prod));
+						 RX_PG_RING_IDX(pg_prod),
+						 GFP_ATOMIC);
 			if (unlikely(err)) {
 				rxr->rx_pg_cons = pg_cons;
 				rxr->rx_pg_prod = pg_prod;
@@ -5179,7 +5180,7 @@ bnx2_init_rx_ring(struct bnx2 *bp, int ring_num)
 
 	ring_prod = prod = rxr->rx_pg_prod;
 	for (i = 0; i < bp->rx_pg_ring_size; i++) {
-		if (bnx2_alloc_rx_page(bp, rxr, ring_prod) < 0) {
+		if (bnx2_alloc_rx_page(bp, rxr, ring_prod, GFP_KERNEL) < 0) {
 			netdev_warn(bp->dev, "init'ed rx page ring %d with %d/%d pages only\n",
 				    ring_num, i, bp->rx_pg_ring_size);
 			break;
@@ -5191,7 +5192,7 @@ bnx2_init_rx_ring(struct bnx2 *bp, int ring_num)
 
 	ring_prod = prod = rxr->rx_prod;
 	for (i = 0; i < bp->rx_ring_size; i++) {
-		if (bnx2_alloc_rx_skb(bp, rxr, ring_prod) < 0) {
+		if (bnx2_alloc_rx_skb(bp, rxr, ring_prod, GFP_KERNEL) < 0) {
 			netdev_warn(bp->dev, "init'ed rx ring %d with %d/%d skbs only\n",
 				    ring_num, i, bp->rx_ring_size);
 			break;
-- 
1.7.1


^ permalink raw reply related

* [rfc] ipvs: Add ip_vs_onepacket_enabled()
From: Simon Horman @ 2010-07-16  8:30 UTC (permalink / raw)
  To: Thomas Graf
  Cc: Nick Chalk, Wensong Zhang, Julian Anastasov, lvs-devel, netdev

Add ip_vs_onepacket_enabled() instead of open-coding the logic three times.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

Compile tested only.

Extracted from a patch by Thomas Graf <tgraf@infradead.org>.
Thomas, feel free to make this patch from you and add a signed-off-by line.

Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c
===================================================================
--- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_core.c	2010-07-16 16:44:49.000000000 +0900
+++ nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c	2010-07-16 16:45:20.000000000 +0900
@@ -165,7 +165,6 @@ ip_vs_conn_stats(struct ip_vs_conn *cp,
 	spin_unlock(&ip_vs_stats.lock);
 }
 
-
 static inline int
 ip_vs_set_state(struct ip_vs_conn *cp, int direction,
 		const struct sk_buff *skb,
@@ -176,6 +175,12 @@ ip_vs_set_state(struct ip_vs_conn *cp, i
 	return pp->state_transition(cp, direction, skb, pp);
 }
 
+static inline __u16
+ip_vs_onepacket_enabled(struct ip_vs_service *svc, struct ip_vs_iphdr iph)
+{
+	return (svc->flags & IP_VS_SVC_F_ONEPACKET &&
+		iph->protocol == IPPROTO_UDP) ? IP_VS_CONN_F_ONE_PACKET : 0;
+}
 
 /*
  *  IPVS persistent scheduling function
@@ -194,7 +199,6 @@ ip_vs_sched_persist(struct ip_vs_service
 	struct ip_vs_dest *dest;
 	struct ip_vs_conn *ct;
 	__be16  dport;			/* destination port to forward */
-	__be16  flags;
 	union nf_inet_addr snet;	/* source network of the client,
 					   after masking */
 
@@ -341,10 +345,6 @@ ip_vs_sched_persist(struct ip_vs_service
 		dport = ports[1];
 	}
 
-	flags = (svc->flags & IP_VS_SVC_F_ONEPACKET
-		 && iph.protocol == IPPROTO_UDP)?
-		IP_VS_CONN_F_ONE_PACKET : 0;
-
 	/*
 	 *    Create a new connection according to the template
 	 */
@@ -352,7 +352,7 @@ ip_vs_sched_persist(struct ip_vs_service
 			    &iph.saddr, ports[0],
 			    &iph.daddr, ports[1],
 			    &dest->addr, dport,
-			    flags,
+			    ip_vs_onepacket_enabled(svc, &iph),
 			    dest);
 	if (cp == NULL) {
 		ip_vs_conn_put(ct);
@@ -382,7 +382,7 @@ ip_vs_schedule(struct ip_vs_service *svc
 	struct ip_vs_conn *cp = NULL;
 	struct ip_vs_iphdr iph;
 	struct ip_vs_dest *dest;
-	__be16 _ports[2], *pptr, flags;
+	__be16 _ports[2], *pptr;
 
 	ip_vs_fill_iphdr(svc->af, skb_network_header(skb), &iph);
 	pptr = skb_header_pointer(skb, iph.len, sizeof(_ports), _ports);
@@ -412,10 +412,6 @@ ip_vs_schedule(struct ip_vs_service *svc
 		return NULL;
 	}
 
-	flags = (svc->flags & IP_VS_SVC_F_ONEPACKET
-		 && iph.protocol == IPPROTO_UDP)?
-		IP_VS_CONN_F_ONE_PACKET : 0;
-
 	/*
 	 *    Create a connection entry.
 	 */
@@ -423,7 +419,7 @@ ip_vs_schedule(struct ip_vs_service *svc
 			    &iph.saddr, pptr[0],
 			    &iph.daddr, pptr[1],
 			    &dest->addr, dest->port ? dest->port : pptr[1],
-			    flags,
+			    ip_vs_onepacket_enabled(svc, &iph),
 			    dest);
 	if (cp == NULL)
 		return NULL;
@@ -473,9 +469,6 @@ int ip_vs_leave(struct ip_vs_service *sv
 	if (sysctl_ip_vs_cache_bypass && svc->fwmark && unicast) {
 		int ret, cs;
 		struct ip_vs_conn *cp;
-		__u16 flags = (svc->flags & IP_VS_SVC_F_ONEPACKET &&
-				iph.protocol == IPPROTO_UDP)?
-				IP_VS_CONN_F_ONE_PACKET : 0;
 		union nf_inet_addr daddr =  { .all = { 0, 0, 0, 0 } };
 
 		ip_vs_service_put(svc);
@@ -486,7 +479,8 @@ int ip_vs_leave(struct ip_vs_service *sv
 				    &iph.saddr, pptr[0],
 				    &iph.daddr, pptr[1],
 				    &daddr, 0,
-				    IP_VS_CONN_F_BYPASS | flags,
+				    IP_VS_CONN_F_BYPASS,
+				    ip_vs_onepacket_enabled(svc, &iph),
 				    NULL);
 		if (cp == NULL)
 			return NF_DROP;

^ permalink raw reply

* Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
From: Shreyas Bhatewara @ 2010-07-16  8:17 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	pv-drivers@vmware.com, Ronghua Zhang, Matthieu Bucchianeri
In-Reply-To: <20100715.183210.226762655.davem@davemloft.net>



On Thu, 15 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Thu, 15 Jul 2010 18:20:52 -0700 (PDT)
> 
> > Is this what you suggest :
> > 
> > ---
> > 
> > Hold rtnl_lock to get the right link state.
> 
> It ought to work, but make sure that it is legal to take the
> RTNL semaphore in all contexts in which this code block
> might be called.
> 

This code block is called only from the workqueue handler, which runs in
process context, so it is legal to take rtnl semaphore.
Tested this code by simulating event interrupts (which schedule this 
code) at considerable frequency while the interface was brought up and
down in a loop. Similar stress testing had revealed the bug originally. 

^ permalink raw reply

* sky2 mac hung again (or still?)
From: Helmut Grohne @ 2010-07-16  7:44 UTC (permalink / raw)
  To: Stephen Hemminger, netdev

Dear sky2 maintainers,

thanks for all your work on the continuously improving stability issues
of the sky2 driver. Unfortunately this battle is not yet over. To aid in
getting this fully working I'd like to give another data point.

Kernel: vanilla 2.6.33.2

lspci output:
05:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8053 PCI-E Gigabit Ethernet Controller (rev 19)
        Subsystem: ASUSTeK Computer Inc. Marvell 88E8053 Gigabit Ethernet controller PCIe (Asus)
        Flags: bus master, fast devsel, latency 0, IRQ 36
        Memory at d1000000 (64-bit, non-prefetchable) [size=16K]
        I/O ports at a000 [size=256]
        [virtual] Expansion ROM at 80800000 [disabled] [size=128K]
        Capabilities: [48] Power Management version 2
        Capabilities: [50] Vital Product Data
        Capabilities: [5c] MSI: Enable- Count=1/2 Maskable- 64bit+
        Capabilities: [e0] Express Legacy Endpoint, MSI 00
        Capabilities: [100] Advanced Error Reporting
        Kernel driver in use: sky2

dmesg output after rmmod and modprobe (for a currently very hung card):
sky2 driver version 1.26
sky2 0000:05:00.0: PCI INT A -> GSI 36 (level, low) -> IRQ 36
sky2 0000:05:00.0: setting latency timer to 64
sky2 0000:05:00.0: PCI: Disallowing DAC for device
sky2 0000:05:00.0: Yukon-2 EC chip revision 2
sky2 0000:05:00.0: irq 53 for MSI/MSI-X
sky2 0000:05:00.0: No interrupt generated using MSI, switching to INTx mode.
sky2 eth0: addr 00:15:f2:36:00:e9

The problem (again/still) is as usual:
sky2 eth0: hung mac 124:91 fifo 194 (182:176)
sky2 eth0: receiver hang detected
sky2 eth0: disabling interface
sky2 eth0: enabling interface

(I can get you more of these hung mac messages with slightly different
numbers if need be.)

This kind of hang happens about once a day for me and it kills my pptp vpn
connection, cause the card takes too long to recover.

Today in contrast the card took the opportunity to crash a bit harder,
so I did not get it up again (maybe a reboot can help later). I was
lucky to have an unused rtl8139too in that machine...

It seems like a precondition for the card to hang is a bit network
traffic. I don't remember getting it crashed with a load of less than
10Mbit symmetric (receive/send) while using this kernel version. On the
other I have seen the card survive this workload for 8 hours a few
times. It does not seem to have any problems with a quick and high
receive load (downloading Debian packages from the next 1Gbit mirror,
usually around 200Mbit).

To me this sounds like a race condition.

Did I miss out any details?

Thanks in advance

Helmut

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox