Netdev List
 help / color / mirror / Atom feed
* 2.6.37-rc2 after KVM shutdown - unregister_netdevice: waiting for vmtst01eth0 to become free. Usage count = 1
From: Nikola Ciprich @ 2010-11-16  8:41 UTC (permalink / raw)
  To: KVM list, netdev list; +Cc: nikola.ciprich, Linux kernel list

Hello,
I just tried running KVM on 2.6.37-rc2 host, and when trying to shutdown guest, it
got to unterruptible state (D) and I'm getting lots of following messages:
[ 1269.392009] unregister_netdevice: waiting for vmtst01eth0 to become free. Usage count = 1
So I just want to report..
n.


-- 
-------------------------------------
Ing. Nikola CIPRICH
LinuxBox.cz, s.r.o.
28. rijna 168, 709 01 Ostrava

tel.:   +420 596 603 142
fax:    +420 596 621 273
mobil:  +420 777 093 799
www.linuxbox.cz

mobil servis: +420 737 238 656
email servis: servis@linuxbox.cz
-------------------------------------

^ permalink raw reply

* Re: ipvs: ipvs update for nf-next-2.6
From: Patrick McHardy @ 2010-11-16  9:21 UTC (permalink / raw)
  To: Simon Horman
  Cc: Eric Dumazet, Hans Schillstrom, Julian Anastasov, lvs-devel,
	netdev, netfilter, netfilter-devel
In-Reply-To: <1289889298-17287-1-git-send-email-horms@verge.net.au>

On 16.11.2010 07:34, Simon Horman wrote:
> git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-test-2.6.git for-patrick

Pulled, thanks Simon!

^ permalink raw reply

* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Tomoya MORINAGA @ 2010-11-16  9:33 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
	Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, LKML,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CE23D5A.5080606@pengutronix.de>

Hi Marc,

On Tuesday, November 16, 2010 5:14 PM, Marc Kleine-Budde wrote :
> Sorry, but there is little chance to get a "this patch changes about 10
> things" into the kernel, please try to split up the patch.

Why !? I say again, it takes much time.
This doesn't make sense, does this?
Have I already updated almost your indications?

How about the following procedure?
I think the latest patch is pretty better than upstream's.
Thus,  firstly, could you accept the latest patch ?
After this, I will post patch per topic.

---
Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply

* Re: 2.6.37-rc2 after KVM shutdown - unregister_netdevice: waiting for vmtst01eth0 to become free. Usage count = 1
From: Eric Dumazet @ 2010-11-16  9:46 UTC (permalink / raw)
  To: Nikola Ciprich; +Cc: KVM list, netdev list, nikola.ciprich, Linux kernel list
In-Reply-To: <20101116084148.GA3781@pcnci.linuxbox.cz>

Le mardi 16 novembre 2010 à 09:41 +0100, Nikola Ciprich a écrit :
> Hello,
> I just tried running KVM on 2.6.37-rc2 host, and when trying to shutdown guest, it
> got to unterruptible state (D) and I'm getting lots of following messages:
> [ 1269.392009] unregister_netdevice: waiting for vmtst01eth0 to become free. Usage count = 1
> So I just want to report..
> n.
> 
> 

Yep, this is a known problem, thanks !

fix is there : 

http://patchwork.ozlabs.org/patch/71354/




^ permalink raw reply

* Re: Vlan communication
From: Liew Kian Yang @ 2010-11-17  9:57 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1289893935.3364.255.camel@edumazet-laptop>

Dear Eric,

My question is, can i have vlan function enable without using the
vconfig command?

Thank you


Regards,
Kian Yang, Liew

On Tue, 2010-11-16 at 08:52 +0100, Eric Dumazet wrote:
> ip link add link eth0 eth0.1 type vlan id 1
> 
> 
> 


^ permalink raw reply

* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Wolfgang Grandegger @ 2010-11-16 10:16 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
	Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <001501cb8567$c7a38820$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>

Hi Tomoya,

On 11/16/2010 09:25 AM, Tomoya MORINAGA wrote:
> On Monday, November 15, 2010 11:21 PM, Wolfgang Grandegger wrote:
> 
>>
>> More comments to the lec handling below.
>>
>>> + cf->data[6] = ioread32(&priv->regs->errc) & PCH_TEC;
>>> + cf->data[7] = (ioread32(&priv->regs->errc) & PCH_REC) >> 8;
>>
>> Could be handle with just *one* register access.
> 
> I will modify.
> 
>>   if (reg_stat & PCH_BUS_OFF ||
>>     (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
>>
>> Your lec handling is still not correc, 
> 
> I will modify like above.
> 
>> I believe. The driver needs to
>> write PCH_LEC_ALL to the "stat" register once in the initialization code
> This is NOT true.
> I heard even if CAN driver detects PCH_LEC_ALL, the driver doesn't have to
> write PCH_LEC_ALL to LEC.
> "PCH_LEC_ALL" means there is no error event.

OK, but...

> In case error is occurred, lec value is updated automatically.

... I doubt that it's updated automatically. At least I understand
chapter "13.4.2.2 CAN Status Register (CANSTAT)" and "Table 507. Last
Error Code (LEC) Details of Operation" differently. Please *check* the
real behavior by adding debugging code.

>> and then after each error observed (lec != PCH_LEC_ALL). I still do not
>> find such code. Could you show us the output of
>>
>>   "# candump any,0:0,#FFFFFFFF"
>>
>> when yo send CAN messages *without* a cable connected?.
> 
> 
> [root@localhost can-utils]# ./candump any,0:0,#FFFFFFFF
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
>   can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME

To inteprete the error messages, please have a look to
http://lxr.linux.no/#linux/include/linux/can/error.h

The id is "CAN_EFF_FLAG | CAN_ERR_ACK | CAN_ERR_CRTL" and data[1] is
"CAN_ERR_CRTL_TX_PASSIVE | CAN_ERR_CRTL_TX_WARNING".

Which is *not* 100% OK.

> ......It seems the same line continues forever.

Yes, it will continue until you connect the cable, that's normal
behavior. But that's not the full sequence. Could you please repeat the
test as shown below:

  First start the following command in a *separate* session.
  # candump any,0:0,#FFFFFFFF"

  Then setup and start the CAN controller:
  # ip link set can0 up type can bitrate 125000
  # cansend can0 123#deadbeef

Wolfgang.

^ permalink raw reply

* Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Wolfgang Grandegger @ 2010-11-16 11:04 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: David S. Miller, Wolfram Sang, Christian Pellegrin, Barry Song,
	Samuel Ortiz, socketcan-core, netdev, linux-kernel,
	andrew.chih.howe.khor, qi.wang, margie.foster, yong.y.wang,
	Masayuki Ohtake, kok.howg.ewe, joel.clark
In-Reply-To: <4CE2434B.5050701@dsn.okisemi.com>

Hi Tomoya,

On 11/16/2010 09:39 AM, Tomoya MORINAGA wrote:
> Hi Marc and Wolf,
> 
> I have updated your indications.
> 
>> * Add Flow control
>> * Fix Data copy issue (endianness)
>> * Add Macro prefix "PCH_"
>> * Separate interface register structure
>> * Some functions are unified.
>> * Change MessageObject indication(PCH_RX_OBJ_START, etc..)
>> * Enumerate LEC macro
>> * Move MSI processing from open/close to probe/remove processing
>> * Use BIT(x)
>> * and more...
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>

Could you please resend the patch (as v4) with a proper subject and
commit message. Then I will add my "Acked-by" to get that
*not-yet-ready* driver updated quickly.

Wolfgang.

^ permalink raw reply

* Re: [PATCH 05/11] drivers/net/irda: Remove unnecessary casts of pci_get_drvdata
From: Samuel Ortiz @ 2010-11-16 11:27 UTC (permalink / raw)
  To: Joe Perches; +Cc: Jiri Kosina, netdev, linux-kernel
In-Reply-To: <11b30b9512bf0438efaf70d6c0a819756dbf0e2f.1289851770.git.joe@perches.com>

Hi Joe,

On Mon, 2010-11-15 at 12:13 -0800, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>
Thanks, I'll apply it to my irda tree.

Cheers,
Samuel.


> ---
>  drivers/net/irda/donauboe.c |    6 +++---
>  drivers/net/irda/vlsi_ir.c  |    2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/irda/donauboe.c b/drivers/net/irda/donauboe.c
> index b626ccc..bee5ccc 100644
> --- a/drivers/net/irda/donauboe.c
> +++ b/drivers/net/irda/donauboe.c
> @@ -1488,7 +1488,7 @@ static void
>  toshoboe_close (struct pci_dev *pci_dev)
>  {
>    int i;
> -  struct toshoboe_cb *self = (struct toshoboe_cb*)pci_get_drvdata(pci_dev);
> +  struct toshoboe_cb *self = pci_get_drvdata(pci_dev);
>  
>    IRDA_DEBUG (4, "%s()\n", __func__);
>  
> @@ -1698,7 +1698,7 @@ freeself:
>  static int
>  toshoboe_gotosleep (struct pci_dev *pci_dev, pm_message_t crap)
>  {
> -  struct toshoboe_cb *self = (struct toshoboe_cb*)pci_get_drvdata(pci_dev);
> +  struct toshoboe_cb *self = pci_get_drvdata(pci_dev);
>    unsigned long flags;
>    int i = 10;
>  
> @@ -1727,7 +1727,7 @@ toshoboe_gotosleep (struct pci_dev *pci_dev, pm_message_t crap)
>  static int
>  toshoboe_wakeup (struct pci_dev *pci_dev)
>  {
> -  struct toshoboe_cb *self = (struct toshoboe_cb*)pci_get_drvdata(pci_dev);
> +  struct toshoboe_cb *self = pci_get_drvdata(pci_dev);
>    unsigned long flags;
>  
>    IRDA_DEBUG (4, "%s()\n", __func__);
> diff --git a/drivers/net/irda/vlsi_ir.c b/drivers/net/irda/vlsi_ir.c
> index c3d0738..62f2d12 100644
> --- a/drivers/net/irda/vlsi_ir.c
> +++ b/drivers/net/irda/vlsi_ir.c
> @@ -542,7 +542,7 @@ static int vlsi_process_rx(struct vlsi_ring *r, struct ring_descr *rd)
>  	int		crclen, len = 0;
>  	struct sk_buff	*skb;
>  	int		ret = 0;
> -	struct net_device *ndev = (struct net_device *)pci_get_drvdata(r->pdev);
> +	struct net_device *ndev = pci_get_drvdata(r->pdev);
>  	vlsi_irda_dev_t *idev = netdev_priv(ndev);
>  
>  	pci_dma_sync_single_for_cpu(r->pdev, rd_get_addr(rd), r->len, r->dir);

^ permalink raw reply

* [PATCH net-next-2.6 v4] can: Topcliff: PCH_CAN driver: Add Flow control/Fix Endianess issue/Separate IF register/Enumerate LEC macro/Move MSI processing/Use BIT(X)/Change Message Object index/Add prefix PCH_
From: Tomoya MORINAGA @ 2010-11-16 12:14 UTC (permalink / raw)
  To: Wolfgang Grandegger, David S. Miller, Wolfram Sang,
	Christian Pellegrin, Barry Song
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, Masayuki Ohtake, margie.foster

Add flow control processing.
Currently, there is no flow control processing.
Thus, Add flow control processing as
when there is no empty of tx buffer,
netif_stop_queue is called.
When there is empty buffer, netif_wake_queue is called.


Fix endianness issue.
there is endianness issue both Tx and Rx.
Currently, data is set like below.
Register:
 MSB---LSB
 x x D0 D1
 x x D2 D3
 x x D4 D5
 x x D6 D7

But Data to be sent must be set like below.
Register:
 MSB---LSB
 x x D1 D0
 x x D3 D2
 x x D5 D4
 x x D7 D6  (x means reserved area.)


Separate interface register from whole of register structure.
CAN register of Intel PCH EG20T has 2 sets of interface register.
To reduce whole of code size, separate interface register.
As a result, the number of function also can be reduced. 


Enumerate LEC macro from #define macro.
For easy to readable, all LEC #define macros are replease to enums like below.
enum pch_can_err {
	PCH_STUF_ERR = 1,
	PCH_FORM_ERR,
	PCH_ACK_ERR,
	PCH_BIT1_ERR,
	PCH_BIT0_ERR,
	PCH_CRC_ERR,
	PCH_LEC_ALL,
};


Move MSI processing to probe/remove processing.
Currently, in case this driver is integrated as module, and 
when this module is re-installed, no interrupts is to be occurred.
For the above issue, move MSI processing to open/release processing.


Replace bit assignment value to BIT(X).
For easy to readable, replace all bit assigned macros to BIT(X)


Change Message Object index macro name.
For easy to readable, add Message Object index like below.
PCH_RX_OBJ_START
PCH_RX_OBJ_END
PCH_TX_OBJ_START
PCH_TX_OBJ_END


Add prefix PCH_ to all of #define macros.
For easy to readable, add prefix "PCH_" to all of #define macros.



Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c | 1341 ++++++++++++++++++++-------------------------
 1 files changed, 586 insertions(+), 755 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 6727182..bb0f873 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 1999 - 2010 Intel Corporation.
- * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
+ * Copyright (C) 2010 OKI SEMICONDUCTOR CO., LTD.
  *
  * 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
@@ -32,106 +32,111 @@
 #include <linux/can/dev.h>
 #include <linux/can/error.h>
 
-#define MAX_MSG_OBJ		32
-#define MSG_OBJ_RX		0 /* The receive message object flag. */
-#define MSG_OBJ_TX		1 /* The transmit message object flag. */
-
-#define ENABLE			1 /* The enable flag */
-#define DISABLE			0 /* The disable flag */
-#define CAN_CTRL_INIT		0x0001 /* The INIT bit of CANCONT register. */
-#define CAN_CTRL_IE		0x0002 /* The IE bit of CAN control register */
-#define CAN_CTRL_IE_SIE_EIE	0x000e
-#define CAN_CTRL_CCE		0x0040
-#define CAN_CTRL_OPT		0x0080 /* The OPT bit of CANCONT register. */
-#define CAN_OPT_SILENT		0x0008 /* The Silent bit of CANOPT reg. */
-#define CAN_OPT_LBACK		0x0010 /* The LoopBack bit of CANOPT reg. */
-#define CAN_CMASK_RX_TX_SET	0x00f3
-#define CAN_CMASK_RX_TX_GET	0x0073
-#define CAN_CMASK_ALL		0xff
-#define CAN_CMASK_RDWR		0x80
-#define CAN_CMASK_ARB		0x20
-#define CAN_CMASK_CTRL		0x10
-#define CAN_CMASK_MASK		0x40
-#define CAN_CMASK_NEWDAT	0x04
-#define CAN_CMASK_CLRINTPND	0x08
-
-#define CAN_IF_MCONT_NEWDAT	0x8000
-#define CAN_IF_MCONT_INTPND	0x2000
-#define CAN_IF_MCONT_UMASK	0x1000
-#define CAN_IF_MCONT_TXIE	0x0800
-#define CAN_IF_MCONT_RXIE	0x0400
-#define CAN_IF_MCONT_RMTEN	0x0200
-#define CAN_IF_MCONT_TXRQXT	0x0100
-#define CAN_IF_MCONT_EOB	0x0080
-#define CAN_IF_MCONT_DLC	0x000f
-#define CAN_IF_MCONT_MSGLOST	0x4000
-#define CAN_MASK2_MDIR_MXTD	0xc000
-#define CAN_ID2_DIR		0x2000
-#define CAN_ID_MSGVAL		0x8000
-
-#define CAN_STATUS_INT		0x8000
-#define CAN_IF_CREQ_BUSY	0x8000
-#define CAN_ID2_XTD		0x4000
-
-#define CAN_REC			0x00007f00
-#define CAN_TEC			0x000000ff
-
-#define PCH_RX_OK		0x00000010
-#define PCH_TX_OK		0x00000008
-#define PCH_BUS_OFF		0x00000080
-#define PCH_EWARN		0x00000040
-#define PCH_EPASSIV		0x00000020
-#define PCH_LEC0		0x00000001
-#define PCH_LEC1		0x00000002
-#define PCH_LEC2		0x00000004
-#define PCH_LEC_ALL		(PCH_LEC0 | PCH_LEC1 | PCH_LEC2)
-#define PCH_STUF_ERR		PCH_LEC0
-#define PCH_FORM_ERR		PCH_LEC1
-#define PCH_ACK_ERR		(PCH_LEC0 | PCH_LEC1)
-#define PCH_BIT1_ERR		PCH_LEC2
-#define PCH_BIT0_ERR		(PCH_LEC0 | PCH_LEC2)
-#define PCH_CRC_ERR		(PCH_LEC1 | PCH_LEC2)
+#define PCH_CTRL_INIT		BIT(0) /* The INIT bit of CANCONT register. */
+#define PCH_CTRL_IE		BIT(1) /* The IE bit of CAN control register */
+#define PCH_CTRL_IE_SIE_EIE	(BIT(3) | BIT(2) | BIT(1))
+#define PCH_CTRL_CCE		BIT(6)
+#define PCH_CTRL_OPT		BIT(7) /* The OPT bit of CANCONT register. */
+#define PCH_OPT_SILENT		BIT(3) /* The Silent bit of CANOPT reg. */
+#define PCH_OPT_LBACK		BIT(4) /* The LoopBack bit of CANOPT reg. */
+
+#define PCH_CMASK_RX_TX_SET	0x00f3
+#define PCH_CMASK_RX_TX_GET	0x0073
+#define PCH_CMASK_ALL		0xff
+#define PCH_CMASK_NEWDAT	BIT(2)
+#define PCH_CMASK_CLRINTPND	BIT(3)
+#define PCH_CMASK_CTRL		BIT(4)
+#define PCH_CMASK_ARB		BIT(5)
+#define PCH_CMASK_MASK		BIT(6)
+#define PCH_CMASK_RDWR		BIT(7)
+#define PCH_IF_MCONT_NEWDAT	BIT(15)
+#define PCH_IF_MCONT_MSGLOST	BIT(14)
+#define PCH_IF_MCONT_INTPND	BIT(13)
+#define PCH_IF_MCONT_UMASK	BIT(12)
+#define PCH_IF_MCONT_TXIE	BIT(11)
+#define PCH_IF_MCONT_RXIE	BIT(10)
+#define PCH_IF_MCONT_RMTEN	BIT(9)
+#define PCH_IF_MCONT_TXRQXT	BIT(8)
+#define PCH_IF_MCONT_EOB	BIT(7)
+#define PCH_IF_MCONT_DLC	(BIT(0) | BIT(1) | BIT(2) | BIT(3))
+#define PCH_MASK2_MDIR_MXTD	(BIT(14) | BIT(15))
+#define PCH_ID2_DIR		BIT(13)
+#define PCH_ID2_XTD		BIT(14)
+#define PCH_ID_MSGVAL		BIT(15)
+#define PCH_IF_CREQ_BUSY	BIT(15)
+
+#define PCH_STATUS_INT		0x8000
+#define PCH_REC			0x00007f00
+#define PCH_TEC			0x000000ff
+
+#define PCH_TX_OK		BIT(3)
+#define PCH_RX_OK		BIT(4)
+#define PCH_EPASSIV		BIT(5)
+#define PCH_EWARN		BIT(6)
+#define PCH_BUS_OFF		BIT(7)
 
 /* bit position of certain controller bits. */
-#define BIT_BITT_BRP		0
-#define BIT_BITT_SJW		6
-#define BIT_BITT_TSEG1		8
-#define BIT_BITT_TSEG2		12
-#define BIT_IF1_MCONT_RXIE	10
-#define BIT_IF2_MCONT_TXIE	11
-#define BIT_BRPE_BRPE		6
-#define BIT_ES_TXERRCNT		0
-#define BIT_ES_RXERRCNT		8
-#define MSK_BITT_BRP		0x3f
-#define MSK_BITT_SJW		0xc0
-#define MSK_BITT_TSEG1		0xf00
-#define MSK_BITT_TSEG2		0x7000
-#define MSK_BRPE_BRPE		0x3c0
-#define MSK_BRPE_GET		0x0f
-#define MSK_CTRL_IE_SIE_EIE	0x07
-#define MSK_MCONT_TXIE		0x08
-#define MSK_MCONT_RXIE		0x10
-#define PCH_CAN_NO_TX_BUFF	1
-#define COUNTER_LIMIT		10
-
-#define PCH_CAN_CLK		50000000	/* 50MHz */
-
-/* Define the number of message object.
+#define PCH_BIT_BRP_SHIFT		0
+#define PCH_BIT_SJW_SHIFT		6
+#define PCH_BIT_TSEG1_SHIFT		8
+#define PCH_BIT_TSEG2_SHIFT		12
+#define PCH_BIT_IF1_MCONT_RXIE_SHIFT	10
+#define PCH_BIT_IF2_MCONT_TXIE_SHIFT	11
+#define PCH_BIT_BRPE_BRPE_SHIFT		6
+
+#define PCH_MSK_BITT_BRP		0x3f
+#define PCH_MSK_BRPE_BRPE		0x3c0
+#define PCH_MSK_CTRL_IE_SIE_EIE		0x07
+#define PCH_COUNTER_LIMIT		10
+
+#define PCH_RX_IFREG			0
+#define PCH_TX_IFREG			1
+
+#define PCH_CAN_CLK			50000000	/* 50MHz */
+
+/*
+ * Define the number of message object.
  * PCH CAN communications are done via Message RAM.
- * The Message RAM consists of 32 message objects. */
-#define PCH_RX_OBJ_NUM		26  /* 1~ PCH_RX_OBJ_NUM is Rx*/
-#define PCH_TX_OBJ_NUM		6  /* PCH_RX_OBJ_NUM is RX ~ Tx*/
-#define PCH_OBJ_NUM		(PCH_TX_OBJ_NUM + PCH_RX_OBJ_NUM)
+ * The Message RAM consists of 32 message objects.
+ */
+#define PCH_RX_OBJ_NUM		26
+#define PCH_TX_OBJ_NUM		6
+#define PCH_RX_OBJ_START	1
+#define PCH_RX_OBJ_END		PCH_RX_OBJ_NUM
+#define PCH_TX_OBJ_START	(PCH_RX_OBJ_END + 1)
+#define PCH_TX_OBJ_END		(PCH_RX_OBJ_NUM + PCH_TX_OBJ_NUM)
 
 #define PCH_FIFO_THRESH		16
 
+enum pch_can_err {
+	PCH_STUF_ERR = 1,
+	PCH_FORM_ERR,
+	PCH_ACK_ERR,
+	PCH_BIT1_ERR,
+	PCH_BIT0_ERR,
+	PCH_CRC_ERR,
+	PCH_LEC_ALL,
+};
+
 enum pch_can_mode {
 	PCH_CAN_ENABLE,
 	PCH_CAN_DISABLE,
 	PCH_CAN_ALL,
 	PCH_CAN_NONE,
 	PCH_CAN_STOP,
-	PCH_CAN_RUN
+	PCH_CAN_RUN,
+};
+
+struct pch_can_if_regs {
+	u32 creq;
+	u32 cmask;
+	u32 mask1;
+	u32 mask2;
+	u32 id1;
+	u32 id2;
+	u32 mcont;
+	u32 data[4];
+	u32 rsv[13];
 };
 
 struct pch_can_regs {
@@ -142,53 +147,34 @@ struct pch_can_regs {
 	u32 intr;
 	u32 opt;
 	u32 brpe;
-	u32 reserve1;
-	u32 if1_creq;
-	u32 if1_cmask;
-	u32 if1_mask1;
-	u32 if1_mask2;
-	u32 if1_id1;
-	u32 if1_id2;
-	u32 if1_mcont;
-	u32 if1_dataa1;
-	u32 if1_dataa2;
-	u32 if1_datab1;
-	u32 if1_datab2;
-	u32 reserve2;
-	u32 reserve3[12];
-	u32 if2_creq;
-	u32 if2_cmask;
-	u32 if2_mask1;
-	u32 if2_mask2;
-	u32 if2_id1;
-	u32 if2_id2;
-	u32 if2_mcont;
-	u32 if2_dataa1;
-	u32 if2_dataa2;
-	u32 if2_datab1;
-	u32 if2_datab2;
-	u32 reserve4;
-	u32 reserve5[20];
+	u32 reserve;
+	struct pch_can_if_regs ifregs[2]; /* [0]=if1  [1]=if2 */
+	u32 reserve1[8];
 	u32 treq1;
 	u32 treq2;
-	u32 reserve6[2];
-	u32 reserve7[56];
-	u32 reserve8[3];
+	u32 reserve2[6];
+	u32 data1;
+	u32 data2;
+	u32 reserve3[6];
+	u32 canipend1;
+	u32 canipend2;
+	u32 reserve4[6];
+	u32 canmval1;
+	u32 canmval2;
+	u32 reserve5[37];
 	u32 srst;
 };
 
 struct pch_can_priv {
 	struct can_priv can;
-	unsigned int can_num;
 	struct pci_dev *dev;
-	unsigned int tx_enable[MAX_MSG_OBJ];
-	unsigned int rx_enable[MAX_MSG_OBJ];
-	unsigned int rx_link[MAX_MSG_OBJ];
+	unsigned int tx_enable[PCH_TX_OBJ_END];
+	unsigned int rx_enable[PCH_TX_OBJ_END];
+	unsigned int rx_link[PCH_TX_OBJ_END];
 	unsigned int int_enables;
 	unsigned int int_stat;
 	struct net_device *ndev;
-	spinlock_t msgif_reg_lock; /* Message Interface Registers Access Lock*/
-	unsigned int msg_obj[MAX_MSG_OBJ];
+	unsigned int msg_obj[PCH_TX_OBJ_END];
 	struct pch_can_regs __iomem *regs;
 	struct napi_struct napi;
 	unsigned int tx_obj;	/* Point next Tx Obj index */
@@ -228,15 +214,15 @@ static void pch_can_set_run_mode(struct pch_can_priv *priv,
 {
 	switch (mode) {
 	case PCH_CAN_RUN:
-		pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_INIT);
+		pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_INIT);
 		break;
 
 	case PCH_CAN_STOP:
-		pch_can_bit_set(&priv->regs->cont, CAN_CTRL_INIT);
+		pch_can_bit_set(&priv->regs->cont, PCH_CTRL_INIT);
 		break;
 
 	default:
-		dev_err(&priv->ndev->dev, "%s -> Invalid Mode.\n", __func__);
+		netdev_err(priv->ndev, "%s -> Invalid Mode.\n", __func__);
 		break;
 	}
 }
@@ -246,357 +232,184 @@ static void pch_can_set_optmode(struct pch_can_priv *priv)
 	u32 reg_val = ioread32(&priv->regs->opt);
 
 	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
-		reg_val |= CAN_OPT_SILENT;
+		reg_val |= PCH_OPT_SILENT;
 
 	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
-		reg_val |= CAN_OPT_LBACK;
+		reg_val |= PCH_OPT_LBACK;
 
-	pch_can_bit_set(&priv->regs->cont, CAN_CTRL_OPT);
+	pch_can_bit_set(&priv->regs->cont, PCH_CTRL_OPT);
 	iowrite32(reg_val, &priv->regs->opt);
 }
 
-static void pch_can_set_int_custom(struct pch_can_priv *priv)
+static void pch_can_rw_msg_obj(void __iomem *creq_addr, u32 num)
 {
-	/* Clearing the IE, SIE and EIE bits of Can control register. */
-	pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
-
-	/* Appropriately setting them. */
-	pch_can_bit_set(&priv->regs->cont,
-			((priv->int_enables & MSK_CTRL_IE_SIE_EIE) << 1));
-}
+	int counter = PCH_COUNTER_LIMIT;
+	u32 ifx_creq;
 
-/* This function retrieves interrupt enabled for the CAN device. */
-static void pch_can_get_int_enables(struct pch_can_priv *priv, u32 *enables)
-{
-	/* Obtaining the status of IE, SIE and EIE interrupt bits. */
-	*enables = ((ioread32(&priv->regs->cont) & CAN_CTRL_IE_SIE_EIE) >> 1);
+	iowrite32(num, creq_addr);
+	while (counter) {
+		ifx_creq = ioread32(creq_addr) & PCH_IF_CREQ_BUSY;
+		if (!ifx_creq)
+			break;
+		counter--;
+		udelay(1);
+	}
+	if (!counter)
+		pr_err("%s:IF1 BUSY Flag is set forever.\n", __func__);
 }
 
 static void pch_can_set_int_enables(struct pch_can_priv *priv,
 				    enum pch_can_mode interrupt_no)
 {
 	switch (interrupt_no) {
-	case PCH_CAN_ENABLE:
-		pch_can_bit_set(&priv->regs->cont, CAN_CTRL_IE);
-		break;
-
 	case PCH_CAN_DISABLE:
-		pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE);
+		pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_IE);
 		break;
 
 	case PCH_CAN_ALL:
-		pch_can_bit_set(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
+		pch_can_bit_set(&priv->regs->cont, PCH_CTRL_IE_SIE_EIE);
 		break;
 
 	case PCH_CAN_NONE:
-		pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
+		pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_IE_SIE_EIE);
 		break;
 
 	default:
-		dev_err(&priv->ndev->dev, "Invalid interrupt number.\n");
+		netdev_err(priv->ndev, "Invalid interrupt number.\n");
 		break;
 	}
 }
 
-static void pch_can_check_if_busy(u32 __iomem *creq_addr, u32 num)
+static void pch_can_set_rxtx(struct pch_can_priv *priv, u32 buff_num,
+				    u32 set, u32 dir)
 {
-	u32 counter = COUNTER_LIMIT;
-	u32 ifx_creq;
+	u32 ie;
 
-	iowrite32(num, creq_addr);
-	while (counter) {
-		ifx_creq = ioread32(creq_addr) & CAN_IF_CREQ_BUSY;
-		if (!ifx_creq)
-			break;
-		counter--;
-		udelay(1);
-	}
-	if (!counter)
-		pr_err("%s:IF1 BUSY Flag is set forever.\n", __func__);
-}
-
-static void pch_can_set_rx_enable(struct pch_can_priv *priv, u32 buff_num,
-				  u32 set)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
-	/* Reading the receive buffer data from RAM to Interface1 registers */
-	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
-	pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
-
-	/* Setting the IF1MASK1 register to access MsgVal and RxIE bits */
-	iowrite32(CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL,
-		  &priv->regs->if1_cmask);
-
-	if (set == ENABLE) {
-		/* Setting the MsgVal and RxIE bits */
-		pch_can_bit_set(&priv->regs->if1_mcont, CAN_IF_MCONT_RXIE);
-		pch_can_bit_set(&priv->regs->if1_id2, CAN_ID_MSGVAL);
-
-	} else if (set == DISABLE) {
-		/* Resetting the MsgVal and RxIE bits */
-		pch_can_bit_clear(&priv->regs->if1_mcont, CAN_IF_MCONT_RXIE);
-		pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID_MSGVAL);
-	}
-
-	pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
-	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
-}
-
-static void pch_can_rx_enable_all(struct pch_can_priv *priv)
-{
-	int i;
-
-	/* Traversing to obtain the object configured as receivers. */
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_RX)
-			pch_can_set_rx_enable(priv, i + 1, ENABLE);
-	}
-}
-
-static void pch_can_rx_disable_all(struct pch_can_priv *priv)
-{
-	int i;
-
-	/* Traversing to obtain the object configured as receivers. */
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_RX)
-			pch_can_set_rx_enable(priv, i + 1, DISABLE);
-	}
-}
-
-static void pch_can_set_tx_enable(struct pch_can_priv *priv, u32 buff_num,
-				 u32 set)
-{
-	unsigned long flags;
+	if (dir)
+		ie = PCH_IF_MCONT_TXIE;
+	else
+		ie = PCH_IF_MCONT_RXIE;
 
-	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
 	/* Reading the Msg buffer from Message RAM to Interface2 registers. */
-	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
-	pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
+	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
+	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
 
 	/* Setting the IF2CMASK register for accessing the
 		MsgVal and TxIE bits */
-	iowrite32(CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL,
-		 &priv->regs->if2_cmask);
+	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_ARB | PCH_CMASK_CTRL,
+		 &priv->regs->ifregs[dir].cmask);
 
-	if (set == ENABLE) {
+	if (set) {
 		/* Setting the MsgVal and TxIE bits */
-		pch_can_bit_set(&priv->regs->if2_mcont, CAN_IF_MCONT_TXIE);
-		pch_can_bit_set(&priv->regs->if2_id2, CAN_ID_MSGVAL);
-	} else if (set == DISABLE) {
+		pch_can_bit_set(&priv->regs->ifregs[dir].mcont, ie);
+		pch_can_bit_set(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
+	} else {
 		/* Resetting the MsgVal and TxIE bits. */
-		pch_can_bit_clear(&priv->regs->if2_mcont, CAN_IF_MCONT_TXIE);
-		pch_can_bit_clear(&priv->regs->if2_id2, CAN_ID_MSGVAL);
+		pch_can_bit_clear(&priv->regs->ifregs[dir].mcont, ie);
+		pch_can_bit_clear(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
 	}
 
-	pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
-	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
 }
 
-static void pch_can_tx_enable_all(struct pch_can_priv *priv)
+static void pch_can_set_rx_all(struct pch_can_priv *priv, u32 set)
 {
 	int i;
 
-	/* Traversing to obtain the object configured as transmit object. */
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_TX)
-			pch_can_set_tx_enable(priv, i + 1, ENABLE);
-	}
+	/* Traversing to obtain the object configured as receivers. */
+	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++)
+		pch_can_set_rxtx(priv, i, set, PCH_RX_IFREG);
 }
 
-static void pch_can_tx_disable_all(struct pch_can_priv *priv)
+static void pch_can_set_tx_all(struct pch_can_priv *priv, u32 set)
 {
 	int i;
 
-	/* Traversing to obtain the object configured as transmit object. */
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_TX)
-			pch_can_set_tx_enable(priv, i + 1, DISABLE);
-	}
-}
-
-static void pch_can_get_rx_enable(struct pch_can_priv *priv, u32 buff_num,
-				 u32 *enable)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
-	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
-	pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
-
-	if (((ioread32(&priv->regs->if1_id2)) & CAN_ID_MSGVAL) &&
-			((ioread32(&priv->regs->if1_mcont)) &
-			CAN_IF_MCONT_RXIE))
-		*enable = ENABLE;
-	else
-		*enable = DISABLE;
-	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
-}
-
-static void pch_can_get_tx_enable(struct pch_can_priv *priv, u32 buff_num,
-				 u32 *enable)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
-	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
-	pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
-
-	if (((ioread32(&priv->regs->if2_id2)) & CAN_ID_MSGVAL) &&
-			((ioread32(&priv->regs->if2_mcont)) &
-			CAN_IF_MCONT_TXIE)) {
-		*enable = ENABLE;
-	} else {
-		*enable = DISABLE;
-	}
-	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+	/* Traversing to obtain the object configured as receivers. */
+	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
+		pch_can_set_rxtx(priv, i, set, PCH_TX_IFREG);
 }
 
-static int pch_can_int_pending(struct pch_can_priv *priv)
+static u32 pch_can_int_pending(struct pch_can_priv *priv)
 {
 	return ioread32(&priv->regs->intr) & 0xffff;
 }
 
-static void pch_can_set_rx_buffer_link(struct pch_can_priv *priv,
-				       u32 buffer_num, u32 set)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
-	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
-	pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
-	iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL, &priv->regs->if1_cmask);
-	if (set == ENABLE)
-		pch_can_bit_clear(&priv->regs->if1_mcont, CAN_IF_MCONT_EOB);
-	else
-		pch_can_bit_set(&priv->regs->if1_mcont, CAN_IF_MCONT_EOB);
-
-	pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
-	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
-}
-
-static void pch_can_get_rx_buffer_link(struct pch_can_priv *priv,
-				       u32 buffer_num, u32 *link)
+static void pch_can_clear_if_buffers(struct pch_can_priv *priv)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
-	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
-	pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
-
-	if (ioread32(&priv->regs->if1_mcont) & CAN_IF_MCONT_EOB)
-		*link = DISABLE;
-	else
-		*link = ENABLE;
-	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
-}
-
-static void pch_can_clear_buffers(struct pch_can_priv *priv)
-{
-	int i;
-
-	for (i = 0; i < PCH_RX_OBJ_NUM; i++) {
-		iowrite32(CAN_CMASK_RX_TX_SET, &priv->regs->if1_cmask);
-		iowrite32(0xffff, &priv->regs->if1_mask1);
-		iowrite32(0xffff, &priv->regs->if1_mask2);
-		iowrite32(0x0, &priv->regs->if1_id1);
-		iowrite32(0x0, &priv->regs->if1_id2);
-		iowrite32(0x0, &priv->regs->if1_mcont);
-		iowrite32(0x0, &priv->regs->if1_dataa1);
-		iowrite32(0x0, &priv->regs->if1_dataa2);
-		iowrite32(0x0, &priv->regs->if1_datab1);
-		iowrite32(0x0, &priv->regs->if1_datab2);
-		iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
-			  CAN_CMASK_ARB | CAN_CMASK_CTRL,
-			  &priv->regs->if1_cmask);
-		pch_can_check_if_busy(&priv->regs->if1_creq, i+1);
-	}
-
-	for (i = i;  i < PCH_OBJ_NUM; i++) {
-		iowrite32(CAN_CMASK_RX_TX_SET, &priv->regs->if2_cmask);
-		iowrite32(0xffff, &priv->regs->if2_mask1);
-		iowrite32(0xffff, &priv->regs->if2_mask2);
-		iowrite32(0x0, &priv->regs->if2_id1);
-		iowrite32(0x0, &priv->regs->if2_id2);
-		iowrite32(0x0, &priv->regs->if2_mcont);
-		iowrite32(0x0, &priv->regs->if2_dataa1);
-		iowrite32(0x0, &priv->regs->if2_dataa2);
-		iowrite32(0x0, &priv->regs->if2_datab1);
-		iowrite32(0x0, &priv->regs->if2_datab2);
-		iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
-			  CAN_CMASK_ARB | CAN_CMASK_CTRL,
-			  &priv->regs->if2_cmask);
-		pch_can_check_if_busy(&priv->regs->if2_creq, i+1);
+	int i; /* Msg Obj ID (1~32) */
+
+	for (i = PCH_RX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
+		iowrite32(PCH_CMASK_RX_TX_SET, &priv->regs->ifregs[0].cmask);
+		iowrite32(0xffff, &priv->regs->ifregs[0].mask1);
+		iowrite32(0xffff, &priv->regs->ifregs[0].mask2);
+		iowrite32(0x0, &priv->regs->ifregs[0].id1);
+		iowrite32(0x0, &priv->regs->ifregs[0].id2);
+		iowrite32(0x0, &priv->regs->ifregs[0].mcont);
+		iowrite32(0x0, &priv->regs->ifregs[0].data[0]);
+		iowrite32(0x0, &priv->regs->ifregs[0].data[1]);
+		iowrite32(0x0, &priv->regs->ifregs[0].data[2]);
+		iowrite32(0x0, &priv->regs->ifregs[0].data[3]);
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
+			  PCH_CMASK_ARB | PCH_CMASK_CTRL,
+			  &priv->regs->ifregs[0].cmask);
+		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
 	}
 }
 
 static void pch_can_config_rx_tx_buffers(struct pch_can_priv *priv)
 {
 	int i;
-	unsigned long flags;
-
-	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
 
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_RX) {
-			iowrite32(CAN_CMASK_RX_TX_GET,
-				&priv->regs->if1_cmask);
-			pch_can_check_if_busy(&priv->regs->if1_creq, i+1);
+	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++) {
+		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
+		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
 
-			iowrite32(0x0, &priv->regs->if1_id1);
-			iowrite32(0x0, &priv->regs->if1_id2);
+		iowrite32(0x0, &priv->regs->ifregs[0].id1);
+		iowrite32(0x0, &priv->regs->ifregs[0].id2);
 
-			pch_can_bit_set(&priv->regs->if1_mcont,
-					CAN_IF_MCONT_UMASK);
+		pch_can_bit_set(&priv->regs->ifregs[0].mcont,
+				PCH_IF_MCONT_UMASK);
 
-			/* Set FIFO mode set to 0 except last Rx Obj*/
-			pch_can_bit_clear(&priv->regs->if1_mcont,
-					  CAN_IF_MCONT_EOB);
-			/* In case FIFO mode, Last EoB of Rx Obj must be 1 */
-			if (i == (PCH_RX_OBJ_NUM - 1))
-				pch_can_bit_set(&priv->regs->if1_mcont,
-						  CAN_IF_MCONT_EOB);
+		if (i == PCH_RX_OBJ_END)
+			pch_can_bit_set(&priv->regs->ifregs[0].mcont,
+					PCH_IF_MCONT_EOB);
+		else
+			pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
+					  PCH_IF_MCONT_EOB);
 
-			iowrite32(0, &priv->regs->if1_mask1);
-			pch_can_bit_clear(&priv->regs->if1_mask2,
-					  0x1fff | CAN_MASK2_MDIR_MXTD);
+		iowrite32(0, &priv->regs->ifregs[0].mask1);
+		pch_can_bit_clear(&priv->regs->ifregs[0].mask2,
+				  0x1fff | PCH_MASK2_MDIR_MXTD);
 
-			/* Setting CMASK for writing */
-			iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
-				  CAN_CMASK_ARB | CAN_CMASK_CTRL,
-				  &priv->regs->if1_cmask);
+		/* Setting CMASK for writing */
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
+			  PCH_CMASK_CTRL, &priv->regs->ifregs[0].cmask);
 
-			pch_can_check_if_busy(&priv->regs->if1_creq, i+1);
-		} else if (priv->msg_obj[i] == MSG_OBJ_TX) {
-			iowrite32(CAN_CMASK_RX_TX_GET,
-				&priv->regs->if2_cmask);
-			pch_can_check_if_busy(&priv->regs->if2_creq, i+1);
+		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
+	}
 
-			/* Resetting DIR bit for reception */
-			iowrite32(0x0, &priv->regs->if2_id1);
-			iowrite32(0x0, &priv->regs->if2_id2);
-			pch_can_bit_set(&priv->regs->if2_id2, CAN_ID2_DIR);
+	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
+		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[1].cmask);
+		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
 
-			/* Setting EOB bit for transmitter */
-			iowrite32(CAN_IF_MCONT_EOB, &priv->regs->if2_mcont);
+		/* Resetting DIR bit for reception */
+		iowrite32(0x0, &priv->regs->ifregs[1].id1);
+		iowrite32(PCH_ID2_DIR, &priv->regs->ifregs[1].id2);
 
-			pch_can_bit_set(&priv->regs->if2_mcont,
-					CAN_IF_MCONT_UMASK);
+		/* Setting EOB bit for transmitter */
+		iowrite32(PCH_IF_MCONT_EOB | PCH_IF_MCONT_UMASK,
+			  &priv->regs->ifregs[1].mcont);
 
-			iowrite32(0, &priv->regs->if2_mask1);
-			pch_can_bit_clear(&priv->regs->if2_mask2, 0x1fff);
+		iowrite32(0, &priv->regs->ifregs[1].mask1);
+		pch_can_bit_clear(&priv->regs->ifregs[1].mask2, 0x1fff);
 
-			/* Setting CMASK for writing */
-			iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
-				  CAN_CMASK_ARB | CAN_CMASK_CTRL,
-				  &priv->regs->if2_cmask);
+		/* Setting CMASK for writing */
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
+			  PCH_CMASK_CTRL, &priv->regs->ifregs[1].cmask);
 
-			pch_can_check_if_busy(&priv->regs->if2_creq, i+1);
-		}
+		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
 	}
-	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
 }
 
 static void pch_can_init(struct pch_can_priv *priv)
@@ -605,7 +418,7 @@ static void pch_can_init(struct pch_can_priv *priv)
 	pch_can_set_run_mode(priv, PCH_CAN_STOP);
 
 	/* Clearing all the message object buffers. */
-	pch_can_clear_buffers(priv);
+	pch_can_clear_if_buffers(priv);
 
 	/* Configuring the respective message object as either rx/tx object. */
 	pch_can_config_rx_tx_buffers(priv);
@@ -623,57 +436,53 @@ static void pch_can_release(struct pch_can_priv *priv)
 	pch_can_set_int_enables(priv, PCH_CAN_NONE);
 
 	/* Disabling all the receive object. */
-	pch_can_rx_disable_all(priv);
+	pch_can_set_rx_all(priv, 0);
 
 	/* Disabling all the transmit object. */
-	pch_can_tx_disable_all(priv);
+	pch_can_set_tx_all(priv, 0);
 }
 
 /* This function clears interrupt(s) from the CAN device. */
 static void pch_can_int_clr(struct pch_can_priv *priv, u32 mask)
 {
-	if (mask == CAN_STATUS_INT) {
-		ioread32(&priv->regs->stat);
-		return;
-	}
-
 	/* Clear interrupt for transmit object */
-	if (priv->msg_obj[mask - 1] == MSG_OBJ_TX) {
-		/* Setting CMASK for clearing interrupts for
-					 frame transmission. */
-		iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL | CAN_CMASK_ARB,
-			  &priv->regs->if2_cmask);
-
-		/* Resetting the ID registers. */
-		pch_can_bit_set(&priv->regs->if2_id2,
-			       CAN_ID2_DIR | (0x7ff << 2));
-		iowrite32(0x0, &priv->regs->if2_id1);
-
-		/* Claring NewDat, TxRqst & IntPnd */
-		pch_can_bit_clear(&priv->regs->if2_mcont,
-				  CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND |
-				  CAN_IF_MCONT_TXRQXT);
-		pch_can_check_if_busy(&priv->regs->if2_creq, mask);
-	} else if (priv->msg_obj[mask - 1] == MSG_OBJ_RX) {
+	if ((mask >= PCH_RX_OBJ_START) && (mask <= PCH_RX_OBJ_END)) {
 		/* Setting CMASK for clearing the reception interrupts. */
-		iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL | CAN_CMASK_ARB,
-			  &priv->regs->if1_cmask);
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL | PCH_CMASK_ARB,
+			  &priv->regs->ifregs[0].cmask);
 
 		/* Clearing the Dir bit. */
-		pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID2_DIR);
+		pch_can_bit_clear(&priv->regs->ifregs[0].id2, PCH_ID2_DIR);
 
 		/* Clearing NewDat & IntPnd */
-		pch_can_bit_clear(&priv->regs->if1_mcont,
-				  CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND);
+		pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
+				  PCH_IF_MCONT_NEWDAT | PCH_IF_MCONT_INTPND);
 
-		pch_can_check_if_busy(&priv->regs->if1_creq, mask);
+		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, mask);
+	} else if ((mask >= PCH_TX_OBJ_START) && (mask <= PCH_TX_OBJ_END)) {
+		/*
+		 * Setting CMASK for clearing interrupts for frame transmission.
+		 */
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL | PCH_CMASK_ARB,
+			  &priv->regs->ifregs[1].cmask);
+
+		/* Resetting the ID registers. */
+		pch_can_bit_set(&priv->regs->ifregs[1].id2,
+			       PCH_ID2_DIR | (0x7ff << 2));
+		iowrite32(0x0, &priv->regs->ifregs[1].id1);
+
+		/* Claring NewDat, TxRqst & IntPnd */
+		pch_can_bit_clear(&priv->regs->ifregs[1].mcont,
+				  PCH_IF_MCONT_NEWDAT | PCH_IF_MCONT_INTPND |
+				  PCH_IF_MCONT_TXRQXT);
+		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, mask);
 	}
 }
 
-static int pch_can_get_buffer_status(struct pch_can_priv *priv)
+static u32 pch_can_get_buffer_status(struct pch_can_priv *priv)
 {
 	return (ioread32(&priv->regs->treq1) & 0xffff) |
-	       ((ioread32(&priv->regs->treq2) & 0xffff) << 16);
+	       (ioread32(&priv->regs->treq2) << 16);
 }
 
 static void pch_can_reset(struct pch_can_priv *priv)
@@ -688,7 +497,7 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 	struct sk_buff *skb;
 	struct pch_can_priv *priv = netdev_priv(ndev);
 	struct can_frame *cf;
-	u32 errc;
+	u32 errc, lec;
 	struct net_device_stats *stats = &(priv->ndev->stats);
 	enum can_state state = priv->can.state;
 
@@ -697,26 +506,24 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 		return;
 
 	if (status & PCH_BUS_OFF) {
-		pch_can_tx_disable_all(priv);
-		pch_can_rx_disable_all(priv);
+		pch_can_set_tx_all(priv, 0);
+		pch_can_set_rx_all(priv, 0);
 		state = CAN_STATE_BUS_OFF;
 		cf->can_id |= CAN_ERR_BUSOFF;
 		can_bus_off(ndev);
-		pch_can_set_run_mode(priv, PCH_CAN_RUN);
-		dev_err(&ndev->dev, "%s -> Bus Off occurres.\n", __func__);
 	}
 
+	errc = ioread32(&priv->regs->errc);
 	/* Warning interrupt. */
 	if (status & PCH_EWARN) {
 		state = CAN_STATE_ERROR_WARNING;
 		priv->can.can_stats.error_warning++;
 		cf->can_id |= CAN_ERR_CRTL;
-		errc = ioread32(&priv->regs->errc);
-		if (((errc & CAN_REC) >> 8) > 96)
+		if (((errc & PCH_REC) >> 8) > 96)
 			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
-		if ((errc & CAN_TEC) > 96)
+		if ((errc & PCH_TEC) > 96)
 			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
-		dev_warn(&ndev->dev,
+		netdev_dbg(ndev,
 			"%s -> Error Counter is more than 96.\n", __func__);
 	}
 	/* Error passive interrupt. */
@@ -724,46 +531,52 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 		priv->can.can_stats.error_passive++;
 		state = CAN_STATE_ERROR_PASSIVE;
 		cf->can_id |= CAN_ERR_CRTL;
-		errc = ioread32(&priv->regs->errc);
-		if (((errc & CAN_REC) >> 8) > 127)
+		if (((errc & PCH_REC) >> 8) > 127)
 			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
-		if ((errc & CAN_TEC) > 127)
+		if ((errc & PCH_TEC) > 127)
 			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
-		dev_err(&ndev->dev,
+		netdev_dbg(ndev,
 			"%s -> CAN controller is ERROR PASSIVE .\n", __func__);
 	}
 
-	if (status & PCH_LEC_ALL) {
+	lec = status & PCH_LEC_ALL;
+	switch (lec) {
+	case PCH_STUF_ERR:
+		cf->data[2] |= CAN_ERR_PROT_STUFF;
 		priv->can.can_stats.bus_error++;
 		stats->rx_errors++;
-		switch (status & PCH_LEC_ALL) {
-		case PCH_STUF_ERR:
-			cf->data[2] |= CAN_ERR_PROT_STUFF;
-			break;
-		case PCH_FORM_ERR:
-			cf->data[2] |= CAN_ERR_PROT_FORM;
-			break;
-		case PCH_ACK_ERR:
-			cf->data[2] |= CAN_ERR_PROT_LOC_ACK |
-				       CAN_ERR_PROT_LOC_ACK_DEL;
-			break;
-		case PCH_BIT1_ERR:
-		case PCH_BIT0_ERR:
-			cf->data[2] |= CAN_ERR_PROT_BIT;
-			break;
-		case PCH_CRC_ERR:
-			cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
-				       CAN_ERR_PROT_LOC_CRC_DEL;
-			break;
-		default:
-			iowrite32(status | PCH_LEC_ALL, &priv->regs->stat);
-			break;
-		}
-
+		break;
+	case PCH_FORM_ERR:
+		cf->data[2] |= CAN_ERR_PROT_FORM;
+		priv->can.can_stats.bus_error++;
+		stats->rx_errors++;
+		break;
+	case PCH_ACK_ERR:
+		cf->can_id |= CAN_ERR_ACK;
+		priv->can.can_stats.bus_error++;
+		stats->rx_errors++;
+		break;
+	case PCH_BIT1_ERR:
+	case PCH_BIT0_ERR:
+		cf->data[2] |= CAN_ERR_PROT_BIT;
+		priv->can.can_stats.bus_error++;
+		stats->rx_errors++;
+		break;
+	case PCH_CRC_ERR:
+		cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
+			       CAN_ERR_PROT_LOC_CRC_DEL;
+		priv->can.can_stats.bus_error++;
+		stats->rx_errors++;
+		break;
+	case PCH_LEC_ALL: /* Written by CPU. No error status */
+		break;
 	}
 
+	cf->data[6] = errc & PCH_TEC;
+	cf->data[7] = (errc & PCH_REC) >> 8;
+
 	priv->can.state = state;
-	netif_rx(skb);
+	netif_receive_skb(skb);
 
 	stats->rx_packets++;
 	stats->rx_bytes += cf->can_dlc;
@@ -775,199 +588,207 @@ static irqreturn_t pch_can_interrupt(int irq, void *dev_id)
 	struct pch_can_priv *priv = netdev_priv(ndev);
 
 	pch_can_set_int_enables(priv, PCH_CAN_NONE);
-
 	napi_schedule(&priv->napi);
 
 	return IRQ_HANDLED;
 }
 
-static int pch_can_rx_normal(struct net_device *ndev, u32 int_stat)
+static void pch_fifo_thresh(struct pch_can_priv *priv, int obj_id)
+{
+	if (obj_id < PCH_FIFO_THRESH) {
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL |
+			  PCH_CMASK_ARB, &priv->regs->ifregs[0].cmask);
+
+		/* Clearing the Dir bit. */
+		pch_can_bit_clear(&priv->regs->ifregs[0].id2, PCH_ID2_DIR);
+
+		/* Clearing NewDat & IntPnd */
+		pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
+				  PCH_IF_MCONT_INTPND);
+		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, obj_id);
+	} else if (obj_id > PCH_FIFO_THRESH) {
+		pch_can_int_clr(priv, obj_id);
+	} else if (obj_id == PCH_FIFO_THRESH) {
+		int cnt;
+		for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
+			pch_can_int_clr(priv, cnt+1);
+	}
+}
+
+static int pch_can_rx_msg_lost(struct net_device *ndev, int obj_id)
+{
+	struct pch_can_priv *priv = netdev_priv(ndev);
+	struct net_device_stats *stats = &(priv->ndev->stats);
+	struct sk_buff *skb;
+	struct can_frame *cf;
+
+	netdev_dbg(priv->ndev, "Msg Obj is overwritten.\n");
+	pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
+			  PCH_IF_MCONT_MSGLOST);
+	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL,
+		  &priv->regs->ifregs[0].cmask);
+	pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, obj_id);
+
+	skb = alloc_can_err_skb(ndev, &cf);
+	if (!skb)
+		return -ENOMEM;
+
+	cf->can_id |= CAN_ERR_CRTL;
+	cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+	stats->rx_over_errors++;
+	stats->rx_errors++;
+
+	netif_receive_skb(skb);
+
+	return 0;
+}
+
+static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int quota)
 {
 	u32 reg;
 	canid_t id;
-	u32 ide;
-	u32 rtr;
-	int i, j, k;
 	int rcv_pkts = 0;
+	int rtn;
+	int next_flag = 0;
 	struct sk_buff *skb;
 	struct can_frame *cf;
 	struct pch_can_priv *priv = netdev_priv(ndev);
 	struct net_device_stats *stats = &(priv->ndev->stats);
+	int i;
+	u32 id2;
+	u16 data_reg;
 
-	/* Reading the messsage object from the Message RAM */
-	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
-	pch_can_check_if_busy(&priv->regs->if1_creq, int_stat);
+	do {
+		/* Reading the messsage object from the Message RAM */
+		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
+		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, obj_num);
 
-	/* Reading the MCONT register. */
-	reg = ioread32(&priv->regs->if1_mcont);
-	reg &= 0xffff;
+		/* Reading the MCONT register. */
+		reg = ioread32(&priv->regs->ifregs[0].mcont);
+
+		if (reg & PCH_IF_MCONT_EOB)
+			break;
 
-	for (k = int_stat; !(reg & CAN_IF_MCONT_EOB); k++) {
 		/* If MsgLost bit set. */
-		if (reg & CAN_IF_MCONT_MSGLOST) {
-			dev_err(&priv->ndev->dev, "Msg Obj is overwritten.\n");
-			pch_can_bit_clear(&priv->regs->if1_mcont,
-					  CAN_IF_MCONT_MSGLOST);
-			iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL,
-				  &priv->regs->if1_cmask);
-			pch_can_check_if_busy(&priv->regs->if1_creq, k);
-
-			skb = alloc_can_err_skb(ndev, &cf);
+		if (reg & PCH_IF_MCONT_MSGLOST) {
+			rtn = pch_can_rx_msg_lost(ndev, obj_num);
+			if (!rtn)
+				return rtn;
+			rcv_pkts++;
+			quota--;
+			next_flag = 1;
+		} else if (!(reg & PCH_IF_MCONT_NEWDAT))
+			next_flag = 1;
+
+		if (!next_flag) {
+			skb = alloc_can_skb(priv->ndev, &cf);
 			if (!skb)
 				return -ENOMEM;
 
-			priv->can.can_stats.error_passive++;
-			priv->can.state = CAN_STATE_ERROR_PASSIVE;
-			cf->can_id |= CAN_ERR_CRTL;
-			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
-			cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
-			stats->rx_packets++;
-			stats->rx_bytes += cf->can_dlc;
+			/* Get Received data */
+			id2 = ioread32(&priv->regs->ifregs[0].id2);
+			if (id2 & PCH_ID2_XTD) {
+				id = (ioread32(&priv->regs->ifregs[0].id1) &
+					       0xffff);
+				id |= (((id2) & 0x1fff) << 16);
+				cf->can_id = id | CAN_EFF_FLAG;
+			} else {
+				id = ((id2 & (CAN_SFF_MASK << 2)) >> 2);
+				cf->can_id = id;
+			}
+
+			if (id2 & PCH_ID2_DIR)
+				cf->can_id |= CAN_RTR_FLAG;
+
+			cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
+						   ifregs[0].mcont)) & 0xF);
+
+			for (i = 0; i < cf->can_dlc; i += 2) {
+				data_reg = ioread16(&priv->regs->ifregs[0].
+						    data[i / 2]);
+				cf->data[i] = data_reg & 0xff;
+				cf->data[i + 1] = (data_reg >> 8) & 0xff;
+			}
 
 			netif_receive_skb(skb);
 			rcv_pkts++;
-			goto RX_NEXT;
-		}
-		if (!(reg & CAN_IF_MCONT_NEWDAT))
-			goto RX_NEXT;
-
-		skb = alloc_can_skb(priv->ndev, &cf);
-		if (!skb)
-			return -ENOMEM;
-
-		/* Get Received data */
-		ide = ((ioread32(&priv->regs->if1_id2)) & CAN_ID2_XTD) >> 14;
-		if (ide) {
-			id = (ioread32(&priv->regs->if1_id1) & 0xffff);
-			id |= (((ioread32(&priv->regs->if1_id2)) &
-					    0x1fff) << 16);
-			cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
-		} else {
-			id = (((ioread32(&priv->regs->if1_id2)) &
-					  (CAN_SFF_MASK << 2)) >> 2);
-			cf->can_id = (id & CAN_SFF_MASK);
-		}
+			stats->rx_packets++;
+			quota--;
+			stats->rx_bytes += cf->can_dlc;
 
-		rtr = (ioread32(&priv->regs->if1_id2) &  CAN_ID2_DIR);
-		if (rtr) {
-			cf->can_dlc = 0;
-			cf->can_id |= CAN_RTR_FLAG;
-		} else {
-			cf->can_dlc = ((ioread32(&priv->regs->if1_mcont)) &
-						   0x0f);
+			pch_fifo_thresh(priv, obj_num);
 		}
+		obj_num++;
+		next_flag = 0;
+	} while (quota > 0);
 
-		for (i = 0, j = 0; i < cf->can_dlc; j++) {
-			reg = ioread32(&priv->regs->if1_dataa1 + j*4);
-			cf->data[i++] = cpu_to_le32(reg & 0xff);
-			if (i == cf->can_dlc)
-				break;
-			cf->data[i++] = cpu_to_le32((reg >> 8) & 0xff);
-		}
+	return rcv_pkts;
+}
 
-		netif_receive_skb(skb);
-		rcv_pkts++;
-		stats->rx_packets++;
-		stats->rx_bytes += cf->can_dlc;
-
-		if (k < PCH_FIFO_THRESH) {
-			iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL |
-				  CAN_CMASK_ARB, &priv->regs->if1_cmask);
-
-			/* Clearing the Dir bit. */
-			pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID2_DIR);
-
-			/* Clearing NewDat & IntPnd */
-			pch_can_bit_clear(&priv->regs->if1_mcont,
-					  CAN_IF_MCONT_INTPND);
-			pch_can_check_if_busy(&priv->regs->if1_creq, k);
-		} else if (k > PCH_FIFO_THRESH) {
-			pch_can_int_clr(priv, k);
-		} else if (k == PCH_FIFO_THRESH) {
-			int cnt;
-			for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
-				pch_can_int_clr(priv, cnt+1);
-		}
-RX_NEXT:
-		/* Reading the messsage object from the Message RAM */
-		iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
-		pch_can_check_if_busy(&priv->regs->if1_creq, k + 1);
-		reg = ioread32(&priv->regs->if1_mcont);
-	}
+static void pch_can_tx_complete(struct net_device *ndev, u32 int_stat)
+{
+	struct pch_can_priv *priv = netdev_priv(ndev);
+	struct net_device_stats *stats = &(priv->ndev->stats);
+	u32 dlc;
 
-	return rcv_pkts;
+	can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_END - 1);
+	iowrite32(PCH_CMASK_RX_TX_GET | PCH_CMASK_CLRINTPND,
+		  &priv->regs->ifregs[1].cmask);
+	dlc = get_can_dlc(ioread32(&priv->regs->ifregs[1].mcont) &
+			  PCH_IF_MCONT_DLC);
+	pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, int_stat);
+	stats->tx_bytes += dlc;
+	stats->tx_packets++;
+	if (int_stat == PCH_TX_OBJ_END)
+		netif_wake_queue(ndev);
 }
+
 static int pch_can_rx_poll(struct napi_struct *napi, int quota)
 {
 	struct net_device *ndev = napi->dev;
 	struct pch_can_priv *priv = netdev_priv(ndev);
-	struct net_device_stats *stats = &(priv->ndev->stats);
-	u32 dlc;
 	u32 int_stat;
 	int rcv_pkts = 0;
 	u32 reg_stat;
-	unsigned long flags;
 
 	int_stat = pch_can_int_pending(priv);
 	if (!int_stat)
-		return 0;
+		goto end;
 
-INT_STAT:
-	if (int_stat == CAN_STATUS_INT) {
+	if ((int_stat == PCH_STATUS_INT) && (quota > 0)) {
 		reg_stat = ioread32(&priv->regs->stat);
 		if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
-			if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)
+			if (reg_stat & PCH_BUS_OFF ||
+			   (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
 				pch_can_error(ndev, reg_stat);
+				quota--;
+			}
 		}
 
-		if (reg_stat & PCH_TX_OK) {
-			spin_lock_irqsave(&priv->msgif_reg_lock, flags);
-			iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
-			pch_can_check_if_busy(&priv->regs->if2_creq,
-					       ioread32(&priv->regs->intr));
-			spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+		if (reg_stat & PCH_TX_OK)
 			pch_can_bit_clear(&priv->regs->stat, PCH_TX_OK);
-		}
 
 		if (reg_stat & PCH_RX_OK)
 			pch_can_bit_clear(&priv->regs->stat, PCH_RX_OK);
 
 		int_stat = pch_can_int_pending(priv);
-		if (int_stat == CAN_STATUS_INT)
-			goto INT_STAT;
 	}
 
-MSG_OBJ:
-	if ((int_stat >= 1) && (int_stat <= PCH_RX_OBJ_NUM)) {
-		spin_lock_irqsave(&priv->msgif_reg_lock, flags);
-		rcv_pkts = pch_can_rx_normal(ndev, int_stat);
-		spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
-		if (rcv_pkts < 0)
-			return 0;
-	} else if ((int_stat > PCH_RX_OBJ_NUM) && (int_stat <= PCH_OBJ_NUM)) {
-		if (priv->msg_obj[int_stat - 1] == MSG_OBJ_TX) {
-			/* Handle transmission interrupt */
-			can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_NUM - 1);
-			spin_lock_irqsave(&priv->msgif_reg_lock, flags);
-			iowrite32(CAN_CMASK_RX_TX_GET | CAN_CMASK_CLRINTPND,
-				  &priv->regs->if2_cmask);
-			dlc = ioread32(&priv->regs->if2_mcont) &
-				       CAN_IF_MCONT_DLC;
-			pch_can_check_if_busy(&priv->regs->if2_creq, int_stat);
-			spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
-			if (dlc > 8)
-				dlc = 8;
-			stats->tx_bytes += dlc;
-			stats->tx_packets++;
-		}
+	if (quota == 0)
+		goto end;
+
+	if ((int_stat >= PCH_RX_OBJ_START) && (int_stat <= PCH_RX_OBJ_END)) {
+		rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
+		quota -= rcv_pkts;
+		if (quota < 0)
+			goto end;
+	} else if ((int_stat >= PCH_TX_OBJ_START) &&
+		   (int_stat <= PCH_TX_OBJ_END)) {
+		/* Handle transmission interrupt */
+		pch_can_tx_complete(ndev, int_stat);
 	}
 
-	int_stat = pch_can_int_pending(priv);
-	if (int_stat == CAN_STATUS_INT)
-		goto INT_STAT;
-	else if (int_stat >= 1 && int_stat <= 32)
-		goto MSG_OBJ;
-
+end:
 	napi_complete(napi);
 	pch_can_set_int_enables(priv, PCH_CAN_ALL);
 
@@ -980,20 +801,18 @@ static int pch_set_bittiming(struct net_device *ndev)
 	const struct can_bittiming *bt = &priv->can.bittiming;
 	u32 canbit;
 	u32 bepe;
-	u32 brp;
 
 	/* Setting the CCE bit for accessing the Can Timing register. */
-	pch_can_bit_set(&priv->regs->cont, CAN_CTRL_CCE);
-
-	brp = (bt->tq) / (1000000000/PCH_CAN_CLK) - 1;
-	canbit = brp & MSK_BITT_BRP;
-	canbit |= (bt->sjw - 1) << BIT_BITT_SJW;
-	canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << BIT_BITT_TSEG1;
-	canbit |= (bt->phase_seg2 - 1) << BIT_BITT_TSEG2;
-	bepe = (brp & MSK_BRPE_BRPE) >> BIT_BRPE_BRPE;
+	pch_can_bit_set(&priv->regs->cont, PCH_CTRL_CCE);
+
+	canbit = (bt->brp - 1) & PCH_MSK_BITT_BRP;
+	canbit |= (bt->sjw - 1) << PCH_BIT_SJW_SHIFT;
+	canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << PCH_BIT_TSEG1_SHIFT;
+	canbit |= (bt->phase_seg2 - 1) << PCH_BIT_TSEG2_SHIFT;
+	bepe = ((bt->brp - 1) & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
 	iowrite32(canbit, &priv->regs->bitt);
 	iowrite32(bepe, &priv->regs->brpe);
-	pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_CCE);
+	pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_CCE);
 
 	return 0;
 }
@@ -1008,8 +827,8 @@ static void pch_can_start(struct net_device *ndev)
 	pch_set_bittiming(ndev);
 	pch_can_set_optmode(priv);
 
-	pch_can_tx_enable_all(priv);
-	pch_can_rx_enable_all(priv);
+	pch_can_set_tx_all(priv, 1);
+	pch_can_set_rx_all(priv, 1);
 
 	/* Setting the CAN to run mode. */
 	pch_can_set_run_mode(priv, PCH_CAN_RUN);
@@ -1041,27 +860,18 @@ static int pch_can_open(struct net_device *ndev)
 	struct pch_can_priv *priv = netdev_priv(ndev);
 	int retval;
 
-	retval = pci_enable_msi(priv->dev);
-	if (retval) {
-		dev_info(&ndev->dev, "PCH CAN opened without MSI\n");
-		priv->use_msi = 0;
-	} else {
-		dev_info(&ndev->dev, "PCH CAN opened with MSI\n");
-		priv->use_msi = 1;
-	}
-
-	/* Regsitering the interrupt. */
+	/* Regstering the interrupt. */
 	retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,
 			     ndev->name, ndev);
 	if (retval) {
-		dev_err(&ndev->dev, "request_irq failed.\n");
+		netdev_err(ndev, "request_irq failed.\n");
 		goto req_irq_err;
 	}
 
 	/* Open common can device */
 	retval = open_candev(ndev);
 	if (retval) {
-		dev_err(ndev->dev.parent, "open_candev() failed %d\n", retval);
+		netdev_err(ndev, "open_candev() failed %d\n", retval);
 		goto err_open_candev;
 	}
 
@@ -1075,9 +885,6 @@ static int pch_can_open(struct net_device *ndev)
 err_open_candev:
 	free_irq(priv->dev->irq, ndev);
 req_irq_err:
-	if (priv->use_msi)
-		pci_disable_msi(priv->dev);
-
 	pch_can_release(priv);
 
 	return retval;
@@ -1091,102 +898,68 @@ static int pch_close(struct net_device *ndev)
 	napi_disable(&priv->napi);
 	pch_can_release(priv);
 	free_irq(priv->dev->irq, ndev);
-	if (priv->use_msi)
-		pci_disable_msi(priv->dev);
 	close_candev(ndev);
 	priv->can.state = CAN_STATE_STOPPED;
 	return 0;
 }
 
-static int pch_get_msg_obj_sts(struct net_device *ndev, u32 obj_id)
-{
-	u32 buffer_status = 0;
-	struct pch_can_priv *priv = netdev_priv(ndev);
-
-	/* Getting the message object status. */
-	buffer_status = (u32) pch_can_get_buffer_status(priv);
-
-	return buffer_status & obj_id;
-}
-
-
 static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
-	int i, j;
-	unsigned long flags;
 	struct pch_can_priv *priv = netdev_priv(ndev);
 	struct can_frame *cf = (struct can_frame *)skb->data;
-	int tx_buffer_avail = 0;
+	int tx_obj_no = 0;
+	int i;
+	u32 id2;
 
 	if (can_dropped_invalid_skb(ndev, skb))
 		return NETDEV_TX_OK;
 
-	if (priv->tx_obj == (PCH_OBJ_NUM + 1)) { /* Point tail Obj */
-		while (pch_get_msg_obj_sts(ndev, (((1 << PCH_TX_OBJ_NUM)-1) <<
-					   PCH_RX_OBJ_NUM)))
-			udelay(500);
+	if (priv->tx_obj == PCH_TX_OBJ_END) {
+		if (ioread32(&priv->regs->treq2) & 0xfc00)
+			netif_stop_queue(ndev);
 
-		priv->tx_obj = PCH_RX_OBJ_NUM + 1; /* Point head of Tx Obj ID */
-		tx_buffer_avail = priv->tx_obj; /* Point Tail of Tx Obj */
+		tx_obj_no = priv->tx_obj;
+		priv->tx_obj = PCH_TX_OBJ_START;
 	} else {
-		tx_buffer_avail = priv->tx_obj;
+		tx_obj_no = priv->tx_obj;
+		priv->tx_obj++;
 	}
-	priv->tx_obj++;
-
-	/* Attaining the lock. */
-	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
 
-	/* Reading the Msg Obj from the Msg RAM to the Interface register. */
-	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
-	pch_can_check_if_busy(&priv->regs->if2_creq, tx_buffer_avail);
-
-	/* Setting the CMASK register. */
-	pch_can_bit_set(&priv->regs->if2_cmask, CAN_CMASK_ALL);
+	/* Setting the CMASK register to set value */
+	iowrite32(PCH_CMASK_RX_TX_SET, &priv->regs->ifregs[1].cmask);
 
 	/* If ID extended is set. */
-	pch_can_bit_clear(&priv->regs->if2_id1, 0xffff);
-	pch_can_bit_clear(&priv->regs->if2_id2, 0x1fff | CAN_ID2_XTD);
 	if (cf->can_id & CAN_EFF_FLAG) {
-		pch_can_bit_set(&priv->regs->if2_id1, cf->can_id & 0xffff);
-		pch_can_bit_set(&priv->regs->if2_id2,
-				((cf->can_id >> 16) & 0x1fff) | CAN_ID2_XTD);
+		iowrite32(cf->can_id & 0xffff, &priv->regs->ifregs[1].id1);
+		id2 = ((cf->can_id >> 16) & 0x1fff) | PCH_ID2_XTD;
 	} else {
-		pch_can_bit_set(&priv->regs->if2_id1, 0);
-		pch_can_bit_set(&priv->regs->if2_id2,
-				(cf->can_id & CAN_SFF_MASK) << 2);
+		iowrite32(0, &priv->regs->ifregs[1].id1);
+		id2 = (cf->can_id & CAN_SFF_MASK) << 2;
 	}
 
+	id2 |= PCH_ID_MSGVAL;
+
 	/* If remote frame has to be transmitted.. */
 	if (cf->can_id & CAN_RTR_FLAG)
-		pch_can_bit_clear(&priv->regs->if2_id2, CAN_ID2_DIR);
-
-	for (i = 0, j = 0; i < cf->can_dlc; j++) {
-		iowrite32(le32_to_cpu(cf->data[i++]),
-			 (&priv->regs->if2_dataa1) + j*4);
-		if (i == cf->can_dlc)
-			break;
-		iowrite32(le32_to_cpu(cf->data[i++] << 8),
-			 (&priv->regs->if2_dataa1) + j*4);
-	}
-
-	can_put_echo_skb(skb, ndev, tx_buffer_avail - PCH_RX_OBJ_NUM - 1);
+		id2 &= ~PCH_ID2_DIR;
+	else
+		id2 |= PCH_ID2_DIR;
 
-	/* Updating the size of the data. */
-	pch_can_bit_clear(&priv->regs->if2_mcont, 0x0f);
-	pch_can_bit_set(&priv->regs->if2_mcont, cf->can_dlc);
+	iowrite32(id2, &priv->regs->ifregs[1].id2);
 
-	/* Clearing IntPend, NewDat & TxRqst */
-	pch_can_bit_clear(&priv->regs->if2_mcont,
-			  CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND |
-			  CAN_IF_MCONT_TXRQXT);
+	/* Copy data to register */
+	for (i = 0; i < cf->can_dlc; i += 2) {
+		iowrite16(cf->data[i] | (cf->data[i + 1] << 8),
+			  &priv->regs->ifregs[1].data[i / 2]);
+	}
 
-	/* Setting NewDat, TxRqst bits */
-	pch_can_bit_set(&priv->regs->if2_mcont,
-			CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_TXRQXT);
+	can_put_echo_skb(skb, ndev, tx_obj_no - PCH_RX_OBJ_END - 1);
 
-	pch_can_check_if_busy(&priv->regs->if2_creq, tx_buffer_avail);
+	/* Set the size of the data. Update if2_mcont*/
+	iowrite32(cf->can_dlc | PCH_IF_MCONT_NEWDAT | PCH_IF_MCONT_TXRQXT |
+		  PCH_IF_MCONT_TXIE, &priv->regs->ifregs[1].mcont);
 
-	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
+	pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, tx_obj_no);
 
 	return NETDEV_TX_OK;
 }
@@ -1203,21 +976,90 @@ static void __devexit pch_can_remove(struct pci_dev *pdev)
 	struct pch_can_priv *priv = netdev_priv(ndev);
 
 	unregister_candev(priv->ndev);
-	free_candev(priv->ndev);
 	pci_iounmap(pdev, priv->regs);
+	if (priv->use_msi)
+		pci_disable_msi(priv->dev);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 	pch_can_reset(priv);
+	free_candev(priv->ndev);
 }
 
 #ifdef CONFIG_PM
+static void pch_can_set_int_custom(struct pch_can_priv *priv)
+{
+	/* Clearing the IE, SIE and EIE bits of Can control register. */
+	pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_IE_SIE_EIE);
+
+	/* Appropriately setting them. */
+	pch_can_bit_set(&priv->regs->cont,
+			((priv->int_enables & PCH_MSK_CTRL_IE_SIE_EIE) << 1));
+}
+
+/* This function retrieves interrupt enabled for the CAN device. */
+static u32 pch_can_get_int_enables(struct pch_can_priv *priv)
+{
+	/* Obtaining the status of IE, SIE and EIE interrupt bits. */
+	return (ioread32(&priv->regs->cont) & PCH_CTRL_IE_SIE_EIE) >> 1;
+}
+
+static u32 pch_can_get_rxtx_ir(struct pch_can_priv *priv, u32 buff_num, u32 dir)
+{
+	u32 ie, enable;
+
+	if (dir)
+		ie = PCH_IF_MCONT_RXIE;
+	else
+		ie = PCH_IF_MCONT_TXIE;
+
+	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
+	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
+
+	if (((ioread32(&priv->regs->ifregs[dir].id2)) & PCH_ID_MSGVAL) &&
+			((ioread32(&priv->regs->ifregs[dir].mcont)) & ie))
+		enable = 1;
+	else
+		enable = 0;
+	return enable;
+}
+
+static void pch_can_set_rx_buffer_link(struct pch_can_priv *priv,
+				       u32 buffer_num, u32 set)
+{
+	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
+	pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, buffer_num);
+	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL,
+		  &priv->regs->ifregs[0].cmask);
+	if (set)
+		pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
+				  PCH_IF_MCONT_EOB);
+	else
+		pch_can_bit_set(&priv->regs->ifregs[0].mcont, PCH_IF_MCONT_EOB);
+
+	pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, buffer_num);
+}
+
+static u32 pch_can_get_rx_buffer_link(struct pch_can_priv *priv, u32 buffer_num)
+{
+	u32 link;
+
+	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
+	pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, buffer_num);
+
+	if (ioread32(&priv->regs->ifregs[0].mcont) & PCH_IF_MCONT_EOB)
+		link = 0;
+	else
+		link = 1;
+	return link;
+}
+
 static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 {
-	int i;			/* Counter variable. */
-	int retval;		/* Return value. */
+	int i;
+	int retval;
 	u32 buf_stat;	/* Variable for reading the transmit buffer status. */
-	u32 counter = 0xFFFFFF;
+	u32 counter = PCH_COUNTER_LIMIT;
 
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct pch_can_priv *priv = netdev_priv(dev);
@@ -1226,7 +1068,7 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 	pch_can_set_run_mode(priv, PCH_CAN_STOP);
 
 	/* Indicate that we are aboutto/in suspend */
-	priv->can.state = CAN_STATE_SLEEPING;
+	priv->can.state = CAN_STATE_STOPPED;
 
 	/* Waiting for all transmission to complete. */
 	while (counter) {
@@ -1240,31 +1082,24 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 		dev_err(&pdev->dev, "%s -> Transmission time out.\n", __func__);
 
 	/* Save interrupt configuration and then disable them */
-	pch_can_get_int_enables(priv, &(priv->int_enables));
+	priv->int_enables = pch_can_get_int_enables(priv);
 	pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
 
 	/* Save Tx buffer enable state */
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_TX)
-			pch_can_get_tx_enable(priv, i + 1,
-					      &(priv->tx_enable[i]));
-	}
+	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
+		priv->tx_enable[i] = pch_can_get_rxtx_ir(priv, i, PCH_TX_IFREG);
 
 	/* Disable all Transmit buffers */
-	pch_can_tx_disable_all(priv);
+	pch_can_set_tx_all(priv, 0);
 
 	/* Save Rx buffer enable state */
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_RX) {
-			pch_can_get_rx_enable(priv, i + 1,
-						&(priv->rx_enable[i]));
-			pch_can_get_rx_buffer_link(priv, i + 1,
-						&(priv->rx_link[i]));
-		}
+	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++) {
+		priv->rx_enable[i] = pch_can_get_rxtx_ir(priv, i, PCH_RX_IFREG);
+		priv->rx_link[i] = pch_can_get_rx_buffer_link(priv, i);
 	}
 
 	/* Disable all Receive buffers */
-	pch_can_rx_disable_all(priv);
+	pch_can_set_rx_all(priv, 0);
 	retval = pci_save_state(pdev);
 	if (retval) {
 		dev_err(&pdev->dev, "pci_save_state failed.\n");
@@ -1279,8 +1114,8 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 
 static int pch_can_resume(struct pci_dev *pdev)
 {
-	int i;			/* Counter variable. */
-	int retval;		/* Return variable. */
+	int i;
+	int retval;
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct pch_can_priv *priv = netdev_priv(dev);
 
@@ -1312,23 +1147,16 @@ static int pch_can_resume(struct pci_dev *pdev)
 	pch_can_set_optmode(priv);
 
 	/* Enabling the transmit buffer. */
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_TX) {
-			pch_can_set_tx_enable(priv, i + 1,
-					      priv->tx_enable[i]);
-		}
-	}
+	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++)
+		pch_can_set_rxtx(priv, i, priv->tx_enable[i], PCH_TX_IFREG);
 
 	/* Configuring the receive buffer and enabling them. */
-	for (i = 0; i < PCH_OBJ_NUM; i++) {
-		if (priv->msg_obj[i] == MSG_OBJ_RX) {
-			/* Restore buffer link */
-			pch_can_set_rx_buffer_link(priv, i + 1,
-						   priv->rx_link[i]);
-
-			/* Restore buffer enables */
-			pch_can_set_rx_enable(priv, i + 1, priv->rx_enable[i]);
-		}
+	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
+		/* Restore buffer link */
+		pch_can_set_rx_buffer_link(priv, i, priv->rx_link[i]);
+
+		/* Restore buffer enables */
+		pch_can_set_rxtx(priv, i, priv->rx_enable[i], PCH_RX_IFREG);
 	}
 
 	/* Enable CAN Interrupts */
@@ -1349,8 +1177,8 @@ static int pch_can_get_berr_counter(const struct net_device *dev,
 {
 	struct pch_can_priv *priv = netdev_priv(dev);
 
-	bec->txerr = ioread32(&priv->regs->errc) & CAN_TEC;
-	bec->rxerr = (ioread32(&priv->regs->errc) & CAN_REC) >> 8;
+	bec->txerr = ioread32(&priv->regs->errc) & PCH_TEC;
+	bec->rxerr = (ioread32(&priv->regs->errc) & PCH_REC) >> 8;
 
 	return 0;
 }
@@ -1361,7 +1189,6 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
 	struct net_device *ndev;
 	struct pch_can_priv *priv;
 	int rc;
-	int index;
 	void __iomem *addr;
 
 	rc = pci_enable_device(pdev);
@@ -1383,7 +1210,7 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
 		goto probe_exit_ipmap;
 	}
 
-	ndev = alloc_candev(sizeof(struct pch_can_priv), PCH_TX_OBJ_NUM);
+	ndev = alloc_candev(sizeof(struct pch_can_priv), PCH_TX_OBJ_END);
 	if (!ndev) {
 		rc = -ENOMEM;
 		dev_err(&pdev->dev, "Failed alloc_candev\n");
@@ -1399,7 +1226,7 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
 	priv->can.do_get_berr_counter = pch_can_get_berr_counter;
 	priv->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
 				       CAN_CTRLMODE_LOOPBACK;
-	priv->tx_obj = PCH_RX_OBJ_NUM + 1; /* Point head of Tx Obj */
+	priv->tx_obj = PCH_TX_OBJ_START; /* Point head of Tx Obj */
 
 	ndev->irq = pdev->irq;
 	ndev->flags |= IFF_ECHO;
@@ -1407,15 +1234,18 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
 	pci_set_drvdata(pdev, ndev);
 	SET_NETDEV_DEV(ndev, &pdev->dev);
 	ndev->netdev_ops = &pch_can_netdev_ops;
-
 	priv->can.clock.freq = PCH_CAN_CLK; /* Hz */
-	for (index = 0; index < PCH_RX_OBJ_NUM;)
-		priv->msg_obj[index++] = MSG_OBJ_RX;
 
-	for (index = index;  index < PCH_OBJ_NUM;)
-		priv->msg_obj[index++] = MSG_OBJ_TX;
+	netif_napi_add(ndev, &priv->napi, pch_can_rx_poll, PCH_RX_OBJ_END);
 
-	netif_napi_add(ndev, &priv->napi, pch_can_rx_poll, PCH_RX_OBJ_NUM);
+	rc = pci_enable_msi(priv->dev);
+	if (rc) {
+		dev_info(&ndev->dev, "PCH CAN opened without MSI\n");
+		priv->use_msi = 0;
+	} else {
+		dev_info(&ndev->dev, "PCH CAN opened with MSI\n");
+		priv->use_msi = 1;
+	}
 
 	rc = register_candev(ndev);
 	if (rc) {
@@ -1426,7 +1256,8 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
 	return 0;
 
 probe_exit_reg_candev:
-	free_candev(ndev);
+	if (priv->use_msi)
+		pci_disable_msi(priv->dev);
 probe_exit_alloc_candev:
 	pci_iounmap(pdev, addr);
 probe_exit_ipmap:
@@ -1458,6 +1289,6 @@ static void __exit pch_can_pci_exit(void)
 }
 module_exit(pch_can_pci_exit);
 
-MODULE_DESCRIPTION("Controller Area Network Driver");
+MODULE_DESCRIPTION("Intel EG20T PCH CAN(Controller Area Network) Driver");
 MODULE_LICENSE("GPL v2");
 MODULE_VERSION("0.94");
-- 
1.6.0.6


^ permalink raw reply related

* Re: [PATCH net-next-2.6 v4] can: Topcliff: PCH_CAN driver: Add Flow control/Fix Endianess issue/Separate IF register/Enumerate LEC macro/Move MSI processing/Use BIT(X)/Change Message Object index/Add prefix PCH_
From: Wolfgang Grandegger @ 2010-11-16 12:22 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
	Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CE275A4.9010400-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>

On 11/16/2010 01:14 PM, Tomoya MORINAGA wrote:
> Add flow control processing.
> Currently, there is no flow control processing.
> Thus, Add flow control processing as
> when there is no empty of tx buffer,
> netif_stop_queue is called.
> When there is empty buffer, netif_wake_queue is called.
> 
> 
> Fix endianness issue.
> there is endianness issue both Tx and Rx.
> Currently, data is set like below.
> Register:
>  MSB---LSB
>  x x D0 D1
>  x x D2 D3
>  x x D4 D5
>  x x D6 D7
> 
> But Data to be sent must be set like below.
> Register:
>  MSB---LSB
>  x x D1 D0
>  x x D3 D2
>  x x D5 D4
>  x x D7 D6  (x means reserved area.)
> 
> 
> Separate interface register from whole of register structure.
> CAN register of Intel PCH EG20T has 2 sets of interface register.
> To reduce whole of code size, separate interface register.
> As a result, the number of function also can be reduced. 
> 
> 
> Enumerate LEC macro from #define macro.
> For easy to readable, all LEC #define macros are replease to enums like below.
> enum pch_can_err {
> 	PCH_STUF_ERR = 1,
> 	PCH_FORM_ERR,
> 	PCH_ACK_ERR,
> 	PCH_BIT1_ERR,
> 	PCH_BIT0_ERR,
> 	PCH_CRC_ERR,
> 	PCH_LEC_ALL,
> };
> 
> 
> Move MSI processing to probe/remove processing.
> Currently, in case this driver is integrated as module, and 
> when this module is re-installed, no interrupts is to be occurred.
> For the above issue, move MSI processing to open/release processing.
> 
> 
> Replace bit assignment value to BIT(X).
> For easy to readable, replace all bit assigned macros to BIT(X)
> 
> 
> Change Message Object index macro name.
> For easy to readable, add Message Object index like below.
> PCH_RX_OBJ_START
> PCH_RX_OBJ_END
> PCH_TX_OBJ_START
> PCH_TX_OBJ_END
> 
> 
> Add prefix PCH_ to all of #define macros.
> For easy to readable, add prefix "PCH_" to all of #define macros.
> 
> 
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>

Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Thanks for your contribution.

Wolfgang.

^ permalink raw reply

* Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Marc Kleine-Budde @ 2010-11-16 12:22 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
	Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CE2434B.5050701-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>


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

On 11/16/2010 09:39 AM, Tomoya MORINAGA wrote:
> Hi Marc and Wolf,
> 
> I have updated your indications.
> 
>> * Add Flow control
>> * Fix Data copy issue (endianness)
>> * Add Macro prefix "PCH_"
>> * Separate interface register structure
>> * Some functions are unified.
>> * Change MessageObject indication(PCH_RX_OBJ_START, etc..)
>> * Enumerate LEC macro
>> * Move MSI processing from open/close to probe/remove processing
>> * Use BIT(x)
>> * and more...
> 
> Signed-off-by: Tomoya MORINAGA <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
> ---
>  drivers/net/can/pch_can.c | 1341 ++++++++++++++++++++-------------------------
>  1 files changed, 586 insertions(+), 755 deletions(-)
> 
> diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
> index 6727182..bb0f873 100644
> --- a/drivers/net/can/pch_can.c
> +++ b/drivers/net/can/pch_can.c
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright (C) 1999 - 2010 Intel Corporation.
> - * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
> + * Copyright (C) 2010 OKI SEMICONDUCTOR CO., LTD.
>   *
>   * 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
> @@ -32,106 +32,111 @@
>  #include <linux/can/dev.h>
>  #include <linux/can/error.h>
>  
> -#define MAX_MSG_OBJ		32
> -#define MSG_OBJ_RX		0 /* The receive message object flag. */
> -#define MSG_OBJ_TX		1 /* The transmit message object flag. */
> -
> -#define ENABLE			1 /* The enable flag */
> -#define DISABLE			0 /* The disable flag */
> -#define CAN_CTRL_INIT		0x0001 /* The INIT bit of CANCONT register. */
> -#define CAN_CTRL_IE		0x0002 /* The IE bit of CAN control register */
> -#define CAN_CTRL_IE_SIE_EIE	0x000e
> -#define CAN_CTRL_CCE		0x0040
> -#define CAN_CTRL_OPT		0x0080 /* The OPT bit of CANCONT register. */
> -#define CAN_OPT_SILENT		0x0008 /* The Silent bit of CANOPT reg. */
> -#define CAN_OPT_LBACK		0x0010 /* The LoopBack bit of CANOPT reg. */
> -#define CAN_CMASK_RX_TX_SET	0x00f3
> -#define CAN_CMASK_RX_TX_GET	0x0073
> -#define CAN_CMASK_ALL		0xff
> -#define CAN_CMASK_RDWR		0x80
> -#define CAN_CMASK_ARB		0x20
> -#define CAN_CMASK_CTRL		0x10
> -#define CAN_CMASK_MASK		0x40
> -#define CAN_CMASK_NEWDAT	0x04
> -#define CAN_CMASK_CLRINTPND	0x08
> -
> -#define CAN_IF_MCONT_NEWDAT	0x8000
> -#define CAN_IF_MCONT_INTPND	0x2000
> -#define CAN_IF_MCONT_UMASK	0x1000
> -#define CAN_IF_MCONT_TXIE	0x0800
> -#define CAN_IF_MCONT_RXIE	0x0400
> -#define CAN_IF_MCONT_RMTEN	0x0200
> -#define CAN_IF_MCONT_TXRQXT	0x0100
> -#define CAN_IF_MCONT_EOB	0x0080
> -#define CAN_IF_MCONT_DLC	0x000f
> -#define CAN_IF_MCONT_MSGLOST	0x4000
> -#define CAN_MASK2_MDIR_MXTD	0xc000
> -#define CAN_ID2_DIR		0x2000
> -#define CAN_ID_MSGVAL		0x8000
> -
> -#define CAN_STATUS_INT		0x8000
> -#define CAN_IF_CREQ_BUSY	0x8000
> -#define CAN_ID2_XTD		0x4000
> -
> -#define CAN_REC			0x00007f00
> -#define CAN_TEC			0x000000ff
> -
> -#define PCH_RX_OK		0x00000010
> -#define PCH_TX_OK		0x00000008
> -#define PCH_BUS_OFF		0x00000080
> -#define PCH_EWARN		0x00000040
> -#define PCH_EPASSIV		0x00000020
> -#define PCH_LEC0		0x00000001
> -#define PCH_LEC1		0x00000002
> -#define PCH_LEC2		0x00000004
> -#define PCH_LEC_ALL		(PCH_LEC0 | PCH_LEC1 | PCH_LEC2)
> -#define PCH_STUF_ERR		PCH_LEC0
> -#define PCH_FORM_ERR		PCH_LEC1
> -#define PCH_ACK_ERR		(PCH_LEC0 | PCH_LEC1)
> -#define PCH_BIT1_ERR		PCH_LEC2
> -#define PCH_BIT0_ERR		(PCH_LEC0 | PCH_LEC2)
> -#define PCH_CRC_ERR		(PCH_LEC1 | PCH_LEC2)
> +#define PCH_CTRL_INIT		BIT(0) /* The INIT bit of CANCONT register. */
> +#define PCH_CTRL_IE		BIT(1) /* The IE bit of CAN control register */
> +#define PCH_CTRL_IE_SIE_EIE	(BIT(3) | BIT(2) | BIT(1))
> +#define PCH_CTRL_CCE		BIT(6)
> +#define PCH_CTRL_OPT		BIT(7) /* The OPT bit of CANCONT register. */
> +#define PCH_OPT_SILENT		BIT(3) /* The Silent bit of CANOPT reg. */
> +#define PCH_OPT_LBACK		BIT(4) /* The LoopBack bit of CANOPT reg. */
> +
> +#define PCH_CMASK_RX_TX_SET	0x00f3
> +#define PCH_CMASK_RX_TX_GET	0x0073
> +#define PCH_CMASK_ALL		0xff
> +#define PCH_CMASK_NEWDAT	BIT(2)
> +#define PCH_CMASK_CLRINTPND	BIT(3)
> +#define PCH_CMASK_CTRL		BIT(4)
> +#define PCH_CMASK_ARB		BIT(5)
> +#define PCH_CMASK_MASK		BIT(6)
> +#define PCH_CMASK_RDWR		BIT(7)
> +#define PCH_IF_MCONT_NEWDAT	BIT(15)
> +#define PCH_IF_MCONT_MSGLOST	BIT(14)
> +#define PCH_IF_MCONT_INTPND	BIT(13)
> +#define PCH_IF_MCONT_UMASK	BIT(12)
> +#define PCH_IF_MCONT_TXIE	BIT(11)
> +#define PCH_IF_MCONT_RXIE	BIT(10)
> +#define PCH_IF_MCONT_RMTEN	BIT(9)
> +#define PCH_IF_MCONT_TXRQXT	BIT(8)
> +#define PCH_IF_MCONT_EOB	BIT(7)
> +#define PCH_IF_MCONT_DLC	(BIT(0) | BIT(1) | BIT(2) | BIT(3))
> +#define PCH_MASK2_MDIR_MXTD	(BIT(14) | BIT(15))
> +#define PCH_ID2_DIR		BIT(13)
> +#define PCH_ID2_XTD		BIT(14)
> +#define PCH_ID_MSGVAL		BIT(15)
> +#define PCH_IF_CREQ_BUSY	BIT(15)
> +
> +#define PCH_STATUS_INT		0x8000
> +#define PCH_REC			0x00007f00
> +#define PCH_TEC			0x000000ff
> +
> +#define PCH_TX_OK		BIT(3)
> +#define PCH_RX_OK		BIT(4)
> +#define PCH_EPASSIV		BIT(5)
> +#define PCH_EWARN		BIT(6)
> +#define PCH_BUS_OFF		BIT(7)
>  
>  /* bit position of certain controller bits. */
> -#define BIT_BITT_BRP		0
> -#define BIT_BITT_SJW		6
> -#define BIT_BITT_TSEG1		8
> -#define BIT_BITT_TSEG2		12
> -#define BIT_IF1_MCONT_RXIE	10
> -#define BIT_IF2_MCONT_TXIE	11
> -#define BIT_BRPE_BRPE		6
> -#define BIT_ES_TXERRCNT		0
> -#define BIT_ES_RXERRCNT		8
> -#define MSK_BITT_BRP		0x3f
> -#define MSK_BITT_SJW		0xc0
> -#define MSK_BITT_TSEG1		0xf00
> -#define MSK_BITT_TSEG2		0x7000
> -#define MSK_BRPE_BRPE		0x3c0
> -#define MSK_BRPE_GET		0x0f
> -#define MSK_CTRL_IE_SIE_EIE	0x07
> -#define MSK_MCONT_TXIE		0x08
> -#define MSK_MCONT_RXIE		0x10
> -#define PCH_CAN_NO_TX_BUFF	1
> -#define COUNTER_LIMIT		10
> -
> -#define PCH_CAN_CLK		50000000	/* 50MHz */
> -
> -/* Define the number of message object.
> +#define PCH_BIT_BRP_SHIFT		0
> +#define PCH_BIT_SJW_SHIFT		6
> +#define PCH_BIT_TSEG1_SHIFT		8
> +#define PCH_BIT_TSEG2_SHIFT		12
> +#define PCH_BIT_IF1_MCONT_RXIE_SHIFT	10
> +#define PCH_BIT_IF2_MCONT_TXIE_SHIFT	11
> +#define PCH_BIT_BRPE_BRPE_SHIFT		6
> +
> +#define PCH_MSK_BITT_BRP		0x3f
> +#define PCH_MSK_BRPE_BRPE		0x3c0
> +#define PCH_MSK_CTRL_IE_SIE_EIE		0x07
> +#define PCH_COUNTER_LIMIT		10
> +
> +#define PCH_RX_IFREG			0
> +#define PCH_TX_IFREG			1

Please make this an enum. Use this enum in the function arguments
instead of an "u32".

> +
> +#define PCH_CAN_CLK			50000000	/* 50MHz */
> +
> +/*
> + * Define the number of message object.
>   * PCH CAN communications are done via Message RAM.
> - * The Message RAM consists of 32 message objects. */
> -#define PCH_RX_OBJ_NUM		26  /* 1~ PCH_RX_OBJ_NUM is Rx*/
> -#define PCH_TX_OBJ_NUM		6  /* PCH_RX_OBJ_NUM is RX ~ Tx*/
> -#define PCH_OBJ_NUM		(PCH_TX_OBJ_NUM + PCH_RX_OBJ_NUM)
> + * The Message RAM consists of 32 message objects.
> + */
> +#define PCH_RX_OBJ_NUM		26
> +#define PCH_TX_OBJ_NUM		6
> +#define PCH_RX_OBJ_START	1
> +#define PCH_RX_OBJ_END		PCH_RX_OBJ_NUM
> +#define PCH_TX_OBJ_START	(PCH_RX_OBJ_END + 1)
> +#define PCH_TX_OBJ_END		(PCH_RX_OBJ_NUM + PCH_TX_OBJ_NUM)
>  
>  #define PCH_FIFO_THRESH		16
>  
> +enum pch_can_err {
> +	PCH_STUF_ERR = 1,
> +	PCH_FORM_ERR,
> +	PCH_ACK_ERR,
> +	PCH_BIT1_ERR,
> +	PCH_BIT0_ERR,
> +	PCH_CRC_ERR,
> +	PCH_LEC_ALL,
> +};
> +
>  enum pch_can_mode {
>  	PCH_CAN_ENABLE,
>  	PCH_CAN_DISABLE,
>  	PCH_CAN_ALL,
>  	PCH_CAN_NONE,
>  	PCH_CAN_STOP,
> -	PCH_CAN_RUN
> +	PCH_CAN_RUN,
> +};
> +
> +struct pch_can_if_regs {
> +	u32 creq;
> +	u32 cmask;
> +	u32 mask1;
> +	u32 mask2;
> +	u32 id1;
> +	u32 id2;
> +	u32 mcont;
> +	u32 data[4];
> +	u32 rsv[13];
>  };
>  
>  struct pch_can_regs {
> @@ -142,53 +147,34 @@ struct pch_can_regs {
>  	u32 intr;
>  	u32 opt;
>  	u32 brpe;
> -	u32 reserve1;
> -	u32 if1_creq;
> -	u32 if1_cmask;
> -	u32 if1_mask1;
> -	u32 if1_mask2;
> -	u32 if1_id1;
> -	u32 if1_id2;
> -	u32 if1_mcont;
> -	u32 if1_dataa1;
> -	u32 if1_dataa2;
> -	u32 if1_datab1;
> -	u32 if1_datab2;
> -	u32 reserve2;
> -	u32 reserve3[12];
> -	u32 if2_creq;
> -	u32 if2_cmask;
> -	u32 if2_mask1;
> -	u32 if2_mask2;
> -	u32 if2_id1;
> -	u32 if2_id2;
> -	u32 if2_mcont;
> -	u32 if2_dataa1;
> -	u32 if2_dataa2;
> -	u32 if2_datab1;
> -	u32 if2_datab2;
> -	u32 reserve4;
> -	u32 reserve5[20];
> +	u32 reserve;
> +	struct pch_can_if_regs ifregs[2]; /* [0]=if1  [1]=if2 */
> +	u32 reserve1[8];
>  	u32 treq1;
>  	u32 treq2;
> -	u32 reserve6[2];
> -	u32 reserve7[56];
> -	u32 reserve8[3];
> +	u32 reserve2[6];
> +	u32 data1;
> +	u32 data2;
> +	u32 reserve3[6];
> +	u32 canipend1;
> +	u32 canipend2;
> +	u32 reserve4[6];
> +	u32 canmval1;
> +	u32 canmval2;
> +	u32 reserve5[37];
>  	u32 srst;
>  };
>  
>  struct pch_can_priv {
>  	struct can_priv can;
> -	unsigned int can_num;
>  	struct pci_dev *dev;
> -	unsigned int tx_enable[MAX_MSG_OBJ];
> -	unsigned int rx_enable[MAX_MSG_OBJ];
> -	unsigned int rx_link[MAX_MSG_OBJ];
> +	unsigned int tx_enable[PCH_TX_OBJ_END];
> +	unsigned int rx_enable[PCH_TX_OBJ_END];
> +	unsigned int rx_link[PCH_TX_OBJ_END];
>  	unsigned int int_enables;
>  	unsigned int int_stat;
>  	struct net_device *ndev;
> -	spinlock_t msgif_reg_lock; /* Message Interface Registers Access Lock*/
> -	unsigned int msg_obj[MAX_MSG_OBJ];
> +	unsigned int msg_obj[PCH_TX_OBJ_END];
>  	struct pch_can_regs __iomem *regs;
>  	struct napi_struct napi;
>  	unsigned int tx_obj;	/* Point next Tx Obj index */
> @@ -228,15 +214,15 @@ static void pch_can_set_run_mode(struct pch_can_priv *priv,
>  {
>  	switch (mode) {
>  	case PCH_CAN_RUN:
> -		pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_INIT);
> +		pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_INIT);
>  		break;
>  
>  	case PCH_CAN_STOP:
> -		pch_can_bit_set(&priv->regs->cont, CAN_CTRL_INIT);
> +		pch_can_bit_set(&priv->regs->cont, PCH_CTRL_INIT);
>  		break;
>  
>  	default:
> -		dev_err(&priv->ndev->dev, "%s -> Invalid Mode.\n", __func__);
> +		netdev_err(priv->ndev, "%s -> Invalid Mode.\n", __func__);
>  		break;
>  	}
>  }
> @@ -246,357 +232,184 @@ static void pch_can_set_optmode(struct pch_can_priv *priv)
>  	u32 reg_val = ioread32(&priv->regs->opt);
>  
>  	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> -		reg_val |= CAN_OPT_SILENT;
> +		reg_val |= PCH_OPT_SILENT;
>  
>  	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
> -		reg_val |= CAN_OPT_LBACK;
> +		reg_val |= PCH_OPT_LBACK;
>  
> -	pch_can_bit_set(&priv->regs->cont, CAN_CTRL_OPT);
> +	pch_can_bit_set(&priv->regs->cont, PCH_CTRL_OPT);
>  	iowrite32(reg_val, &priv->regs->opt);
>  }
>  
> -static void pch_can_set_int_custom(struct pch_can_priv *priv)
> +static void pch_can_rw_msg_obj(void __iomem *creq_addr, u32 num)
>  {
> -	/* Clearing the IE, SIE and EIE bits of Can control register. */
> -	pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
> -
> -	/* Appropriately setting them. */
> -	pch_can_bit_set(&priv->regs->cont,
> -			((priv->int_enables & MSK_CTRL_IE_SIE_EIE) << 1));
> -}
> +	int counter = PCH_COUNTER_LIMIT;
> +	u32 ifx_creq;
>  
> -/* This function retrieves interrupt enabled for the CAN device. */
> -static void pch_can_get_int_enables(struct pch_can_priv *priv, u32 *enables)
> -{
> -	/* Obtaining the status of IE, SIE and EIE interrupt bits. */
> -	*enables = ((ioread32(&priv->regs->cont) & CAN_CTRL_IE_SIE_EIE) >> 1);
> +	iowrite32(num, creq_addr);
> +	while (counter) {
> +		ifx_creq = ioread32(creq_addr) & PCH_IF_CREQ_BUSY;
> +		if (!ifx_creq)
> +			break;
> +		counter--;
> +		udelay(1);
> +	}
> +	if (!counter)
> +		pr_err("%s:IF1 BUSY Flag is set forever.\n", __func__);
>  }
>  
>  static void pch_can_set_int_enables(struct pch_can_priv *priv,
>  				    enum pch_can_mode interrupt_no)
>  {
>  	switch (interrupt_no) {
> -	case PCH_CAN_ENABLE:
> -		pch_can_bit_set(&priv->regs->cont, CAN_CTRL_IE);
> -		break;
> -
>  	case PCH_CAN_DISABLE:
> -		pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE);
> +		pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_IE);
>  		break;
>  
>  	case PCH_CAN_ALL:
> -		pch_can_bit_set(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
> +		pch_can_bit_set(&priv->regs->cont, PCH_CTRL_IE_SIE_EIE);
>  		break;
>  
>  	case PCH_CAN_NONE:
> -		pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_IE_SIE_EIE);
> +		pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_IE_SIE_EIE);
>  		break;
>  
>  	default:
> -		dev_err(&priv->ndev->dev, "Invalid interrupt number.\n");
> +		netdev_err(priv->ndev, "Invalid interrupt number.\n");
>  		break;
>  	}
>  }
>  
> -static void pch_can_check_if_busy(u32 __iomem *creq_addr, u32 num)
> +static void pch_can_set_rxtx(struct pch_can_priv *priv, u32 buff_num,
> +				    u32 set, u32 dir)
                                    ^^^^^^^

Please make "set" a normal int (or bool).

>  {
> -	u32 counter = COUNTER_LIMIT;
> -	u32 ifx_creq;
> +	u32 ie;
>  
> -	iowrite32(num, creq_addr);
> -	while (counter) {
> -		ifx_creq = ioread32(creq_addr) & CAN_IF_CREQ_BUSY;
> -		if (!ifx_creq)
> -			break;
> -		counter--;
> -		udelay(1);
> -	}
> -	if (!counter)
> -		pr_err("%s:IF1 BUSY Flag is set forever.\n", __func__);
> -}
> -
> -static void pch_can_set_rx_enable(struct pch_can_priv *priv, u32 buff_num,
> -				  u32 set)
> -{
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
> -	/* Reading the receive buffer data from RAM to Interface1 registers */
> -	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
> -	pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
> -
> -	/* Setting the IF1MASK1 register to access MsgVal and RxIE bits */
> -	iowrite32(CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL,
> -		  &priv->regs->if1_cmask);
> -
> -	if (set == ENABLE) {
> -		/* Setting the MsgVal and RxIE bits */
> -		pch_can_bit_set(&priv->regs->if1_mcont, CAN_IF_MCONT_RXIE);
> -		pch_can_bit_set(&priv->regs->if1_id2, CAN_ID_MSGVAL);
> -
> -	} else if (set == DISABLE) {
> -		/* Resetting the MsgVal and RxIE bits */
> -		pch_can_bit_clear(&priv->regs->if1_mcont, CAN_IF_MCONT_RXIE);
> -		pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID_MSGVAL);
> -	}
> -
> -	pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
> -	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> -}
> -
> -static void pch_can_rx_enable_all(struct pch_can_priv *priv)
> -{
> -	int i;
> -
> -	/* Traversing to obtain the object configured as receivers. */
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_RX)
> -			pch_can_set_rx_enable(priv, i + 1, ENABLE);
> -	}
> -}
> -
> -static void pch_can_rx_disable_all(struct pch_can_priv *priv)
> -{
> -	int i;
> -
> -	/* Traversing to obtain the object configured as receivers. */
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_RX)
> -			pch_can_set_rx_enable(priv, i + 1, DISABLE);
> -	}
> -}
> -
> -static void pch_can_set_tx_enable(struct pch_can_priv *priv, u32 buff_num,
> -				 u32 set)
> -{
> -	unsigned long flags;
> +	if (dir)
> +		ie = PCH_IF_MCONT_TXIE;
> +	else
> +		ie = PCH_IF_MCONT_RXIE;
>  
> -	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>  	/* Reading the Msg buffer from Message RAM to Interface2 registers. */
> -	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
> -	pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
> +	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
>  
>  	/* Setting the IF2CMASK register for accessing the
>  		MsgVal and TxIE bits */

can you fix this multiline comment, please

> -	iowrite32(CAN_CMASK_RDWR | CAN_CMASK_ARB | CAN_CMASK_CTRL,
> -		 &priv->regs->if2_cmask);
> +	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_ARB | PCH_CMASK_CTRL,
> +		 &priv->regs->ifregs[dir].cmask);
>  
> -	if (set == ENABLE) {
> +	if (set) {
>  		/* Setting the MsgVal and TxIE bits */
> -		pch_can_bit_set(&priv->regs->if2_mcont, CAN_IF_MCONT_TXIE);
> -		pch_can_bit_set(&priv->regs->if2_id2, CAN_ID_MSGVAL);
> -	} else if (set == DISABLE) {
> +		pch_can_bit_set(&priv->regs->ifregs[dir].mcont, ie);
> +		pch_can_bit_set(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
> +	} else {
>  		/* Resetting the MsgVal and TxIE bits. */
> -		pch_can_bit_clear(&priv->regs->if2_mcont, CAN_IF_MCONT_TXIE);
> -		pch_can_bit_clear(&priv->regs->if2_id2, CAN_ID_MSGVAL);
> +		pch_can_bit_clear(&priv->regs->ifregs[dir].mcont, ie);
> +		pch_can_bit_clear(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
>  	}
>  
> -	pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
> -	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
>  }
>  
> -static void pch_can_tx_enable_all(struct pch_can_priv *priv)
> +static void pch_can_set_rx_all(struct pch_can_priv *priv, u32 set)
>  {
>  	int i;
>  
> -	/* Traversing to obtain the object configured as transmit object. */
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_TX)
> -			pch_can_set_tx_enable(priv, i + 1, ENABLE);
> -	}
> +	/* Traversing to obtain the object configured as receivers. */
> +	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++)
> +		pch_can_set_rxtx(priv, i, set, PCH_RX_IFREG);
>  }
>  
> -static void pch_can_tx_disable_all(struct pch_can_priv *priv)
> +static void pch_can_set_tx_all(struct pch_can_priv *priv, u32 set)
>  {
>  	int i;
>  
> -	/* Traversing to obtain the object configured as transmit object. */
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_TX)
> -			pch_can_set_tx_enable(priv, i + 1, DISABLE);
> -	}
> -}
> -
> -static void pch_can_get_rx_enable(struct pch_can_priv *priv, u32 buff_num,
> -				 u32 *enable)
> -{
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
> -	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
> -	pch_can_check_if_busy(&priv->regs->if1_creq, buff_num);
> -
> -	if (((ioread32(&priv->regs->if1_id2)) & CAN_ID_MSGVAL) &&
> -			((ioread32(&priv->regs->if1_mcont)) &
> -			CAN_IF_MCONT_RXIE))
> -		*enable = ENABLE;
> -	else
> -		*enable = DISABLE;
> -	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> -}
> -
> -static void pch_can_get_tx_enable(struct pch_can_priv *priv, u32 buff_num,
> -				 u32 *enable)
> -{
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
> -	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
> -	pch_can_check_if_busy(&priv->regs->if2_creq, buff_num);
> -
> -	if (((ioread32(&priv->regs->if2_id2)) & CAN_ID_MSGVAL) &&
> -			((ioread32(&priv->regs->if2_mcont)) &
> -			CAN_IF_MCONT_TXIE)) {
> -		*enable = ENABLE;
> -	} else {
> -		*enable = DISABLE;
> -	}
> -	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> +	/* Traversing to obtain the object configured as receivers. */
> +	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
> +		pch_can_set_rxtx(priv, i, set, PCH_TX_IFREG);
>  }
>  
> -static int pch_can_int_pending(struct pch_can_priv *priv)
> +static u32 pch_can_int_pending(struct pch_can_priv *priv)
>  {
>  	return ioread32(&priv->regs->intr) & 0xffff;
>  }
>  
> -static void pch_can_set_rx_buffer_link(struct pch_can_priv *priv,
> -				       u32 buffer_num, u32 set)
> -{
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
> -	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
> -	pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
> -	iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL, &priv->regs->if1_cmask);
> -	if (set == ENABLE)
> -		pch_can_bit_clear(&priv->regs->if1_mcont, CAN_IF_MCONT_EOB);
> -	else
> -		pch_can_bit_set(&priv->regs->if1_mcont, CAN_IF_MCONT_EOB);
> -
> -	pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
> -	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> -}
> -
> -static void pch_can_get_rx_buffer_link(struct pch_can_priv *priv,
> -				       u32 buffer_num, u32 *link)
> +static void pch_can_clear_if_buffers(struct pch_can_priv *priv)
>  {
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
> -	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
> -	pch_can_check_if_busy(&priv->regs->if1_creq, buffer_num);
> -
> -	if (ioread32(&priv->regs->if1_mcont) & CAN_IF_MCONT_EOB)
> -		*link = DISABLE;
> -	else
> -		*link = ENABLE;
> -	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> -}
> -
> -static void pch_can_clear_buffers(struct pch_can_priv *priv)
> -{
> -	int i;
> -
> -	for (i = 0; i < PCH_RX_OBJ_NUM; i++) {
> -		iowrite32(CAN_CMASK_RX_TX_SET, &priv->regs->if1_cmask);
> -		iowrite32(0xffff, &priv->regs->if1_mask1);
> -		iowrite32(0xffff, &priv->regs->if1_mask2);
> -		iowrite32(0x0, &priv->regs->if1_id1);
> -		iowrite32(0x0, &priv->regs->if1_id2);
> -		iowrite32(0x0, &priv->regs->if1_mcont);
> -		iowrite32(0x0, &priv->regs->if1_dataa1);
> -		iowrite32(0x0, &priv->regs->if1_dataa2);
> -		iowrite32(0x0, &priv->regs->if1_datab1);
> -		iowrite32(0x0, &priv->regs->if1_datab2);
> -		iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
> -			  CAN_CMASK_ARB | CAN_CMASK_CTRL,
> -			  &priv->regs->if1_cmask);
> -		pch_can_check_if_busy(&priv->regs->if1_creq, i+1);
> -	}
> -
> -	for (i = i;  i < PCH_OBJ_NUM; i++) {
> -		iowrite32(CAN_CMASK_RX_TX_SET, &priv->regs->if2_cmask);
> -		iowrite32(0xffff, &priv->regs->if2_mask1);
> -		iowrite32(0xffff, &priv->regs->if2_mask2);
> -		iowrite32(0x0, &priv->regs->if2_id1);
> -		iowrite32(0x0, &priv->regs->if2_id2);
> -		iowrite32(0x0, &priv->regs->if2_mcont);
> -		iowrite32(0x0, &priv->regs->if2_dataa1);
> -		iowrite32(0x0, &priv->regs->if2_dataa2);
> -		iowrite32(0x0, &priv->regs->if2_datab1);
> -		iowrite32(0x0, &priv->regs->if2_datab2);
> -		iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
> -			  CAN_CMASK_ARB | CAN_CMASK_CTRL,
> -			  &priv->regs->if2_cmask);
> -		pch_can_check_if_busy(&priv->regs->if2_creq, i+1);
> +	int i; /* Msg Obj ID (1~32) */
> +
> +	for (i = PCH_RX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
> +		iowrite32(PCH_CMASK_RX_TX_SET, &priv->regs->ifregs[0].cmask);
> +		iowrite32(0xffff, &priv->regs->ifregs[0].mask1);
> +		iowrite32(0xffff, &priv->regs->ifregs[0].mask2);
> +		iowrite32(0x0, &priv->regs->ifregs[0].id1);
> +		iowrite32(0x0, &priv->regs->ifregs[0].id2);
> +		iowrite32(0x0, &priv->regs->ifregs[0].mcont);
> +		iowrite32(0x0, &priv->regs->ifregs[0].data[0]);
> +		iowrite32(0x0, &priv->regs->ifregs[0].data[1]);
> +		iowrite32(0x0, &priv->regs->ifregs[0].data[2]);
> +		iowrite32(0x0, &priv->regs->ifregs[0].data[3]);
> +		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
> +			  PCH_CMASK_ARB | PCH_CMASK_CTRL,
> +			  &priv->regs->ifregs[0].cmask);
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
>  	}
>  }
>  
>  static void pch_can_config_rx_tx_buffers(struct pch_can_priv *priv)
>  {
>  	int i;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>  
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_RX) {
> -			iowrite32(CAN_CMASK_RX_TX_GET,
> -				&priv->regs->if1_cmask);
> -			pch_can_check_if_busy(&priv->regs->if1_creq, i+1);
> +	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++) {
> +		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
>  
> -			iowrite32(0x0, &priv->regs->if1_id1);
> -			iowrite32(0x0, &priv->regs->if1_id2);
> +		iowrite32(0x0, &priv->regs->ifregs[0].id1);
> +		iowrite32(0x0, &priv->regs->ifregs[0].id2);
>  
> -			pch_can_bit_set(&priv->regs->if1_mcont,
> -					CAN_IF_MCONT_UMASK);
> +		pch_can_bit_set(&priv->regs->ifregs[0].mcont,
> +				PCH_IF_MCONT_UMASK);
>  
> -			/* Set FIFO mode set to 0 except last Rx Obj*/
> -			pch_can_bit_clear(&priv->regs->if1_mcont,
> -					  CAN_IF_MCONT_EOB);
> -			/* In case FIFO mode, Last EoB of Rx Obj must be 1 */
> -			if (i == (PCH_RX_OBJ_NUM - 1))
> -				pch_can_bit_set(&priv->regs->if1_mcont,
> -						  CAN_IF_MCONT_EOB);
> +		if (i == PCH_RX_OBJ_END)
> +			pch_can_bit_set(&priv->regs->ifregs[0].mcont,
> +					PCH_IF_MCONT_EOB);
> +		else
> +			pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> +					  PCH_IF_MCONT_EOB);
>  
> -			iowrite32(0, &priv->regs->if1_mask1);
> -			pch_can_bit_clear(&priv->regs->if1_mask2,
> -					  0x1fff | CAN_MASK2_MDIR_MXTD);
> +		iowrite32(0, &priv->regs->ifregs[0].mask1);
> +		pch_can_bit_clear(&priv->regs->ifregs[0].mask2,
> +				  0x1fff | PCH_MASK2_MDIR_MXTD);
>  
> -			/* Setting CMASK for writing */
> -			iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
> -				  CAN_CMASK_ARB | CAN_CMASK_CTRL,
> -				  &priv->regs->if1_cmask);
> +		/* Setting CMASK for writing */
> +		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
> +			  PCH_CMASK_CTRL, &priv->regs->ifregs[0].cmask);
>  
> -			pch_can_check_if_busy(&priv->regs->if1_creq, i+1);
> -		} else if (priv->msg_obj[i] == MSG_OBJ_TX) {
> -			iowrite32(CAN_CMASK_RX_TX_GET,
> -				&priv->regs->if2_cmask);
> -			pch_can_check_if_busy(&priv->regs->if2_creq, i+1);
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
> +	}
>  
> -			/* Resetting DIR bit for reception */
> -			iowrite32(0x0, &priv->regs->if2_id1);
> -			iowrite32(0x0, &priv->regs->if2_id2);
> -			pch_can_bit_set(&priv->regs->if2_id2, CAN_ID2_DIR);
> +	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
> +		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[1].cmask);
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
>  
> -			/* Setting EOB bit for transmitter */
> -			iowrite32(CAN_IF_MCONT_EOB, &priv->regs->if2_mcont);
> +		/* Resetting DIR bit for reception */
> +		iowrite32(0x0, &priv->regs->ifregs[1].id1);
> +		iowrite32(PCH_ID2_DIR, &priv->regs->ifregs[1].id2);
>  
> -			pch_can_bit_set(&priv->regs->if2_mcont,
> -					CAN_IF_MCONT_UMASK);
> +		/* Setting EOB bit for transmitter */
> +		iowrite32(PCH_IF_MCONT_EOB | PCH_IF_MCONT_UMASK,
> +			  &priv->regs->ifregs[1].mcont);
>  
> -			iowrite32(0, &priv->regs->if2_mask1);
> -			pch_can_bit_clear(&priv->regs->if2_mask2, 0x1fff);
> +		iowrite32(0, &priv->regs->ifregs[1].mask1);
> +		pch_can_bit_clear(&priv->regs->ifregs[1].mask2, 0x1fff);
>  
> -			/* Setting CMASK for writing */
> -			iowrite32(CAN_CMASK_RDWR | CAN_CMASK_MASK |
> -				  CAN_CMASK_ARB | CAN_CMASK_CTRL,
> -				  &priv->regs->if2_cmask);
> +		/* Setting CMASK for writing */
> +		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
> +			  PCH_CMASK_CTRL, &priv->regs->ifregs[1].cmask);
>  
> -			pch_can_check_if_busy(&priv->regs->if2_creq, i+1);
> -		}
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
>  	}
> -	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
>  }
>  
>  static void pch_can_init(struct pch_can_priv *priv)
> @@ -605,7 +418,7 @@ static void pch_can_init(struct pch_can_priv *priv)
>  	pch_can_set_run_mode(priv, PCH_CAN_STOP);
>  
>  	/* Clearing all the message object buffers. */
> -	pch_can_clear_buffers(priv);
> +	pch_can_clear_if_buffers(priv);
>  
>  	/* Configuring the respective message object as either rx/tx object. */
>  	pch_can_config_rx_tx_buffers(priv);
> @@ -623,57 +436,53 @@ static void pch_can_release(struct pch_can_priv *priv)
>  	pch_can_set_int_enables(priv, PCH_CAN_NONE);
>  
>  	/* Disabling all the receive object. */
> -	pch_can_rx_disable_all(priv);
> +	pch_can_set_rx_all(priv, 0);
>  
>  	/* Disabling all the transmit object. */
> -	pch_can_tx_disable_all(priv);
> +	pch_can_set_tx_all(priv, 0);
>  }
>  
>  /* This function clears interrupt(s) from the CAN device. */
>  static void pch_can_int_clr(struct pch_can_priv *priv, u32 mask)
>  {
> -	if (mask == CAN_STATUS_INT) {
> -		ioread32(&priv->regs->stat);
> -		return;
> -	}
> -
>  	/* Clear interrupt for transmit object */
> -	if (priv->msg_obj[mask - 1] == MSG_OBJ_TX) {
> -		/* Setting CMASK for clearing interrupts for
> -					 frame transmission. */
> -		iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL | CAN_CMASK_ARB,
> -			  &priv->regs->if2_cmask);
> -
> -		/* Resetting the ID registers. */
> -		pch_can_bit_set(&priv->regs->if2_id2,
> -			       CAN_ID2_DIR | (0x7ff << 2));
> -		iowrite32(0x0, &priv->regs->if2_id1);
> -
> -		/* Claring NewDat, TxRqst & IntPnd */
> -		pch_can_bit_clear(&priv->regs->if2_mcont,
> -				  CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND |
> -				  CAN_IF_MCONT_TXRQXT);
> -		pch_can_check_if_busy(&priv->regs->if2_creq, mask);
> -	} else if (priv->msg_obj[mask - 1] == MSG_OBJ_RX) {
> +	if ((mask >= PCH_RX_OBJ_START) && (mask <= PCH_RX_OBJ_END)) {
>  		/* Setting CMASK for clearing the reception interrupts. */
> -		iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL | CAN_CMASK_ARB,
> -			  &priv->regs->if1_cmask);
> +		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL | PCH_CMASK_ARB,
> +			  &priv->regs->ifregs[0].cmask);
>  
>  		/* Clearing the Dir bit. */
> -		pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID2_DIR);
> +		pch_can_bit_clear(&priv->regs->ifregs[0].id2, PCH_ID2_DIR);
>  
>  		/* Clearing NewDat & IntPnd */
> -		pch_can_bit_clear(&priv->regs->if1_mcont,
> -				  CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND);
> +		pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> +				  PCH_IF_MCONT_NEWDAT | PCH_IF_MCONT_INTPND);
>  
> -		pch_can_check_if_busy(&priv->regs->if1_creq, mask);
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, mask);
> +	} else if ((mask >= PCH_TX_OBJ_START) && (mask <= PCH_TX_OBJ_END)) {
> +		/*
> +		 * Setting CMASK for clearing interrupts for frame transmission.
> +		 */
> +		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL | PCH_CMASK_ARB,
> +			  &priv->regs->ifregs[1].cmask);
> +
> +		/* Resetting the ID registers. */
> +		pch_can_bit_set(&priv->regs->ifregs[1].id2,
> +			       PCH_ID2_DIR | (0x7ff << 2));
> +		iowrite32(0x0, &priv->regs->ifregs[1].id1);
> +
> +		/* Claring NewDat, TxRqst & IntPnd */
> +		pch_can_bit_clear(&priv->regs->ifregs[1].mcont,
> +				  PCH_IF_MCONT_NEWDAT | PCH_IF_MCONT_INTPND |
> +				  PCH_IF_MCONT_TXRQXT);
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, mask);
>  	}
>  }
>  
> -static int pch_can_get_buffer_status(struct pch_can_priv *priv)
> +static u32 pch_can_get_buffer_status(struct pch_can_priv *priv)
>  {
>  	return (ioread32(&priv->regs->treq1) & 0xffff) |
> -	       ((ioread32(&priv->regs->treq2) & 0xffff) << 16);
> +	       (ioread32(&priv->regs->treq2) << 16);
>  }
>  
>  static void pch_can_reset(struct pch_can_priv *priv)
> @@ -688,7 +497,7 @@ static void pch_can_error(struct net_device *ndev, u32 status)
>  	struct sk_buff *skb;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  	struct can_frame *cf;
> -	u32 errc;
> +	u32 errc, lec;
>  	struct net_device_stats *stats = &(priv->ndev->stats);
>  	enum can_state state = priv->can.state;
>  
> @@ -697,26 +506,24 @@ static void pch_can_error(struct net_device *ndev, u32 status)
>  		return;
>  
>  	if (status & PCH_BUS_OFF) {
> -		pch_can_tx_disable_all(priv);
> -		pch_can_rx_disable_all(priv);
> +		pch_can_set_tx_all(priv, 0);
> +		pch_can_set_rx_all(priv, 0);
>  		state = CAN_STATE_BUS_OFF;
>  		cf->can_id |= CAN_ERR_BUSOFF;
>  		can_bus_off(ndev);
> -		pch_can_set_run_mode(priv, PCH_CAN_RUN);
> -		dev_err(&ndev->dev, "%s -> Bus Off occurres.\n", __func__);
>  	}
>  
> +	errc = ioread32(&priv->regs->errc);
>  	/* Warning interrupt. */
>  	if (status & PCH_EWARN) {
>  		state = CAN_STATE_ERROR_WARNING;
>  		priv->can.can_stats.error_warning++;
>  		cf->can_id |= CAN_ERR_CRTL;
> -		errc = ioread32(&priv->regs->errc);
> -		if (((errc & CAN_REC) >> 8) > 96)
> +		if (((errc & PCH_REC) >> 8) > 96)
>  			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> -		if ((errc & CAN_TEC) > 96)
> +		if ((errc & PCH_TEC) > 96)
>  			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> -		dev_warn(&ndev->dev,
> +		netdev_dbg(ndev,
>  			"%s -> Error Counter is more than 96.\n", __func__);
>  	}
>  	/* Error passive interrupt. */
> @@ -724,46 +531,52 @@ static void pch_can_error(struct net_device *ndev, u32 status)
>  		priv->can.can_stats.error_passive++;
>  		state = CAN_STATE_ERROR_PASSIVE;
>  		cf->can_id |= CAN_ERR_CRTL;
> -		errc = ioread32(&priv->regs->errc);
> -		if (((errc & CAN_REC) >> 8) > 127)
> +		if (((errc & PCH_REC) >> 8) > 127)
>  			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> -		if ((errc & CAN_TEC) > 127)
> +		if ((errc & PCH_TEC) > 127)
>  			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> -		dev_err(&ndev->dev,
> +		netdev_dbg(ndev,
>  			"%s -> CAN controller is ERROR PASSIVE .\n", __func__);
>  	}
>  
> -	if (status & PCH_LEC_ALL) {
> +	lec = status & PCH_LEC_ALL;
> +	switch (lec) {
> +	case PCH_STUF_ERR:
> +		cf->data[2] |= CAN_ERR_PROT_STUFF;
>  		priv->can.can_stats.bus_error++;
>  		stats->rx_errors++;
> -		switch (status & PCH_LEC_ALL) {
> -		case PCH_STUF_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_STUFF;
> -			break;
> -		case PCH_FORM_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_FORM;
> -			break;
> -		case PCH_ACK_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_LOC_ACK |
> -				       CAN_ERR_PROT_LOC_ACK_DEL;
> -			break;
> -		case PCH_BIT1_ERR:
> -		case PCH_BIT0_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_BIT;
> -			break;
> -		case PCH_CRC_ERR:
> -			cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> -				       CAN_ERR_PROT_LOC_CRC_DEL;
> -			break;
> -		default:
> -			iowrite32(status | PCH_LEC_ALL, &priv->regs->stat);
> -			break;
> -		}
> -
> +		break;
> +	case PCH_FORM_ERR:
> +		cf->data[2] |= CAN_ERR_PROT_FORM;
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +		break;
> +	case PCH_ACK_ERR:
> +		cf->can_id |= CAN_ERR_ACK;
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +		break;
> +	case PCH_BIT1_ERR:
> +	case PCH_BIT0_ERR:
> +		cf->data[2] |= CAN_ERR_PROT_BIT;
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +		break;
> +	case PCH_CRC_ERR:
> +		cf->data[2] |= CAN_ERR_PROT_LOC_CRC_SEQ |
> +			       CAN_ERR_PROT_LOC_CRC_DEL;
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +		break;
> +	case PCH_LEC_ALL: /* Written by CPU. No error status */
> +		break;
>  	}
>  
> +	cf->data[6] = errc & PCH_TEC;
> +	cf->data[7] = (errc & PCH_REC) >> 8;
> +
>  	priv->can.state = state;
> -	netif_rx(skb);
> +	netif_receive_skb(skb);
>  
>  	stats->rx_packets++;
>  	stats->rx_bytes += cf->can_dlc;
> @@ -775,199 +588,207 @@ static irqreturn_t pch_can_interrupt(int irq, void *dev_id)
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  
>  	pch_can_set_int_enables(priv, PCH_CAN_NONE);
> -
>  	napi_schedule(&priv->napi);
>  
>  	return IRQ_HANDLED;
>  }
>  
> -static int pch_can_rx_normal(struct net_device *ndev, u32 int_stat)
> +static void pch_fifo_thresh(struct pch_can_priv *priv, int obj_id)
> +{
> +	if (obj_id < PCH_FIFO_THRESH) {
> +		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL |
> +			  PCH_CMASK_ARB, &priv->regs->ifregs[0].cmask);
> +
> +		/* Clearing the Dir bit. */
> +		pch_can_bit_clear(&priv->regs->ifregs[0].id2, PCH_ID2_DIR);
> +
> +		/* Clearing NewDat & IntPnd */
> +		pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> +				  PCH_IF_MCONT_INTPND);
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, obj_id);
> +	} else if (obj_id > PCH_FIFO_THRESH) {
> +		pch_can_int_clr(priv, obj_id);
> +	} else if (obj_id == PCH_FIFO_THRESH) {
> +		int cnt;
> +		for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
> +			pch_can_int_clr(priv, cnt+1);
> +	}
> +}
> +
> +static int pch_can_rx_msg_lost(struct net_device *ndev, int obj_id)
> +{
> +	struct pch_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &(priv->ndev->stats);
> +	struct sk_buff *skb;
> +	struct can_frame *cf;
> +
> +	netdev_dbg(priv->ndev, "Msg Obj is overwritten.\n");
> +	pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> +			  PCH_IF_MCONT_MSGLOST);
> +	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL,
> +		  &priv->regs->ifregs[0].cmask);
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, obj_id);
> +
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	cf->can_id |= CAN_ERR_CRTL;
> +	cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +	stats->rx_over_errors++;
> +	stats->rx_errors++;
> +
> +	netif_receive_skb(skb);
> +
> +	return 0;
> +}
> +
> +static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int quota)
>  {
>  	u32 reg;
>  	canid_t id;
> -	u32 ide;
> -	u32 rtr;
> -	int i, j, k;
>  	int rcv_pkts = 0;
> +	int rtn;
> +	int next_flag = 0;
>  	struct sk_buff *skb;
>  	struct can_frame *cf;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  	struct net_device_stats *stats = &(priv->ndev->stats);
> +	int i;
> +	u32 id2;
> +	u16 data_reg;
>  
> -	/* Reading the messsage object from the Message RAM */
> -	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
> -	pch_can_check_if_busy(&priv->regs->if1_creq, int_stat);
> +	do {
> +		/* Reading the messsage object from the Message RAM */
> +		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> +		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, obj_num);
>  
> -	/* Reading the MCONT register. */
> -	reg = ioread32(&priv->regs->if1_mcont);
> -	reg &= 0xffff;
> +		/* Reading the MCONT register. */
> +		reg = ioread32(&priv->regs->ifregs[0].mcont);
> +
> +		if (reg & PCH_IF_MCONT_EOB)
> +			break;
>  
> -	for (k = int_stat; !(reg & CAN_IF_MCONT_EOB); k++) {
>  		/* If MsgLost bit set. */
> -		if (reg & CAN_IF_MCONT_MSGLOST) {
> -			dev_err(&priv->ndev->dev, "Msg Obj is overwritten.\n");
> -			pch_can_bit_clear(&priv->regs->if1_mcont,
> -					  CAN_IF_MCONT_MSGLOST);
> -			iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL,
> -				  &priv->regs->if1_cmask);
> -			pch_can_check_if_busy(&priv->regs->if1_creq, k);
> -
> -			skb = alloc_can_err_skb(ndev, &cf);
> +		if (reg & PCH_IF_MCONT_MSGLOST) {
> +			rtn = pch_can_rx_msg_lost(ndev, obj_num);
> +			if (!rtn)
> +				return rtn;

I would return "rcv_pkgs" even in case of an error (or continue
working), because "pch_can_rx_poll" does a "quota -= rcv_pkts;"

> +			rcv_pkts++;
> +			quota--;
> +			next_flag = 1;
> +		} else if (!(reg & PCH_IF_MCONT_NEWDAT))
> +			next_flag = 1;
> +
> +		if (!next_flag) {

Regarding the "next_flag", it's just the "obj_num++;" that needs to be
reorganized, then you can get rid of the next_flag and use just "continue".

> +			skb = alloc_can_skb(priv->ndev, &cf);
>  			if (!skb)
>  				return -ENOMEM;
>  
> -			priv->can.can_stats.error_passive++;
> -			priv->can.state = CAN_STATE_ERROR_PASSIVE;
> -			cf->can_id |= CAN_ERR_CRTL;
> -			cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> -			cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> -			stats->rx_packets++;
> -			stats->rx_bytes += cf->can_dlc;
> +			/* Get Received data */
> +			id2 = ioread32(&priv->regs->ifregs[0].id2);
> +			if (id2 & PCH_ID2_XTD) {
> +				id = (ioread32(&priv->regs->ifregs[0].id1) &
> +					       0xffff);
> +				id |= (((id2) & 0x1fff) << 16);
> +				cf->can_id = id | CAN_EFF_FLAG;
> +			} else {
> +				id = ((id2 & (CAN_SFF_MASK << 2)) >> 2);
> +				cf->can_id = id;
> +			}
> +
> +			if (id2 & PCH_ID2_DIR)
> +				cf->can_id |= CAN_RTR_FLAG;
> +
> +			cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
> +						   ifregs[0].mcont)) & 0xF);
> +

Using the idX value several times looks very nice!

> +			for (i = 0; i < cf->can_dlc; i += 2) {
> +				data_reg = ioread16(&priv->regs->ifregs[0].
> +						    data[i / 2]);
> +				cf->data[i] = data_reg & 0xff;
> +				cf->data[i + 1] = (data_reg >> 8) & 0xff;

The & 0xff in not needed "data" is 8 bit wide.

> +			}
>  
>  			netif_receive_skb(skb);
>  			rcv_pkts++;
> -			goto RX_NEXT;
> -		}
> -		if (!(reg & CAN_IF_MCONT_NEWDAT))
> -			goto RX_NEXT;
> -
> -		skb = alloc_can_skb(priv->ndev, &cf);
> -		if (!skb)
> -			return -ENOMEM;
> -
> -		/* Get Received data */
> -		ide = ((ioread32(&priv->regs->if1_id2)) & CAN_ID2_XTD) >> 14;
> -		if (ide) {
> -			id = (ioread32(&priv->regs->if1_id1) & 0xffff);
> -			id |= (((ioread32(&priv->regs->if1_id2)) &
> -					    0x1fff) << 16);
> -			cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
> -		} else {
> -			id = (((ioread32(&priv->regs->if1_id2)) &
> -					  (CAN_SFF_MASK << 2)) >> 2);
> -			cf->can_id = (id & CAN_SFF_MASK);
> -		}
> +			stats->rx_packets++;
> +			quota--;
> +			stats->rx_bytes += cf->can_dlc;
>  
> -		rtr = (ioread32(&priv->regs->if1_id2) &  CAN_ID2_DIR);
> -		if (rtr) {
> -			cf->can_dlc = 0;
> -			cf->can_id |= CAN_RTR_FLAG;
> -		} else {
> -			cf->can_dlc = ((ioread32(&priv->regs->if1_mcont)) &
> -						   0x0f);
> +			pch_fifo_thresh(priv, obj_num);
>  		}
> +		obj_num++;
> +		next_flag = 0;
> +	} while (quota > 0);
>  
> -		for (i = 0, j = 0; i < cf->can_dlc; j++) {
> -			reg = ioread32(&priv->regs->if1_dataa1 + j*4);
> -			cf->data[i++] = cpu_to_le32(reg & 0xff);
> -			if (i == cf->can_dlc)
> -				break;
> -			cf->data[i++] = cpu_to_le32((reg >> 8) & 0xff);
> -		}
> +	return rcv_pkts;
> +}
>  
> -		netif_receive_skb(skb);
> -		rcv_pkts++;
> -		stats->rx_packets++;
> -		stats->rx_bytes += cf->can_dlc;
> -
> -		if (k < PCH_FIFO_THRESH) {
> -			iowrite32(CAN_CMASK_RDWR | CAN_CMASK_CTRL |
> -				  CAN_CMASK_ARB, &priv->regs->if1_cmask);
> -
> -			/* Clearing the Dir bit. */
> -			pch_can_bit_clear(&priv->regs->if1_id2, CAN_ID2_DIR);
> -
> -			/* Clearing NewDat & IntPnd */
> -			pch_can_bit_clear(&priv->regs->if1_mcont,
> -					  CAN_IF_MCONT_INTPND);
> -			pch_can_check_if_busy(&priv->regs->if1_creq, k);
> -		} else if (k > PCH_FIFO_THRESH) {
> -			pch_can_int_clr(priv, k);
> -		} else if (k == PCH_FIFO_THRESH) {
> -			int cnt;
> -			for (cnt = 0; cnt < PCH_FIFO_THRESH; cnt++)
> -				pch_can_int_clr(priv, cnt+1);
> -		}
> -RX_NEXT:
> -		/* Reading the messsage object from the Message RAM */
> -		iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if1_cmask);
> -		pch_can_check_if_busy(&priv->regs->if1_creq, k + 1);
> -		reg = ioread32(&priv->regs->if1_mcont);
> -	}
> +static void pch_can_tx_complete(struct net_device *ndev, u32 int_stat)
> +{
> +	struct pch_can_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &(priv->ndev->stats);
> +	u32 dlc;
>  
> -	return rcv_pkts;
> +	can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_END - 1);
> +	iowrite32(PCH_CMASK_RX_TX_GET | PCH_CMASK_CLRINTPND,
> +		  &priv->regs->ifregs[1].cmask);
> +	dlc = get_can_dlc(ioread32(&priv->regs->ifregs[1].mcont) &
> +			  PCH_IF_MCONT_DLC);
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, int_stat);
> +	stats->tx_bytes += dlc;
> +	stats->tx_packets++;
> +	if (int_stat == PCH_TX_OBJ_END)
> +		netif_wake_queue(ndev);
>  }
> +
>  static int pch_can_rx_poll(struct napi_struct *napi, int quota)

The function does tx, rx and error handling, better call it napi or just
poll.

>  {
>  	struct net_device *ndev = napi->dev;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
> -	struct net_device_stats *stats = &(priv->ndev->stats);
> -	u32 dlc;
>  	u32 int_stat;
>  	int rcv_pkts = 0;
                    ^^^^

can be removed...if you remove the += below

>  	u32 reg_stat;
> -	unsigned long flags;
>  
>  	int_stat = pch_can_int_pending(priv);
>  	if (!int_stat)
> -		return 0;
> +		goto end;
>  
> -INT_STAT:
> -	if (int_stat == CAN_STATUS_INT) {
> +	if ((int_stat == PCH_STATUS_INT) && (quota > 0)) {
                                             ^^^^^^^^^

quota should be >0 here, it's the first usage of quota in the function.

>  		reg_stat = ioread32(&priv->regs->stat);
>  		if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> -			if ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)
> +			if (reg_stat & PCH_BUS_OFF ||
> +			   (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

can you combine these two ifs?

>  				pch_can_error(ndev, reg_stat);
> +				quota--;
> +			}
>  		}
>  
> -		if (reg_stat & PCH_TX_OK) {
> -			spin_lock_irqsave(&priv->msgif_reg_lock, flags);
> -			iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
> -			pch_can_check_if_busy(&priv->regs->if2_creq,
> -					       ioread32(&priv->regs->intr));
> -			spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> +		if (reg_stat & PCH_TX_OK)
>  			pch_can_bit_clear(&priv->regs->stat, PCH_TX_OK);
> -		}

can the PCH_TX_OK and PCH_TX_OK related pch_can_bit_clear be combined
into a single pch_can_bit_clear?

>  
>  		if (reg_stat & PCH_RX_OK)
>  			pch_can_bit_clear(&priv->regs->stat, PCH_RX_OK);
>  
>  		int_stat = pch_can_int_pending(priv);
> -		if (int_stat == CAN_STATUS_INT)
> -			goto INT_STAT;
>  	}
>  
> -MSG_OBJ:
> -	if ((int_stat >= 1) && (int_stat <= PCH_RX_OBJ_NUM)) {
> -		spin_lock_irqsave(&priv->msgif_reg_lock, flags);
> -		rcv_pkts = pch_can_rx_normal(ndev, int_stat);
> -		spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> -		if (rcv_pkts < 0)
> -			return 0;
> -	} else if ((int_stat > PCH_RX_OBJ_NUM) && (int_stat <= PCH_OBJ_NUM)) {
> -		if (priv->msg_obj[int_stat - 1] == MSG_OBJ_TX) {
> -			/* Handle transmission interrupt */
> -			can_get_echo_skb(ndev, int_stat - PCH_RX_OBJ_NUM - 1);
> -			spin_lock_irqsave(&priv->msgif_reg_lock, flags);
> -			iowrite32(CAN_CMASK_RX_TX_GET | CAN_CMASK_CLRINTPND,
> -				  &priv->regs->if2_cmask);
> -			dlc = ioread32(&priv->regs->if2_mcont) &
> -				       CAN_IF_MCONT_DLC;
> -			pch_can_check_if_busy(&priv->regs->if2_creq, int_stat);
> -			spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> -			if (dlc > 8)
> -				dlc = 8;
> -			stats->tx_bytes += dlc;
> -			stats->tx_packets++;
> -		}
> +	if (quota == 0)
> +		goto end;
> +
> +	if ((int_stat >= PCH_RX_OBJ_START) && (int_stat <= PCH_RX_OBJ_END)) {
> +		rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
...here...               ^^

You cannot return -errno in pch_can_rx_normal, it would mess up quota
calculation.

> +		quota -= rcv_pkts;
> +		if (quota < 0)
> +			goto end;

this should not happen.

> +	} else if ((int_stat >= PCH_TX_OBJ_START) &&
> +		   (int_stat <= PCH_TX_OBJ_END)) {
> +		/* Handle transmission interrupt */
> +		pch_can_tx_complete(ndev, int_stat);
>  	}

Is it either RX or TX interrupt? Would it make sense to loop in the
"rx_poll" function to handle both RX and TX without exiting the NAPI
function?

> -	int_stat = pch_can_int_pending(priv);
> -	if (int_stat == CAN_STATUS_INT)
> -		goto INT_STAT;
> -	else if (int_stat >= 1 && int_stat <= 32)
> -		goto MSG_OBJ;
> -
> +end:
>  	napi_complete(napi);
>  	pch_can_set_int_enables(priv, PCH_CAN_ALL);
>  
> @@ -980,20 +801,18 @@ static int pch_set_bittiming(struct net_device *ndev)
>  	const struct can_bittiming *bt = &priv->can.bittiming;
>  	u32 canbit;
>  	u32 bepe;
> -	u32 brp;
>  
>  	/* Setting the CCE bit for accessing the Can Timing register. */
> -	pch_can_bit_set(&priv->regs->cont, CAN_CTRL_CCE);
> -
> -	brp = (bt->tq) / (1000000000/PCH_CAN_CLK) - 1;
> -	canbit = brp & MSK_BITT_BRP;
> -	canbit |= (bt->sjw - 1) << BIT_BITT_SJW;
> -	canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << BIT_BITT_TSEG1;
> -	canbit |= (bt->phase_seg2 - 1) << BIT_BITT_TSEG2;
> -	bepe = (brp & MSK_BRPE_BRPE) >> BIT_BRPE_BRPE;
> +	pch_can_bit_set(&priv->regs->cont, PCH_CTRL_CCE);
> +
> +	canbit = (bt->brp - 1) & PCH_MSK_BITT_BRP;
> +	canbit |= (bt->sjw - 1) << PCH_BIT_SJW_SHIFT;
> +	canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << PCH_BIT_TSEG1_SHIFT;
> +	canbit |= (bt->phase_seg2 - 1) << PCH_BIT_TSEG2_SHIFT;
> +	bepe = ((bt->brp - 1) & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
>  	iowrite32(canbit, &priv->regs->bitt);
>  	iowrite32(bepe, &priv->regs->brpe);
> -	pch_can_bit_clear(&priv->regs->cont, CAN_CTRL_CCE);
> +	pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_CCE);
>  
>  	return 0;
>  }
> @@ -1008,8 +827,8 @@ static void pch_can_start(struct net_device *ndev)
>  	pch_set_bittiming(ndev);
>  	pch_can_set_optmode(priv);
>  
> -	pch_can_tx_enable_all(priv);
> -	pch_can_rx_enable_all(priv);
> +	pch_can_set_tx_all(priv, 1);
> +	pch_can_set_rx_all(priv, 1);
>  
>  	/* Setting the CAN to run mode. */
>  	pch_can_set_run_mode(priv, PCH_CAN_RUN);
> @@ -1041,27 +860,18 @@ static int pch_can_open(struct net_device *ndev)
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  	int retval;
>  
> -	retval = pci_enable_msi(priv->dev);
> -	if (retval) {
> -		dev_info(&ndev->dev, "PCH CAN opened without MSI\n");
> -		priv->use_msi = 0;
> -	} else {
> -		dev_info(&ndev->dev, "PCH CAN opened with MSI\n");
> -		priv->use_msi = 1;
> -	}
> -
> -	/* Regsitering the interrupt. */
> +	/* Regstering the interrupt. */
>  	retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,

Does this driver really support SHARED interrupts? The pch_can_interrupt
always return IRQ_HANDLED.

>  			     ndev->name, ndev);
>  	if (retval) {
> -		dev_err(&ndev->dev, "request_irq failed.\n");
> +		netdev_err(ndev, "request_irq failed.\n");
>  		goto req_irq_err;
>  	}
>  
>  	/* Open common can device */
>  	retval = open_candev(ndev);
>  	if (retval) {
> -		dev_err(ndev->dev.parent, "open_candev() failed %d\n", retval);
> +		netdev_err(ndev, "open_candev() failed %d\n", retval);
>  		goto err_open_candev;
>  	}
>  
> @@ -1075,9 +885,6 @@ static int pch_can_open(struct net_device *ndev)
>  err_open_candev:
>  	free_irq(priv->dev->irq, ndev);
>  req_irq_err:
> -	if (priv->use_msi)
> -		pci_disable_msi(priv->dev);
> -
>  	pch_can_release(priv);
>  
>  	return retval;
> @@ -1091,102 +898,68 @@ static int pch_close(struct net_device *ndev)
>  	napi_disable(&priv->napi);
>  	pch_can_release(priv);
>  	free_irq(priv->dev->irq, ndev);
> -	if (priv->use_msi)
> -		pci_disable_msi(priv->dev);
>  	close_candev(ndev);
>  	priv->can.state = CAN_STATE_STOPPED;
>  	return 0;
>  }
>  
> -static int pch_get_msg_obj_sts(struct net_device *ndev, u32 obj_id)
> -{
> -	u32 buffer_status = 0;
> -	struct pch_can_priv *priv = netdev_priv(ndev);
> -
> -	/* Getting the message object status. */
> -	buffer_status = (u32) pch_can_get_buffer_status(priv);
> -
> -	return buffer_status & obj_id;
> -}
> -
> -
>  static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
>  {
> -	int i, j;
> -	unsigned long flags;
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  	struct can_frame *cf = (struct can_frame *)skb->data;
> -	int tx_buffer_avail = 0;
> +	int tx_obj_no = 0;
                     ^^^^

There should be no need to initialize it with 0.

> +	int i;
> +	u32 id2;
>  
>  	if (can_dropped_invalid_skb(ndev, skb))
>  		return NETDEV_TX_OK;
>  
> -	if (priv->tx_obj == (PCH_OBJ_NUM + 1)) { /* Point tail Obj */
> -		while (pch_get_msg_obj_sts(ndev, (((1 << PCH_TX_OBJ_NUM)-1) <<
> -					   PCH_RX_OBJ_NUM)))
> -			udelay(500);
> +	if (priv->tx_obj == PCH_TX_OBJ_END) {

> +		if (ioread32(&priv->regs->treq2) & 0xfc00)
                                                   ^^^^^^
Please add a oneliner comment what this if checks, can you introduce a
define for this value?

> +			netif_stop_queue(ndev);
>  
> -		priv->tx_obj = PCH_RX_OBJ_NUM + 1; /* Point head of Tx Obj ID */
> -		tx_buffer_avail = priv->tx_obj; /* Point Tail of Tx Obj */
> +		tx_obj_no = priv->tx_obj;
                ^^^^^^^^^^^^^^^^^^^^^^^^^
> +		priv->tx_obj = PCH_TX_OBJ_START;
>  	} else {
> -		tx_buffer_avail = priv->tx_obj;
> +		tx_obj_no = priv->tx_obj;
                ^^^^^^^^^^^^^^^^^^^^^^^^^

can you move this before the if()?

> +		priv->tx_obj++;
>  	}
> -	priv->tx_obj++;
> -
> -	/* Attaining the lock. */
> -	spin_lock_irqsave(&priv->msgif_reg_lock, flags);
>  
> -	/* Reading the Msg Obj from the Msg RAM to the Interface register. */
> -	iowrite32(CAN_CMASK_RX_TX_GET, &priv->regs->if2_cmask);
> -	pch_can_check_if_busy(&priv->regs->if2_creq, tx_buffer_avail);
> -
> -	/* Setting the CMASK register. */
> -	pch_can_bit_set(&priv->regs->if2_cmask, CAN_CMASK_ALL);
> +	/* Setting the CMASK register to set value */
> +	iowrite32(PCH_CMASK_RX_TX_SET, &priv->regs->ifregs[1].cmask);
>  
>  	/* If ID extended is set. */
> -	pch_can_bit_clear(&priv->regs->if2_id1, 0xffff);
> -	pch_can_bit_clear(&priv->regs->if2_id2, 0x1fff | CAN_ID2_XTD);
>  	if (cf->can_id & CAN_EFF_FLAG) {
> -		pch_can_bit_set(&priv->regs->if2_id1, cf->can_id & 0xffff);
> -		pch_can_bit_set(&priv->regs->if2_id2,
> -				((cf->can_id >> 16) & 0x1fff) | CAN_ID2_XTD);
> +		iowrite32(cf->can_id & 0xffff, &priv->regs->ifregs[1].id1);
> +		id2 = ((cf->can_id >> 16) & 0x1fff) | PCH_ID2_XTD;
>  	} else {
> -		pch_can_bit_set(&priv->regs->if2_id1, 0);
> -		pch_can_bit_set(&priv->regs->if2_id2,
> -				(cf->can_id & CAN_SFF_MASK) << 2);
> +		iowrite32(0, &priv->regs->ifregs[1].id1);
> +		id2 = (cf->can_id & CAN_SFF_MASK) << 2;
>  	}
>  
> +	id2 |= PCH_ID_MSGVAL;
> +
>  	/* If remote frame has to be transmitted.. */
>  	if (cf->can_id & CAN_RTR_FLAG)
> -		pch_can_bit_clear(&priv->regs->if2_id2, CAN_ID2_DIR);
> -
> -	for (i = 0, j = 0; i < cf->can_dlc; j++) {
> -		iowrite32(le32_to_cpu(cf->data[i++]),
> -			 (&priv->regs->if2_dataa1) + j*4);
> -		if (i == cf->can_dlc)
> -			break;
> -		iowrite32(le32_to_cpu(cf->data[i++] << 8),
> -			 (&priv->regs->if2_dataa1) + j*4);
> -	}
> -
> -	can_put_echo_skb(skb, ndev, tx_buffer_avail - PCH_RX_OBJ_NUM - 1);
> +		id2 &= ~PCH_ID2_DIR;

As far as I can see, there's no need to clear PCH_ID2_DIR.

> +	else
> +		id2 |= PCH_ID2_DIR;
>  
> -	/* Updating the size of the data. */
> -	pch_can_bit_clear(&priv->regs->if2_mcont, 0x0f);
> -	pch_can_bit_set(&priv->regs->if2_mcont, cf->can_dlc);
> +	iowrite32(id2, &priv->regs->ifregs[1].id2);
>  
> -	/* Clearing IntPend, NewDat & TxRqst */
> -	pch_can_bit_clear(&priv->regs->if2_mcont,
> -			  CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_INTPND |
> -			  CAN_IF_MCONT_TXRQXT);
> +	/* Copy data to register */
> +	for (i = 0; i < cf->can_dlc; i += 2) {
> +		iowrite16(cf->data[i] | (cf->data[i + 1] << 8),
> +			  &priv->regs->ifregs[1].data[i / 2]);
> +	}
>  
> -	/* Setting NewDat, TxRqst bits */
> -	pch_can_bit_set(&priv->regs->if2_mcont,
> -			CAN_IF_MCONT_NEWDAT | CAN_IF_MCONT_TXRQXT);
> +	can_put_echo_skb(skb, ndev, tx_obj_no - PCH_RX_OBJ_END - 1);
>  
> -	pch_can_check_if_busy(&priv->regs->if2_creq, tx_buffer_avail);
> +	/* Set the size of the data. Update if2_mcont*/
> +	iowrite32(cf->can_dlc | PCH_IF_MCONT_NEWDAT | PCH_IF_MCONT_TXRQXT |
> +		  PCH_IF_MCONT_TXIE, &priv->regs->ifregs[1].mcont);
>  
> -	spin_unlock_irqrestore(&priv->msgif_reg_lock, flags);
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, tx_obj_no);

Still we have the busy waiting in the TX path. Maybe you can move the
waiting before accessing the if[1] and remove the busy waiting here.

>  
>  	return NETDEV_TX_OK;
>  }
> @@ -1203,21 +976,90 @@ static void __devexit pch_can_remove(struct pci_dev *pdev)
>  	struct pch_can_priv *priv = netdev_priv(ndev);
>  
>  	unregister_candev(priv->ndev);
> -	free_candev(priv->ndev);
>  	pci_iounmap(pdev, priv->regs);
> +	if (priv->use_msi)
> +		pci_disable_msi(priv->dev);
>  	pci_release_regions(pdev);
>  	pci_disable_device(pdev);
>  	pci_set_drvdata(pdev, NULL);
>  	pch_can_reset(priv);
> +	free_candev(priv->ndev);
>  }
>  
>  #ifdef CONFIG_PM
> +static void pch_can_set_int_custom(struct pch_can_priv *priv)
> +{
> +	/* Clearing the IE, SIE and EIE bits of Can control register. */
> +	pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_IE_SIE_EIE);
> +
> +	/* Appropriately setting them. */
> +	pch_can_bit_set(&priv->regs->cont,
> +			((priv->int_enables & PCH_MSK_CTRL_IE_SIE_EIE) << 1));
> +}
> +
> +/* This function retrieves interrupt enabled for the CAN device. */
> +static u32 pch_can_get_int_enables(struct pch_can_priv *priv)
> +{
> +	/* Obtaining the status of IE, SIE and EIE interrupt bits. */
> +	return (ioread32(&priv->regs->cont) & PCH_CTRL_IE_SIE_EIE) >> 1;
> +}
> +
> +static u32 pch_can_get_rxtx_ir(struct pch_can_priv *priv, u32 buff_num, u32 dir)
> +{
> +	u32 ie, enable;
> +
> +	if (dir)
> +		ie = PCH_IF_MCONT_RXIE;
> +	else
> +		ie = PCH_IF_MCONT_TXIE;
> +
> +	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
> +
> +	if (((ioread32(&priv->regs->ifregs[dir].id2)) & PCH_ID_MSGVAL) &&
> +			((ioread32(&priv->regs->ifregs[dir].mcont)) & ie))
> +		enable = 1;
> +	else
> +		enable = 0;
> +	return enable;
> +}
> +
> +static void pch_can_set_rx_buffer_link(struct pch_can_priv *priv,
> +				       u32 buffer_num, u32 set)
> +{
> +	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, buffer_num);
> +	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL,
> +		  &priv->regs->ifregs[0].cmask);
> +	if (set)
> +		pch_can_bit_clear(&priv->regs->ifregs[0].mcont,
> +				  PCH_IF_MCONT_EOB);
> +	else
> +		pch_can_bit_set(&priv->regs->ifregs[0].mcont, PCH_IF_MCONT_EOB);
> +
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, buffer_num);
> +}
> +
> +static u32 pch_can_get_rx_buffer_link(struct pch_can_priv *priv, u32 buffer_num)
> +{
> +	u32 link;
> +
> +	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
> +	pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, buffer_num);
> +
> +	if (ioread32(&priv->regs->ifregs[0].mcont) & PCH_IF_MCONT_EOB)
> +		link = 0;
> +	else
> +		link = 1;
> +	return link;
> +}
> +
>  static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
>  {
> -	int i;			/* Counter variable. */
> -	int retval;		/* Return value. */
> +	int i;
> +	int retval;
>  	u32 buf_stat;	/* Variable for reading the transmit buffer status. */
> -	u32 counter = 0xFFFFFF;
> +	u32 counter = PCH_COUNTER_LIMIT;
>  
>  	struct net_device *dev = pci_get_drvdata(pdev);
>  	struct pch_can_priv *priv = netdev_priv(dev);
> @@ -1226,7 +1068,7 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
>  	pch_can_set_run_mode(priv, PCH_CAN_STOP);
>  
>  	/* Indicate that we are aboutto/in suspend */
> -	priv->can.state = CAN_STATE_SLEEPING;
> +	priv->can.state = CAN_STATE_STOPPED;
>  
>  	/* Waiting for all transmission to complete. */
>  	while (counter) {
> @@ -1240,31 +1082,24 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
>  		dev_err(&pdev->dev, "%s -> Transmission time out.\n", __func__);
>  
>  	/* Save interrupt configuration and then disable them */
> -	pch_can_get_int_enables(priv, &(priv->int_enables));
> +	priv->int_enables = pch_can_get_int_enables(priv);
>  	pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
>  
>  	/* Save Tx buffer enable state */
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_TX)
> -			pch_can_get_tx_enable(priv, i + 1,
> -					      &(priv->tx_enable[i]));
> -	}
> +	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
> +		priv->tx_enable[i] = pch_can_get_rxtx_ir(priv, i, PCH_TX_IFREG);
>  
>  	/* Disable all Transmit buffers */
> -	pch_can_tx_disable_all(priv);
> +	pch_can_set_tx_all(priv, 0);
>  
>  	/* Save Rx buffer enable state */
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_RX) {
> -			pch_can_get_rx_enable(priv, i + 1,
> -						&(priv->rx_enable[i]));
> -			pch_can_get_rx_buffer_link(priv, i + 1,
> -						&(priv->rx_link[i]));
> -		}
> +	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++) {
> +		priv->rx_enable[i] = pch_can_get_rxtx_ir(priv, i, PCH_RX_IFREG);
> +		priv->rx_link[i] = pch_can_get_rx_buffer_link(priv, i);
>  	}
>  
>  	/* Disable all Receive buffers */
> -	pch_can_rx_disable_all(priv);
> +	pch_can_set_rx_all(priv, 0);
>  	retval = pci_save_state(pdev);
>  	if (retval) {
>  		dev_err(&pdev->dev, "pci_save_state failed.\n");
> @@ -1279,8 +1114,8 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
>  
>  static int pch_can_resume(struct pci_dev *pdev)
>  {
> -	int i;			/* Counter variable. */
> -	int retval;		/* Return variable. */
> +	int i;
> +	int retval;
>  	struct net_device *dev = pci_get_drvdata(pdev);
>  	struct pch_can_priv *priv = netdev_priv(dev);
>  
> @@ -1312,23 +1147,16 @@ static int pch_can_resume(struct pci_dev *pdev)
>  	pch_can_set_optmode(priv);
>  
>  	/* Enabling the transmit buffer. */
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_TX) {
> -			pch_can_set_tx_enable(priv, i + 1,
> -					      priv->tx_enable[i]);
> -		}
> -	}
> +	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++)
> +		pch_can_set_rxtx(priv, i, priv->tx_enable[i], PCH_TX_IFREG);
>  
>  	/* Configuring the receive buffer and enabling them. */
> -	for (i = 0; i < PCH_OBJ_NUM; i++) {
> -		if (priv->msg_obj[i] == MSG_OBJ_RX) {
> -			/* Restore buffer link */
> -			pch_can_set_rx_buffer_link(priv, i + 1,
> -						   priv->rx_link[i]);
> -
> -			/* Restore buffer enables */
> -			pch_can_set_rx_enable(priv, i + 1, priv->rx_enable[i]);
> -		}
> +	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
> +		/* Restore buffer link */
> +		pch_can_set_rx_buffer_link(priv, i, priv->rx_link[i]);
> +
> +		/* Restore buffer enables */
> +		pch_can_set_rxtx(priv, i, priv->rx_enable[i], PCH_RX_IFREG);
>  	}
>  
>  	/* Enable CAN Interrupts */
> @@ -1349,8 +1177,8 @@ static int pch_can_get_berr_counter(const struct net_device *dev,
>  {
>  	struct pch_can_priv *priv = netdev_priv(dev);
>  
> -	bec->txerr = ioread32(&priv->regs->errc) & CAN_TEC;
> -	bec->rxerr = (ioread32(&priv->regs->errc) & CAN_REC) >> 8;
> +	bec->txerr = ioread32(&priv->regs->errc) & PCH_TEC;
> +	bec->rxerr = (ioread32(&priv->regs->errc) & PCH_REC) >> 8;

try to minimize IO, only read errc once.

>  
>  	return 0;
>  }
> @@ -1361,7 +1189,6 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
>  	struct net_device *ndev;
>  	struct pch_can_priv *priv;
>  	int rc;
> -	int index;
>  	void __iomem *addr;
>  
>  	rc = pci_enable_device(pdev);
> @@ -1383,7 +1210,7 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
>  		goto probe_exit_ipmap;
>  	}
>  
> -	ndev = alloc_candev(sizeof(struct pch_can_priv), PCH_TX_OBJ_NUM);
> +	ndev = alloc_candev(sizeof(struct pch_can_priv), PCH_TX_OBJ_END);
>  	if (!ndev) {
>  		rc = -ENOMEM;
>  		dev_err(&pdev->dev, "Failed alloc_candev\n");
> @@ -1399,7 +1226,7 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
>  	priv->can.do_get_berr_counter = pch_can_get_berr_counter;
>  	priv->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
>  				       CAN_CTRLMODE_LOOPBACK;
> -	priv->tx_obj = PCH_RX_OBJ_NUM + 1; /* Point head of Tx Obj */
> +	priv->tx_obj = PCH_TX_OBJ_START; /* Point head of Tx Obj */
>  
>  	ndev->irq = pdev->irq;
>  	ndev->flags |= IFF_ECHO;
> @@ -1407,15 +1234,18 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
>  	pci_set_drvdata(pdev, ndev);
>  	SET_NETDEV_DEV(ndev, &pdev->dev);
>  	ndev->netdev_ops = &pch_can_netdev_ops;
> -
>  	priv->can.clock.freq = PCH_CAN_CLK; /* Hz */
> -	for (index = 0; index < PCH_RX_OBJ_NUM;)
> -		priv->msg_obj[index++] = MSG_OBJ_RX;
>  
> -	for (index = index;  index < PCH_OBJ_NUM;)
> -		priv->msg_obj[index++] = MSG_OBJ_TX;
> +	netif_napi_add(ndev, &priv->napi, pch_can_rx_poll, PCH_RX_OBJ_END);
>  
> -	netif_napi_add(ndev, &priv->napi, pch_can_rx_poll, PCH_RX_OBJ_NUM);
> +	rc = pci_enable_msi(priv->dev);
> +	if (rc) {
> +		dev_info(&ndev->dev, "PCH CAN opened without MSI\n");
> +		priv->use_msi = 0;
> +	} else {
> +		dev_info(&ndev->dev, "PCH CAN opened with MSI\n");
> +		priv->use_msi = 1;
> +	}
>  
>  	rc = register_candev(ndev);
>  	if (rc) {
> @@ -1426,7 +1256,8 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
>  	return 0;
>  
>  probe_exit_reg_candev:
> -	free_candev(ndev);
> +	if (priv->use_msi)
> +		pci_disable_msi(priv->dev);
>  probe_exit_alloc_candev:
>  	pci_iounmap(pdev, addr);
>  probe_exit_ipmap:
> @@ -1458,6 +1289,6 @@ static void __exit pch_can_pci_exit(void)
>  }
>  module_exit(pch_can_pci_exit);
>  
> -MODULE_DESCRIPTION("Controller Area Network Driver");
> +MODULE_DESCRIPTION("Intel EG20T PCH CAN(Controller Area Network) Driver");
>  MODULE_LICENSE("GPL v2");
>  MODULE_VERSION("0.94");

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: 262 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

* Re: [PATCH net-next-2.6] bridge: add __rcu annotations
From: Simon Horman @ 2010-11-16 12:59 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Stephen Hemminger, David Miller, netdev
In-Reply-To: <1289685861.2743.38.camel@edumazet-laptop>

On Sat, Nov 13, 2010 at 11:04:21PM +0100, Eric Dumazet wrote:
> Le samedi 13 novembre 2010 à 10:13 -0800, Stephen Hemminger a écrit :
> > On Sat, 13 Nov 2010 18:58:50 +0100
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > 
> > > Le samedi 13 novembre 2010 à 09:35 -0800, Stephen Hemminger a écrit :
> > > > On Sat, 13 Nov 2010 09:15:28 +0100
> > > > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > > 
> > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > > > index 578debb..ffbd177 100644
> > > > > --- a/include/linux/netdevice.h
> > > > > +++ b/include/linux/netdevice.h
> > > > > @@ -996,7 +996,10 @@ struct net_device {
> > > > >  #endif
> > > > >  
> > > > >  	rx_handler_func_t	*rx_handler;
> > > > > -	void			*rx_handler_data;
> > > > > +	union {
> > > > > +		void				*rx_handler_data;
> > > > > +		struct net_bridge_port __rcu	*br_port_rcu;
> > > > > +	};
> > > > >  
> > > > >  	struct netdev_queue __rcu *ingress_queue;
> > > > 
> > > > I don't like making the generic hook typed again.
> > > > We don't do this for other callbacks, timers, workqueues, ...
> > > > Why is it necessary for RCU notation.
> > > > 
> > > 
> > > because rcu_dereference() needs the type for __CHECKER__/sparse checks
> > > 
> > > #define __rcu_dereference_check(p, c, space) \
> > >         ({ \
> > >                 typeof(*p) *_________p1 = (typeof(*p)*__force )ACCESS_ONCE(p); \
> > >                 rcu_lockdep_assert(c); \
> > >                 rcu_dereference_sparse(p, space); \
> > >                 smp_read_barrier_depends(); \
> > >                 ((typeof(*p) __force __kernel *)(_________p1)); \
> > >         })
> > > 
> > > So using a "void *ptr" is not an option
> > > 
> > > Its also cleaner to use
> > > 
> > > rcu_dereference(dev->br_port_rcu)
> > > 
> > > instead of 
> > > 
> > > (struct net_bridge_port *)rcu_dereference(dev->rx_handler_data)
> > 
> > There must be a better way. What about use of that hook by macvlan and openvswitch?
> 
> macvlan and openvswitch (is it part of linux yet ???)

No, openvswitch's datapath hasn't been submitted to netdev yet.

[snip]

^ permalink raw reply

* [PATCH] filter: Optimize instruction revalidation code.
From: Tetsuo Handa @ 2010-11-16 13:08 UTC (permalink / raw)
  To: davem, eric.dumazet; +Cc: drosenberg, netdev
In-Reply-To: <20101110.103807.39173013.davem@davemloft.net>

In the wake of commit 57fe93b3 "filter: make sure filters dont read
uninitialized memory", I came up with this patch. I don't know how to test.
Please review and do tests.

This patch optimizes sk_chk_filter(). Using gcc 4.1.2 on x86_32, the size of
net/core/filter.o changed from 7416 bytes to 5260 bytes
("make allnoconfig" + CONFIG_NET=y).

By the way, if "struct sock_filter *filter" argument of sk_run_filter() does
not change after it was revalidated at sk_chk_filter(), can't we detect and
reject "BPF_S_LD_MEM/BPF_S_LDX_MEM before BPF_S_ST/BPF_S_STX" at
sk_chk_filter()?
----------------------------------------
 From 2bf36130bd4912590b409b47a6a9a82e2884e035 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 16 Nov 2010 21:39:50 +0900
Subject: [PATCH] filter: Optimize instruction revalidation code.

Since repeating small value to small value conversion using switch() clause's
case statement is wasteful, this patch instroduces u16 to u16 mapping table
and removes most of case statements. As a result, the size of net/core/filter.o
is reduced by about 30% on x86.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 net/core/filter.c |  214 ++++++++++++++++-------------------------------------
 1 files changed, 65 insertions(+), 149 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 23e9b2a..85be3d8 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -383,7 +383,57 @@ EXPORT_SYMBOL(sk_run_filter);
  */
 int sk_chk_filter(struct sock_filter *filter, int flen)
 {
-	struct sock_filter *ftest;
+	/*
+	 * Valid instructions are initialized to non-0.
+	 * Invalid instructions are initialized to 0.
+	 */
+	static u16 codes[] = {
+		[BPF_ALU|BPF_ADD|BPF_K]  = BPF_S_ALU_ADD_K + 1,
+		[BPF_ALU|BPF_ADD|BPF_X]  = BPF_S_ALU_ADD_X + 1,
+		[BPF_ALU|BPF_SUB|BPF_K]  = BPF_S_ALU_SUB_K + 1,
+		[BPF_ALU|BPF_SUB|BPF_X]  = BPF_S_ALU_SUB_X + 1,
+		[BPF_ALU|BPF_MUL|BPF_K]  = BPF_S_ALU_MUL_K + 1,
+		[BPF_ALU|BPF_MUL|BPF_X]  = BPF_S_ALU_MUL_X + 1,
+		[BPF_ALU|BPF_DIV|BPF_X]  = BPF_S_ALU_DIV_X + 1,
+		[BPF_ALU|BPF_AND|BPF_K]  = BPF_S_ALU_AND_K + 1,
+		[BPF_ALU|BPF_AND|BPF_X]  = BPF_S_ALU_AND_X + 1,
+		[BPF_ALU|BPF_OR|BPF_K]   = BPF_S_ALU_OR_K + 1,
+		[BPF_ALU|BPF_OR|BPF_X]   = BPF_S_ALU_OR_X + 1,
+		[BPF_ALU|BPF_LSH|BPF_K]  = BPF_S_ALU_LSH_K + 1,
+		[BPF_ALU|BPF_LSH|BPF_X]  = BPF_S_ALU_LSH_X + 1,
+		[BPF_ALU|BPF_RSH|BPF_K]  = BPF_S_ALU_RSH_K + 1,
+		[BPF_ALU|BPF_RSH|BPF_X]  = BPF_S_ALU_RSH_X + 1,
+		[BPF_ALU|BPF_NEG]        = BPF_S_ALU_NEG + 1,
+		[BPF_LD|BPF_W|BPF_ABS]   = BPF_S_LD_W_ABS + 1,
+		[BPF_LD|BPF_H|BPF_ABS]   = BPF_S_LD_H_ABS + 1,
+		[BPF_LD|BPF_B|BPF_ABS]   = BPF_S_LD_B_ABS + 1,
+		[BPF_LD|BPF_W|BPF_LEN]   = BPF_S_LD_W_LEN + 1,
+		[BPF_LD|BPF_W|BPF_IND]   = BPF_S_LD_W_IND + 1,
+		[BPF_LD|BPF_H|BPF_IND]   = BPF_S_LD_H_IND + 1,
+		[BPF_LD|BPF_B|BPF_IND]   = BPF_S_LD_B_IND + 1,
+		[BPF_LD|BPF_IMM]         = BPF_S_LD_IMM + 1,
+		[BPF_LDX|BPF_W|BPF_LEN]  = BPF_S_LDX_W_LEN + 1,
+		[BPF_LDX|BPF_B|BPF_MSH]  = BPF_S_LDX_B_MSH + 1,
+		[BPF_LDX|BPF_IMM]        = BPF_S_LDX_IMM + 1,
+		[BPF_MISC|BPF_TAX]       = BPF_S_MISC_TAX + 1,
+		[BPF_MISC|BPF_TXA]       = BPF_S_MISC_TXA + 1,
+		[BPF_RET|BPF_K]          = BPF_S_RET_K + 1,
+		[BPF_RET|BPF_A]          = BPF_S_RET_A + 1,
+		[BPF_ALU|BPF_DIV|BPF_K]  = BPF_S_ALU_DIV_K + 1,
+		[BPF_LD|BPF_MEM]         = BPF_S_LD_MEM + 1,
+		[BPF_LDX|BPF_MEM]        = BPF_S_LDX_MEM + 1,
+		[BPF_ST]                 = BPF_S_ST + 1,
+		[BPF_STX]                = BPF_S_STX + 1,
+		[BPF_JMP|BPF_JA]         = BPF_S_JMP_JA + 1,
+		[BPF_JMP|BPF_JEQ|BPF_K]  = BPF_S_JMP_JEQ_K + 1,
+		[BPF_JMP|BPF_JEQ|BPF_X]  = BPF_S_JMP_JEQ_X + 1,
+		[BPF_JMP|BPF_JGE|BPF_K]  = BPF_S_JMP_JGE_K + 1,
+		[BPF_JMP|BPF_JGE|BPF_X]  = BPF_S_JMP_JGE_X + 1,
+		[BPF_JMP|BPF_JGT|BPF_K]  = BPF_S_JMP_JGT_K + 1,
+		[BPF_JMP|BPF_JGT|BPF_X]  = BPF_S_JMP_JGT_X + 1,
+		[BPF_JMP|BPF_JSET|BPF_K] = BPF_S_JMP_JSET_K + 1,
+		[BPF_JMP|BPF_JSET|BPF_X] = BPF_S_JMP_JSET_X + 1,
+	};
 	int pc;
 
 	if (flen == 0 || flen > BPF_MAXINSNS)
@@ -391,135 +441,26 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
 
 	/* check the filter code now */
 	for (pc = 0; pc < flen; pc++) {
-		ftest = &filter[pc];
-
-		/* Only allow valid instructions */
-		switch (ftest->code) {
-		case BPF_ALU|BPF_ADD|BPF_K:
-			ftest->code = BPF_S_ALU_ADD_K;
-			break;
-		case BPF_ALU|BPF_ADD|BPF_X:
-			ftest->code = BPF_S_ALU_ADD_X;
-			break;
-		case BPF_ALU|BPF_SUB|BPF_K:
-			ftest->code = BPF_S_ALU_SUB_K;
-			break;
-		case BPF_ALU|BPF_SUB|BPF_X:
-			ftest->code = BPF_S_ALU_SUB_X;
-			break;
-		case BPF_ALU|BPF_MUL|BPF_K:
-			ftest->code = BPF_S_ALU_MUL_K;
-			break;
-		case BPF_ALU|BPF_MUL|BPF_X:
-			ftest->code = BPF_S_ALU_MUL_X;
-			break;
-		case BPF_ALU|BPF_DIV|BPF_X:
-			ftest->code = BPF_S_ALU_DIV_X;
-			break;
-		case BPF_ALU|BPF_AND|BPF_K:
-			ftest->code = BPF_S_ALU_AND_K;
-			break;
-		case BPF_ALU|BPF_AND|BPF_X:
-			ftest->code = BPF_S_ALU_AND_X;
-			break;
-		case BPF_ALU|BPF_OR|BPF_K:
-			ftest->code = BPF_S_ALU_OR_K;
-			break;
-		case BPF_ALU|BPF_OR|BPF_X:
-			ftest->code = BPF_S_ALU_OR_X;
-			break;
-		case BPF_ALU|BPF_LSH|BPF_K:
-			ftest->code = BPF_S_ALU_LSH_K;
-			break;
-		case BPF_ALU|BPF_LSH|BPF_X:
-			ftest->code = BPF_S_ALU_LSH_X;
-			break;
-		case BPF_ALU|BPF_RSH|BPF_K:
-			ftest->code = BPF_S_ALU_RSH_K;
-			break;
-		case BPF_ALU|BPF_RSH|BPF_X:
-			ftest->code = BPF_S_ALU_RSH_X;
-			break;
-		case BPF_ALU|BPF_NEG:
-			ftest->code = BPF_S_ALU_NEG;
-			break;
-		case BPF_LD|BPF_W|BPF_ABS:
-			ftest->code = BPF_S_LD_W_ABS;
-			break;
-		case BPF_LD|BPF_H|BPF_ABS:
-			ftest->code = BPF_S_LD_H_ABS;
-			break;
-		case BPF_LD|BPF_B|BPF_ABS:
-			ftest->code = BPF_S_LD_B_ABS;
-			break;
-		case BPF_LD|BPF_W|BPF_LEN:
-			ftest->code = BPF_S_LD_W_LEN;
-			break;
-		case BPF_LD|BPF_W|BPF_IND:
-			ftest->code = BPF_S_LD_W_IND;
-			break;
-		case BPF_LD|BPF_H|BPF_IND:
-			ftest->code = BPF_S_LD_H_IND;
-			break;
-		case BPF_LD|BPF_B|BPF_IND:
-			ftest->code = BPF_S_LD_B_IND;
-			break;
-		case BPF_LD|BPF_IMM:
-			ftest->code = BPF_S_LD_IMM;
-			break;
-		case BPF_LDX|BPF_W|BPF_LEN:
-			ftest->code = BPF_S_LDX_W_LEN;
-			break;
-		case BPF_LDX|BPF_B|BPF_MSH:
-			ftest->code = BPF_S_LDX_B_MSH;
-			break;
-		case BPF_LDX|BPF_IMM:
-			ftest->code = BPF_S_LDX_IMM;
-			break;
-		case BPF_MISC|BPF_TAX:
-			ftest->code = BPF_S_MISC_TAX;
-			break;
-		case BPF_MISC|BPF_TXA:
-			ftest->code = BPF_S_MISC_TXA;
-			break;
-		case BPF_RET|BPF_K:
-			ftest->code = BPF_S_RET_K;
-			break;
-		case BPF_RET|BPF_A:
-			ftest->code = BPF_S_RET_A;
-			break;
+		struct sock_filter *ftest = &filter[pc];
+		u16 code = ftest->code;
 
+		if (code >= ARRAY_SIZE(codes))
+			return 0;
 		/* Some instructions need special checks */
-
-			/* check for division by zero */
+		switch (code) {
 		case BPF_ALU|BPF_DIV|BPF_K:
+			/* check for division by zero */
 			if (ftest->k == 0)
 				return -EINVAL;
-			ftest->code = BPF_S_ALU_DIV_K;
 			break;
-
-		/* check for invalid memory addresses */
 		case BPF_LD|BPF_MEM:
-			if (ftest->k >= BPF_MEMWORDS)
-				return -EINVAL;
-			ftest->code = BPF_S_LD_MEM;
-			break;
 		case BPF_LDX|BPF_MEM:
-			if (ftest->k >= BPF_MEMWORDS)
-				return -EINVAL;
-			ftest->code = BPF_S_LDX_MEM;
-			break;
 		case BPF_ST:
-			if (ftest->k >= BPF_MEMWORDS)
-				return -EINVAL;
-			ftest->code = BPF_S_ST;
-			break;
 		case BPF_STX:
+			/* check for invalid memory addresses */
 			if (ftest->k >= BPF_MEMWORDS)
 				return -EINVAL;
-			ftest->code = BPF_S_STX;
 			break;
-
 		case BPF_JMP|BPF_JA:
 			/*
 			 * Note, the large ftest->k might cause loops.
@@ -528,40 +469,14 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
 			 */
 			if (ftest->k >= (unsigned)(flen-pc-1))
 				return -EINVAL;
-			ftest->code = BPF_S_JMP_JA;
-			break;
-
-		case BPF_JMP|BPF_JEQ|BPF_K:
-			ftest->code = BPF_S_JMP_JEQ_K;
-			break;
-		case BPF_JMP|BPF_JEQ|BPF_X:
-			ftest->code = BPF_S_JMP_JEQ_X;
-			break;
-		case BPF_JMP|BPF_JGE|BPF_K:
-			ftest->code = BPF_S_JMP_JGE_K;
-			break;
-		case BPF_JMP|BPF_JGE|BPF_X:
-			ftest->code = BPF_S_JMP_JGE_X;
-			break;
-		case BPF_JMP|BPF_JGT|BPF_K:
-			ftest->code = BPF_S_JMP_JGT_K;
-			break;
-		case BPF_JMP|BPF_JGT|BPF_X:
-			ftest->code = BPF_S_JMP_JGT_X;
 			break;
-		case BPF_JMP|BPF_JSET|BPF_K:
-			ftest->code = BPF_S_JMP_JSET_K;
-			break;
-		case BPF_JMP|BPF_JSET|BPF_X:
-			ftest->code = BPF_S_JMP_JSET_X;
-			break;
-
-		default:
-			return -EINVAL;
 		}
-
-			/* for conditionals both must be safe */
-		switch (ftest->code) {
+		code = codes[code];
+		if (!code--)
+			return -EINVAL;
+
+		/* for conditionals both must be safe */
+		switch (code) {
 		case BPF_S_JMP_JEQ_K:
 		case BPF_S_JMP_JEQ_X:
 		case BPF_S_JMP_JGE_K:
@@ -574,6 +489,7 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
 			    pc + ftest->jf + 1 >= flen)
 				return -EINVAL;
 		}
+		ftest->code = code;
 	}
 
 	/* last instruction must be a RET code */
-- 
1.6.1

^ permalink raw reply related

* Re: [PATCH] filter: Optimize instruction revalidation code.
From: Michael Tokarev @ 2010-11-16 13:11 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: davem, eric.dumazet, drosenberg, netdev
In-Reply-To: <201011162208.BHC17628.SVtFMJOOLFQFOH@I-love.SAKURA.ne.jp>

16.11.2010 16:08, Tetsuo Handa wrote:
[]
>  net/core/filter.c |  214 ++++++++++++++++-------------------------------------
>  1 files changed, 65 insertions(+), 149 deletions(-)
> 
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 23e9b2a..85be3d8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -383,7 +383,57 @@ EXPORT_SYMBOL(sk_run_filter);
>   */
>  int sk_chk_filter(struct sock_filter *filter, int flen)
>  {
> -	struct sock_filter *ftest;
> +	/*
> +	 * Valid instructions are initialized to non-0.
> +	 * Invalid instructions are initialized to 0.
> +	 */
> +	static u16 codes[] = {
> +		[BPF_ALU|BPF_ADD|BPF_K]  = BPF_S_ALU_ADD_K + 1,

How about using some "const" there?  :)

/mjt

^ permalink raw reply

* Re: [PATCH] filter: Optimize instruction revalidation code.
From: Eric Dumazet @ 2010-11-16 13:44 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: davem, drosenberg, netdev
In-Reply-To: <201011162208.BHC17628.SVtFMJOOLFQFOH@I-love.SAKURA.ne.jp>

Le mardi 16 novembre 2010 à 22:08 +0900, Tetsuo Handa a écrit :
> In the wake of commit 57fe93b3 "filter: make sure filters dont read
> uninitialized memory", I came up with this patch. I don't know how to test.
> Please review and do tests.
> 
> This patch optimizes sk_chk_filter(). Using gcc 4.1.2 on x86_32, the size of
> net/core/filter.o changed from 7416 bytes to 5260 bytes
> ("make allnoconfig" + CONFIG_NET=y).
> 
> By the way, if "struct sock_filter *filter" argument of sk_run_filter() does
> not change after it was revalidated at sk_chk_filter(), can't we detect and
> reject "BPF_S_LD_MEM/BPF_S_LDX_MEM before BPF_S_ST/BPF_S_STX" at
> sk_chk_filter()?

Sure, this was the idea, but seems complex because of branches.

Patches are welcomed ;)

> ----------------------------------------
>  From 2bf36130bd4912590b409b47a6a9a82e2884e035 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Tue, 16 Nov 2010 21:39:50 +0900
> Subject: [PATCH] filter: Optimize instruction revalidation code.
> 
> Since repeating small value to small value conversion using switch() clause's
> case statement is wasteful, this patch instroduces u16 to u16 mapping table
> and removes most of case statements. As a result, the size of net/core/filter.o
> is reduced by about 30% on x86.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---

Patch seems fine to me, with the 'const' codes[] Michael Tokarev already
spotted.

Thanks



^ permalink raw reply

* (unknown), , 
From: Ming-Yang Lee @ 2010-11-16 13:59 UTC (permalink / raw)




Do you need a loan to pay your bills or to start up a business or for Xmas?.
Kindly apply now for a low rate loan of 3%. for more information contact:
ming.yangfundsservice@qatar.io
We Await Your Response.
Mr Ming-Yang Lee

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.



-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.


^ permalink raw reply

* [PATCH net-next 1/4] rtnetlink: Link address family API
From: Thomas Graf @ 2010-11-16 14:30 UTC (permalink / raw)
  To: davem; +Cc: netdev

Each net_device contains address family specific data such as
per device settings and statistics. We already expose this data
via procfs/sysfs and partially netlink.

The netlink method requires the requester to send one RTM_GETLINK
request for each address family it wishes to receive data of
and then merge this data itself.

This patch implements a new API which combines all address family
specific link data in a new netlink attribute IFLA_AF_SPEC.
IFLA_AF_SPEC contains a sequence of nested attributes, one for each
address family which in turn defines the structure of its own
attribute. Example:

   [IFLA_AF_SPEC] = {
       [AF_INET] = {
           [IFLA_INET_CONF] = ...,
       },
       [AF_INET6] = {
           [IFLA_INET6_FLAGS] = ...,
           [IFLA_INET6_CONF] = ...,
       }
   }

The API also allows for address families to implement a function
which parses the IFLA_AF_SPEC attribute sent by userspace to
implement address family specific link options.

Signed-off-by: Thomas Graf <tgraf@infradead.org>

Index: net-2.6/include/linux/if_link.h
===================================================================
--- net-2.6.orig/include/linux/if_link.h
+++ net-2.6/include/linux/if_link.h
@@ -80,6 +80,24 @@ struct rtnl_link_ifmap {
 	__u8	port;
 };
 
+/*
+ * IFLA_AF_SPEC
+ *   Contains nested attributes for address family specific attributes.
+ *   Each address family may create a attribute with the address family
+ *   number as type and create its own attribute structure in it.
+ *
+ *   Example:
+ *   [IFLA_AF_SPEC] = {
+ *       [AF_INET] = {
+ *           [IFLA_INET_CONF] = ...,
+ *       },
+ *       [AF_INET6] = {
+ *           [IFLA_INET6_FLAGS] = ...,
+ *           [IFLA_INET6_CONF] = ...,
+ *       }
+ *   }
+ */
+
 enum {
 	IFLA_UNSPEC,
 	IFLA_ADDRESS,
@@ -116,6 +134,7 @@ enum {
 	IFLA_STATS64,
 	IFLA_VF_PORTS,
 	IFLA_PORT_SELF,
+	IFLA_AF_SPEC,
 	__IFLA_MAX
 };
 
Index: net-2.6/include/net/rtnetlink.h
===================================================================
--- net-2.6.orig/include/net/rtnetlink.h
+++ net-2.6/include/net/rtnetlink.h
@@ -83,6 +83,37 @@ extern void	__rtnl_link_unregister(struc
 extern int	rtnl_link_register(struct rtnl_link_ops *ops);
 extern void	rtnl_link_unregister(struct rtnl_link_ops *ops);
 
+/**
+ * 	struct rtnl_af_ops - rtnetlink address family operations
+ *
+ *	@list: Used internally
+ * 	@family: Address family
+ * 	@fill_link_af: Function to fill IFLA_AF_SPEC with address family
+ * 		       specific netlink attributes.
+ * 	@get_link_af_size: Function to calculate size of address family specific
+ * 			   netlink attributes exlusive the container attribute.
+ * 	@parse_link_af: Function to parse a IFLA_AF_SPEC attribute and modify
+ *			net_device accordingly.
+ */
+struct rtnl_af_ops {
+	struct list_head	list;
+	int			family;
+
+	int			(*fill_link_af)(struct sk_buff *skb,
+						const struct net_device *dev);
+	size_t			(*get_link_af_size)(const struct net_device *dev);
+
+	int			(*parse_link_af)(struct net_device *dev,
+						 const struct nlattr *attr);
+};
+
+extern int	__rtnl_af_register(struct rtnl_af_ops *ops);
+extern void	__rtnl_af_unregister(struct rtnl_af_ops *ops);
+
+extern int	rtnl_af_register(struct rtnl_af_ops *ops);
+extern void	rtnl_af_unregister(struct rtnl_af_ops *ops);
+
+
 extern struct net *rtnl_link_get_net(struct net *src_net, struct nlattr *tb[]);
 extern struct net_device *rtnl_create_link(struct net *src_net, struct net *net,
 	char *ifname, const struct rtnl_link_ops *ops, struct nlattr *tb[]);
Index: net-2.6/net/core/rtnetlink.c
===================================================================
--- net-2.6.orig/net/core/rtnetlink.c
+++ net-2.6/net/core/rtnetlink.c
@@ -362,6 +362,95 @@ static size_t rtnl_link_get_size(const s
 	return size;
 }
 
+static LIST_HEAD(rtnl_af_ops);
+
+static const struct rtnl_af_ops *rtnl_af_lookup(const int family)
+{
+	const struct rtnl_af_ops *ops;
+
+	list_for_each_entry(ops, &rtnl_af_ops, list) {
+		if (ops->family == family)
+			return ops;
+	}
+
+	return NULL;
+}
+
+/**
+ * __rtnl_af_register - Register rtnl_af_ops with rtnetlink.
+ * @ops: struct rtnl_af_ops * to register
+ *
+ * The caller must hold the rtnl_mutex.
+ *
+ * Returns 0 on success or a negative error code.
+ */
+int __rtnl_af_register(struct rtnl_af_ops *ops)
+{
+	list_add_tail(&ops->list, &rtnl_af_ops);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__rtnl_af_register);
+
+/**
+ * rtnl_af_register - Register rtnl_af_ops with rtnetlink.
+ * @ops: struct rtnl_af_ops * to register
+ *
+ * Returns 0 on success or a negative error code.
+ */
+int rtnl_af_register(struct rtnl_af_ops *ops)
+{
+	int err;
+
+	rtnl_lock();
+	err = __rtnl_af_register(ops);
+	rtnl_unlock();
+	return err;
+}
+EXPORT_SYMBOL_GPL(rtnl_af_register);
+
+/**
+ * __rtnl_af_unregister - Unregister rtnl_af_ops from rtnetlink.
+ * @ops: struct rtnl_af_ops * to unregister
+ *
+ * The caller must hold the rtnl_mutex.
+ */
+void __rtnl_af_unregister(struct rtnl_af_ops *ops)
+{
+	list_del(&ops->list);
+}
+EXPORT_SYMBOL_GPL(__rtnl_af_unregister);
+
+/**
+ * rtnl_af_unregister - Unregister rtnl_af_ops from rtnetlink.
+ * @ops: struct rtnl_af_ops * to unregister
+ */
+void rtnl_af_unregister(struct rtnl_af_ops *ops)
+{
+	rtnl_lock();
+	__rtnl_af_unregister(ops);
+	rtnl_unlock();
+}
+EXPORT_SYMBOL_GPL(rtnl_af_unregister);
+
+static size_t rtnl_link_get_af_size(const struct net_device *dev)
+{
+	struct rtnl_af_ops *af_ops;
+	size_t size;
+
+	/* IFLA_AF_SPEC */
+	size = nla_total_size(sizeof(struct nlattr));
+
+	list_for_each_entry(af_ops, &rtnl_af_ops, list) {
+		if (af_ops->get_link_af_size) {
+			/* AF_* + nested data */
+			size += nla_total_size(sizeof(struct nlattr)) +
+				af_ops->get_link_af_size(dev);
+		}
+	}
+
+	return size;
+}
+
 static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
 {
 	const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
@@ -671,7 +760,8 @@ static noinline size_t if_nlmsg_size(con
 	       + nla_total_size(4) /* IFLA_NUM_VF */
 	       + rtnl_vfinfo_size(dev) /* IFLA_VFINFO_LIST */
 	       + rtnl_port_size(dev) /* IFLA_VF_PORTS + IFLA_PORT_SELF */
-	       + rtnl_link_get_size(dev); /* IFLA_LINKINFO */
+	       + rtnl_link_get_size(dev) /* IFLA_LINKINFO */
+	       + rtnl_link_get_af_size(dev); /* IFLA_AF_SPEC */
 }
 
 static int rtnl_vf_ports_fill(struct sk_buff *skb, struct net_device *dev)
@@ -757,7 +847,8 @@ static int rtnl_fill_ifinfo(struct sk_bu
 	struct nlmsghdr *nlh;
 	struct rtnl_link_stats64 temp;
 	const struct rtnl_link_stats64 *stats;
-	struct nlattr *attr;
+	struct nlattr *attr, *af_spec;
+	struct rtnl_af_ops *af_ops;
 
 	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags);
 	if (nlh == NULL)
@@ -866,6 +957,36 @@ static int rtnl_fill_ifinfo(struct sk_bu
 			goto nla_put_failure;
 	}
 
+	if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
+		goto nla_put_failure;
+
+	list_for_each_entry(af_ops, &rtnl_af_ops, list) {
+		if (af_ops->fill_link_af) {
+			struct nlattr *af;
+			int err;
+
+			if (!(af = nla_nest_start(skb, af_ops->family)))
+				goto nla_put_failure;
+
+			err = af_ops->fill_link_af(skb, dev);
+
+			/*
+			 * Caller may return ENODATA to indicate that there
+			 * was no data to be dumped. This is not an error, it
+			 * means we should trim the attribute header and
+			 * continue.
+			 */
+			if (err == -ENODATA)
+				nla_nest_cancel(skb, af);
+			else if (err < 0)
+				goto nla_put_failure;
+
+			nla_nest_end(skb, af);
+		}
+	}
+
+	nla_nest_end(skb, af_spec);
+
 	return nlmsg_end(skb, nlh);
 
 nla_put_failure:
@@ -924,6 +1045,7 @@ const struct nla_policy ifla_policy[IFLA
 	[IFLA_VFINFO_LIST]	= {. type = NLA_NESTED },
 	[IFLA_VF_PORTS]		= { .type = NLA_NESTED },
 	[IFLA_PORT_SELF]	= { .type = NLA_NESTED },
+	[IFLA_AF_SPEC]		= { .type = NLA_NESTED },
 };
 EXPORT_SYMBOL(ifla_policy);
 
@@ -1225,6 +1347,27 @@ static int do_setlink(struct net_device 
 			goto errout;
 		modified = 1;
 	}
+
+	if (tb[IFLA_AF_SPEC]) {
+		struct nlattr *af;
+		int rem;
+
+		nla_for_each_nested(af, tb[IFLA_AF_SPEC], rem) {
+			const struct rtnl_af_ops *af_ops;
+
+			if (!(af_ops = rtnl_af_lookup(nla_type(af))))
+				continue;
+
+			if (!af_ops->parse_link_af)
+				continue;
+
+			err = af_ops->parse_link_af(dev, af);
+			if (err < 0)
+				goto errout;
+
+			modified = 1;
+		}
+	}
 	err = 0;
 
 errout:

^ permalink raw reply

* [PATCH net-next 2/4] inet: Define IPV4_DEVCONF_MAX
From: Thomas Graf @ 2010-11-16 14:31 UTC (permalink / raw)
  To: davem; +Cc: netdev

Define IPV4_DEVCONF_MAX to get rid of MAX - 1 notation.

Signed-off-by: Thomas Graf <tgraf@infradead.org>

Index: net-next-2.6/include/linux/inetdevice.h
===================================================================
--- net-next-2.6.orig/include/linux/inetdevice.h
+++ net-next-2.6/include/linux/inetdevice.h
@@ -41,10 +41,12 @@ enum
 	__IPV4_DEVCONF_MAX
 };
 
+#define IPV4_DEVCONF_MAX (__IPV4_DEVCONF_MAX - 1)
+
 struct ipv4_devconf {
 	void	*sysctl;
-	int	data[__IPV4_DEVCONF_MAX - 1];
-	DECLARE_BITMAP(state, __IPV4_DEVCONF_MAX - 1);
+	int	data[IPV4_DEVCONF_MAX];
+	DECLARE_BITMAP(state, IPV4_DEVCONF_MAX);
 };
 
 struct in_device {
@@ -90,7 +92,7 @@ static inline void ipv4_devconf_set(stru
 
 static inline void ipv4_devconf_setall(struct in_device *in_dev)
 {
-	bitmap_fill(in_dev->cnf.state, __IPV4_DEVCONF_MAX - 1);
+	bitmap_fill(in_dev->cnf.state, IPV4_DEVCONF_MAX);
 }
 
 #define IN_DEV_CONF_GET(in_dev, attr) \

^ permalink raw reply

* [PATCH v2] filter: Optimize instruction revalidation code.
From: Tetsuo Handa @ 2010-11-16 14:31 UTC (permalink / raw)
  To: eric.dumazet, mjt; +Cc: davem, drosenberg, netdev
In-Reply-To: <1289915053.5372.183.camel@edumazet-laptop>

Eric Dumazet wrote:
> Patch seems fine to me, with the 'const' codes[] Michael Tokarev already
> spotted.

Sorry, I forgot to evaluate

	default:
		return -EINVAL;
	}

case. If caller passed ftest->code == BPF_S_ALU_DIV_K (translated value)
rather than ftest->code == BPF_ALU|BPF_DIV|BPF_K (original value), the check

	case BPF_ALU|BPF_DIV|BPF_K:
		if (ftest->k == 0)
			return -EINVAL;
		ftest->code = BPF_S_ALU_DIV_K;
		break;

is bypassed. The problem is that original value and translated value overwraps.
Can we change translated value in order to guarantee that these values never
overwraps?
----------------------------------------
 From 7b6a7b784fa096383aadc86d32bff6b8329a2e66 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 16 Nov 2010 22:37:45 +0900
Subject: [PATCH v2] filter: optimize instruction revalidation code.

Since repeating small value to small value conversion using switch() clause's
case statement is wasteful, this patch instroduces u16 to u16 mapping table
and removes most of case statements. As a result, the size of net/core/filter.o
is reduced by about 27% on x86.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 net/core/filter.c |  223 ++++++++++++++++-------------------------------------
 1 files changed, 68 insertions(+), 155 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 23e9b2a..ef1d226 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -383,7 +383,57 @@ EXPORT_SYMBOL(sk_run_filter);
  */
 int sk_chk_filter(struct sock_filter *filter, int flen)
 {
-	struct sock_filter *ftest;
+	/*
+	 * Valid instructions are initialized to non-0.
+	 * Invalid instructions are initialized to 0.
+	 */
+	static const u16 codes[] = {
+		[BPF_ALU|BPF_ADD|BPF_K]  = BPF_S_ALU_ADD_K + 1,
+		[BPF_ALU|BPF_ADD|BPF_X]  = BPF_S_ALU_ADD_X + 1,
+		[BPF_ALU|BPF_SUB|BPF_K]  = BPF_S_ALU_SUB_K + 1,
+		[BPF_ALU|BPF_SUB|BPF_X]  = BPF_S_ALU_SUB_X + 1,
+		[BPF_ALU|BPF_MUL|BPF_K]  = BPF_S_ALU_MUL_K + 1,
+		[BPF_ALU|BPF_MUL|BPF_X]  = BPF_S_ALU_MUL_X + 1,
+		[BPF_ALU|BPF_DIV|BPF_X]  = BPF_S_ALU_DIV_X + 1,
+		[BPF_ALU|BPF_AND|BPF_K]  = BPF_S_ALU_AND_K + 1,
+		[BPF_ALU|BPF_AND|BPF_X]  = BPF_S_ALU_AND_X + 1,
+		[BPF_ALU|BPF_OR|BPF_K]   = BPF_S_ALU_OR_K + 1,
+		[BPF_ALU|BPF_OR|BPF_X]   = BPF_S_ALU_OR_X + 1,
+		[BPF_ALU|BPF_LSH|BPF_K]  = BPF_S_ALU_LSH_K + 1,
+		[BPF_ALU|BPF_LSH|BPF_X]  = BPF_S_ALU_LSH_X + 1,
+		[BPF_ALU|BPF_RSH|BPF_K]  = BPF_S_ALU_RSH_K + 1,
+		[BPF_ALU|BPF_RSH|BPF_X]  = BPF_S_ALU_RSH_X + 1,
+		[BPF_ALU|BPF_NEG]        = BPF_S_ALU_NEG + 1,
+		[BPF_LD|BPF_W|BPF_ABS]   = BPF_S_LD_W_ABS + 1,
+		[BPF_LD|BPF_H|BPF_ABS]   = BPF_S_LD_H_ABS + 1,
+		[BPF_LD|BPF_B|BPF_ABS]   = BPF_S_LD_B_ABS + 1,
+		[BPF_LD|BPF_W|BPF_LEN]   = BPF_S_LD_W_LEN + 1,
+		[BPF_LD|BPF_W|BPF_IND]   = BPF_S_LD_W_IND + 1,
+		[BPF_LD|BPF_H|BPF_IND]   = BPF_S_LD_H_IND + 1,
+		[BPF_LD|BPF_B|BPF_IND]   = BPF_S_LD_B_IND + 1,
+		[BPF_LD|BPF_IMM]         = BPF_S_LD_IMM + 1,
+		[BPF_LDX|BPF_W|BPF_LEN]  = BPF_S_LDX_W_LEN + 1,
+		[BPF_LDX|BPF_B|BPF_MSH]  = BPF_S_LDX_B_MSH + 1,
+		[BPF_LDX|BPF_IMM]        = BPF_S_LDX_IMM + 1,
+		[BPF_MISC|BPF_TAX]       = BPF_S_MISC_TAX + 1,
+		[BPF_MISC|BPF_TXA]       = BPF_S_MISC_TXA + 1,
+		[BPF_RET|BPF_K]          = BPF_S_RET_K + 1,
+		[BPF_RET|BPF_A]          = BPF_S_RET_A + 1,
+		[BPF_ALU|BPF_DIV|BPF_K]  = BPF_S_ALU_DIV_K + 1,
+		[BPF_LD|BPF_MEM]         = BPF_S_LD_MEM + 1,
+		[BPF_LDX|BPF_MEM]        = BPF_S_LDX_MEM + 1,
+		[BPF_ST]                 = BPF_S_ST + 1,
+		[BPF_STX]                = BPF_S_STX + 1,
+		[BPF_JMP|BPF_JA]         = BPF_S_JMP_JA + 1,
+		[BPF_JMP|BPF_JEQ|BPF_K]  = BPF_S_JMP_JEQ_K + 1,
+		[BPF_JMP|BPF_JEQ|BPF_X]  = BPF_S_JMP_JEQ_X + 1,
+		[BPF_JMP|BPF_JGE|BPF_K]  = BPF_S_JMP_JGE_K + 1,
+		[BPF_JMP|BPF_JGE|BPF_X]  = BPF_S_JMP_JGE_X + 1,
+		[BPF_JMP|BPF_JGT|BPF_K]  = BPF_S_JMP_JGT_K + 1,
+		[BPF_JMP|BPF_JGT|BPF_X]  = BPF_S_JMP_JGT_X + 1,
+		[BPF_JMP|BPF_JSET|BPF_K] = BPF_S_JMP_JSET_K + 1,
+		[BPF_JMP|BPF_JSET|BPF_X] = BPF_S_JMP_JSET_X + 1,
+	};
 	int pc;
 
 	if (flen == 0 || flen > BPF_MAXINSNS)
@@ -391,136 +441,30 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
 
 	/* check the filter code now */
 	for (pc = 0; pc < flen; pc++) {
-		ftest = &filter[pc];
-
-		/* Only allow valid instructions */
-		switch (ftest->code) {
-		case BPF_ALU|BPF_ADD|BPF_K:
-			ftest->code = BPF_S_ALU_ADD_K;
-			break;
-		case BPF_ALU|BPF_ADD|BPF_X:
-			ftest->code = BPF_S_ALU_ADD_X;
-			break;
-		case BPF_ALU|BPF_SUB|BPF_K:
-			ftest->code = BPF_S_ALU_SUB_K;
-			break;
-		case BPF_ALU|BPF_SUB|BPF_X:
-			ftest->code = BPF_S_ALU_SUB_X;
-			break;
-		case BPF_ALU|BPF_MUL|BPF_K:
-			ftest->code = BPF_S_ALU_MUL_K;
-			break;
-		case BPF_ALU|BPF_MUL|BPF_X:
-			ftest->code = BPF_S_ALU_MUL_X;
-			break;
-		case BPF_ALU|BPF_DIV|BPF_X:
-			ftest->code = BPF_S_ALU_DIV_X;
-			break;
-		case BPF_ALU|BPF_AND|BPF_K:
-			ftest->code = BPF_S_ALU_AND_K;
-			break;
-		case BPF_ALU|BPF_AND|BPF_X:
-			ftest->code = BPF_S_ALU_AND_X;
-			break;
-		case BPF_ALU|BPF_OR|BPF_K:
-			ftest->code = BPF_S_ALU_OR_K;
-			break;
-		case BPF_ALU|BPF_OR|BPF_X:
-			ftest->code = BPF_S_ALU_OR_X;
-			break;
-		case BPF_ALU|BPF_LSH|BPF_K:
-			ftest->code = BPF_S_ALU_LSH_K;
-			break;
-		case BPF_ALU|BPF_LSH|BPF_X:
-			ftest->code = BPF_S_ALU_LSH_X;
-			break;
-		case BPF_ALU|BPF_RSH|BPF_K:
-			ftest->code = BPF_S_ALU_RSH_K;
-			break;
-		case BPF_ALU|BPF_RSH|BPF_X:
-			ftest->code = BPF_S_ALU_RSH_X;
-			break;
-		case BPF_ALU|BPF_NEG:
-			ftest->code = BPF_S_ALU_NEG;
-			break;
-		case BPF_LD|BPF_W|BPF_ABS:
-			ftest->code = BPF_S_LD_W_ABS;
-			break;
-		case BPF_LD|BPF_H|BPF_ABS:
-			ftest->code = BPF_S_LD_H_ABS;
-			break;
-		case BPF_LD|BPF_B|BPF_ABS:
-			ftest->code = BPF_S_LD_B_ABS;
-			break;
-		case BPF_LD|BPF_W|BPF_LEN:
-			ftest->code = BPF_S_LD_W_LEN;
-			break;
-		case BPF_LD|BPF_W|BPF_IND:
-			ftest->code = BPF_S_LD_W_IND;
-			break;
-		case BPF_LD|BPF_H|BPF_IND:
-			ftest->code = BPF_S_LD_H_IND;
-			break;
-		case BPF_LD|BPF_B|BPF_IND:
-			ftest->code = BPF_S_LD_B_IND;
-			break;
-		case BPF_LD|BPF_IMM:
-			ftest->code = BPF_S_LD_IMM;
-			break;
-		case BPF_LDX|BPF_W|BPF_LEN:
-			ftest->code = BPF_S_LDX_W_LEN;
-			break;
-		case BPF_LDX|BPF_B|BPF_MSH:
-			ftest->code = BPF_S_LDX_B_MSH;
-			break;
-		case BPF_LDX|BPF_IMM:
-			ftest->code = BPF_S_LDX_IMM;
-			break;
-		case BPF_MISC|BPF_TAX:
-			ftest->code = BPF_S_MISC_TAX;
-			break;
-		case BPF_MISC|BPF_TXA:
-			ftest->code = BPF_S_MISC_TXA;
-			break;
-		case BPF_RET|BPF_K:
-			ftest->code = BPF_S_RET_K;
-			break;
-		case BPF_RET|BPF_A:
-			ftest->code = BPF_S_RET_A;
-			break;
+		struct sock_filter *ftest = &filter[pc];
+		u16 code = ftest->code;
 
+		if (code >= ARRAY_SIZE(codes))
+			return -EINVAL;
+		code = codes[code];
+		if (!code--)
+			return -EINVAL;
 		/* Some instructions need special checks */
-
+		switch (code) {
+		case BPF_S_ALU_DIV_K:
 			/* check for division by zero */
-		case BPF_ALU|BPF_DIV|BPF_K:
 			if (ftest->k == 0)
 				return -EINVAL;
-			ftest->code = BPF_S_ALU_DIV_K;
-			break;
-
-		/* check for invalid memory addresses */
-		case BPF_LD|BPF_MEM:
-			if (ftest->k >= BPF_MEMWORDS)
-				return -EINVAL;
-			ftest->code = BPF_S_LD_MEM;
-			break;
-		case BPF_LDX|BPF_MEM:
-			if (ftest->k >= BPF_MEMWORDS)
-				return -EINVAL;
-			ftest->code = BPF_S_LDX_MEM;
 			break;
-		case BPF_ST:
-			if (ftest->k >= BPF_MEMWORDS)
-				return -EINVAL;
-			ftest->code = BPF_S_ST;
-			break;
-		case BPF_STX:
+		case BPF_S_LD_MEM:
+		case BPF_S_LDX_MEM:
+		case BPF_S_ST:
+		case BPF_S_STX:
+			/* check for invalid memory addresses */
 			if (ftest->k >= BPF_MEMWORDS)
 				return -EINVAL;
-			ftest->code = BPF_S_STX;
 			break;
-
-		case BPF_JMP|BPF_JA:
+		case BPF_S_JMP_JA:
 			/*
 			 * Note, the large ftest->k might cause loops.
 			 * Compare this with conditional jumps below,
@@ -528,40 +472,7 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
 			 */
 			if (ftest->k >= (unsigned)(flen-pc-1))
 				return -EINVAL;
-			ftest->code = BPF_S_JMP_JA;
-			break;
-
-		case BPF_JMP|BPF_JEQ|BPF_K:
-			ftest->code = BPF_S_JMP_JEQ_K;
-			break;
-		case BPF_JMP|BPF_JEQ|BPF_X:
-			ftest->code = BPF_S_JMP_JEQ_X;
-			break;
-		case BPF_JMP|BPF_JGE|BPF_K:
-			ftest->code = BPF_S_JMP_JGE_K;
-			break;
-		case BPF_JMP|BPF_JGE|BPF_X:
-			ftest->code = BPF_S_JMP_JGE_X;
-			break;
-		case BPF_JMP|BPF_JGT|BPF_K:
-			ftest->code = BPF_S_JMP_JGT_K;
-			break;
-		case BPF_JMP|BPF_JGT|BPF_X:
-			ftest->code = BPF_S_JMP_JGT_X;
-			break;
-		case BPF_JMP|BPF_JSET|BPF_K:
-			ftest->code = BPF_S_JMP_JSET_K;
 			break;
-		case BPF_JMP|BPF_JSET|BPF_X:
-			ftest->code = BPF_S_JMP_JSET_X;
-			break;
-
-		default:
-			return -EINVAL;
-		}
-
-			/* for conditionals both must be safe */
-		switch (ftest->code) {
 		case BPF_S_JMP_JEQ_K:
 		case BPF_S_JMP_JEQ_X:
 		case BPF_S_JMP_JGE_K:
@@ -570,10 +481,12 @@ int sk_chk_filter(struct sock_filter *filter, int flen)
 		case BPF_S_JMP_JGT_X:
 		case BPF_S_JMP_JSET_X:
 		case BPF_S_JMP_JSET_K:
+			/* for conditionals both must be safe */
 			if (pc + ftest->jt + 1 >= flen ||
 			    pc + ftest->jf + 1 >= flen)
 				return -EINVAL;
 		}
+		ftest->code = code;
 	}
 
 	/* last instruction must be a RET code */
-- 
1.6.1

^ permalink raw reply related

* [PATCH net-next 3/4] ipv4: AF_INET link address family
From: Thomas Graf @ 2010-11-16 14:32 UTC (permalink / raw)
  To: davem; +Cc: netdev

Implements the AF_INET link address family exposing the per
device configuration settings via netlink using the attribute
IFLA_INET_CONF.

The format of IFLA_INET_CONF differs depending on the direction
the attribute is sent. The attribute sent by the kernel consists
of a u32 array, basically a 1:1 copy of in_device->cnf.data[].
The attribute expected by the kernel must consist of a sequence
of nested u32 attributes, each representing a change request,
e.g.
	[IFLA_INET_CONF] = {
		[IPV4_DEVCONF_FORWARDING] = 1,
		[IPV4_DEVCONF_NOXFRM] = 0,
	}

libnl userspace API documentation and example available from:
http://www.infradead.org/~tgr/libnl/doc-git/group__link__inet.html

Signed-off-by: Thomas Graf <tgraf@infradead.org>

Index: net-next-2.6/net/ipv4/devinet.c
===================================================================
--- net-next-2.6.orig/net/ipv4/devinet.c
+++ net-next-2.6/net/ipv4/devinet.c
@@ -1256,6 +1256,72 @@ errout:
 		rtnl_set_sk_err(net, RTNLGRP_IPV4_IFADDR, err);
 }
 
+static size_t inet_get_link_af_size(const struct net_device *dev)
+{
+	struct in_device *in_dev = __in_dev_get_rcu(dev);
+
+	if (!in_dev)
+		return 0;
+
+	return nla_total_size(IPV4_DEVCONF_MAX * 4); /* IFLA_INET_CONF */
+}
+
+static int inet_fill_link_af(struct sk_buff *skb, const struct net_device *dev)
+{
+	struct in_device *in_dev = __in_dev_get_rcu(dev);
+	struct nlattr *nla;
+	int i;
+
+	if (!in_dev)
+		return -ENODATA;
+
+	nla = nla_reserve(skb, IFLA_INET_CONF, IPV4_DEVCONF_MAX * 4);
+	if (nla == NULL)
+		return -EMSGSIZE;
+
+	for (i = 0; i < IPV4_DEVCONF_MAX; i++)
+		((u32 *) nla_data(nla))[i] = in_dev->cnf.data[i];
+
+	return 0;
+}
+
+static const struct nla_policy inet_af_policy[IFLA_INET_MAX+1] = {
+	[IFLA_INET_CONF]	= { .type = NLA_NESTED },
+};
+
+static int inet_parse_link_af(struct net_device *dev, const struct nlattr *nla)
+{
+	struct in_device *in_dev = __in_dev_get_rcu(dev);
+	struct nlattr *a, *tb[IFLA_INET_MAX+1];
+	int err, rem;
+
+	if (!in_dev)
+		return -EOPNOTSUPP;
+
+	err = nla_parse_nested(tb, IFLA_INET_MAX, nla, inet_af_policy);
+	if (err < 0)
+		return err;
+
+	if (tb[IFLA_INET_CONF]) {
+		nla_for_each_nested(a, tb[IFLA_INET_CONF], rem) {
+			int cfgid = nla_type(a);
+
+			if (nla_len(a) < 4)
+				return -EINVAL;
+
+			if (cfgid <= 0 || cfgid > IPV4_DEVCONF_MAX)
+				return -EINVAL;
+		}
+	}
+
+	if (tb[IFLA_INET_CONF]) {
+		nla_for_each_nested(a, tb[IFLA_INET_CONF], rem)
+			ipv4_devconf_set(in_dev, nla_type(a), nla_get_u32(a));
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_SYSCTL
 
 static void devinet_copy_dflt_conf(struct net *net, int i)
@@ -1619,6 +1685,13 @@ static __net_initdata struct pernet_oper
 	.exit = devinet_exit_net,
 };
 
+static struct rtnl_af_ops inet_af_ops = {
+	.family		  = AF_INET,
+	.fill_link_af	  = inet_fill_link_af,
+	.get_link_af_size = inet_get_link_af_size,
+	.parse_link_af	  = inet_parse_link_af,
+};
+
 void __init devinet_init(void)
 {
 	register_pernet_subsys(&devinet_ops);
@@ -1626,6 +1699,8 @@ void __init devinet_init(void)
 	register_gifconf(PF_INET, inet_gifconf);
 	register_netdevice_notifier(&ip_netdev_notifier);
 
+	rtnl_af_register(&inet_af_ops);
+
 	rtnl_register(PF_INET, RTM_NEWADDR, inet_rtm_newaddr, NULL);
 	rtnl_register(PF_INET, RTM_DELADDR, inet_rtm_deladdr, NULL);
 	rtnl_register(PF_INET, RTM_GETADDR, NULL, inet_dump_ifaddr);
Index: net-next-2.6/include/linux/if_link.h
===================================================================
--- net-next-2.6.orig/include/linux/if_link.h
+++ net-next-2.6/include/linux/if_link.h
@@ -147,6 +147,14 @@ enum {
 #define IFLA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifinfomsg))
 #endif
 
+enum {
+	IFLA_INET_UNSPEC,
+	IFLA_INET_CONF,
+	__IFLA_INET_MAX,
+};
+
+#define IFLA_INET_MAX (__IFLA_INET_MAX - 1)
+
 /* ifi_flags.
 
    IFF_* flags.

^ permalink raw reply

* [PATCH net-next 4/4] ipv6: AF_INET6 link address family
From: Thomas Graf @ 2010-11-16 14:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, YOSHIFUJI Hideaki

IPv6 already exposes some address family data via netlink in the
IFLA_PROTINFO attribute if RTM_GETLINK request is sent with the
address family set to AF_INET6. We take over this format and
reuse all the code.

Signed-off-by: Thomas Graf <tgraf@infradead.org>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

Index: net-next-2.6/net/ipv6/addrconf.c
===================================================================
--- net-next-2.6.orig/net/ipv6/addrconf.c
+++ net-next-2.6/net/ipv6/addrconf.c
@@ -3831,6 +3831,15 @@ static inline void ipv6_store_devconf(st
 	array[DEVCONF_FORCE_TLLAO] = cnf->force_tllao;
 }
 
+static inline size_t inet6_ifla6_size(void)
+{
+	return nla_total_size(4) /* IFLA_INET6_FLAGS */
+	     + nla_total_size(sizeof(struct ifla_cacheinfo))
+	     + nla_total_size(DEVCONF_MAX * 4) /* IFLA_INET6_CONF */
+	     + nla_total_size(IPSTATS_MIB_MAX * 8) /* IFLA_INET6_STATS */
+	     + nla_total_size(ICMP6_MIB_MAX * 8); /* IFLA_INET6_ICMP6STATS */
+}
+
 static inline size_t inet6_if_nlmsg_size(void)
 {
 	return NLMSG_ALIGN(sizeof(struct ifinfomsg))
@@ -3838,13 +3847,7 @@ static inline size_t inet6_if_nlmsg_size
 	       + nla_total_size(MAX_ADDR_LEN) /* IFLA_ADDRESS */
 	       + nla_total_size(4) /* IFLA_MTU */
 	       + nla_total_size(4) /* IFLA_LINK */
-	       + nla_total_size( /* IFLA_PROTINFO */
-			nla_total_size(4) /* IFLA_INET6_FLAGS */
-			+ nla_total_size(sizeof(struct ifla_cacheinfo))
-			+ nla_total_size(DEVCONF_MAX * 4) /* IFLA_INET6_CONF */
-			+ nla_total_size(IPSTATS_MIB_MAX * 8) /* IFLA_INET6_STATS */
-			+ nla_total_size(ICMP6_MIB_MAX * 8) /* IFLA_INET6_ICMP6STATS */
-		 );
+	       + nla_total_size(inet6_ifla6_size()); /* IFLA_PROTINFO */
 }
 
 static inline void __snmp6_fill_stats(u64 *stats, void __percpu **mib,
@@ -3891,15 +3894,76 @@ static void snmp6_fill_stats(u64 *stats,
 	}
 }
 
+static int inet6_fill_ifla6_attrs(struct sk_buff *skb, struct inet6_dev *idev)
+{
+	struct nlattr *nla;
+	struct ifla_cacheinfo ci;
+
+	NLA_PUT_U32(skb, IFLA_INET6_FLAGS, idev->if_flags);
+
+	ci.max_reasm_len = IPV6_MAXPLEN;
+	ci.tstamp = (__u32)(TIME_DELTA(idev->tstamp, INITIAL_JIFFIES) / HZ * 100
+		    + TIME_DELTA(idev->tstamp, INITIAL_JIFFIES) % HZ * 100 / HZ);
+	ci.reachable_time = idev->nd_parms->reachable_time;
+	ci.retrans_time = idev->nd_parms->retrans_time;
+	NLA_PUT(skb, IFLA_INET6_CACHEINFO, sizeof(ci), &ci);
+
+	nla = nla_reserve(skb, IFLA_INET6_CONF, DEVCONF_MAX * sizeof(s32));
+	if (nla == NULL)
+		goto nla_put_failure;
+	ipv6_store_devconf(&idev->cnf, nla_data(nla), nla_len(nla));
+
+	/* XXX - MC not implemented */
+
+	nla = nla_reserve(skb, IFLA_INET6_STATS, IPSTATS_MIB_MAX * sizeof(u64));
+	if (nla == NULL)
+		goto nla_put_failure;
+	snmp6_fill_stats(nla_data(nla), idev, IFLA_INET6_STATS, nla_len(nla));
+
+	nla = nla_reserve(skb, IFLA_INET6_ICMP6STATS, ICMP6_MIB_MAX * sizeof(u64));
+	if (nla == NULL)
+		goto nla_put_failure;
+	snmp6_fill_stats(nla_data(nla), idev, IFLA_INET6_ICMP6STATS, nla_len(nla));
+
+	return 0;
+
+nla_put_failure:
+	return -EMSGSIZE;
+}
+
+static size_t inet6_get_link_af_size(const struct net_device *dev)
+{
+	if (!__in6_dev_get(dev))
+		return 0;
+
+	return inet6_ifla6_size();
+}
+
+static int inet6_fill_link_af(struct sk_buff *skb, const struct net_device *dev)
+{
+	struct inet6_dev *idev = __in6_dev_get(dev);
+
+	if (!idev)
+		return -ENODATA;
+
+	if (inet6_fill_ifla6_attrs(skb, idev) < 0)
+		return -EMSGSIZE;
+
+	return 0;
+}
+
+static int inet6_parse_link_af(struct net_device *dev, const struct nlattr *nla)
+{
+	return -EOPNOTSUPP;
+}
+
 static int inet6_fill_ifinfo(struct sk_buff *skb, struct inet6_dev *idev,
 			     u32 pid, u32 seq, int event, unsigned int flags)
 {
 	struct net_device *dev = idev->dev;
-	struct nlattr *nla;
 	struct ifinfomsg *hdr;
 	struct nlmsghdr *nlh;
 	void *protoinfo;
-	struct ifla_cacheinfo ci;
 
 	nlh = nlmsg_put(skb, pid, seq, event, sizeof(*hdr), flags);
 	if (nlh == NULL)
@@ -3926,31 +3990,8 @@ static int inet6_fill_ifinfo(struct sk_b
 	if (protoinfo == NULL)
 		goto nla_put_failure;
 
-	NLA_PUT_U32(skb, IFLA_INET6_FLAGS, idev->if_flags);
-
-	ci.max_reasm_len = IPV6_MAXPLEN;
-	ci.tstamp = (__u32)(TIME_DELTA(idev->tstamp, INITIAL_JIFFIES) / HZ * 100
-		    + TIME_DELTA(idev->tstamp, INITIAL_JIFFIES) % HZ * 100 / HZ);
-	ci.reachable_time = idev->nd_parms->reachable_time;
-	ci.retrans_time = idev->nd_parms->retrans_time;
-	NLA_PUT(skb, IFLA_INET6_CACHEINFO, sizeof(ci), &ci);
-
-	nla = nla_reserve(skb, IFLA_INET6_CONF, DEVCONF_MAX * sizeof(s32));
-	if (nla == NULL)
+	if (inet6_fill_ifla6_attrs(skb, idev) < 0)
 		goto nla_put_failure;
-	ipv6_store_devconf(&idev->cnf, nla_data(nla), nla_len(nla));
-
-	/* XXX - MC not implemented */
-
-	nla = nla_reserve(skb, IFLA_INET6_STATS, IPSTATS_MIB_MAX * sizeof(u64));
-	if (nla == NULL)
-		goto nla_put_failure;
-	snmp6_fill_stats(nla_data(nla), idev, IFLA_INET6_STATS, nla_len(nla));
-
-	nla = nla_reserve(skb, IFLA_INET6_ICMP6STATS, ICMP6_MIB_MAX * sizeof(u64));
-	if (nla == NULL)
-		goto nla_put_failure;
-	snmp6_fill_stats(nla_data(nla), idev, IFLA_INET6_ICMP6STATS, nla_len(nla));
 
 	nla_nest_end(skb, protoinfo);
 	return nlmsg_end(skb, nlh);
@@ -4621,6 +4662,13 @@ int unregister_inet6addr_notifier(struct
 }
 EXPORT_SYMBOL(unregister_inet6addr_notifier);
 
+static struct rtnl_af_ops inet6_ops = {
+	.family		  = AF_INET6,
+	.fill_link_af	  = inet6_fill_link_af,
+	.get_link_af_size = inet6_get_link_af_size,
+	.parse_link_af	  = inet6_parse_link_af,
+};
+
 /*
  *	Init / cleanup code
  */
@@ -4672,6 +4720,10 @@ int __init addrconf_init(void)
 
 	addrconf_verify(0);
 
+	err = rtnl_af_register(&inet6_ops);
+	if (err < 0)
+		goto errout_af;
+
 	err = __rtnl_register(PF_INET6, RTM_GETLINK, NULL, inet6_dump_ifinfo);
 	if (err < 0)
 		goto errout;
@@ -4687,6 +4739,8 @@ int __init addrconf_init(void)
 
 	return 0;
 errout:
+	rtnl_af_unregister(&inet6_ops);
+errout_af:
 	unregister_netdevice_notifier(&ipv6_dev_notf);
 errlo:
 	unregister_pernet_subsys(&addrconf_ops);
@@ -4707,6 +4761,8 @@ void addrconf_cleanup(void)
 
 	rtnl_lock();
 
+	__rtnl_af_unregister(&inet6_ops);
+
 	/* clean dev list */
 	for_each_netdev(&init_net, dev) {
 		if (__in6_dev_get(dev) == NULL)

^ permalink raw reply

* Re: Remaining problems in firewire-net
From: Stefan Richter @ 2010-11-16 15:11 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: netdev, linux1394-devel, linux-kernel
In-Reply-To: <1289878275.16486.57.camel@maxim-laptop>

On Nov 16 Maxim Levitsky wrote:
> The IP over 1394 tells that unicast can also use ISO transactions.
> However, it doesn't say how to negotiate the ISO channel number, thus a
> logical conclusion is that its not possible to use ISO transactions for
> unicast.
> Is that right?

To my understanding, any unicast transport method other than block write
to the unicast FIFO are outside the scope of RFC 2734.  At the end of
section 7:
"""
   Unicast IP datagrams whose quality of service is best-effort SHALL be
   contained within the data payload of 1394 block write transactions [...]
"""
"""
   Unicast IP datagrams that require quality of service other than
   best-effort are beyond the scope of this standard.
"""
-- 
Stefan Richter
-=====-==-=- =-== =----
http://arcgraph.de/sr/

------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
Spend less time writing and  rewriting code and more time creating great
experiences on the web. Be a part of the beta today
http://p.sf.net/sfu/msIE9-sfdev2dev

^ permalink raw reply

* [PATCH] 3c59x: fix build failure on !CONFIG_PCI
From: Namhyung Kim @ 2010-11-16 15:27 UTC (permalink / raw)
  To: Steffen Klassert, netdev; +Cc: linux-kernel

VORTEX_PCI() could return NULL so it needs to be casted before
accessing any member of struct pci_dev. This fixes following
build failure. Likewise VORTEX_EISA() was changed also.

  CC [M]  drivers/net/3c59x.o
drivers/net/3c59x.c: In function 'acpi_set_WOL':
drivers/net/3c59x.c:3211:39: warning: dereferencing 'void *' pointer
drivers/net/3c59x.c:3211:39: error: request for member 'current_state' in something not a structure or union
make[3]: *** [drivers/net/3c59x.o] Error 1
make[2]: *** [drivers/net/3c59x.o] Error 2
make[1]: *** [sub-make] Error 2
make: *** [all] Error 2

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 drivers/net/3c59x.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
index e1da258..0a92436 100644
--- a/drivers/net/3c59x.c
+++ b/drivers/net/3c59x.c
@@ -699,7 +699,8 @@ DEFINE_WINDOW_IO(32)
 #define DEVICE_PCI(dev) NULL
 #endif
 
-#define VORTEX_PCI(vp) (((vp)->gendev) ? DEVICE_PCI((vp)->gendev) : NULL)
+#define VORTEX_PCI(vp)							\
+	((struct pci_dev *) (((vp)->gendev) ? DEVICE_PCI((vp)->gendev) : NULL))
 
 #ifdef CONFIG_EISA
 #define DEVICE_EISA(dev) (((dev)->bus == &eisa_bus_type) ? to_eisa_device((dev)) : NULL)
@@ -707,7 +708,8 @@ DEFINE_WINDOW_IO(32)
 #define DEVICE_EISA(dev) NULL
 #endif
 
-#define VORTEX_EISA(vp) (((vp)->gendev) ? DEVICE_EISA((vp)->gendev) : NULL)
+#define VORTEX_EISA(vp)							\
+	((struct eisa_device *) (((vp)->gendev) ? DEVICE_EISA((vp)->gendev) : NULL))
 
 /* The action to take with a media selection timer tick.
    Note that we deviate from the 3Com order by checking 10base2 before AUI.
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH net-next-2.6 V2] net: reorder struct sock fields
From: Eric Dumazet @ 2010-11-16 15:56 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <1289891544.3364.193.camel@edumazet-laptop>

Le mardi 16 novembre 2010 à 08:12 +0100, Eric Dumazet a écrit :
> Right now, fields in struct sock are not optimally ordered, because each
> path (RX softirq, TX completion, RX user,  TX user) has to touch fields
> that are contained in many different cache lines.
> 

Performance testing pointed out I forgot :

- sk_drops, read for each queued packet, and incremented on packet drops
- sk_flags tested in sock_def_readable()
- sk_policy[0]

Here is V2 of patch :

[PATCH net-next-2.6 V2] net: reorder struct sock fields

Right now, fields in struct sock are not optimally ordered, because each
path (RX softirq, TX completion, RX user,  TX user) has to touch fields
that are contained in many different cache lines.

The really critical thing is to shrink number of cache lines that are
used at RX softirq time : CPU handling softirqs for a device can receive
many frames per second for many sockets. If load is too big, we can drop
frames at NIC level. RPS or multiqueue cards can help, but better reduce
latency if possible.

This patch starts with UDP protocol, then additional patches will try to
reduce latencies of other ones as well.

At RX softirq time, fields of interest for UDP protocol are :
(not counting ones in inet struct for the lookup)

Read/Written:
sk_refcnt   (atomic increment/decrement)
sk_rmem_alloc & sk_backlog.len (to check if there is room in queues)
sk_receive_queue
sk_backlog (if socket locked by user program)
sk_rxhash
sk_forward_alloc
sk_drops

Read only:
sk_rcvbuf (sk_rcvqueues_full())
sk_filter
sk_wq
sk_policy[0]
sk_flags

Additional notes :

- sk_backlog has one hole on 64bit arches. We can fill it to save 8
bytes.
- sk_backlog is used only if RX sofirq handler finds the socket while
locked by user.
- sk_rxhash is written only once per flow.
- sk_drops is written only if queues are full

Final layout :

[1] One section grouping all read/write fields, but placing rxhash and
sk_backlog at the end of this section.

[2] One section grouping all read fields in RX handler 
   (sk_filter, sk_rcv_buf, sk_wq)

[3] Section used by other paths

I'll post a patch on its own to put sk_refcnt at the end of struct
sock_common so that it shares same cache line than section [1]

New offsets on 64bit arch :

sizeof(struct sock)=0x268
offsetof(struct sock, sk_refcnt)  =0x10
offsetof(struct sock, sk_lock)    =0x48
offsetof(struct sock, sk_receive_queue)=0x68
offsetof(struct sock, sk_backlog)=0x80
offsetof(struct sock, sk_rmem_alloc)=0x80
offsetof(struct sock, sk_forward_alloc)=0x98
offsetof(struct sock, sk_rxhash)=0x9c
offsetof(struct sock, sk_rcvbuf)=0xa4
offsetof(struct sock, sk_drops) =0xa0
offsetof(struct sock, sk_filter)=0xa8
offsetof(struct sock, sk_wq)=0xb0
offsetof(struct sock, sk_policy)=0xd0
offsetof(struct sock, sk_flags) =0xe0

Instead of :

sizeof(struct sock)=0x270
offsetof(struct sock, sk_refcnt)  =0x10
offsetof(struct sock, sk_lock)    =0x50
offsetof(struct sock, sk_receive_queue)=0xc0
offsetof(struct sock, sk_backlog)=0x70
offsetof(struct sock, sk_rmem_alloc)=0xac
offsetof(struct sock, sk_forward_alloc)=0x10c
offsetof(struct sock, sk_rxhash)=0x128
offsetof(struct sock, sk_rcvbuf)=0x4c
offsetof(struct sock, sk_drops) =0x16c
offsetof(struct sock, sk_filter)=0x198
offsetof(struct sock, sk_wq)=0x88
offsetof(struct sock, sk_policy)=0x98
offsetof(struct sock, sk_flags) =0x130

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/sock.h |   55 ++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index a6338d0..a2825eb 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -241,59 +241,67 @@ struct sock {
 #define sk_bind_node		__sk_common.skc_bind_node
 #define sk_prot			__sk_common.skc_prot
 #define sk_net			__sk_common.skc_net
-	kmemcheck_bitfield_begin(flags);
-	unsigned int		sk_shutdown  : 2,
-				sk_no_check  : 2,
-				sk_userlocks : 4,
-				sk_protocol  : 8,
-				sk_type      : 16;
-	kmemcheck_bitfield_end(flags);
-	int			sk_rcvbuf;
 	socket_lock_t		sk_lock;
+	struct sk_buff_head	sk_receive_queue;
 	/*
 	 * The backlog queue is special, it is always used with
 	 * the per-socket spinlock held and requires low latency
 	 * access. Therefore we special case it's implementation.
+	 * Note : rmem_alloc is in this structure to fill a hole
+	 * on 64bit arches, not because its logically part of
+	 * backlog.
 	 */
 	struct {
-		struct sk_buff *head;
-		struct sk_buff *tail;
-		int len;
+		atomic_t	rmem_alloc;
+		int		len;
+		struct sk_buff	*head;
+		struct sk_buff	*tail;
 	} sk_backlog;
+#define sk_rmem_alloc sk_backlog.rmem_alloc
+	int			sk_forward_alloc;
+#ifdef CONFIG_RPS
+	__u32			sk_rxhash;
+#endif
+	atomic_t		sk_drops;
+	int			sk_rcvbuf;
+
+	struct sk_filter __rcu	*sk_filter;
 	struct socket_wq	*sk_wq;
-	struct dst_entry	*sk_dst_cache;
+
+#ifdef CONFIG_NET_DMA
+	struct sk_buff_head	sk_async_wait_queue;
+#endif
+
 #ifdef CONFIG_XFRM
 	struct xfrm_policy	*sk_policy[2];
 #endif
+	unsigned long 		sk_flags;
+	struct dst_entry	*sk_dst_cache;
 	spinlock_t		sk_dst_lock;
-	atomic_t		sk_rmem_alloc;
 	atomic_t		sk_wmem_alloc;
 	atomic_t		sk_omem_alloc;
 	int			sk_sndbuf;
-	struct sk_buff_head	sk_receive_queue;
 	struct sk_buff_head	sk_write_queue;
-#ifdef CONFIG_NET_DMA
-	struct sk_buff_head	sk_async_wait_queue;
-#endif
+	kmemcheck_bitfield_begin(flags);
+	unsigned int		sk_shutdown  : 2,
+				sk_no_check  : 2,
+				sk_userlocks : 4,
+				sk_protocol  : 8,
+				sk_type      : 16;
+	kmemcheck_bitfield_end(flags);
 	int			sk_wmem_queued;
-	int			sk_forward_alloc;
 	gfp_t			sk_allocation;
 	int			sk_route_caps;
 	int			sk_route_nocaps;
 	int			sk_gso_type;
 	unsigned int		sk_gso_max_size;
 	int			sk_rcvlowat;
-#ifdef CONFIG_RPS
-	__u32			sk_rxhash;
-#endif
-	unsigned long 		sk_flags;
 	unsigned long	        sk_lingertime;
 	struct sk_buff_head	sk_error_queue;
 	struct proto		*sk_prot_creator;
 	rwlock_t		sk_callback_lock;
 	int			sk_err,
 				sk_err_soft;
-	atomic_t		sk_drops;
 	unsigned short		sk_ack_backlog;
 	unsigned short		sk_max_ack_backlog;
 	__u32			sk_priority;
@@ -301,7 +309,6 @@ struct sock {
 	const struct cred	*sk_peer_cred;
 	long			sk_rcvtimeo;
 	long			sk_sndtimeo;
-	struct sk_filter __rcu	*sk_filter;
 	void			*sk_protinfo;
 	struct timer_list	sk_timer;
 	ktime_t			sk_stamp;



^ permalink raw reply related

* Re: Fwd: a Great Idea - include Kademlia networking protocol in kernel -- REVISITED
From: Honglei Cong @ 2010-11-16 16:11 UTC (permalink / raw)
  To: Marcos; +Cc: Eric Dumazet, netdev
In-Reply-To: <AANLkTimoZRYssbop-JhryUVae5zy+KFsxny9T4ssHnqE@mail.gmail.com>

On Tue, Nov 16, 2010 at 2:21 PM, Marcos <stalkingtime@gmail.com> wrote:
> [Eric Dumazet wrote:]
>> But we dont want a "super operating system". We want a good one.
>
> Yes, well you have that, I think, speaking at the kernel level.  But
> the thing is, people are building tools that mimic such anyway, and
> the next wave of new value will be found there.
>
>> Memory stores done in userland are as fast as memory stores done in
>> kernel.
>
> Really? And how about the abstraction-level?  because that will either
> make it lucrative or not for developers to build applications for
> it.....
>
>> Once you need to access files, perform complex searches, timers,
>> logging, and all the stuff, you really want to do it from userland, in
>> high level language that many programmers master, or get something that
>> is too complex/buggy.
>
> Yes, of course, all that will have to be considered.   But I'm
> suggesting that such a move is an investment in the future, that the
> the number of machines that will want or request peer-2-peer
> connectivity will (or should) only increase.  Done right, such a move
> should *simplify* things.  We're biased to think in centralized ways
> because of the centuries-old history of *who* has the resources.  But
> as networking, computation, and storage become commodified further,
> whole new topologies for the *right* architecture become available.
> The idea of "the OS" itself morphs.   And the *only* way maximize the
Agree with u.  But 'kernel' is not.

> value of the network is to make it easy to connect and communicate
> between peers -- what happens after that is so radical it can hardly
> be speculated because it gets into the realm of emergent complexity.
> Again, I refer you to Reed's law on the value of such networks.
>
> marcos
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ 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