LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: regarding pci  vga card output with powerpc
From: Andrey Volkov @ 2006-08-09 23:06 UTC (permalink / raw)
  To: urwithsudheer; +Cc: linuxppc-embedded
In-Reply-To: <44D7097C.7040907@gmail.com>

urwithsudheer wrote:
> Hello All,
> I am working with a pci based vga card. I have connected the vga card 
> next to the  pci slot of my target processor on the baseboard.
> I have connected a monitor to the vga card.
> When i boot linux from minicom, i could see all the kernel log messages 
> on both the minicom and vga card monitor. But on the vga monitor, i 
> could not see any messages after asking for login as below:
> .
> .
> 
> Mounting filesystems
> Running depmod
> Setting up networking on loopback device:
> Setting up networking on eth0:
> eth0: PHY is Marvell 88E11x1 (1410cc2)
> Adding static route for default gateway to 192.168.3.1:
> Setting nameserver to 192.168.0.1 in /etc/resolv.conf:
> Starting inetd:
> 
> login:
> 
> 
> Though i am entering my login and passwd at the minicom, nothing comes 
> on to the vga monitor. But i am able to write to vga monitor using the 
> following commands from minicom:
> 
> # echo "Hello" > /dev/tty1
> # echo "Hi " > /dev/tty1
> My requirement is to get on the vga monitor whatever i am typing on the 
> minicom after logging into the linux like .
> ls or pwd commands.
> 
> Anyone pl help me.
> 
> Thanks & Regards
> Sudheer
> 

See Documentation/kernel-parameters.txt.

If be short, add:

     console=/dev/ttySxx console=/dev/tty1

(both) to a kernel parameters string.

--
Regards
Andrey

^ permalink raw reply

* Re: SystemAce Driver.
From: sudheer @ 2006-08-10  3:06 UTC (permalink / raw)
  To: Frank D Lombardo; +Cc: linuxppc-embedded
In-Reply-To: <44DA40FF.90504@mdivac.com>

Hi Frank D Lombardo

Thanks for the reply.

I have done the same and  got the support files for system ace.

Thanks & Regards
Sudheer



Frank D Lombardo wrote:
> sudheer wrote:
>> Hello Ameet Patil
>>
>> I am looking for linux kernel source 2.6.16 with system ace 
>> controller support. I downloaded the linux-2.6.16 and linux-2.6.17-1 
>> source from kernel.org but could not find any files related to system 
>> ace controller  ( No xilinx_sysace directory in  drivers/block/) .  I 
>> have checked penguinppc.org also but could not get it.
>>
>> Can you please send to me the link where i could download the 
>> linuxppc-2.6.16 source with system ace support.
>>
>> Thanks & Regards
>> Sudheer
>>
>> Ameet Patil wrote:
>>> Hi Raja,
>>>     I have ported the Xilinx System ACE driver to 2.6 kernel. Find 
>>> the latest one here:
>>> http://www.cs.york.ac.uk/rtslab/demos/amos/xupv2pro/patches/linuxppc-2.6.17.1-sysace-1.2.patch 
>>>
>>>
>>> NOTE: this patch wouldn't work if you are using the TEMAC driver. In 
>>> which case use the -after-TEMAC patch found in the patches folder 
>>> above.
>>>
>>> Check the following discussions (threads) for more details:
>>> 1. "Xilinx SystemACE driver for 2.6"
>>> 2. "Xilinx BSP for linux 2.6"
>>> 3. "Kernel hangs after "Now booting the kernel"."
>>>
>>> cheers,
>>> -Ameet
>>>
>>> Raja Chidambaram wrote:
>>>  
>>>>  Hi all,
>>>>  We are working on customized board with amcc 440SPe
>>>> processor & xilinx System Ace controller. The System
>>>> Ace controller is connected to compact flash driver.
>>>>
>>>> We use u-boot 1.2 as bootloader & linux kernel
>>>> 2.6.16-2.
>>>>
>>>> On the process the u-boot is able to detect compact
>>>> flash through Xilinx SystemAce controller & able to
>>>> load the kernel image into compact flash.But when the
>>>> linux boot's up it not able to detect the System Ace
>>>> controller or compact flash.
>>>>
>>>> Note:we need to have the root file system in compact
>>>> flash.
>>>>
>>>> Is their any drivers available for SystemAce
>>>> controller on linux 2.6,if their how to get it.please
>>>> help me in this
>>>>                                     with regards
>>>>                                      raja
>>>>
>>>>
>>>>
>>>> __________________________________________________
>>>> Do You Yahoo!?
>>>> Tired of spam?  Yahoo! Mail has the best spam protection around 
>>>> http://mail.yahoo.com _______________________________________________
>>>> Linuxppc-embedded mailing list
>>>> Linuxppc-embedded@ozlabs.org
>>>> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>>>>
>>>>     
>>> _______________________________________________
>>> Linuxppc-embedded mailing list
>>> Linuxppc-embedded@ozlabs.org
>>> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>>>
>>>   
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Linuxppc-embedded mailing list
>> Linuxppc-embedded@ozlabs.org
>> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
> Sudheer,
>
> You need to download the SystemAce patch and apply it to the 2.6.17.1 
> Kernel.  The patch can be downloaded from: 
> http://www.cs.york.ac.uk/rtslab/demos/amos/xupv2pro/patches/linuxppc-2.6.17.1-sysace-1.2.patch 
>
>
> Save the patch to the root of the Kernel sources and run the following 
> command:
> patch -p1 < linuxppc-2.6.17.1-sysace-1.2.patch
>
> Frank
> <http://www.cs.york.ac.uk/rtslab/demos/amos/xupv2pro/patches/linuxppc-2.6.17.1-sysace-1.2.patch> 
>
>
> //
> //
>
>
>

^ permalink raw reply

* Re: regarding pci  vga card output with powerpc
From: sudheer @ 2006-08-10  5:08 UTC (permalink / raw)
  To: Andrey Volkov; +Cc: linuxppc-embedded
In-Reply-To: <44DA6A72.7020502@varma-el.com>

[-- Attachment #1: Type: text/plain, Size: 2862 bytes --]

Andrey Volkov wrote:
> urwithsudheer wrote:
>> Hello All,
>> I am working with a pci based vga card. I have connected the vga card 
>> next to the  pci slot of my target processor on the baseboard.
>> I have connected a monitor to the vga card.
>> When i boot linux from minicom, i could see all the kernel log 
>> messages on both the minicom and vga card monitor. But on the vga 
>> monitor, i could not see any messages after asking for login as below:
>> .
>> .
>>
>> Mounting filesystems
>> Running depmod
>> Setting up networking on loopback device:
>> Setting up networking on eth0:
>> eth0: PHY is Marvell 88E11x1 (1410cc2)
>> Adding static route for default gateway to 192.168.3.1:
>> Setting nameserver to 192.168.0.1 in /etc/resolv.conf:
>> Starting inetd:
>>
>> login:
>>
>>
>> Though i am entering my login and passwd at the minicom, nothing 
>> comes on to the vga monitor. But i am able to write to vga monitor 
>> using the following commands from minicom:
>>
>> # echo "Hello" > /dev/tty1
>> # echo "Hi " > /dev/tty1
>> My requirement is to get on the vga monitor whatever i am typing on 
>> the minicom after logging into the linux like .
>> ls or pwd commands.
>>
>> Anyone pl help me.
>>
>> Thanks & Regards
>> Sudheer
>>
>
> See Documentation/kernel-parameters.txt.
>
> If be short, add:
>
>     console=/dev/ttySxx console=/dev/tty1
>
> (both) to a kernel parameters string.
>
I got upto login prompt on the vga monitor only after giving the above 
parameters in my u-boot bootargs.
u-boot=> set bootargs console=tty1 console=ttyS0,115200.


I have added in the serial driver  linux/drivers/serial/serial_core.c
   in the function  uart_write(struct tty_struct *tty, const unsigned 
char * buf, int count)
   The lines in the if directive.
  
if (c <= 0)
                        break;
                memcpy(circ->buf + circ->head, buf, c);
/* added to test vga */
#if 1
                memcpy(tempbuf, circ->buf + circ->head, c);
                  for(i=0;i<c;i++)
                  printk("%c", tempbuf[i]);
#endif

 circ->head = (circ->head + c) & (UART_XMIT_SIZE - 1);
                buf += c;

Then as i expected, all the letters i enter in the minicom came twice on 
the minicom.

login:rroooott
Password: Password:
login  on `ttyS0'ogin  on `ttyS0'login[83]: root

 ~ # ~ # llss
 ~ # ~ # iiffccoonnffiigg

And whatever i enter on the minicom also gets print on the vga monitor  
but only once.
When i comment the if directive, On the vga monitor i could see only 
upto login prompt.

I don't know why the printk statement i added in the serial driver 
prints on the vga monitor.

As said by shkatikannan, i am going through the vga driver to copy to 
the frambuffer. But  yet couldn't get it exactly in the code where to copy.

Please tell me if i am in the wrong path.

Thanks & Regards
Sudheer





> -- 
> Regards
> Andrey
>
>


[-- Attachment #2: Type: text/html, Size: 4464 bytes --]

^ permalink raw reply

* Re: [PATCH 6/6] ehea: Kernel build (Kconfig / Makefile)
From: Michael Neuling @ 2006-08-10  5:30 UTC (permalink / raw)
  To: Jan-Bernd Themann; +Cc: Thomas Klein, linux-ppc, Christoph Raisch
In-Reply-To: <44D99F92.60407@de.ibm.com>

> --- linux-2.6.18-rc4/drivers/net/Makefile	2006-08-06 11:20:11.000000000 -
0700
> +++ patched_kernel/drivers/net/Makefile	2006-08-08 03:00:30.061451584 -
0700
> @@ -10,6 +10,7 @@ obj-$(CONFIG_E1000) += e1000/
>   obj-$(CONFIG_IBM_EMAC) += ibm_emac/
>   obj-$(CONFIG_IXGB) += ixgb/
>   obj-$(CONFIG_CHELSIO_T1) += chelsio/
> +obj-$(CONFIG_EHEA) += ehea/
>   obj-$(CONFIG_BONDING) += bonding/
>   obj-$(CONFIG_GIANFAR) += gianfar_driver.o

Looks like the whitespace has been munged here... 

Mikey

^ permalink raw reply

* Re: regarding pci  vga card output with powerpc
From: Andrey Volkov @ 2006-08-10  6:11 UTC (permalink / raw)
  To: sudheer; +Cc: linuxppc-embedded
In-Reply-To: <44DABF53.5020908@gmail.com>

sudheer wrote:
> Andrey Volkov wrote:
>> urwithsudheer wrote:
>>> Hello All,
>>> I am working with a pci based vga card. I have connected the vga card
>>> next to the  pci slot of my target processor on the baseboard.
>>> I have connected a monitor to the vga card.
>>> When i boot linux from minicom, i could see all the kernel log
>>> messages on both the minicom and vga card monitor. But on the vga
>>> monitor, i could not see any messages after asking for login as below:
>>> .
>>> .
>>>
>>> Mounting filesystems
>>> Running depmod
>>> Setting up networking on loopback device:
>>> Setting up networking on eth0:
>>> eth0: PHY is Marvell 88E11x1 (1410cc2)
>>> Adding static route for default gateway to 192.168.3.1:
>>> Setting nameserver to 192.168.0.1 in /etc/resolv.conf:
>>> Starting inetd:
>>>
>>> login:
>>>
>>>
>>> Though i am entering my login and passwd at the minicom, nothing
>>> comes on to the vga monitor. But i am able to write to vga monitor
>>> using the following commands from minicom:
>>>
>>> # echo "Hello" > /dev/tty1
>>> # echo "Hi " > /dev/tty1
>>> My requirement is to get on the vga monitor whatever i am typing on
>>> the minicom after logging into the linux like .
>>> ls or pwd commands.
>>>
>>> Anyone pl help me.
>>>
>>> Thanks & Regards
>>> Sudheer
>>>
>>
>> See Documentation/kernel-parameters.txt.
>>
>> If be short, add:
>>
>>     console=/dev/ttySxx console=/dev/tty1
>>
>> (both) to a kernel parameters string.
>>
> I got upto login prompt on the vga monitor only after giving the above
> parameters in my u-boot bootargs.
> u-boot=> set bootargs console=tty1 console=ttyS0,115200.
> 
> 
> I have added in the serial driver  linux/drivers/serial/serial_core.c
>    in the function  uart_write(struct tty_struct *tty, const unsigned
> char * buf, int count)
>    The lines in the if directive.
>   
> if (c <= 0)
>                         break;
>                 memcpy(circ->buf + circ->head, buf, c);
> /* added to test vga */
> #if 1
>                 memcpy(tempbuf, circ->buf + circ->head, c);
>                   for(i=0;i<c;i++)
>                   printk("%c", tempbuf[i]);
> #endif
> 
>  circ->head = (circ->head + c) & (UART_XMIT_SIZE - 1);
>                 buf += c;
> 
> Then as i expected, all the letters i enter in the minicom came twice on
> the minicom.

Because printk didn't print to an dedicated device, but to a klog
stream. So check your /etc/inittab and /etc/securetty, its similar
that something wrong in this files.

> 
> login:rroooott
> Password: Password:
> login  on `ttyS0'ogin  on `ttyS0'login[83]: root
> 
>  ~ # ~ # llss
>  ~ # ~ # iiffccoonnffiigg
> 
> And whatever i enter on the minicom also gets print on the vga monitor 
> but only once.
> When i comment the if directive, On the vga monitor i could see only
> upto login prompt.
> 
> I don't know why the printk statement i added in the serial driver
> prints on the vga monitor.
> 
> As said by shkatikannan, i am going through the vga driver to copy to
> the frambuffer. But  yet couldn't get it exactly in the code where to copy.
> 
> Please tell me if i am in the wrong path.
> 
> Thanks & Regards
> Sudheer
> 
-- 
Regards
Andrey

^ permalink raw reply

* Re: [PATCH 1/6] ehea: interface to network stack
From: Michael Ellerman @ 2006-08-10  6:15 UTC (permalink / raw)
  To: Jan-Bernd Themann
  Cc: Thomas Klein, netdev, linux-kernel, linux-ppc, Christoph Raisch,
	Marcus Eder
In-Reply-To: <44D99EFC.3000105@de.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 50528 bytes --]

Hi Jan-Bernd,

Comments below the code they refer to.

On Wed, 2006-08-09 at 10:38 +0200, Jan-Bernd Themann wrote: 
> Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com>

>   drivers/net/ehea/ehea_main.c | 2738 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 2738 insertions(+)

> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_main.c	1969-12-31 16:00:00.000000000 -0800
> +++ kernel/drivers/net/ehea/ehea_main.c	2006-08-08 23:59:39.683357016 -0700
> @@ -0,0 +1,2738 @@
> +/*
> + *  linux/drivers/net/ehea/ehea_main.c

Putting the file name in the file is fairly redundant IMHO.

> + *  eHEA ethernet device driver for IBM eServer System p

What's the actual hardware that this is for? System p covers a whole
range of machines, do they really all support this driver?

> + *  (C) Copyright IBM Corp. 2006
> + *
> + *  Authors:
> + *       Christoph Raisch <raisch@de.ibm.com>
> + *       Jan-Bernd Themann <themann@de.ibm.com>
> + *       Heiko-Joerg Schick <schickhj@de.ibm.com>
> + *       Thomas Klein <tklein@de.ibm.com>
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#define DEB_PREFIX "main"
> +
> +#include <linux/in.h>
> +#include <linux/ip.h>
> +#include <linux/tcp.h>
> +#include <linux/udp.h>
> +#include <linux/if.h>
> +#include <linux/list.h>
> +#include <net/ip.h>
> +
> +#include "ehea.h"
> +#include "ehea_qmr.h"
> +#include "ehea_phyp.h"
> +
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Christoph Raisch <raisch@de.ibm.com>");
> +MODULE_DESCRIPTION("IBM eServer HEA Driver");
> +MODULE_VERSION(EHEA_DRIVER_VERSION);
> +
> +static int __devinit ehea_probe(struct ibmebus_dev *dev,
> +				const struct of_device_id *id);
> +static int __devexit ehea_remove(struct ibmebus_dev *dev);
> +static int ehea_sense_port_attr(struct ehea_adapter *adapter, int portnum);

I haven't looked closely, but can you rearrange the functions so you
don't need these forward declarations?

> +int ehea_trace_level = 5;
> +
> +static struct net_device_stats *ehea_get_stats(struct net_device *dev)
> +{
> +	int i;
> +	u64 hret = H_HARDWARE;

You unconditionally assign to hret below.

> +	u64 rx_packets = 0;

Why not just update stats->rx_packets directly?

> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
> +	struct ehea_adapter *adapter = port->adapter;

I don't think you need adapter, you only use it in one place, just
access it through port->adapter->handle (below).

> +	struct hcp_query_ehea_port_cb_2 *cb2 = NULL;
> +	struct net_device_stats *stats = &port->stats;
> +
> +	EDEB_EN(7, "net_device=%p", dev);
> +
> +	cb2 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> +	if (!cb2) {
> +		EDEB_ERR(4, "No memory for cb2");
> +		goto get_stat_exit;

You leak cb2 here.

> +	}
> +
> +	hret = ehea_h_query_ehea_port(adapter->handle,
> +				      port->logical_port_id,
> +				      H_PORT_CB2,
> +				      H_PORT_CB2_ALL,
> +				      cb2);
> +
> +	if (hret != H_SUCCESS) {
> +		EDEB_ERR(4, "query_ehea_port failed for cb2");
> +		goto get_stat_exit;
> +	}
> +
> +	EDEB_DMP(7, (u8*)cb2,
> +		 sizeof(struct hcp_query_ehea_port_cb_2), "After HCALL");
> +
> +	for (i = 0; i < port->num_def_qps; i++) {
> +		rx_packets += port->port_res[i].rx_packets;
> +	}
> +
> +	stats->tx_packets = cb2->txucp + cb2->txmcp + cb2->txbcp;
> +	stats->multicast = cb2->rxmcp;
> +	stats->rx_errors = cb2->rxuerr;
> +	stats->rx_bytes = cb2->rxo;
> +	stats->tx_bytes = cb2->txo;
> +	stats->rx_packets = rx_packets;
> +
> +get_stat_exit:
> +	EDEB_EX(7, "");
> +	return stats;
> +}
> +
> +static inline u32 ehea_get_send_lkey(struct ehea_port_res *pr)
> +{
> +	return pr->send_mr.lkey;
> +}

Get rid of this, it's only used once.

> +static inline u32 ehea_get_recv_lkey(struct ehea_port_res *pr)
> +{
> +	return pr->recv_mr.lkey;
> +}

And this one only twice? Is it really useful?

> +
> +#define EHEA_OD_ADDR(address, segment) (((address) & (PAGE_SIZE - 1)) \
> +					| ((segment) << 12));
> +
> +static inline u64 get_swqe_addr(u64 tmp_addr, int addr_seg)
> +{
> +	u64 addr;
> +	addr = tmp_addr;
> +	return addr;
> +}
> +
> +static inline u64 get_rwqe_addr(u64 tmp_addr)
> +{
> +	return tmp_addr;
> +}

This two functions seem useless or wrong.

> +static inline int ehea_refill_rq1(struct ehea_port_res *port_res, int arr_index,
> +				  int nr_of_wqes)
> +{
> +	int i;
> +	int ret = 0;
> +	struct ehea_qp *qp;
> +	int skb_arr_rq1_len = port_res->skb_arr_rq1_len;
> +	struct sk_buff **skb_arr_rq1 = port_res->skb_arr_rq1;
> +	EDEB_EN(7, "port_res=%p, arr_index=%d, nr_of_wqes=%d, arr_rq1_len=%d",
> +		port_res, arr_index, nr_of_wqes, skb_arr_rq1_len);
> +
> +	qp = port_res->qp;

You don't need the qp variable, just use port_res->qp below.

> +	if (unlikely(nr_of_wqes == 0))
> +		return -EINVAL;

Newline needed here.

> +	for (i = 0; i < nr_of_wqes; i++) {
> +		int index = ((skb_arr_rq1_len + arr_index) - i)
> +		    % skb_arr_rq1_len;

The wrapped line should be indented further to the right.

> +		if (!skb_arr_rq1[index]) {
> +			skb_arr_rq1[index] = dev_alloc_skb(EHEA_LL_PKT_SIZE);
> +
> +			if (!skb_arr_rq1[index]) {
> +				EDEB_ERR(4, "No mem for skb/%d wqes filled", i);
> +				ret = -ENOMEM;
> +				break;
> +			}
> +		}
> +	}
> +	/* Ring doorbell */
> +	ehea_update_rq1a(qp, nr_of_wqes);
> +	EDEB_EX(7, "");
> +	return ret;
> +}
> +
> +static int ehea_init_fill_rq1(struct ehea_port_res *port_res, int nr_rq1a)
> +{
> +	int i;
> +	int ret = 0;
> +	struct ehea_qp *qp;
> +	EDEB_EN(7, "port_res=%p, nr_rq1a=%d", port_res, nr_rq1a);
> +	qp = port_res->qp;
> +
> +	for (i = 0; i < port_res->skb_arr_rq1_len; i++) {
> +		port_res->skb_arr_rq1[i] = dev_alloc_skb(EHEA_LL_PKT_SIZE);
> +		if (!port_res->skb_arr_rq1[i]) {
> +			EDEB_ERR(4, "dev_alloc_skb failed. Only %d skb filled.",
> +				 i);
> +			ret = -ENOMEM;
> +			break;
> +		}
> +	}
> +	/* Ring doorbell */
> +	ehea_update_rq1a(qp, nr_rq1a);
> +	EDEB_EX(7, "");
> +	return ret;
> +}
> +
> +static inline int ehea_refill_rq2_def(struct ehea_port_res *pr, int nr_of_wqes)
> +{
> +	int i;
> +	int ret = 0;
> +	struct ehea_qp *qp;
> +	struct ehea_rwqe *rwqe;
> +	int skb_arr_rq2_len = pr->skb_arr_rq2_len;
> +	struct sk_buff **skb_arr_rq2 = pr->skb_arr_rq2;
> +
> +	EDEB_EN(8, "pr=%p, nr_of_wqes=%d", pr, nr_of_wqes);
> +	if (nr_of_wqes == 0)
> +		return -EINVAL;
> +	qp = pr->qp;
> +	for (i = 0; i < nr_of_wqes; i++) {
> +		int index = pr->skb_rq2_index++;
> +		struct sk_buff *skb = dev_alloc_skb(EHEA_RQ2_PKT_SIZE
> +						    + NET_IP_ALIGN);
> +
> +		if (!skb) {
> +			EDEB_ERR(4, "dev_alloc_skb nr %d failed", i);
> +			ret = -ENOMEM;
> +			break;
> +		}
> +		skb_reserve(skb, NET_IP_ALIGN);
> +		pr->skb_rq2_index %= skb_arr_rq2_len;
> +		skb_arr_rq2[index] = skb;
> +		rwqe = ehea_get_next_rwqe(qp, 2);
> +		rwqe->wr_id = EHEA_BMASK_SET(EHEA_WR_ID_TYPE, EHEA_RWQE2_TYPE)
> +		            | EHEA_BMASK_SET(EHEA_WR_ID_INDEX, index);
> +		rwqe->sg_list[0].l_key = ehea_get_recv_lkey(pr);
> +		rwqe->sg_list[0].vaddr = get_rwqe_addr((u64)skb->data);
> +		rwqe->sg_list[0].len = EHEA_RQ2_PKT_SIZE;
> +		rwqe->data_segments = 1;
> +	}
> +
> +	/* Ring doorbell */
> +	iosync();
> +	ehea_update_rq2a(qp, i);
> +	EDEB_EX(8, "");
> +	return ret;
> +}
> +
> +
> +static inline int ehea_refill_rq2(struct ehea_port_res *pr, int nr_of_wqes)
> +{
> +	return ehea_refill_rq2_def(pr, nr_of_wqes);
> +}

Why do you need this function?

> +static inline int ehea_refill_rq3_def(struct ehea_port_res *pr, int nr_of_wqes)
> +{
> +	int i;
> +	int ret = 0;
> +	struct ehea_qp *qp;
> +	struct ehea_rwqe *rwqe;
> +	int skb_arr_rq3_len = pr->skb_arr_rq3_len;
> +	struct sk_buff **skb_arr_rq3 = pr->skb_arr_rq3;
> +	EDEB_EN(8, "pr=%p, nr_of_wqes=%d", pr, nr_of_wqes);
> +	if (nr_of_wqes == 0)
> +		return -EINVAL;
> +	qp = pr->qp;
> +	for (i = 0; i < nr_of_wqes; i++) {
> +		int index = pr->skb_rq3_index++;
> +		struct sk_buff *skb = dev_alloc_skb(EHEA_MAX_PACKET_SIZE
> +						    + NET_IP_ALIGN);
> +
> +		if (!skb) {
> +			EDEB_ERR(4, "No memory for skb. Only %d rwqe filled.",
> +				 i);
> +			ret = -ENOMEM;
> +			break;
> +		}
> +		skb_reserve(skb, NET_IP_ALIGN);
> +
> +		rwqe = ehea_get_next_rwqe(qp, 3);
> +		pr->skb_rq3_index %= skb_arr_rq3_len;
> +		skb_arr_rq3[index] = skb;
> +		rwqe->wr_id = EHEA_BMASK_SET(EHEA_WR_ID_TYPE, EHEA_RWQE3_TYPE)
> +		    | EHEA_BMASK_SET(EHEA_WR_ID_INDEX, index);
> +		rwqe->sg_list[0].l_key = ehea_get_recv_lkey(pr);
> +		rwqe->sg_list[0].vaddr = get_rwqe_addr((u64)skb->data);
> +		rwqe->sg_list[0].len = EHEA_MAX_PACKET_SIZE;
> +		rwqe->data_segments = 1;
> +	}
> +
> +	/* Ring doorbell */
> +	iosync();
> +	ehea_update_rq3a(qp, i);
> +	EDEB_EX(8, "");
> +	return ret;
> +}

You "Ring doorbell" in four places, but only in two do you do iosync(),
that might be ok, but it looks suspicious. Also, what is the iosync
trying to achieve?

ehea_refill_rq3_def() and ehea_refill_rq2_def() look quite similar, can
they be combined?

> +
> +
> +static inline int ehea_refill_rq3(struct ehea_port_res *pr, int nr_of_wqes)
> +{
> +	return ehea_refill_rq3_def(pr, nr_of_wqes);
> +}
> +
> +
> +static inline int ehea_check_cqe(struct ehea_cqe *cqe, int *rq_num)
> +{
> +	*rq_num = (cqe->type & EHEA_CQE_TYPE_RQ) >> 5;
> +	EDEB(7, "RQ used=%d, status=%X", *rq_num, cqe->status);
> +	if ((cqe->status & EHEA_CQE_STAT_ERR_MASK) == 0)
> +		return 0;
> +	if (((cqe->status & EHEA_CQE_STAT_ERR_TCP) != 0)
> +	    && (cqe->header_length == 0))
> +		return 0;
> +	else
> +		printk("WARNING: Packet discarded. Wrong TCP/UDP chksum"
> +		       "and header_length != 0. cqe->status=%X", cqe->status);
> +
> +	return -EINVAL;
> +}
> +
> +static inline void ehea_fill_skb_ll(struct net_device *dev,
> +				    struct sk_buff *skb, struct ehea_cqe *cqe)
> +{
> +	int length = cqe->num_bytes_transfered - 4;	/*remove CRC */
> +	EDEB_EN(7, "dev=%p, skb=%p, cqe=%p", dev, skb, cqe);
> +	memcpy(skb->data, ((char*)cqe) + 64, length);
> +	skb_put(skb, length);
> +	skb->dev = dev;
> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
> +	skb->protocol = eth_type_trans(skb, dev);
> +	EDEB_EX(7, "");
> +}
> +
> +static inline void ehea_fill_skb(struct net_device *dev,
> +				 struct sk_buff *skb, struct ehea_cqe *cqe)
> +{
> +	int length = cqe->num_bytes_transfered - 4;	/*remove CRC */
> +	EDEB_EN(7, "dev=%p, skb=%p, cqe=%p", dev, skb, cqe);
> +	skb_put(skb, length);
> +	skb->dev = dev;
> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
> +	skb->protocol = eth_type_trans(skb, dev);
> +	EDEB_EX(7, "");
> +}

What's the difference between the above two functions?

> +#define EHEA_MAX_RWQE 1000

Should probably be in a header.

> +static int ehea_poll(struct net_device *dev, int *budget)
> +{
> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
> +	struct ehea_port_res *port_res = &port->port_res[0];
> +	struct ehea_cqe *cqe;
> +	struct ehea_qp *qp = port_res->qp;
> +	int wqe_index = 0;
> +	int last_wqe_index = 0;
> +	int x = 0;
> +	int processed = 0;
> +	int processed_RQ1 = 0;
> +	int processed_RQ2 = 0;
> +	int processed_RQ3 = 0;
> +	int rq;
> +	int intreq;
> +	struct sk_buff **skb_arr_rq1 = port_res->skb_arr_rq1;
> +	struct sk_buff **skb_arr_rq2 = port_res->skb_arr_rq2;
> +	struct sk_buff **skb_arr_rq3 = port_res->skb_arr_rq3;
> +	int skb_arr_rq1_len = port_res->skb_arr_rq1_len;
> +	int my_quota = min(*budget, dev->quota);
> +
> +	EDEB_EN(7, "dev=%p, port_res=%p, budget=%d, quota=%d, qp_nr=%x",
> +		dev, port_res, *budget, dev->quota,
> +		port_res->qp->init_attr.qp_nr);
> +	my_quota = min(my_quota, EHEA_MAX_RWQE);
> +
> +	/* rq0 is low latency RQ */
> +	cqe = ehea_poll_rq1(qp, &wqe_index);
> +	while ((my_quota > 0) && cqe) {
> +		ehea_inc_rq1(qp);
> +		processed_RQ1++;
> +		processed++;
> +		my_quota--;
> +
> +		EDEB_DMP(6, (u8*)cqe, 4 * 16, "CQE");
> +		last_wqe_index = wqe_index;
> +		rmb();
> +		if (!ehea_check_cqe(cqe, &rq)) {
> +			struct sk_buff *skb;
> +			if (rq == 1) {	/* LL RQ1 */
> +				void *pref;
> +
> +				x = (wqe_index + 1) % skb_arr_rq1_len;
> +				pref = (void*)skb_arr_rq1[x];
> +				prefetchw(pref);
> +				prefetchw(pref + EHEA_CACHE_LINE);
> +
> +				x = (wqe_index + 1) % skb_arr_rq1_len;
> +				pref = (void*)(skb_arr_rq1[x]->data);
> +				prefetchw(pref);
> +				prefetchw(pref + EHEA_CACHE_LINE);
> +
> +				skb = skb_arr_rq1[wqe_index];
> +				if (unlikely(!skb)) {
> +					EDEB_ERR(4, "LL SBK=NULL, wqe_index=%d",
> +						 wqe_index);
> +					skb = dev_alloc_skb(EHEA_LL_PKT_SIZE);
> +					if (!skb)
> +						panic("Alloc SKB failed");
> +				}
> +				skb_arr_rq1[wqe_index] = NULL;
> +				ehea_fill_skb_ll(dev, skb, cqe);
> +			} else if (rq == 2) {	/* RQ2 */
> +				void *pref;
> +				int skb_index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
> +							       cqe->wr_id);
> +				x = (skb_index + 1) % port_res->skb_arr_rq2_len;
> +				pref = (void*)skb_arr_rq2[x];
> +				prefetchw(pref);
> +				prefetchw(pref + EHEA_CACHE_LINE);
> +
> +				x = (skb_index + 1) % port_res->skb_arr_rq2_len;
> +				pref = (void*)(skb_arr_rq2[x]->data);
> +
> +				prefetch(pref);
> +				prefetch(pref + EHEA_CACHE_LINE);
> +				prefetch(pref + EHEA_CACHE_LINE * 2);
> +				prefetch(pref + EHEA_CACHE_LINE * 3);
> +				skb = skb_arr_rq2[skb_index];
> +
> +				if (unlikely(!skb)) {
> +					EDEB_ERR(4, "rq2: SKB=NULL, index=%d",
> +						 skb_index);
> +					break;
> +				}
> +				skb_arr_rq2[skb_index] = NULL;
> +				ehea_fill_skb(dev, skb, cqe);
> +				processed_RQ2++;
> +			} else {
> +				void *pref;
> +				int skb_index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
> +							       cqe->wr_id);
> +				x = (skb_index + 1) % port_res->skb_arr_rq3_len;
> +				pref = (void*)skb_arr_rq3[x];
> +				prefetchw(pref);
> +				prefetchw(pref + EHEA_CACHE_LINE);
> +
> +				x = (skb_index + 1) % port_res->skb_arr_rq3_len;
> +				pref = (void*)(skb_arr_rq3[x]->data);
> +				prefetch(pref);
> +				prefetch(pref + EHEA_CACHE_LINE);
> +				prefetch(pref + EHEA_CACHE_LINE * 2);
> +				prefetch(pref + EHEA_CACHE_LINE * 3);
> +
> +				skb = skb_arr_rq3[skb_index];
> +				if (unlikely(!skb)) {
> +					EDEB_ERR(4, "rq3: SKB=NULL, index=%d",
> +						 skb_index);
> +					break;
> +				}
> +				skb_arr_rq3[skb_index] = NULL;
> +				ehea_fill_skb(dev, skb, cqe);
> +				processed_RQ3++;
> +			}
> +
> +			EDEB(6, "About to pass SKB: dev=%p\n"
> +			     "skb=%p skb->data=%p skb->len=%d"
> +			     " skb->data_len=0x%x nr_frags=%d",
> +			     dev,
> +			     skb,
> +			     skb->data,
> +			     skb->len,
> +			     skb->data_len, skb_shinfo(skb)->nr_frags);
> +			if (cqe->status & EHEA_CQE_VLAN_TAG_XTRACT) {
> +				EDEB(7, "VLAN TAG extracted: %4x, vgrp=%p",
> +				     cqe->vlan_tag, port->vgrp);
> +				EDEB(7, "vlan_devices[vlan_tag]=%p",
> +				     port->vgrp->vlan_devices[cqe->vlan_tag]);
> +				vlan_hwaccel_receive_skb(skb, port->vgrp,
> +							 cqe->vlan_tag);
> +			} else {
> +				EDEB(7, "netif_receive_skb");
> +				netif_receive_skb(skb);
> +			}
> +			EDEB(7, "SKB passed (netif_receive(skb) called)");
> +
> +		} else {
> +			struct sk_buff *skb;
> +
> +			EDEB_ERR(4, "cqe->status indicating error: CQE:");
> +			EDEB_DMP(4, (u8*)cqe, 4 * 16, "");
> +			if (rq == 2) {
> +				processed_RQ2++;
> +				skb = skb_arr_rq2[
> +					EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
> +							  cqe->wr_id)];
> +				skb_arr_rq2[EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
> +							  cqe->wr_id)] = NULL;
> +				dev_kfree_skb(skb);
> +			}
> +			if (rq == 3) {
> +				processed_RQ3++;
> +				skb = skb_arr_rq3[
> +					EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
> +								cqe->wr_id)];
> +				skb_arr_rq3[EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
> +							  cqe->wr_id)] = NULL;
> +				dev_kfree_skb(skb);
> +			}
> +		}
> +		cqe = ehea_poll_rq1(qp, &wqe_index);
> +	}
> +
> +	dev->quota -= processed;
> +	*budget -= processed;
> +
> +	port_res->p_state.ehea_poll += 1;
> +
> +	port_res->rx_packets += processed;
> +
> +	ehea_refill_rq1(port_res, last_wqe_index, processed_RQ1);
> +	ehea_refill_rq2(port_res, processed_RQ2);
> +	ehea_refill_rq3(port_res, processed_RQ3);
> +
> +	intreq = ((port_res->p_state.ehea_poll & 0xF) == 0xF);
> +
> +	EDEB_EX(7, "processed=%d, *budget=%d, dev->quota=%d",
> +		processed, *budget, dev->quota);
> +
> +	if (!cqe || intreq) {
> +		netif_rx_complete(dev);
> +		ehea_reset_cq_ep(port_res->recv_cq);
> +		ehea_reset_cq_n1(port_res->recv_cq);
> +		cqe = ipz_qeit_get_valid(&qp->ipz_rqueue1);
> +		EDEB(7, "CQE=%p, break ehea_poll while loop", cqe);
> +		if (!cqe || intreq)
> +			return 0;
> +		if (!netif_rx_reschedule(dev, my_quota))
> +			return 0;
> +	}
> +	return 1;
> +}

This function is _way_ too big.

> +#define MAX_SENDCOMP_QUOTA 400
> +void ehea_send_irq_tasklet(unsigned long data)
> +{
> +	int quota = MAX_SENDCOMP_QUOTA;
> +	int skb_index;
> +	int cqe_counter = 0;
> +	int swqe_av = 0;
> +	unsigned long flags;
> +	struct sk_buff *skb;
> +	struct ehea_cqe *cqe;
> +	struct ehea_port_res *port_res = (struct ehea_port_res*)data;
> +	struct ehea_cq *send_cq = port_res->send_cq;
> +	struct net_device *dev = port_res->port->netdev;
> +
> +	EDEB_EN(7, "port_res=%p", port_res);
> +
> +	do {
> +		cqe = ehea_poll_cq(send_cq);
> +		if (!cqe) {
> +			EDEB(7, "No further cqe found");
> +			ehea_reset_cq_ep(send_cq);
> +			ehea_reset_cq_n1(send_cq);
> +			cqe = ehea_poll_cq(send_cq);
> +			if (!cqe) {
> +				EDEB(7, "No cqe found after having"
> +				     " reset N1/EP\n");
> +				break;
> +			}
> +		}
> +		cqe_counter++;
> +		EDEB(7, "CQE found on Send-CQ:");
> +		EDEB_DMP(7, (u8*)cqe, 4 * 16, "");
> +		rmb();
> +		if (likely(EHEA_BMASK_GET(EHEA_WR_ID_TYPE, cqe->wr_id)
> +			   == EHEA_SWQE2_TYPE)) {	/* is swqe format 2 */
> +			int i;
> +			int index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
> +						     cqe->wr_id);
> +			for (i = 0; i < EHEA_BMASK_GET(EHEA_WR_ID_REFILL,
> +						       cqe->wr_id); i++) {
> +
> +				skb_index = ((index - i
> +					      + port_res->skb_arr_sq_len)
> +					     % port_res->skb_arr_sq_len);
> +				skb = port_res->skb_arr_sq[skb_index];
> +				port_res->skb_arr_sq[skb_index] = NULL;
> +
> +				if (unlikely(!skb)) {
> +					EDEB_ERR(4, "s_irq_tasklet: SKB=NULL "
> +						 "WQ_ID=%lX, loop=%d, index=%d",
> +						 cqe->wr_id, i, skb_index);
> +					break;
> +				}
> +				dev_kfree_skb(skb);
> +			}
> +		}
> +		swqe_av += EHEA_BMASK_GET(EHEA_WR_ID_REFILL, cqe->wr_id);
> +		quota--;
> +	} while (quota > 0);
> +
> +	ehea_update_feca(send_cq, cqe_counter);
> +
> +	atomic_add(swqe_av, &port_res->swqe_avail);
> +	EDEB(7, "port_res->swqe_avail=%d",
> +	     atomic_read(&port_res->swqe_avail));
> +
> +	if (unlikely(netif_queue_stopped(dev))) {
> +		spin_lock_irqsave(&port_res->netif_queue, flags);
> +		if (unlikely((atomic_read(&port_res->swqe_avail)
> +			      >= EHEA_SWQE_REFILL_TH))) {
> +			EDEB(7, "port %d swqes_avail >=10 (%d),"
> +			     "netif_wake_queue called",
> +			     port_res->port->logical_port_id,
> +			     atomic_read(&port_res->swqe_avail));
> +			netif_wake_queue(port_res->port->netdev);
> +		}
> +		spin_unlock_irqrestore(&port_res->netif_queue, flags);
> +	}
> +
> +	if (unlikely(cqe))
> +		tasklet_hi_schedule(&port_res->send_comp_task);
> +
> +	EDEB_EX(7, "");
> +}

This one's pretty big too.

> +
> +irqreturn_t ehea_send_irq_handler(int irq, void *param, struct pt_regs *regs)
> +{
> +	struct ehea_port_res *pr = (struct ehea_port_res*)param;
> +	EDEB_EN(7, "irq=%d, param=%p, pt_regs=%p", irq, param, regs);
> +	tasklet_hi_schedule(&pr->send_comp_task);
> +	EDEB_EX(7, "");
> +	return IRQ_HANDLED;
> +}
> +
> +irqreturn_t ehea_recv_irq_handler(int irq, void *param, struct pt_regs * regs)
> +{
> +	struct ehea_port_res *pr = (struct ehea_port_res*)param;
> +	struct ehea_port *port = pr->port;
> +	EDEB_EN(7, "irq=%d, param=%p, pt_regs=%p", irq, param, regs);
> +	netif_rx_schedule(port->netdev);
> +	EDEB_EX(7, "");
> +	return IRQ_HANDLED;
> +}
> +
> +irqreturn_t ehea_qp_aff_irq_handler(int irq, void *param, struct pt_regs * regs)
> +{
> +	struct ehea_port *port = (struct ehea_port*)param;
> +	struct ehea_eqe *eqe;
> +	u32 qp_token;
> +
> +	EDEB_EN(7, "irq=%d, param=%p, pt_regs=%p", irq, param, regs);
> +	eqe = (struct ehea_eqe*)ehea_poll_eq(port->qp_eq);

Suspicious cast?

> +	EDEB(7, "eqe=%p", eqe);
> +	while (eqe) {
> +		EDEB(7, "*eqe=%lx", *(u64*)eqe);
> +		eqe = (struct ehea_eqe*)ehea_poll_eq(port->qp_eq);
> +		qp_token = EHEA_BMASK_GET(EHEA_EQE_QP_TOKEN, eqe->entry);

You don't seem to use qp_token?

> +		EDEB(7, "next eqe=%p", eqe);
> +	}
> +
> +	EDEB_EX(7, "");
> +	return IRQ_HANDLED;
> +}
> +
> +static struct ehea_port *ehea_get_port(struct ehea_adapter *adapter,
> +				       int logical_port)
> +{
> +	int i;
> +
> +	for (i = 0; i < adapter->num_ports; i++)
> +		if (adapter->port[i]->logical_port_id == logical_port)
> +			return adapter->port[i];
> +	return NULL;
> +}
> +
> +static void ehea_parse_eqe(struct ehea_adapter *adapter, u64 eqe)
> +{
> +	int ret = -EINVAL;
> +	u8 ec = 0;
> +	u8 portnum = 0;
> +	struct ehea_port *port = NULL;

You don't need to initialise these three variables.

> +
> +	EDEB_EN(7, "eqe=%lx", eqe);
> +
> +	ec = EHEA_BMASK_GET(NEQE_EVENT_CODE, eqe);
> +
> +	switch (ec) {
> +	case EHEA_EC_PORTSTATE_CHG:	/* port state change */
> +		EDEB(7, "Port state change");
> +		portnum = EHEA_BMASK_GET(NEQE_PORTNUM, eqe);
> +		port = ehea_get_port(adapter, portnum);
> +
> +		if (!port) {
> +			EDEB_ERR(4, "Unknown portnum %x", portnum);
> +			break;
> +		}
> +
> +		if (EHEA_BMASK_GET(NEQE_PORT_UP, eqe)) {
> +			if (!netif_carrier_ok(port->netdev)) {
> +				ret = ehea_sense_port_attr(adapter, portnum);
> +				if (ret) {
> +					EDEB_ERR(4, "Failed resensing port");
> +					break;
> +				}
> +
> +				printk("%s: Logical port up: %dMbps %s Duplex",
> +				       port->netdev->name,
> +				       port->port_speed,
> +				       port->full_duplex ==
> +				       1 ? "Full" : "Half");
> +
> +				netif_carrier_on(port->netdev);
> +				netif_wake_queue(port->netdev);
> +			}
> +		} else
> +			if (netif_carrier_ok(port->netdev)) {
> +				printk("%s: Logical port down",
> +				       port->netdev->name);
> +				netif_carrier_off(port->netdev);
> +				netif_stop_queue(port->netdev);
> +			}
> +
> +		if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PORT_UP, eqe))
> +			printk("%s: Physical port up", port->netdev->name);
> +		else
> +			printk("%s: Physical port down", port->netdev->name);
> +
> +		if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PRIMARY, eqe))
> +			printk("Externel switch port is primary port");
> +		else
> +			printk("Externel switch port is backup port");

These printks should probably use KERN_DEBUG, or netif_msg_foo().

> +		break;
> +	case EHEA_EC_ADAPTER_MALFUNC:	/* adapter malfunction */
> +		EDEB_ERR(4, "Adapter malfunction");
> +		break;
> +	case EHEA_EC_PORT_MALFUNC:	/* port malfunction */
> +		EDEB_ERR(4, "Port malfunction");
> +		/* TODO Determine the port structure of the malfunctioning port
> +		 * netif_carrier_off(port->netdev);
> +		 * netif_stop_queue(port->netdev);
> +		 */
> +		break;

The two malfunction states could probably use a printk(KERN_WARNING .. )
for now.

> +	default:
> +		EDEB_ERR(4, "Unknown event code %x", ec);
> +		break;
> +	}
> +
> +	EDEB_EX(7, "");
> +}
> +
> +void ehea_neq_tasklet(unsigned long data)
> +{
> +	struct ehea_adapter *adapter = (struct ehea_adapter*)data;
> +	struct ehea_eqe *eqe;
> +	u64 event_mask;
> +
> +	EDEB_EN(7, "");
> +	eqe = (struct ehea_eqe*)ehea_poll_eq(adapter->neq);
> +	EDEB(7, "eqe=%p", eqe);
> +
> +	while (eqe) {
> +		EDEB(7, "*eqe=%lx", eqe->entry);
> +		ehea_parse_eqe(adapter, eqe->entry);
> +		eqe = (struct ehea_eqe*)ehea_poll_eq(adapter->neq);
> +		EDEB(7, "next eqe=%p", eqe);
> +	}

I think I've seen this code before.

> +
> +	event_mask = EHEA_BMASK_SET(NELR_PORTSTATE_CHG, 1)
> +		   | EHEA_BMASK_SET(NELR_ADAPTER_MALFUNC, 1)
> +		   | EHEA_BMASK_SET(NELR_PORT_MALFUNC, 1);
> +
> +	ehea_h_reset_events(adapter->handle,
> +			    adapter->neq->ipz_eq_handle, event_mask);
> +	EDEB_EX(7, "");
> +}
> +
> +irqreturn_t ehea_interrupt_neq(int irq, void *param, struct pt_regs *regs)
> +{
> +	struct ehea_adapter *adapter = (struct ehea_adapter*)param;
> +	EDEB_EN(7, "dev_id=%p", adapter);
> +	tasklet_hi_schedule(&adapter->neq_tasklet);
> +	EDEB_EX(7, "");
> +	return IRQ_HANDLED;
> +}
> +
> +
> +
> +static void ehea_fill_port_res(struct ehea_port_res *pr)
> +{
> +	struct ehea_qp_init_attr *init_attr = &pr->qp->init_attr;
> +
> +	/* RQ 1 */
> +	ehea_init_fill_rq1(pr, init_attr->act_nr_rwqes_rq1
> +			   - init_attr->act_nr_rwqes_rq2
> +			   - init_attr->act_nr_rwqes_rq3 - 1);
> +
> +	/* RQ 2 */
> +	ehea_refill_rq2(pr, init_attr->act_nr_rwqes_rq2);
> +
> +	/* RQ 3 */
> +	ehea_refill_rq3(pr, init_attr->act_nr_rwqes_rq3);
> +}
> +
> +static int ehea_reg_interrupts(struct net_device *dev)
> +{
> +	int ret = -EINVAL;
> +	int i;
> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
> +	struct ehea_port_res *pr = &port->port_res[0];

Don't need to initalise pr.

> +
> +	EDEB_EN(7, "");
> +	for (i = 0; i < port->num_def_qps; i++) {
> +		pr = &port->port_res[i];
> +		snprintf(pr->int_recv_name, EHEA_IRQ_NAME_SIZE - 1
> +			 , "%s-recv%d", dev->name, i);
> +		ret = ibmebus_request_irq(NULL,
> +					  pr->recv_eq->attr.ist1,
> +					  ehea_recv_irq_handler,
> +					  SA_INTERRUPT, pr->int_recv_name, pr);
> +		if (ret) {
> +			EDEB_ERR(4, "Failed registering irq for ehea_recv_int:"
> +				 "port_res_nr:%d, ist=%X", i,
> +				 pr->recv_eq->attr.ist1);
> +			goto failure;
> +		}
> +		EDEB(7, "irq_handle 0x%X for function ehea_recv_int %d "
> +		     " registered", pr->recv_eq->attr.ist1, i);
> +	}
> +
> +	snprintf(port->int_aff_name, EHEA_IRQ_NAME_SIZE - 1,
> +		 "%s-aff", dev->name);
> +	ret = ibmebus_request_irq(NULL,
> +				  port->qp_eq->attr.ist1,
> +				  ehea_qp_aff_irq_handler,
> +				  SA_INTERRUPT, port->int_aff_name, port);
> +	if (ret) {
> +		EDEB_ERR(4, "Failed registering irq for qp_aff_irq_handler:"
> +			 " ist=%X", port->qp_eq->attr.ist1);
> +		goto failure;
> +	}
> +	EDEB(7, "irq_handle 0x%X for function qp_aff_irq_handler registered",
> +	     port->qp_eq->attr.ist1);
> +
> +	for (i = 0; i < port->num_def_qps + port->num_tx_qps; i++) {
> +		pr = &port->port_res[i];
> +		snprintf(pr->int_send_name, EHEA_IRQ_NAME_SIZE - 1,
> +			 "%s-send%d", dev->name, i);
> +		ret = ibmebus_request_irq(NULL,
> +					  pr->send_eq->attr.ist1,
> +					  ehea_send_irq_handler,
> +					  SA_INTERRUPT, pr->int_send_name,
> +					  pr);
> +		if (ret) {
> +			EDEB_ERR(4, "Registering irq for ehea_send failed"
> +				 " port_res_nr:%d, ist=%X", i,
> +				 pr->send_eq->attr.ist1);
> +			goto failure;
> +		}
> +		EDEB(7, "irq_handle 0x%X for function ehea_send_int %d"
> +		     " registered", pr->send_eq->attr.ist1, i);
> +	}

You seem to be iterating over some of the port_res structs twice, is
that what you intended?

> +failure:
> +	EDEB_EX(7, "ret=%d", ret);
> +	return ret;

No further cleanup required? Should you be calling ehea_free_interrupts?

> +}
> +
> +static void ehea_free_interrupts(struct net_device *dev)
> +{
> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
> +	int i;
> +
> +	EDEB_EN(7, "");
> +	/* send */
> +	for (i = 0; i < port->num_def_qps + port->num_tx_qps; i++) {
> +		ibmebus_free_irq(NULL, port->port_res[i].send_eq->attr.ist1,
> +				 &port->port_res[i]);
> +		EDEB(7, "free send interrupt for res %d with handle 0x%X",
> +		     i, port->port_res[i].send_eq->attr.ist1);
> +	}
> +
> +	/* receive */
> +	for (i = 0; i < port->num_def_qps; i++) {
> +		ibmebus_free_irq(NULL, port->port_res[i].recv_eq->attr.ist1,
> +				 &port->port_res[i]);
> +		EDEB(7, "free recv intterupt for res %d with handle 0x%X",
> +		     i, port->port_res[i].recv_eq->attr.ist1);
> +	}
> +	/* associated events */
> +	ibmebus_free_irq(NULL, port->qp_eq->attr.ist1, port);
> +	EDEB(7, "associated event interrupt for handle 0x%X freed",
> +	     port->qp_eq->attr.ist1);
> +	EDEB_EX(7, "");
> +}
> +
> +static int ehea_configure_port(struct ehea_port *port)
> +{
> +	int ret = -EINVAL;
> +	u64 hret = H_HARDWARE;
> +	struct hcp_query_ehea_port_cb_0 *ehea_port_cb_0 = NULL;
> +	u64 mask = 0;
> +	int i;
> +
> +	EDEB_EN(7, "");
> +
> +	ehea_port_cb_0 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> +
> +	if (!ehea_port_cb_0) {
> +		EDEB_ERR(4, "No memory for ehea_port control block");
> +		ret = -ENOMEM;
> +		goto kzalloc_failed;
> +	}
> +
> +	ehea_port_cb_0->port_rc = EHEA_BMASK_SET(PXLY_RC_VALID, 1)
> +				| EHEA_BMASK_SET(PXLY_RC_IP_CHKSUM, 1)
> +				| EHEA_BMASK_SET(PXLY_RC_TCP_UDP_CHKSUM, 1)
> +                		| EHEA_BMASK_SET(PXLY_RC_VLAN_XTRACT, 1)
> +                		| EHEA_BMASK_SET(PXLY_RC_VLAN_TAG_FILTER,
> +				                 PXLY_RC_VLAN_FILTER)
> +				| EHEA_BMASK_SET(PXLY_RC_JUMBO_FRAME, 1);
> +
> +	for (i = 0; i < port->num_def_qps; i++) {
> +		ehea_port_cb_0->default_qpn_array[i] =
> +		    port->port_res[i].qp->init_attr.qp_nr;
> +
> +		EDEB(7, "default_qpn_array[%d]=%d",
> +		     i, port->port_res[i].qp->init_attr.qp_nr);
> +	}
> +
> +	EDEB(7, "ehea_port_cb_0");
> +	EDEB_DMP(7, (u8*)ehea_port_cb_0, sizeof(*ehea_port_cb_0), "");
> +
> +	mask = EHEA_BMASK_SET(H_PORT_CB0_PRC, 1)
> +	       | EHEA_BMASK_SET(H_PORT_CB0_DEFQPNARRAY, 1);
> +
> +	hret = ehea_h_modify_ehea_port(port->adapter->handle,
> +				       port->logical_port_id,
> +				       H_PORT_CB0,
> +				       mask,
> +				       (void*)ehea_port_cb_0);
> +
> +	if (hret != H_SUCCESS) {
> +		goto modify_ehea_port_failed;
> +	}
> +
> +	ret = 0;
> +
> +modify_ehea_port_failed:
> +	kfree(ehea_port_cb_0);
> +
> +kzalloc_failed:
> +	EDEB_EX(7, "ret=%d", ret);
> +	return ret;
> +}
> +
> +
> +static int ehea_gen_smrs(struct ehea_port_res *pr)
> +{
> +	u64 hret = H_HARDWARE;

No need to set hret. You do that a lot, int foo = 0; foo = bar();

> +	struct ehea_adapter *adapter = pr->port->adapter;
> +	EDEB_EN(7, "ehea_port_res=%p", pr);
> +	hret = hipz_h_register_smr(adapter->handle,
> +				   adapter->mr.handle,
> +				   adapter->mr.vaddr,
> +				   EHEA_MR_ACC_CTRL,
> +				   adapter->pd,
> +				   &pr->send_mr);
> +	if (hret != H_SUCCESS)
> +		goto ehea_gen_smrs_err1;
> +
> +	hret = hipz_h_register_smr(adapter->handle,
> +				   adapter->mr.handle,
> +				   adapter->mr.vaddr,
> +				   EHEA_MR_ACC_CTRL,
> +				   adapter->pd,
> +				   &pr->recv_mr);
> +	if (hret != H_SUCCESS)
> +		goto ehea_gen_smrs_err2;
> +	EDEB_EX(7, "");
> +	return 0;
> +
> +ehea_gen_smrs_err2:
> +	hret = ehea_h_free_resource_mr(adapter->handle, pr->send_mr.handle);
> +	if (hret != H_SUCCESS)
> +		EDEB_ERR(4, "Could not free SMR");
> +ehea_gen_smrs_err1:
> +	return -EINVAL;
> +}
> +
> +static void ehea_rem_smrs(struct ehea_port_res *pr)
> +{
> +	u64 hret = H_HARDWARE;
> +	struct ehea_adapter *adapter = pr->port->adapter;
> +	EDEB_EN(7, "ehea_port_res=%p", pr);
> +	hret = ehea_h_free_resource_mr(adapter->handle, pr->send_mr.handle);
> +	if (hret != H_SUCCESS)
> +		EDEB_ERR(4, "Could not free send SMR for pr=%p", pr);
> +
> +	hret = ehea_h_free_resource_mr(adapter->handle, pr->recv_mr.handle);
> +	if (hret != H_SUCCESS)
> +		EDEB_ERR(4, "Could not free receive SMR for pr=%p", pr);
> +	EDEB_EX(7, "");
> +}

It's a bit worrying that this routine can fail, but returns void, ie. it
can't report failure to its caller. (but perhaps it's ok).

> +
> +static int ehea_init_port_res(struct ehea_port *port, struct ehea_port_res *pr,
> +			      struct port_res_cfg *pr_cfg, int queue_token)
> +{
> +	int ret = -EINVAL;
> +	int max_rq_entries = 0;
> +	enum ehea_eq_type eq_type = EHEA_EQ;
> +	struct ehea_qp_init_attr *init_attr;
> +	struct ehea_adapter *adapter = port->adapter;
> +
> +	EDEB_EN(7, "port=%p, pr=%p", port, pr);
> +
> +	memset(pr, 0, sizeof(struct ehea_port_res));
> +
> +	pr->port = port;
> +	spin_lock_init(&pr->send_lock);
> +	spin_lock_init(&pr->recv_lock);
> +	spin_lock_init(&pr->xmit_lock);
> +	spin_lock_init(&pr->netif_queue);
> +
> +	pr->recv_eq = ehea_create_eq(adapter, eq_type,
> +				     EHEA_MAX_ENTRIES_EQ, 0);
> +	if (!pr->recv_eq) {
> +		EDEB_ERR(4, "ehea_create_eq failed (recv_eq)");
> +		goto ehea_init_port_res_err1;
> +	}
> +	pr->send_eq = ehea_create_eq(adapter, eq_type,
> +				     EHEA_MAX_ENTRIES_EQ, 0);
> +	if (!pr->send_eq) {
> +		EDEB_ERR(4, "ehea_create_eq failed (send_eq)");
> +		goto ehea_init_port_res_err2;
> +	}
> +
> +	pr->recv_cq = ehea_create_cq(adapter, pr_cfg->max_entries_rcq,
> +				     pr->recv_eq->ipz_eq_handle,
> +				     port->logical_port_id);
> +	if (!pr->recv_cq) {
> +		EDEB_ERR(4, "ehea_create_cq failed (cq_recv)");
> +		goto ehea_init_port_res_err3;
> +	}
> +
> +	pr->send_cq = ehea_create_cq(adapter, pr_cfg->max_entries_scq,
> +				     pr->send_eq->ipz_eq_handle,
> +				     port->logical_port_id);
> +	if (!pr->send_cq) {
> +		EDEB_ERR(4, "ehea_create_cq failed (cq_send)");
> +		goto ehea_init_port_res_err4;
> +	}
> +
> +	init_attr = (struct ehea_qp_init_attr*)
> +	    kzalloc(sizeof(struct ehea_qp_init_attr), GFP_KERNEL);
> +
> +	if (!init_attr) {
> +		EDEB_ERR(4, "no mem for init_attr struct");
> +		ret = -ENOMEM;
> +		goto ehea_init_port_res_err5;
> +	}
> +
> +	init_attr->low_lat_rq1 = 1;
> +	init_attr->signalingtype = 1;	/* generate CQE if specified in WQE */
> +	init_attr->rq_count = 3;
> +	init_attr->qp_token = queue_token;
> +
> +	init_attr->max_nr_send_wqes = pr_cfg->max_entries_sq;
> +	init_attr->max_nr_rwqes_rq1 = pr_cfg->max_entries_rq1;
> +	init_attr->max_nr_rwqes_rq2 = pr_cfg->max_entries_rq2;
> +	init_attr->max_nr_rwqes_rq3 = pr_cfg->max_entries_rq3;
> +
> +	init_attr->wqe_size_enc_sq = EHEA_SG_SQ;
> +	init_attr->wqe_size_enc_rq1 = EHEA_SG_RQ1;
> +	init_attr->wqe_size_enc_rq2 = EHEA_SG_RQ2;
> +	init_attr->wqe_size_enc_rq3 = EHEA_SG_RQ3;
> +
> +	init_attr->rq2_threshold = EHEA_RQ2_THRESHOLD;
> +	init_attr->rq3_threshold = EHEA_RQ3_THRESHOLD;
> +	init_attr->port_nr = port->logical_port_id;
> +	init_attr->send_cq_handle = pr->send_cq->ipz_cq_handle;
> +	init_attr->recv_cq_handle = pr->recv_cq->ipz_cq_handle;
> +	init_attr->aff_eq_handle = port->qp_eq->ipz_eq_handle;
> +
> +	pr->qp = ehea_create_qp(adapter, adapter->pd, init_attr);
> +	if (!pr->qp) {
> +		EDEB_ERR(4, "could not create queue pair");
> +		goto ehea_init_port_res_err6;
> +	}
> +
> +	/* SQ */
> +	max_rq_entries = init_attr->act_nr_send_wqes;
> +	pr->skb_arr_sq = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
> +						    * (max_rq_entries + 1));
> +	if (!pr->skb_arr_sq) {
> +		EDEB_ERR(4, "vmalloc for skb_arr_sq failed");
> +		goto ehea_init_port_res_err7;
> +	}
> +	memset(pr->skb_arr_sq, 0, sizeof(void*) * (max_rq_entries + 1));
> +	pr->skb_sq_index = 0;
> +	pr->skb_arr_sq_len = max_rq_entries + 1;
> +
> +	/* RQ 1 */
> +	max_rq_entries = init_attr->act_nr_rwqes_rq1;
> +	pr->skb_arr_rq1 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
> +						     * (max_rq_entries + 1));
> +	if (!pr->skb_arr_rq1) {
> +		EDEB_ERR(4, "vmalloc for skb_arr_rq1 failed");
> +		goto ehea_init_port_res_err8;
> +	}
> +	memset(pr->skb_arr_rq1, 0, sizeof(void*) * (max_rq_entries + 1));
> +	pr->skb_arr_rq1_len = max_rq_entries + 1;
> +
> +	/* RQ 2 */
> +	max_rq_entries = init_attr->act_nr_rwqes_rq2;
> +	pr->skb_arr_rq2 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
> +						     * (max_rq_entries + 1));
> +	if (!pr->skb_arr_rq2) {
> +		EDEB_ERR(4, "vmalloc for skb_arr_rq2 failed");
> +		goto ehea_init_port_res_err9;
> +	}
> +	memset(pr->skb_arr_rq2, 0, sizeof(void*) * (max_rq_entries + 1));
> +	pr->skb_arr_rq2_len = max_rq_entries;
> +	pr->skb_rq2_index = 0;
> +
> +	/* RQ 3 */
> +	max_rq_entries = init_attr->act_nr_rwqes_rq3;
> +	pr->skb_arr_rq3 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
> +						     * (max_rq_entries + 1));
> +	if (!pr->skb_arr_rq3) {
> +		EDEB_ERR(4, "vmalloc for skb_arr_rq3 failed");
> +		goto ehea_init_port_res_err10;
> +	}
> +	memset(pr->skb_arr_rq3, 0, sizeof(void*) * (max_rq_entries + 1));
> +	pr->skb_arr_rq3_len = max_rq_entries;
> +	pr->skb_rq3_index = 0;

You shouldn't need the sk_buff** casts here. You probably should use
kzalloc here, instead of vmalloc + memset. Depends on how big
max_rq_entries can be.

> +
> +	if (ehea_gen_smrs(pr) != 0)
> +		goto ehea_init_port_res_err11;
> +	tasklet_init(&pr->send_comp_task, ehea_send_irq_tasklet,
> +		     (unsigned long)pr);
> +	atomic_set(&pr->swqe_avail, EHEA_MAX_ENTRIES_SQ - 1);
> +
> +	kfree(init_attr);
> +	ret = 0;
> +	goto done;
> +
> +ehea_init_port_res_err11:
> +	vfree(pr->skb_arr_rq3);
> +ehea_init_port_res_err10:
> +	vfree(pr->skb_arr_rq2);
> +ehea_init_port_res_err9:
> +	vfree(pr->skb_arr_rq1);
> +ehea_init_port_res_err8:
> +	vfree(pr->skb_arr_sq);
> +ehea_init_port_res_err7:
> +	ehea_destroy_qp(pr->qp);
> +ehea_init_port_res_err6:
> +	kfree(init_attr);
> +ehea_init_port_res_err5:
> +	ehea_destroy_cq(pr->send_cq);
> +ehea_init_port_res_err4:
> +	ehea_destroy_cq(pr->recv_cq);
> +ehea_init_port_res_err3:
> +	ehea_destroy_eq(pr->send_eq);
> +ehea_init_port_res_err2:
> +	ehea_destroy_eq(pr->recv_eq);
> +ehea_init_port_res_err1:
> +done:
> +	EDEB_EX(7, "ret=%d", ret);
> +	return ret;
> +}

You can vfree/kfree a NULL pointer, so you can probably amalgamate some
of these error cases if you initialise the pointers to NULL at the
beginning.

> +
> +static int ehea_clean_port_res(struct ehea_port *port, struct ehea_port_res *pr)
> +{
> +	int i;
> +	int ret = -EINVAL;
> +
> +	EDEB_EN(7, "Not completed yet...");
> +
> +	ret = ehea_destroy_qp(pr->qp);
> +	if (ret)
> +		EDEB_ERR(4, "could not destroy queue pair");
> +
> +	ret = ehea_destroy_cq(pr->send_cq);
> +	if (ret)
> +		EDEB_ERR(4, "could not destroy send_cq");
> +
> +	ret = ehea_destroy_cq(pr->recv_cq);
> +	if (ret)
> +		EDEB_ERR(4, "could not destroy recv_cq");
> +
> +	ret = ehea_destroy_eq(pr->send_eq);
> +	if (ret)
> +		EDEB_ERR(4, "could not destroy send_eq");
> +
> +	ret = ehea_destroy_eq(pr->recv_eq);
> +	if (ret)
> +		EDEB_ERR(4, "could not destroy recv_eq");

If these are leaking memory they're probably worth a printk.

> +	for (i = 0; i < pr->skb_arr_rq1_len; i++) {
> +		if (pr->skb_arr_rq1[i])
> +			dev_kfree_skb(pr->skb_arr_rq1[i]);
> +	}
> +
> +	for (i = 0; i < pr->skb_arr_rq2_len; i++)
> +		if (pr->skb_arr_rq2[i])
> +			dev_kfree_skb(pr->skb_arr_rq2[i]);
> +
> +	for (i = 0; i < pr->skb_arr_rq3_len; i++)
> +		if (pr->skb_arr_rq3[i])
> +			dev_kfree_skb(pr->skb_arr_rq3[i]);
> +
> +	for (i = 0; i < pr->skb_arr_sq_len; i++)
> +		if (pr->skb_arr_sq[i])
> +			dev_kfree_skb(pr->skb_arr_sq[i]);
> +
> +	vfree(pr->skb_arr_sq);
> +	vfree(pr->skb_arr_rq1);
> +	vfree(pr->skb_arr_rq2);
> +	vfree(pr->skb_arr_rq3);
> +
> +	ehea_rem_smrs(pr);
> +	EDEB_EX(7, "ret=%d", ret);
> +	return ret;
> +}
> +
> +static inline void write_ip_start_end(struct ehea_swqe *swqe,
> +				      const struct sk_buff *skb)
> +{
> +
> +	swqe->ip_start = (u8)(((u64)skb->nh.iph) - ((u64)skb->data));
> +	swqe->ip_end = (u8)(swqe->ip_start + skb->nh.iph->ihl * 4 - 1);
> +}

> +static inline void write_tcp_offset_end(struct ehea_swqe *swqe,
> +					const struct sk_buff *skb)
> +{
> +	swqe->tcp_offset = (u8)(swqe->ip_end + 1 + offsetof(struct tcphdr,
> +							    check));
> +	swqe->tcp_end = (u16)skb->len - 1;
> +}
> +
> +static inline void write_udp_offset_end(struct ehea_swqe *swqe,
> +					const struct sk_buff *skb)
> +{
> +	swqe->tcp_offset = (u8)(swqe->ip_end + 1 + offsetof(struct udphdr,
> +							    check));
> +	swqe->tcp_end = (u16)skb->len - 1;
> +}

These three need some explanation at best.

> +
> +static inline void write_swqe2_data(struct sk_buff *skb,
> +				    struct net_device *dev,
> +				    struct ehea_swqe *swqe,
> +				    u32 lkey)
> +{
> +	int skb_data_size, nfrags, headersize, i, sg1entry_contains_frag_data;
> +	struct ehea_vsgentry *sg_list;
> +	struct ehea_vsgentry *sg1entry;
> +	struct ehea_vsgentry *sgentry;
> +	u8 *imm_data;
> +	u64 tmp_addr;
> +	skb_frag_t *frag;
> +	EDEB_EN(7, "");
> +
> +	skb_data_size = skb->len - skb->data_len;
> +	nfrags = skb_shinfo(skb)->nr_frags;
> +	sg1entry = &swqe->u.immdata_desc.sg_entry;
> +	sg_list = (struct ehea_vsgentry*)&swqe->u.immdata_desc.sg_list;
> +	imm_data = &swqe->u.immdata_desc.immediate_data[0];
> +	swqe->descriptors = 0;
> +	sg1entry_contains_frag_data = 0;
> +
> +	if ((dev->features & NETIF_F_TSO) && skb_shinfo(skb)->gso_size) {
> +		/* Packet is TCP with TSO enabled */
> +		swqe->tx_control |= EHEA_SWQE_TSO;
> +		swqe->mss = skb_shinfo(skb)->gso_size;
> +		/* copy only eth/ip/tcp headers to immediate data and
> +		 * the rest of skb->data to sg1entry
> +		 */
> +		headersize = ETH_HLEN + (skb->nh.iph->ihl * 4)
> +		    + skb->h.th->doff * 4;
> +		skb_data_size = skb->len - skb->data_len;
> +
> +		if (skb_data_size >= headersize) {
> +			/* copy immediate data */
> +			memcpy(imm_data, skb->data, headersize);
> +			swqe->immediate_data_length = headersize;
> +
> +			if (skb_data_size > headersize) {
> +				/* set sg1entry data */
> +				sg1entry->l_key = lkey;
> +				sg1entry->len = skb_data_size - headersize;
> +
> +				tmp_addr = (u64)(skb->data + headersize);
> +				sg1entry->vaddr =
> +					get_swqe_addr(tmp_addr, 0);
> +
> +				swqe->descriptors++;
> +			}
> +		} else
> +			EDEB_ERR(4, "Cannot handle fragmented headers");
> +	} else {
> +		/* Packet is any nonTSO type
> +		 *
> +		 * Copy as much as possible skb->data to immediate data and
> +		 * the rest to sg1entry
> +		 */
> +		if (skb_data_size >= SWQE2_MAX_IMM) {
> +			/* copy immediate data */
> +			memcpy(imm_data, skb->data, SWQE2_MAX_IMM);
> +
> +			swqe->immediate_data_length = SWQE2_MAX_IMM;
> +
> +			if (skb_data_size > SWQE2_MAX_IMM) {
> +				/* copy sg1entry data */
> +				sg1entry->l_key = lkey;
> +				sg1entry->len = skb_data_size - SWQE2_MAX_IMM;
> +				tmp_addr = (u64)(skb->data + SWQE2_MAX_IMM);
> +				sg1entry->vaddr = get_swqe_addr(tmp_addr, 0);
> +				swqe->descriptors++;
> +			}
> +		} else {
> +			memcpy(imm_data, skb->data, skb_data_size);
> +			swqe->immediate_data_length = skb_data_size;
> +		}
> +	}
> +
> +	/* write descriptors */
> +	if (nfrags > 0) {
> +		if (swqe->descriptors == 0) {
> +			/* sg1entry not yet used */
> +			frag = &skb_shinfo(skb)->frags[0];
> +
> +			/* copy sg1entry data */
> +			sg1entry->l_key = lkey;
> +			sg1entry->len = frag->size;
> +			tmp_addr =  (u64)(page_address(frag->page) +
> +					    frag->page_offset);
> +			sg1entry->vaddr = get_swqe_addr(tmp_addr,
> +							EHEA_MR_TX_DATA_PN);
> +			swqe->descriptors++;
> +			sg1entry_contains_frag_data = 1;
> +		}
> +
> +		for (i = sg1entry_contains_frag_data; i < nfrags; i++) {
> +
> +			frag = &skb_shinfo(skb)->frags[i];
> +			sgentry = &sg_list[i - sg1entry_contains_frag_data];
> +
> +			sgentry->l_key = lkey;
> +			sgentry->len = frag->size;
> +
> +			tmp_addr = (u64)(page_address(frag->page)
> +					 + frag->page_offset);
> +			sgentry->vaddr = get_swqe_addr(tmp_addr,
> +						       EHEA_MR_TX_DATA_PN + i);
> +		}
> +	}
> +
> +	EDEB_EX(7, "");
> +}
> +
> +static int ehea_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> +{
> +	EDEB_ERR(4, "ioctl not supported: dev=%s cmd=%d", dev->name, cmd);
> +	return -EOPNOTSUPP;
> +}
> +
> +static u64 ehea_broadcast_reg_helper(struct ehea_port *port, u32 hcallid)
> +{
> +	u64 hret = H_HARDWARE;
> +	u8 reg_type = 0;
> +
> +	if (hcallid == H_REG_BCMC) {
> +		EDEB(7, "REGistering MAC for broadcast");
> +	} else {
> +		EDEB(7, "DEREGistering MAC for broadcast");
> +	}
> +
> +	/* De/Register untagged packets */
> +	reg_type = EHEA_BCMC_BROADCAST | EHEA_BCMC_UNTAGGED;
> +	hret = ehea_h_reg_dereg_bcmc(port->adapter->handle,
> +				     port->logical_port_id,
> +				     reg_type, port->mac_addr, 0, hcallid);
> +	if (hret != H_SUCCESS)
> +		goto hcall_failed;
> +
> +	/* De/Register VLAN packets */
> +	reg_type = EHEA_BCMC_BROADCAST | EHEA_BCMC_VLANID_ALL;
> +	hret = ehea_h_reg_dereg_bcmc(port->adapter->handle,
> +				     port->logical_port_id,
> +				     reg_type, port->mac_addr, 0, hcallid);
> +hcall_failed:

unregister here?

> +	return hret;
> +}
> +
> +static int ehea_set_mac_addr(struct net_device *dev, void *sa)
> +{
> +	int ret = -EOPNOTSUPP;
> +	u64 hret = H_HARDWARE;
> +	struct hcp_query_ehea_port_cb_0 *ehea_port_cb_0 = NULL;
> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
> +	struct sockaddr *mac_addr = (struct sockaddr*)sa;
> +
> +	EDEB_EN(7, "devname=%s", dev->name);
> +	EDEB_DMP(7, (u8*)&(mac_addr->sa_data[0]), 14, "");
> +
> +	if (!is_valid_ether_addr(mac_addr->sa_data)) {
> +		ret = -EADDRNOTAVAIL;
> +		goto invalid_mac;
> +	}
> +
> +	ehea_port_cb_0 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> +
> +	if (!ehea_port_cb_0) {
> +		EDEB_ERR(4, "No memory for ehea_port control block");
> +		ret = -ENOMEM;
> +		goto kzalloc_failed;
> +	}
> +
> +	memcpy((u8*)(&(ehea_port_cb_0->port_mac_addr)),
> +	       (u8*)&(mac_addr->sa_data[0]), 6);

Don't need casts, memcpy takes void *. You should use ETH_ALEN
(include/linux/if_ether.h) instead of 6.

> +
> +	ehea_port_cb_0->port_mac_addr = ehea_port_cb_0->port_mac_addr >> 16;
> +
> +	EDEB(7, "ehea_port_cb_0");
> +	EDEB_DMP(7, (u8*)ehea_port_cb_0,
> +		 sizeof(struct hcp_query_ehea_port_cb_0), "");
> +
> +	hret = ehea_h_modify_ehea_port(port->adapter->handle,
> +				       port->logical_port_id,
> +				       H_PORT_CB0,
> +				       EHEA_BMASK_SET(H_PORT_CB0_MAC, 1),
> +				       (void*)ehea_port_cb_0);
> +	if (hret != H_SUCCESS) {
> +		ret = -EOPNOTSUPP;
> +		goto hcall_failed;
> +	}
> +
> +	memcpy(dev->dev_addr, mac_addr->sa_data, dev->addr_len);
> +
> +	/* Deregister old MAC in PHYP */
> +	hret = ehea_broadcast_reg_helper(port, H_DEREG_BCMC);
> +	if (hret) {
> +		ret = -EOPNOTSUPP;
> +		goto hcall_failed;

What happens here? No mac? Old mac?

> +	}
> +
> +	port->mac_addr = ehea_port_cb_0->port_mac_addr << 16;
> +
> +	/* Register new MAC in PHYP */
> +	hret = ehea_broadcast_reg_helper(port, H_REG_BCMC);
> +	if (hret) {
> +		ret = -EOPNOTSUPP;
> +		goto hcall_failed;
> +	}
> +
> +	ret = 0;
> +
> +hcall_failed:
> +	kfree(ehea_port_cb_0);
> +
> +kzalloc_failed:
> +invalid_mac:
> +	EDEB_EX(7, "ret=%d", ret);
> +	return ret;
> +}
> +
> +static void ehea_promiscuous(struct net_device *dev, int enable)
> +{
> +	struct ehea_port *port = dev->priv;
> +
> +	if (!port->promisc) {
> +		if (enable) {
> +			/* Enable promiscuous mode */
> +			EDEB(7, "Enabling IFF_PROMISC");
> +			EDEB_ERR(4, "Enable promiscuous mode: "
> +				 "not yet implemented");
> +			port->promisc = EHEA_ENABLE;
> +		}
> +	} else {
> +		if (!enable) {
> +			/* Disable promiscuous mode */
> +			EDEB(7, "Disabling IFF_PROMISC");
> +			EDEB_ERR(4, "Disable promiscuous mode: "
> +				 "not yet implemented");
> +			port->promisc = EHEA_DISABLE;
> +		}
> +	}
> +}
> +
> +static u64 ehea_multicast_reg_helper(struct ehea_port *port,
> +				     u64 mc_mac_addr,
> +				     u32 hcallid)
> +{
> +	u64 hret = H_HARDWARE;
> +	u8 reg_type = 0;
> +
> +	reg_type = EHEA_BCMC_SCOPE_ALL | EHEA_BCMC_MULTICAST
> +		 | EHEA_BCMC_UNTAGGED;
> +
> +	hret = ehea_h_reg_dereg_bcmc(port->adapter->handle,
> +				     port->logical_port_id,
> +				     reg_type, mc_mac_addr, 0, hcallid);
> +	if (hret)
> +		goto hcall_failed;
> +
> +	reg_type = EHEA_BCMC_SCOPE_ALL | EHEA_BCMC_MULTICAST
> +		 | EHEA_BCMC_VLANID_ALL;
> +
> +	hret = ehea_h_reg_dereg_bcmc(port->adapter->handle,
> +				     port->logical_port_id,
> +				     reg_type, mc_mac_addr, 0, hcallid);
> +hcall_failed:
> +	return hret;
> +}
> +
> +static int ehea_drop_multicast_list(struct net_device *dev)
> +{
> +	int ret = 0;
> +	u64 hret = H_HARDWARE;
> +	struct ehea_port *port = dev->priv;
> +	struct ehea_mc_list *mc_entry = port->mc_list;
> +	struct list_head *pos;
> +	struct list_head *temp;
> +
> +	EDEB_EN(7, "devname=%s", dev->name);
> +
> +	if (!list_empty(&mc_entry->list)) {
> +		list_for_each_safe(pos, temp, &(port->mc_list->list)) {
> +			mc_entry = list_entry(pos, struct ehea_mc_list, list);
> +
> +			EDEB(7, "Deregistering MAC %lx", mc_entry->macaddr);
> +
> +			hret = ehea_multicast_reg_helper(port,
> +							 mc_entry->macaddr,
> +							 H_DEREG_BCMC);
> +			if (hret) {
> +				EDEB_ERR(4, "Failed deregistering mcast MAC");
> +				ret = -EINVAL;
> +			}
> +
> +			list_del(pos);
> +			kfree(mc_entry);
> +		}
> +	}

You don't need the if.


Ok, I'm tired now, I might read the rest later.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

^ permalink raw reply

* Re: [PATCH 4/6] ehea: header files
From: Michael Ellerman @ 2006-08-10  6:22 UTC (permalink / raw)
  To: Jan-Bernd Themann
  Cc: Thomas Klein, netdev, linux-kernel, linux-ppc, Christoph Raisch,
	Marcus Eder
In-Reply-To: <44D99F56.7010201@de.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 13819 bytes --]

Hi Jan-Bernd,

I haven't read all of this, but a few things caught my eye ...

cheers

On Wed, 2006-08-09 at 10:39 +0200, Jan-Bernd Themann wrote:
> Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com>
> 
> 
>   drivers/net/ehea/ehea.h    |  452 +++++++++++++++++++++++++++++++++++++++++++++
>   drivers/net/ehea/ehea_hw.h |  319 +++++++++++++++++++++++++++++++
>   2 files changed, 771 insertions(+)
> 
> 
> 
> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea.h	1969-12-31 16:00:00.000000000 -0800
> +++ kernel/drivers/net/ehea/ehea.h	2006-08-08 23:59:39.927452928 -0700
> @@ -0,0 +1,452 @@
> +/*
> + *  linux/drivers/net/ehea/ehea.h
> + *
> + *  eHEA ethernet device driver for IBM eServer System p
> + *
> + *  (C) Copyright IBM Corp. 2006
> + *
> + *  Authors:
> + *       Christoph Raisch <raisch@de.ibm.com>
> + *       Jan-Bernd Themann <themann@de.ibm.com>
> + *       Heiko-Joerg Schick <schickhj@de.ibm.com>
> + *       Thomas Klein <tklein@de.ibm.com>
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#ifndef __EHEA_H__
> +#define __EHEA_H__
> +
> +#include <linux/version.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/kernel.h>
> +#include <linux/vmalloc.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/sched.h>
> +#include <linux/err.h>
> +#include <linux/list.h>
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <linux/kthread.h>
> +#include <linux/ethtool.h>
> +#include <linux/if_vlan.h>
> +#include <asm/ibmebus.h>
> +#include <asm/of_device.h>
> +#include <asm/abs_addr.h>
> +#include <asm/semaphore.h>
> +#include <asm/current.h>
> +#include <asm/io.h>
> +
> +#define EHEA_DRIVER_NAME	"IBM eHEA"
> +#define EHEA_DRIVER_VERSION	"EHEA_0015"
> +
> +#define NET_IP_ALIGN 0
> +#define EHEA_NUM_TX_QP 1
> +#ifdef EHEA_SMALL_QUEUES
> +#define EHEA_MAX_CQE_COUNT     1020
> +#define EHEA_MAX_ENTRIES_SQ    1020
> +#define EHEA_MAX_ENTRIES_RQ1   4080
> +#define EHEA_MAX_ENTRIES_RQ2   1020
> +#define EHEA_MAX_ENTRIES_RQ3   1020
> +#define EHEA_SWQE_REFILL_TH     100
> +#else
> +#define EHEA_MAX_CQE_COUNT    32000
> +#define EHEA_MAX_ENTRIES_SQ   16000
> +#define EHEA_MAX_ENTRIES_RQ1  32080
> +#define EHEA_MAX_ENTRIES_RQ2   4020
> +#define EHEA_MAX_ENTRIES_RQ3   4020
> +#define EHEA_SWQE_REFILL_TH    1000
> +#endif
> +
> +#define EHEA_MAX_ENTRIES_EQ       20
> +
> +#define EHEA_SG_SQ  2
> +#define EHEA_SG_RQ1 1
> +#define EHEA_SG_RQ2 0
> +#define EHEA_SG_RQ3 0
> +
> +#define EHEA_MAX_PACKET_SIZE    9022	/* for jumbo frame */
> +#define EHEA_RQ2_PKT_SIZE       1522
> +#define EHEA_LL_PKT_SIZE         256
> +
> +/* Send completion signaling */
> +#define EHEA_SIG_IV 1000
> +#define EHEA_SIG_IV_LONG 4
> +
> +/* Protection Domain Identifier */
> +#define EHEA_PD_ID        0xaabcdeff
> +
> +#define EHEA_RQ2_THRESHOLD         1
> +/* use RQ3 threshold of 1522 bytes */
> +#define EHEA_RQ3_THRESHOLD         9
> +
> +#define EHEA_SPEED_10G         10000
> +#define EHEA_SPEED_1G           1000
> +#define EHEA_SPEED_100M          100
> +#define EHEA_SPEED_10M            10
> +
> +/* Broadcast/Multicast registration types */
> +#define EHEA_BCMC_SCOPE_ALL	0x08
> +#define EHEA_BCMC_SCOPE_SINGLE	0x00
> +#define EHEA_BCMC_MULTICAST	0x04
> +#define EHEA_BCMC_BROADCAST	0x00
> +#define EHEA_BCMC_UNTAGGED	0x02
> +#define EHEA_BCMC_TAGGED	0x00
> +#define EHEA_BCMC_VLANID_ALL	0x01
> +#define EHEA_BCMC_VLANID_SINGLE	0x00
> +
> +/* Use this define to kmallocate PHYP control blocks */
> +#define H_CB_ALIGNMENT		4096
> +
> +#define EHEA_PAGESHIFT  12
> +#define EHEA_PAGESIZE   4096UL
> +#define EHEA_CACHE_LINE 128

This looks like a very bad idea, what happens if you're running on a
machine with 64K pages?

> +
> +#define EHEA_ENABLE	1
> +#define EHEA_DISABLE	0

Do you really need hash defines for 0 and 1 ? They're fairly well
understood in C as meaning true and false.

> +
> +/* Memory Regions */
> +#define EHEA_MR_MAX_TX_PAGES 20
> +#define EHEA_MR_TX_DATA_PN 3
> +#define EHEA_MR_ACC_CTRL 0x00800000
> +#define EHEA_RWQES_PER_MR_RQ2 10
> +#define EHEA_RWQES_PER_MR_RQ3 10
> +
> +
> +void ehea_set_ethtool_ops(struct net_device *netdev);
> +
> +#ifndef KEEP_EDEBS_BELOW
> +#define KEEP_EDEBS_BELOW 8
> +#endif
> +
> +extern int ehea_trace_level;
> +
> +#ifdef EHEA_NO_EDEB
> +#define EDEB_P_GENERIC(level, idstring, format, args...) \
> +	while (0 == 1) { \
> +	    if(unlikely (level <= ehea_trace_level)) { \
> +			printk("%s " idstring " "format "\n", \
> +				__func__, ##args); \
> +	  } \
> +	} \
> +
> +#else
> +
> +#define EDEB_P_GENERIC(level,idstring,format,args...) \
> +if (level < KEEP_EDEBS_BELOW) { \
> +	do { \
> +	    if(unlikely (level <= ehea_trace_level)) { \
> +			printk("%s " idstring " "format "\n", \
> +				__func__, ##args); \
> +	  } \
> +	} while (1 == 0); \
> +}
> +#endif
> +
> +#define EDEB(level, format, args...) \
> +        EDEB_P_GENERIC(level, "", format, ##args)
> +
> +#define EDEB_ERR(level, format, args...) \
> +        EDEB_P_GENERIC(level, "EHEA_ERROR", format, ##args)
> +
> +#define EDEB_EN(level, format, args...) \
> +        EDEB_P_GENERIC(level, ">>>", format, ##args)
> +
> +#define EDEB_EX(level, format, args...) \
> +        EDEB_P_GENERIC(level, "<<<", format, ##args)
> +
> +#define EHEA_BMASK(pos, length) (((pos) << 16) + (length))
> +#define EHEA_BMASK_IBM(from, to) (((63 - to) << 16) + ((to) - (from) + 1))
> +#define EHEA_BMASK_SHIFTPOS(mask) (((mask) >> 16) & 0xffff)
> +#define EHEA_BMASK_MASK(mask) \
> +	(0xffffffffffffffffULL >> ((64 - (mask)) & 0xffff))
> +#define EHEA_BMASK_SET(mask, value) \
> +        ((EHEA_BMASK_MASK(mask) & ((u64)(value))) << EHEA_BMASK_SHIFTPOS(mask))
> +#define EHEA_BMASK_GET(mask, value) \
> +        (EHEA_BMASK_MASK(mask) & (((u64)(value)) >> EHEA_BMASK_SHIFTPOS(mask)))
> +
> +extern void exit(int);
> +
> +#define EDEB_DMP(level, adr, len, format, args...) \
> +if (level < KEEP_EDEBS_BELOW) { \
> +     if(unlikely (level <= ehea_trace_level)) {  \
> +        do { \
> +                unsigned int x; \
> +		unsigned int l = (unsigned int)(len); \
> +                unsigned char *deb = (unsigned char*)(adr); \
> +		for (x = 0; x < l; x += 16) { \
> +		        EDEB(level, format " adr=%p ofs=%04x %016lx %016lx", \
> +			     ##args, deb, x, *((u64 *)&deb[0]), \
> +			     *((u64 *)&deb[8])); \
> +			deb += 16; \
> +		} \
> +        } while (0); \
> +     } \
> +}
> +
> +
> +/*
> + * struct generic ehea page
> + */
> +struct ipz_page {
> +	u8 entries[PAGE_SIZE];
> +};
> +
> +/*
> + * struct generic queue in linux kernel virtual memory
> + */
> +struct ipz_queue {
> +	u64 current_q_offset;		/* current queue entry */
> +	struct ipz_page **queue_pages;	/* array of pages belonging to queue */
> +	u32 qe_size;			/* queue entry size */
> +	u32 act_nr_of_sg;
> +	u32 queue_length;      		/* queue length allocated in bytes */
> +	u32 pagesize;
> +	u32 toggle_state;		/* toggle flag - per page */
> +	u32 reserved;			/* 64 bit alignment */
> +};
> +
> +
> +/*
> + *  h_galpa:
> + *  for pSeries this is a 64bit memory address where
> + *  I/O memory is mapped into CPU address space
> + */
> +
> +struct h_galpa {
> +	u64 fw_handle;
> +};

What is a h_galpa? And why does it need a struct if it's just a u64?

> +
> +struct h_galpas {
> +	struct h_galpa kernel;	/* kernel space accessible resource,
> +				   set to 0 if unused */
> +	struct h_galpa user;	/* user space accessible resource
> +				   set to 0 if unused */
> +	u32 pid;		/* PID of userspace galpa checking */
> +};
> +
> +struct ehea_qp;
> +struct ehea_cq;
> +struct ehea_eq;
> +struct ehea_port;
> +struct ehea_av;
> +
> +struct ehea_qp_init_attr {
> +        /* input parameter */
> +	u32 qp_token;
> +	u8 low_lat_rq1;
> +	u8 signalingtype;
> +	u8 rq_count;
> +	u8 eqe_gen;
> +	u16 max_nr_send_wqes;
> +	u16 max_nr_rwqes_rq1;
> +	u16 max_nr_rwqes_rq2;
> +	u16 max_nr_rwqes_rq3;
> +	u8 wqe_size_enc_sq;
> +	u8 wqe_size_enc_rq1;
> +	u8 wqe_size_enc_rq2;
> +	u8 wqe_size_enc_rq3;
> +	u8 swqe_imm_data_len;
> +	u16 port_nr;
> +	u16 rq2_threshold;
> +	u16 rq3_threshold;
> +	u64 send_cq_handle;
> +	u64 recv_cq_handle;
> +	u64 aff_eq_handle;
> +
> +        /* output parameter */
> +	u32 qp_nr;
> +	u16 act_nr_send_wqes;
> +	u16 act_nr_rwqes_rq1;
> +	u16 act_nr_rwqes_rq2;
> +	u16 act_nr_rwqes_rq3;
> +	u8 act_wqe_size_enc_sq;
> +	u8 act_wqe_size_enc_rq1;
> +	u8 act_wqe_size_enc_rq2;
> +	u8 act_wqe_size_enc_rq3;
> +	u32 nr_sq_pages;
> +	u32 nr_rq1_pages;
> +	u32 nr_rq2_pages;
> +	u32 nr_rq3_pages;
> +	u32 liobn_sq;
> +	u32 liobn_rq1;
> +	u32 liobn_rq2;
> +	u32 liobn_rq3;
> +};
> +
> +struct ehea_eq_attr {
> +	u32 type;
> +	u32 max_nr_of_eqes;
> +	u8 eqe_gen;
> +	u64 eq_handle;
> +	u32 act_nr_of_eqes;
> +	u32 nr_pages;
> +	u32 ist1;
> +	u32 ist2;
> +	u32 ist3;
> +	u32 ist4;
> +};
> +
> +struct ehea_eq {
> +	struct ehea_adapter *adapter;
> +	struct ipz_queue ipz_queue;
> +	u64 ipz_eq_handle;
> +	struct h_galpas galpas;
> +	spinlock_t spinlock;
> +	struct ehea_eq_attr attr;
> +};
> +
> +struct ehea_qp {
> +	struct ehea_adapter *adapter;
> +	u64 ipz_qp_handle;	/* QP handle for h-calls */
> +	struct ipz_queue ipz_squeue;
> +	struct ipz_queue ipz_rqueue1;
> +	struct ipz_queue ipz_rqueue2;
> +	struct ipz_queue ipz_rqueue3;
> +	struct h_galpas galpas;
> +	struct ehea_qp_init_attr init_attr;
> +};
> +
> +struct ehea_cq_attr {
> +        /* input parameter */
> +	u32 max_nr_of_cqes;
> +	u32 cq_token;
> +	u64 eq_handle;
> +
> +        /* output parameter */
> +	u32 act_nr_of_cqes;
> +	u32 nr_pages;
> +};
> +
> +struct ehea_cq {
> +	struct ehea_adapter *adapter;
> +	u64 ipz_cq_handle;
> +	struct ipz_queue ipz_queue;
> +	struct h_galpas galpas;
> +	struct ehea_cq_attr attr;
> +};
> +
> +struct ehea_mr {
> +	u64 handle;
> +	u64 vaddr;
> +	u32 lkey;
> +};
> +
> +struct port_state {
> +	int poll_max_processed;
> +	int poll_receive_errors;
> +	int ehea_poll;
> +	int queue_stopped;
> +	int min_swqe_avail;
> +	u64 sqc_stop_sum;
> +	int pkt_send;
> +	int pkt_xmit;
> +	int send_tasklet;
> +	int nwqe;
> +};
> +
> +#define EHEA_IRQ_NAME_SIZE 20
> +struct ehea_port_res {
> +	struct ehea_mr send_mr;
> +	struct ehea_mr recv_mr;
> +	spinlock_t xmit_lock;
> +	struct ehea_port *port;
> +	char int_recv_name[EHEA_IRQ_NAME_SIZE];
> +	char int_send_name[EHEA_IRQ_NAME_SIZE];
> +	struct ehea_qp *qp;
> +	struct ehea_cq *send_cq;
> +	struct ehea_cq *recv_cq;
> +	struct ehea_eq *send_eq;
> +	struct ehea_eq *recv_eq;
> +	spinlock_t send_lock;
> +	struct sk_buff **skb_arr_rq1;
> +	struct sk_buff **skb_arr_rq2;
> +	struct sk_buff **skb_arr_rq3;
> +	struct sk_buff **skb_arr_sq;
> +	int skb_arr_rq1_len;
> +	int skb_arr_rq2_len;
> +	int skb_arr_rq3_len;
> +	int skb_arr_sq_len;
> +	int skb_rq2_index;
> +	int skb_rq3_index;
> +	int skb_sq_index;
> +	spinlock_t netif_queue;
> +	atomic_t swqe_avail;
> +	int swqe_ll_count;
> +	int swqe_count;
> +	u32 swqe_id_counter;
> +	u64 tx_packets;
> +	struct tasklet_struct send_comp_task;
> +	spinlock_t recv_lock;
> +	struct timer_list timer;	/* polling mode, no interrupts */
> +	struct timer_list skb_timer;	/* skb cleanup timer */
> +	struct port_state p_state;
> +	u64 rx_packets;
> +	u32 poll_counter;
> +};
> +
> +
> +struct ehea_adapter {
> +	u64 handle;
> +	u8 num_ports;
> +	struct ehea_port *port[16];
> +	struct ehea_eq *neq;
> +	struct tasklet_struct neq_tasklet;
> +	struct ehea_mr mr;
> +	u32 pd;
> +	u64 max_mc_mac;
> +};
> +
> +
> +struct ehea_mc_list {
> +	struct list_head list;
> +	u64 macaddr;
> +};
> +
> +#define EHEA_MAX_PORT_RES 16
> +struct ehea_port {
> +	struct ehea_adapter *adapter;	 /* adapter that owns this port */
> +	struct net_device *netdev;
> +	struct net_device_stats stats;
> +	struct ehea_port_res port_res[EHEA_MAX_PORT_RES];
> +	struct device_node *of_dev_node; /* Open Firmware Device Node */
> +	struct ehea_mc_list *mc_list;	 /* Multicast MAC addresses */
> +	struct vlan_group *vgrp;
> +	struct ehea_eq *qp_eq;
> +	char int_aff_name[EHEA_IRQ_NAME_SIZE];
> +	int allmulti;			 /* Indicates IFF_ALLMULTI state */
> +	int promisc;		 	 /* Indicates IFF_PROMISC state */
> +	int kernel_l_key;
> +	int num_tx_qps;
> +	u64 mac_addr;
> +	u32 logical_port_id;
> +	u32 port_speed;
> +	u8 full_duplex;
> +	u8 num_def_qps;
> +};
> +
> +struct port_res_cfg {
> +	int max_entries_rcq;
> +	int max_entries_scq;
> +	int max_entries_sq;
> +	int max_entries_rq1;
> +	int max_entries_rq2;
> +	int max_entries_rq3;
> +};

Enormous structs with no comments.

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

^ permalink raw reply

* Re: regarding pci  vga card output with powerpc
From: sudheer @ 2006-08-10  7:10 UTC (permalink / raw)
  To: Andrey Volkov; +Cc: linuxppc-embedded
In-Reply-To: <44DACE0E.2050505@varma-el.com>

[-- Attachment #1: Type: text/plain, Size: 3839 bytes --]

Andrey Volkov wrote:
> sudheer wrote:
>   
>> Andrey Volkov wrote:
>>     
>>> urwithsudheer wrote:
>>>       
>>>> Hello All,
>>>> I am working with a pci based vga card. I have connected the vga card
>>>> next to the  pci slot of my target processor on the baseboard.
>>>> I have connected a monitor to the vga card.
>>>> When i boot linux from minicom, i could see all the kernel log
>>>> messages on both the minicom and vga card monitor. But on the vga
>>>> monitor, i could not see any messages after asking for login as below:
>>>> .
>>>> .
>>>>
>>>> Mounting filesystems
>>>> Running depmod
>>>> Setting up networking on loopback device:
>>>> Setting up networking on eth0:
>>>> eth0: PHY is Marvell 88E11x1 (1410cc2)
>>>> Adding static route for default gateway to 192.168.3.1:
>>>> Setting nameserver to 192.168.0.1 in /etc/resolv.conf:
>>>> Starting inetd:
>>>>
>>>> login:
>>>>
>>>>
>>>> Though i am entering my login and passwd at the minicom, nothing
>>>> comes on to the vga monitor. But i am able to write to vga monitor
>>>> using the following commands from minicom:
>>>>
>>>> # echo "Hello" > /dev/tty1
>>>> # echo "Hi " > /dev/tty1
>>>> My requirement is to get on the vga monitor whatever i am typing on
>>>> the minicom after logging into the linux like .
>>>> ls or pwd commands.
>>>>
>>>> Anyone pl help me.
>>>>
>>>> Thanks & Regards
>>>> Sudheer
>>>>
>>>>         
>>> See Documentation/kernel-parameters.txt.
>>>
>>> If be short, add:
>>>
>>>     console=/dev/ttySxx console=/dev/tty1
>>>
>>> (both) to a kernel parameters string.
>>>
>>>       
>> I got upto login prompt on the vga monitor only after giving the above
>> parameters in my u-boot bootargs.
>> u-boot=> set bootargs console=tty1 console=ttyS0,115200.
>>
>>
>> I have added in the serial driver  linux/drivers/serial/serial_core.c
>>    in the function  uart_write(struct tty_struct *tty, const unsigned
>> char * buf, int count)
>>    The lines in the if directive.
>>   
>> if (c <= 0)
>>                         break;
>>                 memcpy(circ->buf + circ->head, buf, c);
>> /* added to test vga */
>> #if 1
>>                 memcpy(tempbuf, circ->buf + circ->head, c);
>>                   for(i=0;i<c;i++)
>>                   printk("%c", tempbuf[i]);
>> #endif
>>
>>  circ->head = (circ->head + c) & (UART_XMIT_SIZE - 1);
>>                 buf += c;
>>
>> Then as i expected, all the letters i enter in the minicom came twice on
>> the minicom.
>>     
>
> Because printk didn't print to an dedicated device, but to a klog
> stream. So check your /etc/inittab and /etc/securetty, its similar
> that something wrong in this files.
>   
oh..sorry, i forgot to tell you about the other two modifications done 
in the kernel config and ramdisk.
In the kernel config-- I have selected the virtual terminal option.
In the ramdisk- etc/inittab - i added the following line:
::respawn:/sbin/getty tty1 115200 vt100.
The etc/securetty contains the following:
console
fb0
tty1
tty2
tty3
tty4
tty5
tty6
tty7
tty8
ttyS0
ttyS1
ttyS2
ttyS3
ttyp0
ttyp1
ttyp2
ttyp3
ttyAM0
ttyAM1
tts/0
tts/1

Do i need to use fb0, to send the  serial data to frame buffer ?

>   
>> login:rroooott
>> Password: Password:
>> login  on `ttyS0'ogin  on `ttyS0'login[83]: root
>>
>>  ~ # ~ # llss
>>  ~ # ~ # iiffccoonnffiigg
>>
>> And whatever i enter on the minicom also gets print on the vga monitor 
>> but only once.
>> When i comment the if directive, On the vga monitor i could see only
>> upto login prompt.
>>
>> I don't know why the printk statement i added in the serial driver
>> prints on the vga monitor.
>>
>> As said by shkatikannan, i am going through the vga driver to copy to
>> the frambuffer. But  yet couldn't get it exactly in the code where to copy.
>>
>> Please tell me if i am in the wrong path.
>>
>> Thanks & Regards
>> Sudheer
>>
>>     


[-- Attachment #2: Type: text/html, Size: 4405 bytes --]

^ permalink raw reply

* Re: [PATCH 1/6] ehea: interface to network stack
From: Michael Ellerman @ 2006-08-10  7:30 UTC (permalink / raw)
  To: Jan-Bernd Themann
  Cc: Thomas Klein, netdev, linux-kernel, linux-ppc, Christoph Raisch,
	Marcus Eder
In-Reply-To: <1155190553.9801.38.camel@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Thu, 2006-08-10 at 16:15 +1000, Michael Ellerman wrote:
> > +	struct hcp_query_ehea_port_cb_2 *cb2 = NULL;
> > +	struct net_device_stats *stats = &port->stats;
> > +
> > +	EDEB_EN(7, "net_device=%p", dev);
> > +
> > +	cb2 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> > +	if (!cb2) {
> > +		EDEB_ERR(4, "No memory for cb2");
> > +		goto get_stat_exit;
> 
> You leak cb2 here.
> 
> > +	}
> > +
> > +	hret = ehea_h_query_ehea_port(adapter->handle,
> > +				      port->logical_port_id,
> > +				      H_PORT_CB2,
> > +				      H_PORT_CB2_ALL,
> > +				      cb2);
> > +
> > +	if (hret != H_SUCCESS) {
> > +		EDEB_ERR(4, "query_ehea_port failed for cb2");
> > +		goto get_stat_exit;

Sorry, here.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

^ permalink raw reply

* Re: [PATCH][2/2] RTAS MSI
From: Michael Ellerman @ 2006-08-10  8:03 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, PaulMackerras
In-Reply-To: <1155117003.4040.69.camel@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 1695 bytes --]

On Wed, 2006-08-09 at 11:50 +0200, Benjamin Herrenschmidt wrote:
> > I'm only just starting to get benh's new irq code, but I think
> > irq_find_host(dn) isn't doing what we want here. It's probably harmless,
> > but AFAICT irq_find_host() is only meant to be called when you have the
> > node of the irq controller, not for an arbitrary dn. The doco's a bit
> > ambiguous:
> > 
> >  * irq_find_host - Locates a host for a given device node
> >  * @node: device-tree node of the interrupt controller
> > 
> > But looking at the implementation, it doesn't do a search up the tree or
> > anything, it just checks node against each host.
> 
> For pSeries, passing NULL is fine for host anyway as there is only one
> domain that is relevant for MSIs (there might be a 8259 legacy domain
> too but it's not relevant) and that domain is set to be the default
> host.

OK, will fix it. In the long run we probably want a function that takes
any dn and finds the host for it.

> 
> > Also, since's benh's latest patch went in we'll have to split this into
> > two calls, I think we want:
> > 
> > virq = irq_create_mapping(NULL ???, ret[0]);
> > set_irq_type(virq, ret[1] ? IRQ_TYPE_EDGE_RISING : IRQ_TYPE_LEVEL_LOW);
> 
> MSIs are always edge (though there might be an issue with some P5IOC
> errata lurking here...). The xics code doesn't care much at this point
> though.

Err, great. Anymore info? I'll just set it to EDGE for now.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

^ permalink raw reply

* Re: [PATCH][2/2] RTAS MSI
From: Benjamin Herrenschmidt @ 2006-08-10  8:18 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev, PaulMackerras
In-Reply-To: <1155197015.9801.82.camel@localhost.localdomain>

On Thu, 2006-08-10 at 18:03 +1000, Michael Ellerman wrote:

> OK, will fix it. In the long run we probably want a function that takes
> any dn and finds the host for it.

That's what the OF irq parsing functions do...  You can't just walk up
the device-tree for interrupts, you have to walk up the interrupt
tree...

> Err, great. Anymore info? I'll just set it to EDGE for now.

Or just don't call set_irq_type()... xics doesn't care.

Ben.

^ permalink raw reply

* Re: [PATCH][0/2] RTAS MSI
From: Michael Ellerman @ 2006-08-10  8:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1155138084.17187.53.camel@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

On Wed, 2006-08-09 at 17:41 +0200, Benjamin Herrenschmidt wrote:
> > So I think we could have pci_enable_msi(dev), which would enable a
> > single MSI _or_ MSI-X vector, depending on what's available. That'd be
> > used by drivers that just want a simple replacement for LSI. And then
> > pci_enable_multi_msi(dev, num_irqs) which would give the driver num_irqs
> > MSI or MSI-X vectors.
> 
> You cannot lie and have pci_enable_msi() enable an MSI-X vector. Some
> cards need additional tweaking when enabling MSI or MSI-X and if the
> system enables MSI-X while the driver thinks it's MSI, bad things might
> happen.

Hmm. As I read the PAPR the firmware calls may do just that (pp 122).
ie. it doesn't differentiate between MSIs and MSI-Xs as far as I can
tell. So if we implement pci_enable_msi() via the RTAS calls we might be
violating that constraint.

> pci_enable_msi() should call the firmware to reconfigure for only one
> MSI and enable just that. MSI-X is the only really sexy thing anyway
> (that and a way to spread MSI-X accross CPUs from the kernel but that's
> another topic)

And the current implementation doesn't do that either, so we should fix
it to only allocate 1 MSI, regardless of what firmware has set.

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

^ permalink raw reply

* U-boot on ML403
From: Ming Liu @ 2006-08-10  8:57 UTC (permalink / raw)
  To: lombardo; +Cc: linuxppc-embedded

Dear Frank,
I know you are using U-boot on ML403 now. So can you say something about 
how to configure U-boot as ML403 board? I noticed that in U-boot 1.1.4, 
there is only ML300 supported. So if I want to configure the board as ML403 
and some other customed options(e.g. no EEPROM), what shall I do? Thanks 
for your help.

Regards
Ming

_________________________________________________________________
免费下载 MSN Explorer:   http://explorer.msn.com/lccn  

^ permalink raw reply

* how to generate kernel.img
From: tony @ 2006-08-10  9:00 UTC (permalink / raw)
  To: linuxppc-embedded@ozlabs.org

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]

Dear all
    sometimes I met the "kernel.img", I guess it may generated by the tool "mkimage", right? and how to generate a  kernel.img file during we compile the linux kernel?
    any idea is welcome.
    thanks.                




Sincerely
Tony

               

[-- Attachment #2: Type: text/html, Size: 943 bytes --]

^ permalink raw reply

* Re: [PATCH][0/2] RTAS MSI
From: Benjamin Herrenschmidt @ 2006-08-10  9:23 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1155198129.9801.92.camel@localhost.localdomain>


> Hmm. As I read the PAPR the firmware calls may do just that (pp 122).
> ie. it doesn't differentiate between MSIs and MSI-Xs as far as I can
> tell. So if we implement pci_enable_msi() via the RTAS calls we might be
> violating that constraint.

It's pretty screwed up .... I don't know what is a proper way out there.

> > pci_enable_msi() should call the firmware to reconfigure for only one
> > MSI and enable just that. MSI-X is the only really sexy thing anyway
> > (that and a way to spread MSI-X accross CPUs from the kernel but that's
> > another topic)
> 
> And the current implementation doesn't do that either, so we should fix
> it to only allocate 1 MSI, regardless of what firmware has set.

Yes.

Ben.

^ permalink raw reply

* RE: how to generate kernel.img
From: Li Yang-r58472 @ 2006-08-10  9:42 UTC (permalink / raw)
  To: tony; +Cc: linuxppc-embedded
In-Reply-To: <44DAF90D.00FB12.02709>

I don't think "kernel.img" is a standard name for any kind of images.
Maybe you have to look into the content to find out which kind of image
it is.

Best Regards,
Leo
> -----Original Message-----
> From: linuxppc-embedded-bounces+leoli=3Dfreescale.com@ozlabs.org
> [mailto:linuxppc-embedded-bounces+leoli=3Dfreescale.com@ozlabs.org] On
Behalf Of
> tony
> Sent: Thursday, August 10, 2006 5:01 PM
> To: linuxppc-embedded@ozlabs.org
> Subject: how to generate kernel.img
>=20
> Dear all
>     sometimes I met the "kernel.img", I guess it may generated by the
tool "mkimage",
> right? and how to generate a  kernel.img file during we compile the
linux kernel?
>     any idea is welcome.
>     thanks.
>=20
>=20
>=20
>=20
> Sincerely
> Tony
>=20
>=20

^ permalink raw reply

* Gianfar eth driver on 8540 ppc - for 2.4 and 2.6 : different outputs
From: Prashant Yendigeri @ 2006-08-10 11:18 UTC (permalink / raw)
  To: linuxppc-embedded

[-- Attachment #1: Type: text/plain, Size: 1683 bytes --]

Hi,

The gianfar driver of 2.6.12 and 2.4.20 give different outputs on the same 
PPC 8540 board.

What could be the reason ? 

Output on 2.4.20 : 
/root # ifconfig eth0 172.28.8.254 up
eth0: PHY is Marvell 88E1011S (1410c62)
eth0: Auto-negotiation done
eth0: Half Duplex
eth0: Speed 10BT
eth0: Link is up

Output on 2.6.12
/ # ifconfig eth0 172.28.8.254 up
 eth0: PHY is Generic MII (ffffffff)

Kernel bootup mesages on 2.6.12 for eth initialisation :
[    1.706436] eth0: Gianfar Ethernet Controller Version 1.1, 
00:01:af:07:9b:8a

[    1.713769] eth0: Running with NAPI disabled
[    1.718137] eth0: 64/64 RX/TX BD ring size
[    1.722483] eth1: Gianfar Ethernet Controller Version 1.1, 
fd:e4:75:9d:72:6f

[    1.729812] eth1: Running with NAPI disabled
[    1.734180] eth1: 64/64 RX/TX BD ring size
[    1.738521] eth2: Gianfar Ethernet Controller Version 1.1, 
6f:74:3d:2f:64:65

[    1.745851] eth2: Running with NAPI disabled
[    1.750218] eth2: 64/64 RX/TX BD ring size

On 2.4.20 it is the same :
eth0: Gianfar Ethernet Controller Version 1.0, 00:01:af:07:9b:8a
eth0: Running with NAPI disabled
eth0: 64/64 RX/TX BD ring size
eth1: Gianfar Ethernet Controller Version 1.0, fd:e4:75:9d:72:6f
eth1: Running with NAPI disabled
eth1: 64/64 RX/TX BD ring size
eth2: Gianfar Ethernet Controller Version 1.0, 6f:74:3d:2f:64:65
eth2: Running with NAPI disabled
eth2: 64/64 RX/TX BD ring size

I am able to ping and use telnet,ftp etc when booted from 2.4.20 but ping, 
telnet,ftp, don't work eventhough i use same ramdisk in both the cases.
ifconfig shows card is up in 2.6.12 too.


Regards,
Prashant 







______________________________________________________________________

[-- Attachment #2: Type: text/html, Size: 3410 bytes --]

^ permalink raw reply

* Problems of compiling KGDB patched kernel for 405EP and 440EP boards
From: Zhou Rui @ 2006-08-10 13:47 UTC (permalink / raw)
  To: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 2655 bytes --]

Hi,
I'm trying to enable KGDB between x86 host and PPC target (405EP and 440EP boards are used here). I'm using 2.6.15 kernel from ELDK4.0 patched with linux-2.6.15.5-kgdb-2.4.tar.bz2 and cross-compiling toolchain from ELDK4.0. I run "make menuconfig ARCH=ppc CROSS_COMPILE=ppc_4xx-" for configuration. For 405EP board, PPChameleonEVB is used in "Machine Type"; for 440EP board, Yosemite is used. After configuration, I run "make all ARCH=ppc CROSS_COMPILE=ppc_4xx-". But during compilation, similar errors will occur.

For 405EP, there are:
  LD      arch/powerpc/lib/built-in.o
  CC      arch/ppc/platforms/4xx/ppchameleon.o
arch/ppc/platforms/4xx/ppchameleon.c: In function 'platform_init':
arch/ppc/platforms/4xx/ppchameleon.c:252: error: 'struct machdep_calls' has no member named 'early_serial_map'
make[1]: *** [arch/ppc/platforms/4xx/ppchameleon.o] Error 1
make: *** [arch/ppc/platforms/4xx] Error 2
And the lines cause the errors are:
251  #ifdef CONFIG_KGDB
252          ppc_md.early_serial_map = ppchameleon_early_serial_map;
253  #endif


For 440EP, there are:
  LD      arch/powerpc/lib/built-in.o
  CC      arch/ppc/platforms/4xx/yosemite.o
arch/ppc/platforms/4xx/yosemite.c: In function 'yosemite_early_serial_map':
arch/ppc/platforms/4xx/yosemite.c:288: warning: implicit declaration of function 'gen550_init'
arch/ppc/platforms/4xx/yosemite.c: In function 'platform_init':
arch/ppc/platforms/4xx/yosemite.c:349: error: 'struct machdep_calls' has no member named 'early_serial_map'
make[1]: *** [arch/ppc/platforms/4xx/yosemite.o] Error 1
make: *** [arch/ppc/platforms/4xx] Error 2
And the lines cause the errors are:
348  #ifdef CONFIG_KGDB
349          ppc_md.early_serial_map = yosemite_early_serial_map;
350  #endif


I find that for ARCH=ppc, both ppchameleon.c and yosemite.c include the file ${2.6.15_SOURCE_DIR}/include/asm-ppc/machdep.h. There is "struct machdep_calls" but no member "early_serial_map" in it. However, in the file ${2.6.15_SOURCE_DIR}/include/asm-powerpc/machdep.h, there is "struct machdep_calls" and also member "early_serial_map" in it. If I run "make menuconfig ARCH=powerpc CROSS_COMPILE=ppc_4xx-" instead of "make menuconfig ARCH=ppc CROSS_COMPILE=ppc_4xx-", there is no PPChameleonEVB or Yosemite in "Machine Type". So I can't compile kernel for the boards I have.

Would someone like to give me any advice about this problem? Thank you very much.
  

Zhou Rui
Distributed & Embedded System Lab
School of Information Science & Engineering
Lanzhou University, P. R. China
http://dslab.lzu.edu.cn/~zr/
 __________________________________________________
赶快注册雅虎超大容量免费邮箱?
http://cn.mail.yahoo.com

[-- Attachment #2: Type: text/html, Size: 3028 bytes --]

^ permalink raw reply

* [PATCH] ehea: PPC - New hcall opcode defines
From: Thomas Klein @ 2006-08-10 15:05 UTC (permalink / raw)
  To: linux-ppc; +Cc: Thomas Klein, Christoph Raisch, Marcus Eder, Jan-Bernd Themann

Hi,

this patch adds additional hcall opcode defines in asm-powerpc/hvcall.h
which are required for the IBM eHEA Ethernet Device Driver which is
targeted for kernel inclusion in the near future.

Including those defines in hvcall.h was a request we got in reply to
posting our driver.

Driver post: http://ozlabs.org/pipermail/linuxppc-dev/2006-August/024947.html
Reply: http://ozlabs.org/pipermail/linuxppc-dev/2006-August/024971.html

I'm aware this does not fix a bug and we're already at rc4 but since it
adds only a few innocent defines it would be great if it could be included
in 2.6.18-rc5.

Kind regards
Thomas


Signed-off-by: Thomas Klein <tklein@de.ibm.com>


  include/asm-powerpc/hvcall.h |   13 +++++++++++++
  1 file changed, 13 insertions(+)


diff -Nurp -X dontdiff linux-2.6.18-rc4/include/asm-powerpc/hvcall.h patched_kernel/include/asm-powerpc/hvcall.h
--- linux-2.6.18-rc4/include/asm-powerpc/hvcall.h	2006-08-06 11:20:11.000000000 -0700
+++ patched_kernel/include/asm-powerpc/hvcall.h	2006-08-10 06:26:33.018907062 -0700
@@ -201,6 +201,19 @@
  #define H_JOIN			0x298
  #define H_VASI_STATE            0x2A4
  #define H_ENABLE_CRQ		0x2B0
+#define H_ALLOC_HEA_RESOURCE	0x278
+#define H_MODIFY_HEA_QP		0x250
+#define H_QUERY_HEA_QP		0x254
+#define H_QUERY_HEA		0x258
+#define H_QUERY_HEA_PORT	0x25C
+#define H_MODIFY_HEA_PORT	0x260
+#define H_REG_BCMC		0x264
+#define H_DEREG_BCMC		0x268
+#define H_REGISTER_HEA_RPAGES	0x26C
+#define H_DISABLE_AND_GET_HEA	0x270
+#define H_GET_HEA_INFO		0x274
+#define H_ADD_CONN		0x284
+#define H_DEL_CONN		0x288

  #ifndef __ASSEMBLY__

^ permalink raw reply

* Re: [PATCH 1/6] ehea: interface to network stack
From: Jan-Bernd Themann @ 2006-08-10 14:28 UTC (permalink / raw)
  To: michael
  Cc: Thomas Klein, netdev, linux-kernel, linux-ppc, Christoph Raisch,
	Marcus Eder
In-Reply-To: <1155190553.9801.38.camel@localhost.localdomain>

Hi Michael,

thanks for your very helpful comments so far, we'll provide a patch with these and other fixes
very soon.

See comments below.

Jan-Bernd

Michael Ellerman wrote:
 > Hi Jan-Bernd,
 >
 > Comments below the code they refer to.
 >
 > On Wed, 2006-08-09 at 10:38 +0200, Jan-Bernd Themann wrote:
 >> Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com>
 >
 >>   drivers/net/ehea/ehea_main.c | 2738 +++++++++++++++++++++++++++++++++++++++++++
 >>   1 file changed, 2738 insertions(+)
 >
 >> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_main.c	1969-12-31 16:00:00.000000000 -0800
 >> +++ kernel/drivers/net/ehea/ehea_main.c	2006-08-08 23:59:39.683357016 -0700
 >> @@ -0,0 +1,2738 @@
 >> +/*
 >> + *  linux/drivers/net/ehea/ehea_main.c
 >
 > Putting the file name in the file is fairly redundant IMHO.
 >
 >> + *  eHEA ethernet device driver for IBM eServer System p
 >
 > What's the actual hardware that this is for? System p covers a whole
 > range of machines, do they really all support this driver?
 >
 >> + *  (C) Copyright IBM Corp. 2006
 >> + *
 >> + *  Authors:
 >> + *       Christoph Raisch <raisch@de.ibm.com>
 >> + *       Jan-Bernd Themann <themann@de.ibm.com>
 >> + *       Heiko-Joerg Schick <schickhj@de.ibm.com>
 >> + *       Thomas Klein <tklein@de.ibm.com>
 >> + *
 >> + *
 >> + * This program is free software; you can redistribute it and/or modify
 >> + * it under the terms of the GNU General Public License as published by
 >> + * the Free Software Foundation; either version 2, or (at your option)
 >> + * any later version.
 >> + *
 >> + * This program is distributed in the hope that it will be useful,
 >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
 >> + * GNU General Public License for more details.
 >> + *
 >> + * You should have received a copy of the GNU General Public License
 >> + * along with this program; if not, write to the Free Software
 >> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 >> + */
 >> +
 >> +#define DEB_PREFIX "main"
 >> +
 >> +#include <linux/in.h>
 >> +#include <linux/ip.h>
 >> +#include <linux/tcp.h>
 >> +#include <linux/udp.h>
 >> +#include <linux/if.h>
 >> +#include <linux/list.h>
 >> +#include <net/ip.h>
 >> +
 >> +#include "ehea.h"
 >> +#include "ehea_qmr.h"
 >> +#include "ehea_phyp.h"
 >> +
 >> +
 >> +MODULE_LICENSE("GPL");
 >> +MODULE_AUTHOR("Christoph Raisch <raisch@de.ibm.com>");
 >> +MODULE_DESCRIPTION("IBM eServer HEA Driver");
 >> +MODULE_VERSION(EHEA_DRIVER_VERSION);
 >> +
 >> +static int __devinit ehea_probe(struct ibmebus_dev *dev,
 >> +				const struct of_device_id *id);
 >> +static int __devexit ehea_remove(struct ibmebus_dev *dev);
 >> +static int ehea_sense_port_attr(struct ehea_adapter *adapter, int portnum);
 >
 > I haven't looked closely, but can you rearrange the functions so you
 > don't need these forward declarations?

yes, rearrangement is possible. Done.

 >
 >> +int ehea_trace_level = 5;
 >> +
 >> +static struct net_device_stats *ehea_get_stats(struct net_device *dev)
 >> +{
 >> +	int i;
 >> +	u64 hret = H_HARDWARE;
 >
 > You unconditionally assign to hret below.
 >
 >> +	u64 rx_packets = 0;
 >
 > Why not just update stats->rx_packets directly?
 >
 >> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
 >> +	struct ehea_adapter *adapter = port->adapter;
 >
 > I don't think you need adapter, you only use it in one place, just
 > access it through port->adapter->handle (below).

done

 >
 >> +	struct hcp_query_ehea_port_cb_2 *cb2 = NULL;
 >> +	struct net_device_stats *stats = &port->stats;
 >> +
 >> +	EDEB_EN(7, "net_device=%p", dev);
 >> +
 >> +	cb2 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
 >> +	if (!cb2) {
 >> +		EDEB_ERR(4, "No memory for cb2");
 >> +		goto get_stat_exit;
 >
 > You leak cb2 here.
 >

done

 >> +	}
 >> +
 >> +	hret = ehea_h_query_ehea_port(adapter->handle,
 >> +				      port->logical_port_id,
 >> +				      H_PORT_CB2,
 >> +				      H_PORT_CB2_ALL,
 >> +				      cb2);
 >> +
 >> +	if (hret != H_SUCCESS) {
 >> +		EDEB_ERR(4, "query_ehea_port failed for cb2");
 >> +		goto get_stat_exit;
 >> +	}
 >> +
 >> +	EDEB_DMP(7, (u8*)cb2,
 >> +		 sizeof(struct hcp_query_ehea_port_cb_2), "After HCALL");
 >> +
 >> +	for (i = 0; i < port->num_def_qps; i++) {
 >> +		rx_packets += port->port_res[i].rx_packets;
 >> +	}
 >> +
 >> +	stats->tx_packets = cb2->txucp + cb2->txmcp + cb2->txbcp;
 >> +	stats->multicast = cb2->rxmcp;
 >> +	stats->rx_errors = cb2->rxuerr;
 >> +	stats->rx_bytes = cb2->rxo;
 >> +	stats->tx_bytes = cb2->txo;
 >> +	stats->rx_packets = rx_packets;
 >> +
 >> +get_stat_exit:
 >> +	EDEB_EX(7, "");
 >> +	return stats;
 >> +}
 >> +
 >> +static inline u32 ehea_get_send_lkey(struct ehea_port_res *pr)
 >> +{
 >> +	return pr->send_mr.lkey;
 >> +}
 >
 > Get rid of this, it's only used once.
 >

done

 >> +static inline u32 ehea_get_recv_lkey(struct ehea_port_res *pr)
 >> +{
 >> +	return pr->recv_mr.lkey;
 >> +}
 >
 > And this one only twice? Is it really useful?
 >

done

 >> +
 >> +#define EHEA_OD_ADDR(address, segment) (((address) & (PAGE_SIZE - 1)) \
 >> +					| ((segment) << 12));
 >> +
 >> +static inline u64 get_swqe_addr(u64 tmp_addr, int addr_seg)
 >> +{
 >> +	u64 addr;
 >> +	addr = tmp_addr;
 >> +	return addr;
 >> +}
 >> +
 >> +static inline u64 get_rwqe_addr(u64 tmp_addr)
 >> +{
 >> +	return tmp_addr;
 >> +}
 >
 > This two functions seem useless or wrong.
 >

removed

 >> +static inline int ehea_refill_rq1(struct ehea_port_res *port_res, int arr_index,
 >> +				  int nr_of_wqes)
 >> +{
 >> +	int i;
 >> +	int ret = 0;
 >> +	struct ehea_qp *qp;
 >> +	int skb_arr_rq1_len = port_res->skb_arr_rq1_len;
 >> +	struct sk_buff **skb_arr_rq1 = port_res->skb_arr_rq1;
 >> +	EDEB_EN(7, "port_res=%p, arr_index=%d, nr_of_wqes=%d, arr_rq1_len=%d",
 >> +		port_res, arr_index, nr_of_wqes, skb_arr_rq1_len);
 >> +
 >> +	qp = port_res->qp;
 >
 > You don't need the qp variable, just use port_res->qp below.
 >

done

 >> +	if (unlikely(nr_of_wqes == 0))
 >> +		return -EINVAL;
 >
 > Newline needed here.
 >

done

 >> +	for (i = 0; i < nr_of_wqes; i++) {
 >> +		int index = ((skb_arr_rq1_len + arr_index) - i)
 >> +		    % skb_arr_rq1_len;
 >
 > The wrapped line should be indented further to the right.
 >

done

 >> +		if (!skb_arr_rq1[index]) {
 >> +			skb_arr_rq1[index] = dev_alloc_skb(EHEA_LL_PKT_SIZE);
 >> +
 >> +			if (!skb_arr_rq1[index]) {
 >> +				EDEB_ERR(4, "No mem for skb/%d wqes filled", i);
 >> +				ret = -ENOMEM;
 >> +				break;
 >> +			}
 >> +		}
 >> +	}
 >> +	/* Ring doorbell */
 >> +	ehea_update_rq1a(qp, nr_of_wqes);
 >> +	EDEB_EX(7, "");
 >> +	return ret;
 >> +}
 >> +
 >> +static int ehea_init_fill_rq1(struct ehea_port_res *port_res, int nr_rq1a)
 >> +{
 >> +	int i;
 >> +	int ret = 0;
 >> +	struct ehea_qp *qp;
 >> +	EDEB_EN(7, "port_res=%p, nr_rq1a=%d", port_res, nr_rq1a);
 >> +	qp = port_res->qp;
 >> +
 >> +	for (i = 0; i < port_res->skb_arr_rq1_len; i++) {
 >> +		port_res->skb_arr_rq1[i] = dev_alloc_skb(EHEA_LL_PKT_SIZE);
 >> +		if (!port_res->skb_arr_rq1[i]) {
 >> +			EDEB_ERR(4, "dev_alloc_skb failed. Only %d skb filled.",
 >> +				 i);
 >> +			ret = -ENOMEM;
 >> +			break;
 >> +		}
 >> +	}
 >> +	/* Ring doorbell */
 >> +	ehea_update_rq1a(qp, nr_rq1a);
 >> +	EDEB_EX(7, "");
 >> +	return ret;
 >> +}
 >> +
 >> +static inline int ehea_refill_rq2_def(struct ehea_port_res *pr, int nr_of_wqes)
 >> +{
 >> +	int i;
 >> +	int ret = 0;
 >> +	struct ehea_qp *qp;
 >> +	struct ehea_rwqe *rwqe;
 >> +	int skb_arr_rq2_len = pr->skb_arr_rq2_len;
 >> +	struct sk_buff **skb_arr_rq2 = pr->skb_arr_rq2;
 >> +
 >> +	EDEB_EN(8, "pr=%p, nr_of_wqes=%d", pr, nr_of_wqes);
 >> +	if (nr_of_wqes == 0)
 >> +		return -EINVAL;
 >> +	qp = pr->qp;
 >> +	for (i = 0; i < nr_of_wqes; i++) {
 >> +		int index = pr->skb_rq2_index++;
 >> +		struct sk_buff *skb = dev_alloc_skb(EHEA_RQ2_PKT_SIZE
 >> +						    + NET_IP_ALIGN);
 >> +
 >> +		if (!skb) {
 >> +			EDEB_ERR(4, "dev_alloc_skb nr %d failed", i);
 >> +			ret = -ENOMEM;
 >> +			break;
 >> +		}
 >> +		skb_reserve(skb, NET_IP_ALIGN);
 >> +		pr->skb_rq2_index %= skb_arr_rq2_len;
 >> +		skb_arr_rq2[index] = skb;
 >> +		rwqe = ehea_get_next_rwqe(qp, 2);
 >> +		rwqe->wr_id = EHEA_BMASK_SET(EHEA_WR_ID_TYPE, EHEA_RWQE2_TYPE)
 >> +		            | EHEA_BMASK_SET(EHEA_WR_ID_INDEX, index);
 >> +		rwqe->sg_list[0].l_key = ehea_get_recv_lkey(pr);
 >> +		rwqe->sg_list[0].vaddr = get_rwqe_addr((u64)skb->data);
 >> +		rwqe->sg_list[0].len = EHEA_RQ2_PKT_SIZE;
 >> +		rwqe->data_segments = 1;
 >> +	}
 >> +
 >> +	/* Ring doorbell */
 >> +	iosync();
 >> +	ehea_update_rq2a(qp, i);
 >> +	EDEB_EX(8, "");
 >> +	return ret;
 >> +}
 >> +
 >> +
 >> +static inline int ehea_refill_rq2(struct ehea_port_res *pr, int nr_of_wqes)
 >> +{
 >> +	return ehea_refill_rq2_def(pr, nr_of_wqes);
 >> +}
 >
 > Why do you need this function?
 >

this is for near future features

 >> +static inline int ehea_refill_rq3_def(struct ehea_port_res *pr, int nr_of_wqes)
 >> +{
 >> +	int i;
 >> +	int ret = 0;
 >> +	struct ehea_qp *qp;
 >> +	struct ehea_rwqe *rwqe;
 >> +	int skb_arr_rq3_len = pr->skb_arr_rq3_len;
 >> +	struct sk_buff **skb_arr_rq3 = pr->skb_arr_rq3;
 >> +	EDEB_EN(8, "pr=%p, nr_of_wqes=%d", pr, nr_of_wqes);
 >> +	if (nr_of_wqes == 0)
 >> +		return -EINVAL;
 >> +	qp = pr->qp;
 >> +	for (i = 0; i < nr_of_wqes; i++) {
 >> +		int index = pr->skb_rq3_index++;
 >> +		struct sk_buff *skb = dev_alloc_skb(EHEA_MAX_PACKET_SIZE
 >> +						    + NET_IP_ALIGN);
 >> +
 >> +		if (!skb) {
 >> +			EDEB_ERR(4, "No memory for skb. Only %d rwqe filled.",
 >> +				 i);
 >> +			ret = -ENOMEM;
 >> +			break;
 >> +		}
 >> +		skb_reserve(skb, NET_IP_ALIGN);
 >> +
 >> +		rwqe = ehea_get_next_rwqe(qp, 3);
 >> +		pr->skb_rq3_index %= skb_arr_rq3_len;
 >> +		skb_arr_rq3[index] = skb;
 >> +		rwqe->wr_id = EHEA_BMASK_SET(EHEA_WR_ID_TYPE, EHEA_RWQE3_TYPE)
 >> +		    | EHEA_BMASK_SET(EHEA_WR_ID_INDEX, index);
 >> +		rwqe->sg_list[0].l_key = ehea_get_recv_lkey(pr);
 >> +		rwqe->sg_list[0].vaddr = get_rwqe_addr((u64)skb->data);
 >> +		rwqe->sg_list[0].len = EHEA_MAX_PACKET_SIZE;
 >> +		rwqe->data_segments = 1;
 >> +	}
 >> +
 >> +	/* Ring doorbell */
 >> +	iosync();
 >> +	ehea_update_rq3a(qp, i);
 >> +	EDEB_EX(8, "");
 >> +	return ret;
 >> +}
 >
 > You "Ring doorbell" in four places, but only in two do you do iosync(),
 > that might be ok, but it looks suspicious. Also, what is the iosync
 > trying to achieve?
 >

That is ok. We do a iosync to make sure that the hardware sees the entire
wqe we put on the queue. For RQ1, we don't put something on the queue that
has to be read by the hea hardware -> no iosync needed

 > ehea_refill_rq3_def() and ehea_refill_rq2_def() look quite similar, can
 > they be combined?
 >

Well, it seems to be a tradeoff between a function with lots of parameters
and two separate functions. Our choice was the last option.

 >> +
 >> +
 >> +static inline int ehea_refill_rq3(struct ehea_port_res *pr, int nr_of_wqes)
 >> +{
 >> +	return ehea_refill_rq3_def(pr, nr_of_wqes);
 >> +}
 >> +
 >> +
 >> +static inline int ehea_check_cqe(struct ehea_cqe *cqe, int *rq_num)
 >> +{
 >> +	*rq_num = (cqe->type & EHEA_CQE_TYPE_RQ) >> 5;
 >> +	EDEB(7, "RQ used=%d, status=%X", *rq_num, cqe->status);
 >> +	if ((cqe->status & EHEA_CQE_STAT_ERR_MASK) == 0)
 >> +		return 0;
 >> +	if (((cqe->status & EHEA_CQE_STAT_ERR_TCP) != 0)
 >> +	    && (cqe->header_length == 0))
 >> +		return 0;
 >> +	else
 >> +		printk("WARNING: Packet discarded. Wrong TCP/UDP chksum"
 >> +		       "and header_length != 0. cqe->status=%X", cqe->status);
 >> +
 >> +	return -EINVAL;
 >> +}
 >> +
 >> +static inline void ehea_fill_skb_ll(struct net_device *dev,
 >> +				    struct sk_buff *skb, struct ehea_cqe *cqe)
 >> +{
 >> +	int length = cqe->num_bytes_transfered - 4;	/*remove CRC */
 >> +	EDEB_EN(7, "dev=%p, skb=%p, cqe=%p", dev, skb, cqe);
 >> +	memcpy(skb->data, ((char*)cqe) + 64, length);
 >> +	skb_put(skb, length);
 >> +	skb->dev = dev;
 >> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
 >> +	skb->protocol = eth_type_trans(skb, dev);
 >> +	EDEB_EX(7, "");
 >> +}
 >> +
 >> +static inline void ehea_fill_skb(struct net_device *dev,
 >> +				 struct sk_buff *skb, struct ehea_cqe *cqe)
 >> +{
 >> +	int length = cqe->num_bytes_transfered - 4;	/*remove CRC */
 >> +	EDEB_EN(7, "dev=%p, skb=%p, cqe=%p", dev, skb, cqe);
 >> +	skb_put(skb, length);
 >> +	skb->dev = dev;
 >> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
 >> +	skb->protocol = eth_type_trans(skb, dev);
 >> +	EDEB_EX(7, "");
 >> +}
 >
 > What's the difference between the above two functions?
 >

the first one has an additional copy. But you are right, we get rid of
ehea_fill_skb_ll

 >> +#define EHEA_MAX_RWQE 1000
 >
 > Should probably be in a header.
 >

agreed

 >> +static int ehea_poll(struct net_device *dev, int *budget)
 >> +{
 >> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
 >> +	struct ehea_port_res *port_res = &port->port_res[0];
 >> +	struct ehea_cqe *cqe;
 >> +	struct ehea_qp *qp = port_res->qp;
 >> +	int wqe_index = 0;
 >> +	int last_wqe_index = 0;
 >> +	int x = 0;
 >> +	int processed = 0;
 >> +	int processed_RQ1 = 0;
 >> +	int processed_RQ2 = 0;
 >> +	int processed_RQ3 = 0;
 >> +	int rq;
 >> +	int intreq;
 >> +	struct sk_buff **skb_arr_rq1 = port_res->skb_arr_rq1;
 >> +	struct sk_buff **skb_arr_rq2 = port_res->skb_arr_rq2;
 >> +	struct sk_buff **skb_arr_rq3 = port_res->skb_arr_rq3;
 >> +	int skb_arr_rq1_len = port_res->skb_arr_rq1_len;
 >> +	int my_quota = min(*budget, dev->quota);
 >> +
 >> +	EDEB_EN(7, "dev=%p, port_res=%p, budget=%d, quota=%d, qp_nr=%x",
 >> +		dev, port_res, *budget, dev->quota,
 >> +		port_res->qp->init_attr.qp_nr);
 >> +	my_quota = min(my_quota, EHEA_MAX_RWQE);
 >> +
 >> +	/* rq0 is low latency RQ */
 >> +	cqe = ehea_poll_rq1(qp, &wqe_index);
 >> +	while ((my_quota > 0) && cqe) {
 >> +		ehea_inc_rq1(qp);
 >> +		processed_RQ1++;
 >> +		processed++;
 >> +		my_quota--;
 >> +
 >> +		EDEB_DMP(6, (u8*)cqe, 4 * 16, "CQE");
 >> +		last_wqe_index = wqe_index;
 >> +		rmb();
 >> +		if (!ehea_check_cqe(cqe, &rq)) {
 >> +			struct sk_buff *skb;
 >> +			if (rq == 1) {	/* LL RQ1 */
 >> +				void *pref;
 >> +
 >> +				x = (wqe_index + 1) % skb_arr_rq1_len;
 >> +				pref = (void*)skb_arr_rq1[x];
 >> +				prefetchw(pref);
 >> +				prefetchw(pref + EHEA_CACHE_LINE);
 >> +
 >> +				x = (wqe_index + 1) % skb_arr_rq1_len;
 >> +				pref = (void*)(skb_arr_rq1[x]->data);
 >> +				prefetchw(pref);
 >> +				prefetchw(pref + EHEA_CACHE_LINE);
 >> +
 >> +				skb = skb_arr_rq1[wqe_index];
 >> +				if (unlikely(!skb)) {
 >> +					EDEB_ERR(4, "LL SBK=NULL, wqe_index=%d",
 >> +						 wqe_index);
 >> +					skb = dev_alloc_skb(EHEA_LL_PKT_SIZE);
 >> +					if (!skb)
 >> +						panic("Alloc SKB failed");
 >> +				}
 >> +				skb_arr_rq1[wqe_index] = NULL;
 >> +				ehea_fill_skb_ll(dev, skb, cqe);
 >> +			} else if (rq == 2) {	/* RQ2 */
 >> +				void *pref;
 >> +				int skb_index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
 >> +							       cqe->wr_id);
 >> +				x = (skb_index + 1) % port_res->skb_arr_rq2_len;
 >> +				pref = (void*)skb_arr_rq2[x];
 >> +				prefetchw(pref);
 >> +				prefetchw(pref + EHEA_CACHE_LINE);
 >> +
 >> +				x = (skb_index + 1) % port_res->skb_arr_rq2_len;
 >> +				pref = (void*)(skb_arr_rq2[x]->data);
 >> +
 >> +				prefetch(pref);
 >> +				prefetch(pref + EHEA_CACHE_LINE);
 >> +				prefetch(pref + EHEA_CACHE_LINE * 2);
 >> +				prefetch(pref + EHEA_CACHE_LINE * 3);
 >> +				skb = skb_arr_rq2[skb_index];
 >> +
 >> +				if (unlikely(!skb)) {
 >> +					EDEB_ERR(4, "rq2: SKB=NULL, index=%d",
 >> +						 skb_index);
 >> +					break;
 >> +				}
 >> +				skb_arr_rq2[skb_index] = NULL;
 >> +				ehea_fill_skb(dev, skb, cqe);
 >> +				processed_RQ2++;
 >> +			} else {
 >> +				void *pref;
 >> +				int skb_index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
 >> +							       cqe->wr_id);
 >> +				x = (skb_index + 1) % port_res->skb_arr_rq3_len;
 >> +				pref = (void*)skb_arr_rq3[x];
 >> +				prefetchw(pref);
 >> +				prefetchw(pref + EHEA_CACHE_LINE);
 >> +
 >> +				x = (skb_index + 1) % port_res->skb_arr_rq3_len;
 >> +				pref = (void*)(skb_arr_rq3[x]->data);
 >> +				prefetch(pref);
 >> +				prefetch(pref + EHEA_CACHE_LINE);
 >> +				prefetch(pref + EHEA_CACHE_LINE * 2);
 >> +				prefetch(pref + EHEA_CACHE_LINE * 3);
 >> +
 >> +				skb = skb_arr_rq3[skb_index];
 >> +				if (unlikely(!skb)) {
 >> +					EDEB_ERR(4, "rq3: SKB=NULL, index=%d",
 >> +						 skb_index);
 >> +					break;
 >> +				}
 >> +				skb_arr_rq3[skb_index] = NULL;
 >> +				ehea_fill_skb(dev, skb, cqe);
 >> +				processed_RQ3++;
 >> +			}
 >> +
 >> +			EDEB(6, "About to pass SKB: dev=%p\n"
 >> +			     "skb=%p skb->data=%p skb->len=%d"
 >> +			     " skb->data_len=0x%x nr_frags=%d",
 >> +			     dev,
 >> +			     skb,
 >> +			     skb->data,
 >> +			     skb->len,
 >> +			     skb->data_len, skb_shinfo(skb)->nr_frags);
 >> +			if (cqe->status & EHEA_CQE_VLAN_TAG_XTRACT) {
 >> +				EDEB(7, "VLAN TAG extracted: %4x, vgrp=%p",
 >> +				     cqe->vlan_tag, port->vgrp);
 >> +				EDEB(7, "vlan_devices[vlan_tag]=%p",
 >> +				     port->vgrp->vlan_devices[cqe->vlan_tag]);
 >> +				vlan_hwaccel_receive_skb(skb, port->vgrp,
 >> +							 cqe->vlan_tag);
 >> +			} else {
 >> +				EDEB(7, "netif_receive_skb");
 >> +				netif_receive_skb(skb);
 >> +			}
 >> +			EDEB(7, "SKB passed (netif_receive(skb) called)");
 >> +
 >> +		} else {
 >> +			struct sk_buff *skb;
 >> +
 >> +			EDEB_ERR(4, "cqe->status indicating error: CQE:");
 >> +			EDEB_DMP(4, (u8*)cqe, 4 * 16, "");
 >> +			if (rq == 2) {
 >> +				processed_RQ2++;
 >> +				skb = skb_arr_rq2[
 >> +					EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
 >> +							  cqe->wr_id)];
 >> +				skb_arr_rq2[EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
 >> +							  cqe->wr_id)] = NULL;
 >> +				dev_kfree_skb(skb);
 >> +			}
 >> +			if (rq == 3) {
 >> +				processed_RQ3++;
 >> +				skb = skb_arr_rq3[
 >> +					EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
 >> +								cqe->wr_id)];
 >> +				skb_arr_rq3[EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
 >> +							  cqe->wr_id)] = NULL;
 >> +				dev_kfree_skb(skb);
 >> +			}
 >> +		}
 >> +		cqe = ehea_poll_rq1(qp, &wqe_index);
 >> +	}
 >> +
 >> +	dev->quota -= processed;
 >> +	*budget -= processed;
 >> +
 >> +	port_res->p_state.ehea_poll += 1;
 >> +
 >> +	port_res->rx_packets += processed;
 >> +
 >> +	ehea_refill_rq1(port_res, last_wqe_index, processed_RQ1);
 >> +	ehea_refill_rq2(port_res, processed_RQ2);
 >> +	ehea_refill_rq3(port_res, processed_RQ3);
 >> +
 >> +	intreq = ((port_res->p_state.ehea_poll & 0xF) == 0xF);
 >> +
 >> +	EDEB_EX(7, "processed=%d, *budget=%d, dev->quota=%d",
 >> +		processed, *budget, dev->quota);
 >> +
 >> +	if (!cqe || intreq) {
 >> +		netif_rx_complete(dev);
 >> +		ehea_reset_cq_ep(port_res->recv_cq);
 >> +		ehea_reset_cq_n1(port_res->recv_cq);
 >> +		cqe = ipz_qeit_get_valid(&qp->ipz_rqueue1);
 >> +		EDEB(7, "CQE=%p, break ehea_poll while loop", cqe);
 >> +		if (!cqe || intreq)
 >> +			return 0;
 >> +		if (!netif_rx_reschedule(dev, my_quota))
 >> +			return 0;
 >> +	}
 >> +	return 1;
 >> +}
 >
 > This function is _way_ too big.
 >

Ok, we will make it smaller

 >> +#define MAX_SENDCOMP_QUOTA 400
 >> +void ehea_send_irq_tasklet(unsigned long data)
 >> +{
 >> +	int quota = MAX_SENDCOMP_QUOTA;
 >> +	int skb_index;
 >> +	int cqe_counter = 0;
 >> +	int swqe_av = 0;
 >> +	unsigned long flags;
 >> +	struct sk_buff *skb;
 >> +	struct ehea_cqe *cqe;
 >> +	struct ehea_port_res *port_res = (struct ehea_port_res*)data;
 >> +	struct ehea_cq *send_cq = port_res->send_cq;
 >> +	struct net_device *dev = port_res->port->netdev;
 >> +
 >> +	EDEB_EN(7, "port_res=%p", port_res);
 >> +
 >> +	do {
 >> +		cqe = ehea_poll_cq(send_cq);
 >> +		if (!cqe) {
 >> +			EDEB(7, "No further cqe found");
 >> +			ehea_reset_cq_ep(send_cq);
 >> +			ehea_reset_cq_n1(send_cq);
 >> +			cqe = ehea_poll_cq(send_cq);
 >> +			if (!cqe) {
 >> +				EDEB(7, "No cqe found after having"
 >> +				     " reset N1/EP\n");
 >> +				break;
 >> +			}
 >> +		}
 >> +		cqe_counter++;
 >> +		EDEB(7, "CQE found on Send-CQ:");
 >> +		EDEB_DMP(7, (u8*)cqe, 4 * 16, "");
 >> +		rmb();
 >> +		if (likely(EHEA_BMASK_GET(EHEA_WR_ID_TYPE, cqe->wr_id)
 >> +			   == EHEA_SWQE2_TYPE)) {	/* is swqe format 2 */
 >> +			int i;
 >> +			int index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX,
 >> +						     cqe->wr_id);
 >> +			for (i = 0; i < EHEA_BMASK_GET(EHEA_WR_ID_REFILL,
 >> +						       cqe->wr_id); i++) {
 >> +
 >> +				skb_index = ((index - i
 >> +					      + port_res->skb_arr_sq_len)
 >> +					     % port_res->skb_arr_sq_len);
 >> +				skb = port_res->skb_arr_sq[skb_index];
 >> +				port_res->skb_arr_sq[skb_index] = NULL;
 >> +
 >> +				if (unlikely(!skb)) {
 >> +					EDEB_ERR(4, "s_irq_tasklet: SKB=NULL "
 >> +						 "WQ_ID=%lX, loop=%d, index=%d",
 >> +						 cqe->wr_id, i, skb_index);
 >> +					break;
 >> +				}
 >> +				dev_kfree_skb(skb);
 >> +			}
 >> +		}
 >> +		swqe_av += EHEA_BMASK_GET(EHEA_WR_ID_REFILL, cqe->wr_id);
 >> +		quota--;
 >> +	} while (quota > 0);
 >> +
 >> +	ehea_update_feca(send_cq, cqe_counter);
 >> +
 >> +	atomic_add(swqe_av, &port_res->swqe_avail);
 >> +	EDEB(7, "port_res->swqe_avail=%d",
 >> +	     atomic_read(&port_res->swqe_avail));
 >> +
 >> +	if (unlikely(netif_queue_stopped(dev))) {
 >> +		spin_lock_irqsave(&port_res->netif_queue, flags);
 >> +		if (unlikely((atomic_read(&port_res->swqe_avail)
 >> +			      >= EHEA_SWQE_REFILL_TH))) {
 >> +			EDEB(7, "port %d swqes_avail >=10 (%d),"
 >> +			     "netif_wake_queue called",
 >> +			     port_res->port->logical_port_id,
 >> +			     atomic_read(&port_res->swqe_avail));
 >> +			netif_wake_queue(port_res->port->netdev);
 >> +		}
 >> +		spin_unlock_irqrestore(&port_res->netif_queue, flags);
 >> +	}
 >> +
 >> +	if (unlikely(cqe))
 >> +		tasklet_hi_schedule(&port_res->send_comp_task);
 >> +
 >> +	EDEB_EX(7, "");
 >> +}
 >
 > This one's pretty big too.
 >

Done

 >> +
 >> +irqreturn_t ehea_send_irq_handler(int irq, void *param, struct pt_regs *regs)
 >> +{
 >> +	struct ehea_port_res *pr = (struct ehea_port_res*)param;
 >> +	EDEB_EN(7, "irq=%d, param=%p, pt_regs=%p", irq, param, regs);
 >> +	tasklet_hi_schedule(&pr->send_comp_task);
 >> +	EDEB_EX(7, "");
 >> +	return IRQ_HANDLED;
 >> +}
 >> +
 >> +irqreturn_t ehea_recv_irq_handler(int irq, void *param, struct pt_regs * regs)
 >> +{
 >> +	struct ehea_port_res *pr = (struct ehea_port_res*)param;
 >> +	struct ehea_port *port = pr->port;
 >> +	EDEB_EN(7, "irq=%d, param=%p, pt_regs=%p", irq, param, regs);
 >> +	netif_rx_schedule(port->netdev);
 >> +	EDEB_EX(7, "");
 >> +	return IRQ_HANDLED;
 >> +}
 >> +
 >> +irqreturn_t ehea_qp_aff_irq_handler(int irq, void *param, struct pt_regs * regs)
 >> +{
 >> +	struct ehea_port *port = (struct ehea_port*)param;
 >> +	struct ehea_eqe *eqe;
 >> +	u32 qp_token;
 >> +
 >> +	EDEB_EN(7, "irq=%d, param=%p, pt_regs=%p", irq, param, regs);
 >> +	eqe = (struct ehea_eqe*)ehea_poll_eq(port->qp_eq);
 >
 > Suspicious cast?
 >

ehea_poll_eq changed so that this cast is not necessary anymore

 >> +	EDEB(7, "eqe=%p", eqe);
 >> +	while (eqe) {
 >> +		EDEB(7, "*eqe=%lx", *(u64*)eqe);
 >> +		eqe = (struct ehea_eqe*)ehea_poll_eq(port->qp_eq);
 >> +		qp_token = EHEA_BMASK_GET(EHEA_EQE_QP_TOKEN, eqe->entry);
 >
 > You don't seem to use qp_token?
 >

planned for recovery, we are working on that

 >> +		EDEB(7, "next eqe=%p", eqe);
 >> +	}
 >> +
 >> +	EDEB_EX(7, "");
 >> +	return IRQ_HANDLED;
 >> +}
 >> +
 >> +static struct ehea_port *ehea_get_port(struct ehea_adapter *adapter,
 >> +				       int logical_port)
 >> +{
 >> +	int i;
 >> +
 >> +	for (i = 0; i < adapter->num_ports; i++)
 >> +		if (adapter->port[i]->logical_port_id == logical_port)
 >> +			return adapter->port[i];
 >> +	return NULL;
 >> +}
 >> +
 >> +static void ehea_parse_eqe(struct ehea_adapter *adapter, u64 eqe)
 >> +{
 >> +	int ret = -EINVAL;
 >> +	u8 ec = 0;
 >> +	u8 portnum = 0;
 >> +	struct ehea_port *port = NULL;
 >
 > You don't need to initialise these three variables.
 >

true

 >> +
 >> +	EDEB_EN(7, "eqe=%lx", eqe);
 >> +
 >> +	ec = EHEA_BMASK_GET(NEQE_EVENT_CODE, eqe);
 >> +
 >> +	switch (ec) {
 >> +	case EHEA_EC_PORTSTATE_CHG:	/* port state change */
 >> +		EDEB(7, "Port state change");
 >> +		portnum = EHEA_BMASK_GET(NEQE_PORTNUM, eqe);
 >> +		port = ehea_get_port(adapter, portnum);
 >> +
 >> +		if (!port) {
 >> +			EDEB_ERR(4, "Unknown portnum %x", portnum);
 >> +			break;
 >> +		}
 >> +
 >> +		if (EHEA_BMASK_GET(NEQE_PORT_UP, eqe)) {
 >> +			if (!netif_carrier_ok(port->netdev)) {
 >> +				ret = ehea_sense_port_attr(adapter, portnum);
 >> +				if (ret) {
 >> +					EDEB_ERR(4, "Failed resensing port");
 >> +					break;
 >> +				}
 >> +
 >> +				printk("%s: Logical port up: %dMbps %s Duplex",
 >> +				       port->netdev->name,
 >> +				       port->port_speed,
 >> +				       port->full_duplex ==
 >> +				       1 ? "Full" : "Half");
 >> +
 >> +				netif_carrier_on(port->netdev);
 >> +				netif_wake_queue(port->netdev);
 >> +			}
 >> +		} else
 >> +			if (netif_carrier_ok(port->netdev)) {
 >> +				printk("%s: Logical port down",
 >> +				       port->netdev->name);
 >> +				netif_carrier_off(port->netdev);
 >> +				netif_stop_queue(port->netdev);
 >> +			}
 >> +
 >> +		if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PORT_UP, eqe))
 >> +			printk("%s: Physical port up", port->netdev->name);
 >> +		else
 >> +			printk("%s: Physical port down", port->netdev->name);
 >> +
 >> +		if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PRIMARY, eqe))
 >> +			printk("Externel switch port is primary port");
 >> +		else
 >> +			printk("Externel switch port is backup port");
 >
 > These printks should probably use KERN_DEBUG, or netif_msg_foo().
 >

I guess, KERN_INFO would be a good choice

 >> +		break;
 >> +	case EHEA_EC_ADAPTER_MALFUNC:	/* adapter malfunction */
 >> +		EDEB_ERR(4, "Adapter malfunction");
 >> +		break;
 >> +	case EHEA_EC_PORT_MALFUNC:	/* port malfunction */
 >> +		EDEB_ERR(4, "Port malfunction");
 >> +		/* TODO Determine the port structure of the malfunctioning port
 >> +		 * netif_carrier_off(port->netdev);
 >> +		 * netif_stop_queue(port->netdev);
 >> +		 */
 >> +		break;
 >
 > The two malfunction states could probably use a printk(KERN_WARNING .. )
 > for now.
 >

done

 >> +	default:
 >> +		EDEB_ERR(4, "Unknown event code %x", ec);
 >> +		break;
 >> +	}
 >> +
 >> +	EDEB_EX(7, "");
 >> +}
 >> +
 >> +void ehea_neq_tasklet(unsigned long data)
 >> +{
 >> +	struct ehea_adapter *adapter = (struct ehea_adapter*)data;
 >> +	struct ehea_eqe *eqe;
 >> +	u64 event_mask;
 >> +
 >> +	EDEB_EN(7, "");
 >> +	eqe = (struct ehea_eqe*)ehea_poll_eq(adapter->neq);
 >> +	EDEB(7, "eqe=%p", eqe);
 >> +
 >> +	while (eqe) {
 >> +		EDEB(7, "*eqe=%lx", eqe->entry);
 >> +		ehea_parse_eqe(adapter, eqe->entry);
 >> +		eqe = (struct ehea_eqe*)ehea_poll_eq(adapter->neq);
 >> +		EDEB(7, "next eqe=%p", eqe);
 >> +	}
 >
 > I think I've seen this code before.
 >

there is a similar function, but it will be used for a different
error recovery

 >> +
 >> +	event_mask = EHEA_BMASK_SET(NELR_PORTSTATE_CHG, 1)
 >> +		   | EHEA_BMASK_SET(NELR_ADAPTER_MALFUNC, 1)
 >> +		   | EHEA_BMASK_SET(NELR_PORT_MALFUNC, 1);
 >> +
 >> +	ehea_h_reset_events(adapter->handle,
 >> +			    adapter->neq->ipz_eq_handle, event_mask);
 >> +	EDEB_EX(7, "");
 >> +}
 >> +
 >> +irqreturn_t ehea_interrupt_neq(int irq, void *param, struct pt_regs *regs)
 >> +{
 >> +	struct ehea_adapter *adapter = (struct ehea_adapter*)param;
 >> +	EDEB_EN(7, "dev_id=%p", adapter);
 >> +	tasklet_hi_schedule(&adapter->neq_tasklet);
 >> +	EDEB_EX(7, "");
 >> +	return IRQ_HANDLED;
 >> +}
 >> +
 >> +
 >> +
 >> +static void ehea_fill_port_res(struct ehea_port_res *pr)
 >> +{
 >> +	struct ehea_qp_init_attr *init_attr = &pr->qp->init_attr;
 >> +
 >> +	/* RQ 1 */
 >> +	ehea_init_fill_rq1(pr, init_attr->act_nr_rwqes_rq1
 >> +			   - init_attr->act_nr_rwqes_rq2
 >> +			   - init_attr->act_nr_rwqes_rq3 - 1);
 >> +
 >> +	/* RQ 2 */
 >> +	ehea_refill_rq2(pr, init_attr->act_nr_rwqes_rq2);
 >> +
 >> +	/* RQ 3 */
 >> +	ehea_refill_rq3(pr, init_attr->act_nr_rwqes_rq3);
 >> +}
 >> +
 >> +static int ehea_reg_interrupts(struct net_device *dev)
 >> +{
 >> +	int ret = -EINVAL;
 >> +	int i;
 >> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
 >> +	struct ehea_port_res *pr = &port->port_res[0];
 >
 > Don't need to initalise pr.
 >

done

 >> +
 >> +	EDEB_EN(7, "");
 >> +	for (i = 0; i < port->num_def_qps; i++) {
 >> +		pr = &port->port_res[i];
 >> +		snprintf(pr->int_recv_name, EHEA_IRQ_NAME_SIZE - 1
 >> +			 , "%s-recv%d", dev->name, i);
 >> +		ret = ibmebus_request_irq(NULL,
 >> +					  pr->recv_eq->attr.ist1,
 >> +					  ehea_recv_irq_handler,
 >> +					  SA_INTERRUPT, pr->int_recv_name, pr);
 >> +		if (ret) {
 >> +			EDEB_ERR(4, "Failed registering irq for ehea_recv_int:"
 >> +				 "port_res_nr:%d, ist=%X", i,
 >> +				 pr->recv_eq->attr.ist1);
 >> +			goto failure;
 >> +		}
 >> +		EDEB(7, "irq_handle 0x%X for function ehea_recv_int %d "
 >> +		     " registered", pr->recv_eq->attr.ist1, i);
 >> +	}
 >> +
 >> +	snprintf(port->int_aff_name, EHEA_IRQ_NAME_SIZE - 1,
 >> +		 "%s-aff", dev->name);
 >> +	ret = ibmebus_request_irq(NULL,
 >> +				  port->qp_eq->attr.ist1,
 >> +				  ehea_qp_aff_irq_handler,
 >> +				  SA_INTERRUPT, port->int_aff_name, port);
 >> +	if (ret) {
 >> +		EDEB_ERR(4, "Failed registering irq for qp_aff_irq_handler:"
 >> +			 " ist=%X", port->qp_eq->attr.ist1);
 >> +		goto failure;
 >> +	}
 >> +	EDEB(7, "irq_handle 0x%X for function qp_aff_irq_handler registered",
 >> +	     port->qp_eq->attr.ist1);
 >> +
 >> +	for (i = 0; i < port->num_def_qps + port->num_tx_qps; i++) {
 >> +		pr = &port->port_res[i];
 >> +		snprintf(pr->int_send_name, EHEA_IRQ_NAME_SIZE - 1,
 >> +			 "%s-send%d", dev->name, i);
 >> +		ret = ibmebus_request_irq(NULL,
 >> +					  pr->send_eq->attr.ist1,
 >> +					  ehea_send_irq_handler,
 >> +					  SA_INTERRUPT, pr->int_send_name,
 >> +					  pr);
 >> +		if (ret) {
 >> +			EDEB_ERR(4, "Registering irq for ehea_send failed"
 >> +				 " port_res_nr:%d, ist=%X", i,
 >> +				 pr->send_eq->attr.ist1);
 >> +			goto failure;
 >> +		}
 >> +		EDEB(7, "irq_handle 0x%X for function ehea_send_int %d"
 >> +		     " registered", pr->send_eq->attr.ist1, i);
 >> +	}
 >
 > You seem to be iterating over some of the port_res structs twice, is
 > that what you intended?
 >

yes

 >> +failure:
 >> +	EDEB_EX(7, "ret=%d", ret);
 >> +	return ret;
 >
 > No further cleanup required? Should you be calling ehea_free_interrupts?
 >

good point, a proper error handling will be included

 >> +}
 >> +
 >> +static void ehea_free_interrupts(struct net_device *dev)
 >> +{
 >> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
 >> +	int i;
 >> +
 >> +	EDEB_EN(7, "");
 >> +	/* send */
 >> +	for (i = 0; i < port->num_def_qps + port->num_tx_qps; i++) {
 >> +		ibmebus_free_irq(NULL, port->port_res[i].send_eq->attr.ist1,
 >> +				 &port->port_res[i]);
 >> +		EDEB(7, "free send interrupt for res %d with handle 0x%X",
 >> +		     i, port->port_res[i].send_eq->attr.ist1);
 >> +	}
 >> +
 >> +	/* receive */
 >> +	for (i = 0; i < port->num_def_qps; i++) {
 >> +		ibmebus_free_irq(NULL, port->port_res[i].recv_eq->attr.ist1,
 >> +				 &port->port_res[i]);
 >> +		EDEB(7, "free recv intterupt for res %d with handle 0x%X",
 >> +		     i, port->port_res[i].recv_eq->attr.ist1);
 >> +	}
 >> +	/* associated events */
 >> +	ibmebus_free_irq(NULL, port->qp_eq->attr.ist1, port);
 >> +	EDEB(7, "associated event interrupt for handle 0x%X freed",
 >> +	     port->qp_eq->attr.ist1);
 >> +	EDEB_EX(7, "");
 >> +}
 >> +
 >> +static int ehea_configure_port(struct ehea_port *port)
 >> +{
 >> +	int ret = -EINVAL;
 >> +	u64 hret = H_HARDWARE;
 >> +	struct hcp_query_ehea_port_cb_0 *ehea_port_cb_0 = NULL;
 >> +	u64 mask = 0;
 >> +	int i;
 >> +
 >> +	EDEB_EN(7, "");
 >> +
 >> +	ehea_port_cb_0 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
 >> +
 >> +	if (!ehea_port_cb_0) {
 >> +		EDEB_ERR(4, "No memory for ehea_port control block");
 >> +		ret = -ENOMEM;
 >> +		goto kzalloc_failed;
 >> +	}
 >> +
 >> +	ehea_port_cb_0->port_rc = EHEA_BMASK_SET(PXLY_RC_VALID, 1)
 >> +				| EHEA_BMASK_SET(PXLY_RC_IP_CHKSUM, 1)
 >> +				| EHEA_BMASK_SET(PXLY_RC_TCP_UDP_CHKSUM, 1)
 >> +                		| EHEA_BMASK_SET(PXLY_RC_VLAN_XTRACT, 1)
 >> +                		| EHEA_BMASK_SET(PXLY_RC_VLAN_TAG_FILTER,
 >> +				                 PXLY_RC_VLAN_FILTER)
 >> +				| EHEA_BMASK_SET(PXLY_RC_JUMBO_FRAME, 1);
 >> +
 >> +	for (i = 0; i < port->num_def_qps; i++) {
 >> +		ehea_port_cb_0->default_qpn_array[i] =
 >> +		    port->port_res[i].qp->init_attr.qp_nr;
 >> +
 >> +		EDEB(7, "default_qpn_array[%d]=%d",
 >> +		     i, port->port_res[i].qp->init_attr.qp_nr);
 >> +	}
 >> +
 >> +	EDEB(7, "ehea_port_cb_0");
 >> +	EDEB_DMP(7, (u8*)ehea_port_cb_0, sizeof(*ehea_port_cb_0), "");
 >> +
 >> +	mask = EHEA_BMASK_SET(H_PORT_CB0_PRC, 1)
 >> +	       | EHEA_BMASK_SET(H_PORT_CB0_DEFQPNARRAY, 1);
 >> +
 >> +	hret = ehea_h_modify_ehea_port(port->adapter->handle,
 >> +				       port->logical_port_id,
 >> +				       H_PORT_CB0,
 >> +				       mask,
 >> +				       (void*)ehea_port_cb_0);
 >> +
 >> +	if (hret != H_SUCCESS) {
 >> +		goto modify_ehea_port_failed;
 >> +	}
 >> +
 >> +	ret = 0;
 >> +
 >> +modify_ehea_port_failed:
 >> +	kfree(ehea_port_cb_0);
 >> +
 >> +kzalloc_failed:
 >> +	EDEB_EX(7, "ret=%d", ret);
 >> +	return ret;
 >> +}
 >> +
 >> +
 >> +static int ehea_gen_smrs(struct ehea_port_res *pr)
 >> +{
 >> +	u64 hret = H_HARDWARE;
 >
 > No need to set hret. You do that a lot, int foo = 0; foo = bar();
 >
 >> +	struct ehea_adapter *adapter = pr->port->adapter;
 >> +	EDEB_EN(7, "ehea_port_res=%p", pr);
 >> +	hret = hipz_h_register_smr(adapter->handle,
 >> +				   adapter->mr.handle,
 >> +				   adapter->mr.vaddr,
 >> +				   EHEA_MR_ACC_CTRL,
 >> +				   adapter->pd,
 >> +				   &pr->send_mr);
 >> +	if (hret != H_SUCCESS)
 >> +		goto ehea_gen_smrs_err1;
 >> +
 >> +	hret = hipz_h_register_smr(adapter->handle,
 >> +				   adapter->mr.handle,
 >> +				   adapter->mr.vaddr,
 >> +				   EHEA_MR_ACC_CTRL,
 >> +				   adapter->pd,
 >> +				   &pr->recv_mr);
 >> +	if (hret != H_SUCCESS)
 >> +		goto ehea_gen_smrs_err2;
 >> +	EDEB_EX(7, "");
 >> +	return 0;
 >> +
 >> +ehea_gen_smrs_err2:
 >> +	hret = ehea_h_free_resource_mr(adapter->handle, pr->send_mr.handle);
 >> +	if (hret != H_SUCCESS)
 >> +		EDEB_ERR(4, "Could not free SMR");
 >> +ehea_gen_smrs_err1:
 >> +	return -EINVAL;
 >> +}
 >> +
 >> +static void ehea_rem_smrs(struct ehea_port_res *pr)
 >> +{
 >> +	u64 hret = H_HARDWARE;
 >> +	struct ehea_adapter *adapter = pr->port->adapter;
 >> +	EDEB_EN(7, "ehea_port_res=%p", pr);
 >> +	hret = ehea_h_free_resource_mr(adapter->handle, pr->send_mr.handle);
 >> +	if (hret != H_SUCCESS)
 >> +		EDEB_ERR(4, "Could not free send SMR for pr=%p", pr);
 >> +
 >> +	hret = ehea_h_free_resource_mr(adapter->handle, pr->recv_mr.handle);
 >> +	if (hret != H_SUCCESS)
 >> +		EDEB_ERR(4, "Could not free receive SMR for pr=%p", pr);
 >> +	EDEB_EX(7, "");
 >> +}
 >
 > It's a bit worrying that this routine can fail, but returns void, ie. it
 > can't report failure to its caller. (but perhaps it's ok).
 >
 >> +
 >> +static int ehea_init_port_res(struct ehea_port *port, struct ehea_port_res *pr,
 >> +			      struct port_res_cfg *pr_cfg, int queue_token)
 >> +{
 >> +	int ret = -EINVAL;
 >> +	int max_rq_entries = 0;
 >> +	enum ehea_eq_type eq_type = EHEA_EQ;
 >> +	struct ehea_qp_init_attr *init_attr;
 >> +	struct ehea_adapter *adapter = port->adapter;
 >> +
 >> +	EDEB_EN(7, "port=%p, pr=%p", port, pr);
 >> +
 >> +	memset(pr, 0, sizeof(struct ehea_port_res));
 >> +
 >> +	pr->port = port;
 >> +	spin_lock_init(&pr->send_lock);
 >> +	spin_lock_init(&pr->recv_lock);
 >> +	spin_lock_init(&pr->xmit_lock);
 >> +	spin_lock_init(&pr->netif_queue);
 >> +
 >> +	pr->recv_eq = ehea_create_eq(adapter, eq_type,
 >> +				     EHEA_MAX_ENTRIES_EQ, 0);
 >> +	if (!pr->recv_eq) {
 >> +		EDEB_ERR(4, "ehea_create_eq failed (recv_eq)");
 >> +		goto ehea_init_port_res_err1;
 >> +	}
 >> +	pr->send_eq = ehea_create_eq(adapter, eq_type,
 >> +				     EHEA_MAX_ENTRIES_EQ, 0);
 >> +	if (!pr->send_eq) {
 >> +		EDEB_ERR(4, "ehea_create_eq failed (send_eq)");
 >> +		goto ehea_init_port_res_err2;
 >> +	}
 >> +
 >> +	pr->recv_cq = ehea_create_cq(adapter, pr_cfg->max_entries_rcq,
 >> +				     pr->recv_eq->ipz_eq_handle,
 >> +				     port->logical_port_id);
 >> +	if (!pr->recv_cq) {
 >> +		EDEB_ERR(4, "ehea_create_cq failed (cq_recv)");
 >> +		goto ehea_init_port_res_err3;
 >> +	}
 >> +
 >> +	pr->send_cq = ehea_create_cq(adapter, pr_cfg->max_entries_scq,
 >> +				     pr->send_eq->ipz_eq_handle,
 >> +				     port->logical_port_id);
 >> +	if (!pr->send_cq) {
 >> +		EDEB_ERR(4, "ehea_create_cq failed (cq_send)");
 >> +		goto ehea_init_port_res_err4;
 >> +	}
 >> +
 >> +	init_attr = (struct ehea_qp_init_attr*)
 >> +	    kzalloc(sizeof(struct ehea_qp_init_attr), GFP_KERNEL);
 >> +
 >> +	if (!init_attr) {
 >> +		EDEB_ERR(4, "no mem for init_attr struct");
 >> +		ret = -ENOMEM;
 >> +		goto ehea_init_port_res_err5;
 >> +	}
 >> +
 >> +	init_attr->low_lat_rq1 = 1;
 >> +	init_attr->signalingtype = 1;	/* generate CQE if specified in WQE */
 >> +	init_attr->rq_count = 3;
 >> +	init_attr->qp_token = queue_token;
 >> +
 >> +	init_attr->max_nr_send_wqes = pr_cfg->max_entries_sq;
 >> +	init_attr->max_nr_rwqes_rq1 = pr_cfg->max_entries_rq1;
 >> +	init_attr->max_nr_rwqes_rq2 = pr_cfg->max_entries_rq2;
 >> +	init_attr->max_nr_rwqes_rq3 = pr_cfg->max_entries_rq3;
 >> +
 >> +	init_attr->wqe_size_enc_sq = EHEA_SG_SQ;
 >> +	init_attr->wqe_size_enc_rq1 = EHEA_SG_RQ1;
 >> +	init_attr->wqe_size_enc_rq2 = EHEA_SG_RQ2;
 >> +	init_attr->wqe_size_enc_rq3 = EHEA_SG_RQ3;
 >> +
 >> +	init_attr->rq2_threshold = EHEA_RQ2_THRESHOLD;
 >> +	init_attr->rq3_threshold = EHEA_RQ3_THRESHOLD;
 >> +	init_attr->port_nr = port->logical_port_id;
 >> +	init_attr->send_cq_handle = pr->send_cq->ipz_cq_handle;
 >> +	init_attr->recv_cq_handle = pr->recv_cq->ipz_cq_handle;
 >> +	init_attr->aff_eq_handle = port->qp_eq->ipz_eq_handle;
 >> +
 >> +	pr->qp = ehea_create_qp(adapter, adapter->pd, init_attr);
 >> +	if (!pr->qp) {
 >> +		EDEB_ERR(4, "could not create queue pair");
 >> +		goto ehea_init_port_res_err6;
 >> +	}
 >> +
 >> +	/* SQ */
 >> +	max_rq_entries = init_attr->act_nr_send_wqes;
 >> +	pr->skb_arr_sq = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
 >> +						    * (max_rq_entries + 1));
 >> +	if (!pr->skb_arr_sq) {
 >> +		EDEB_ERR(4, "vmalloc for skb_arr_sq failed");
 >> +		goto ehea_init_port_res_err7;
 >> +	}
 >> +	memset(pr->skb_arr_sq, 0, sizeof(void*) * (max_rq_entries + 1));
 >> +	pr->skb_sq_index = 0;
 >> +	pr->skb_arr_sq_len = max_rq_entries + 1;
 >> +
 >> +	/* RQ 1 */
 >> +	max_rq_entries = init_attr->act_nr_rwqes_rq1;
 >> +	pr->skb_arr_rq1 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
 >> +						     * (max_rq_entries + 1));
 >> +	if (!pr->skb_arr_rq1) {
 >> +		EDEB_ERR(4, "vmalloc for skb_arr_rq1 failed");
 >> +		goto ehea_init_port_res_err8;
 >> +	}
 >> +	memset(pr->skb_arr_rq1, 0, sizeof(void*) * (max_rq_entries + 1));
 >> +	pr->skb_arr_rq1_len = max_rq_entries + 1;
 >> +
 >> +	/* RQ 2 */
 >> +	max_rq_entries = init_attr->act_nr_rwqes_rq2;
 >> +	pr->skb_arr_rq2 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
 >> +						     * (max_rq_entries + 1));
 >> +	if (!pr->skb_arr_rq2) {
 >> +		EDEB_ERR(4, "vmalloc for skb_arr_rq2 failed");
 >> +		goto ehea_init_port_res_err9;
 >> +	}
 >> +	memset(pr->skb_arr_rq2, 0, sizeof(void*) * (max_rq_entries + 1));
 >> +	pr->skb_arr_rq2_len = max_rq_entries;
 >> +	pr->skb_rq2_index = 0;
 >> +
 >> +	/* RQ 3 */
 >> +	max_rq_entries = init_attr->act_nr_rwqes_rq3;
 >> +	pr->skb_arr_rq3 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
 >> +						     * (max_rq_entries + 1));
 >> +	if (!pr->skb_arr_rq3) {
 >> +		EDEB_ERR(4, "vmalloc for skb_arr_rq3 failed");
 >> +		goto ehea_init_port_res_err10;
 >> +	}
 >> +	memset(pr->skb_arr_rq3, 0, sizeof(void*) * (max_rq_entries + 1));
 >> +	pr->skb_arr_rq3_len = max_rq_entries;
 >> +	pr->skb_rq3_index = 0;
 >
 > You shouldn't need the sk_buff** casts here. You probably should use
 > kzalloc here, instead of vmalloc + memset. Depends on how big
 > max_rq_entries can be.
 >
 >> +
 >> +	if (ehea_gen_smrs(pr) != 0)
 >> +		goto ehea_init_port_res_err11;
 >> +	tasklet_init(&pr->send_comp_task, ehea_send_irq_tasklet,
 >> +		     (unsigned long)pr);
 >> +	atomic_set(&pr->swqe_avail, EHEA_MAX_ENTRIES_SQ - 1);
 >> +
 >> +	kfree(init_attr);
 >> +	ret = 0;
 >> +	goto done;
 >> +
 >> +ehea_init_port_res_err11:
 >> +	vfree(pr->skb_arr_rq3);
 >> +ehea_init_port_res_err10:
 >> +	vfree(pr->skb_arr_rq2);
 >> +ehea_init_port_res_err9:
 >> +	vfree(pr->skb_arr_rq1);
 >> +ehea_init_port_res_err8:
 >> +	vfree(pr->skb_arr_sq);
 >> +ehea_init_port_res_err7:
 >> +	ehea_destroy_qp(pr->qp);
 >> +ehea_init_port_res_err6:
 >> +	kfree(init_attr);
 >> +ehea_init_port_res_err5:
 >> +	ehea_destroy_cq(pr->send_cq);
 >> +ehea_init_port_res_err4:
 >> +	ehea_destroy_cq(pr->recv_cq);
 >> +ehea_init_port_res_err3:
 >> +	ehea_destroy_eq(pr->send_eq);
 >> +ehea_init_port_res_err2:
 >> +	ehea_destroy_eq(pr->recv_eq);
 >> +ehea_init_port_res_err1:
 >> +done:
 >> +	EDEB_EX(7, "ret=%d", ret);
 >> +	return ret;
 >> +}
 >
 > You can vfree/kfree a NULL pointer, so you can probably amalgamate some
 > of these error cases if you initialise the pointers to NULL at the
 > beginning.
 >

very good point, this mechanism can also be used for ehea_destroy_xx
which will simplify the error path.
done

 >> +
 >> +static int ehea_clean_port_res(struct ehea_port *port, struct ehea_port_res *pr)
 >> +{
 >> +	int i;
 >> +	int ret = -EINVAL;
 >> +
 >> +	EDEB_EN(7, "Not completed yet...");
 >> +
 >> +	ret = ehea_destroy_qp(pr->qp);
 >> +	if (ret)
 >> +		EDEB_ERR(4, "could not destroy queue pair");
 >> +
 >> +	ret = ehea_destroy_cq(pr->send_cq);
 >> +	if (ret)
 >> +		EDEB_ERR(4, "could not destroy send_cq");
 >> +
 >> +	ret = ehea_destroy_cq(pr->recv_cq);
 >> +	if (ret)
 >> +		EDEB_ERR(4, "could not destroy recv_cq");
 >> +
 >> +	ret = ehea_destroy_eq(pr->send_eq);
 >> +	if (ret)
 >> +		EDEB_ERR(4, "could not destroy send_eq");
 >> +
 >> +	ret = ehea_destroy_eq(pr->recv_eq);
 >> +	if (ret)
 >> +		EDEB_ERR(4, "could not destroy recv_eq");
 >
 > If these are leaking memory they're probably worth a printk.
 >
 >> +	for (i = 0; i < pr->skb_arr_rq1_len; i++) {
 >> +		if (pr->skb_arr_rq1[i])
 >> +			dev_kfree_skb(pr->skb_arr_rq1[i]);
 >> +	}
 >> +
 >> +	for (i = 0; i < pr->skb_arr_rq2_len; i++)
 >> +		if (pr->skb_arr_rq2[i])
 >> +			dev_kfree_skb(pr->skb_arr_rq2[i]);
 >> +
 >> +	for (i = 0; i < pr->skb_arr_rq3_len; i++)
 >> +		if (pr->skb_arr_rq3[i])
 >> +			dev_kfree_skb(pr->skb_arr_rq3[i]);
 >> +
 >> +	for (i = 0; i < pr->skb_arr_sq_len; i++)
 >> +		if (pr->skb_arr_sq[i])
 >> +			dev_kfree_skb(pr->skb_arr_sq[i]);
 >> +
 >> +	vfree(pr->skb_arr_sq);
 >> +	vfree(pr->skb_arr_rq1);
 >> +	vfree(pr->skb_arr_rq2);
 >> +	vfree(pr->skb_arr_rq3);
 >> +
 >> +	ehea_rem_smrs(pr);
 >> +	EDEB_EX(7, "ret=%d", ret);
 >> +	return ret;
 >> +}
 >> +
 >> +static inline void write_ip_start_end(struct ehea_swqe *swqe,
 >> +				      const struct sk_buff *skb)
 >> +{
 >> +
 >> +	swqe->ip_start = (u8)(((u64)skb->nh.iph) - ((u64)skb->data));
 >> +	swqe->ip_end = (u8)(swqe->ip_start + skb->nh.iph->ihl * 4 - 1);
 >> +}
 >
 >> +static inline void write_tcp_offset_end(struct ehea_swqe *swqe,
 >> +					const struct sk_buff *skb)
 >> +{
 >> +	swqe->tcp_offset = (u8)(swqe->ip_end + 1 + offsetof(struct tcphdr,
 >> +							    check));
 >> +	swqe->tcp_end = (u16)skb->len - 1;
 >> +}
 >> +
 >> +static inline void write_udp_offset_end(struct ehea_swqe *swqe,
 >> +					const struct sk_buff *skb)
 >> +{
 >> +	swqe->tcp_offset = (u8)(swqe->ip_end + 1 + offsetof(struct udphdr,
 >> +							    check));
 >> +	swqe->tcp_end = (u16)skb->len - 1;
 >> +}
 >
 > These three need some explanation at best.
 >

ok, done

 >> +
 >> +static inline void write_swqe2_data(struct sk_buff *skb,
 >> +				    struct net_device *dev,
 >> +				    struct ehea_swqe *swqe,
 >> +				    u32 lkey)
 >> +{
 >> +	int skb_data_size, nfrags, headersize, i, sg1entry_contains_frag_data;
 >> +	struct ehea_vsgentry *sg_list;
 >> +	struct ehea_vsgentry *sg1entry;
 >> +	struct ehea_vsgentry *sgentry;
 >> +	u8 *imm_data;
 >> +	u64 tmp_addr;
 >> +	skb_frag_t *frag;
 >> +	EDEB_EN(7, "");
 >> +
 >> +	skb_data_size = skb->len - skb->data_len;
 >> +	nfrags = skb_shinfo(skb)->nr_frags;
 >> +	sg1entry = &swqe->u.immdata_desc.sg_entry;
 >> +	sg_list = (struct ehea_vsgentry*)&swqe->u.immdata_desc.sg_list;
 >> +	imm_data = &swqe->u.immdata_desc.immediate_data[0];
 >> +	swqe->descriptors = 0;
 >> +	sg1entry_contains_frag_data = 0;
 >> +
 >> +	if ((dev->features & NETIF_F_TSO) && skb_shinfo(skb)->gso_size) {
 >> +		/* Packet is TCP with TSO enabled */
 >> +		swqe->tx_control |= EHEA_SWQE_TSO;
 >> +		swqe->mss = skb_shinfo(skb)->gso_size;
 >> +		/* copy only eth/ip/tcp headers to immediate data and
 >> +		 * the rest of skb->data to sg1entry
 >> +		 */
 >> +		headersize = ETH_HLEN + (skb->nh.iph->ihl * 4)
 >> +		    + skb->h.th->doff * 4;
 >> +		skb_data_size = skb->len - skb->data_len;
 >> +
 >> +		if (skb_data_size >= headersize) {
 >> +			/* copy immediate data */
 >> +			memcpy(imm_data, skb->data, headersize);
 >> +			swqe->immediate_data_length = headersize;
 >> +
 >> +			if (skb_data_size > headersize) {
 >> +				/* set sg1entry data */
 >> +				sg1entry->l_key = lkey;
 >> +				sg1entry->len = skb_data_size - headersize;
 >> +
 >> +				tmp_addr = (u64)(skb->data + headersize);
 >> +				sg1entry->vaddr =
 >> +					get_swqe_addr(tmp_addr, 0);
 >> +
 >> +				swqe->descriptors++;
 >> +			}
 >> +		} else
 >> +			EDEB_ERR(4, "Cannot handle fragmented headers");
 >> +	} else {
 >> +		/* Packet is any nonTSO type
 >> +		 *
 >> +		 * Copy as much as possible skb->data to immediate data and
 >> +		 * the rest to sg1entry
 >> +		 */
 >> +		if (skb_data_size >= SWQE2_MAX_IMM) {
 >> +			/* copy immediate data */
 >> +			memcpy(imm_data, skb->data, SWQE2_MAX_IMM);
 >> +
 >> +			swqe->immediate_data_length = SWQE2_MAX_IMM;
 >> +
 >> +			if (skb_data_size > SWQE2_MAX_IMM) {
 >> +				/* copy sg1entry data */
 >> +				sg1entry->l_key = lkey;
 >> +				sg1entry->len = skb_data_size - SWQE2_MAX_IMM;
 >> +				tmp_addr = (u64)(skb->data + SWQE2_MAX_IMM);
 >> +				sg1entry->vaddr = get_swqe_addr(tmp_addr, 0);
 >> +				swqe->descriptors++;
 >> +			}
 >> +		} else {
 >> +			memcpy(imm_data, skb->data, skb_data_size);
 >> +			swqe->immediate_data_length = skb_data_size;
 >> +		}
 >> +	}
 >> +
 >> +	/* write descriptors */
 >> +	if (nfrags > 0) {
 >> +		if (swqe->descriptors == 0) {
 >> +			/* sg1entry not yet used */
 >> +			frag = &skb_shinfo(skb)->frags[0];
 >> +
 >> +			/* copy sg1entry data */
 >> +			sg1entry->l_key = lkey;
 >> +			sg1entry->len = frag->size;
 >> +			tmp_addr =  (u64)(page_address(frag->page) +
 >> +					    frag->page_offset);
 >> +			sg1entry->vaddr = get_swqe_addr(tmp_addr,
 >> +							EHEA_MR_TX_DATA_PN);
 >> +			swqe->descriptors++;
 >> +			sg1entry_contains_frag_data = 1;
 >> +		}
 >> +
 >> +		for (i = sg1entry_contains_frag_data; i < nfrags; i++) {
 >> +
 >> +			frag = &skb_shinfo(skb)->frags[i];
 >> +			sgentry = &sg_list[i - sg1entry_contains_frag_data];
 >> +
 >> +			sgentry->l_key = lkey;
 >> +			sgentry->len = frag->size;
 >> +
 >> +			tmp_addr = (u64)(page_address(frag->page)
 >> +					 + frag->page_offset);
 >> +			sgentry->vaddr = get_swqe_addr(tmp_addr,
 >> +						       EHEA_MR_TX_DATA_PN + i);
 >> +		}
 >> +	}
 >> +
 >> +	EDEB_EX(7, "");
 >> +}
 >> +
 >> +static int ehea_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 >> +{
 >> +	EDEB_ERR(4, "ioctl not supported: dev=%s cmd=%d", dev->name, cmd);
 >> +	return -EOPNOTSUPP;
 >> +}
 >> +
 >> +static u64 ehea_broadcast_reg_helper(struct ehea_port *port, u32 hcallid)
 >> +{
 >> +	u64 hret = H_HARDWARE;
 >> +	u8 reg_type = 0;
 >> +
 >> +	if (hcallid == H_REG_BCMC) {
 >> +		EDEB(7, "REGistering MAC for broadcast");
 >> +	} else {
 >> +		EDEB(7, "DEREGistering MAC for broadcast");
 >> +	}
 >> +
 >> +	/* De/Register untagged packets */
 >> +	reg_type = EHEA_BCMC_BROADCAST | EHEA_BCMC_UNTAGGED;
 >> +	hret = ehea_h_reg_dereg_bcmc(port->adapter->handle,
 >> +				     port->logical_port_id,
 >> +				     reg_type, port->mac_addr, 0, hcallid);
 >> +	if (hret != H_SUCCESS)
 >> +		goto hcall_failed;
 >> +
 >> +	/* De/Register VLAN packets */
 >> +	reg_type = EHEA_BCMC_BROADCAST | EHEA_BCMC_VLANID_ALL;
 >> +	hret = ehea_h_reg_dereg_bcmc(port->adapter->handle,
 >> +				     port->logical_port_id,
 >> +				     reg_type, port->mac_addr, 0, hcallid);
 >> +hcall_failed:
 >
 > unregister here?
 >
 >> +	return hret;
 >> +}
 >> +
 >> +static int ehea_set_mac_addr(struct net_device *dev, void *sa)
 >> +{
 >> +	int ret = -EOPNOTSUPP;
 >> +	u64 hret = H_HARDWARE;
 >> +	struct hcp_query_ehea_port_cb_0 *ehea_port_cb_0 = NULL;
 >> +	struct ehea_port *port = (struct ehea_port*)dev->priv;
 >> +	struct sockaddr *mac_addr = (struct sockaddr*)sa;
 >> +
 >> +	EDEB_EN(7, "devname=%s", dev->name);
 >> +	EDEB_DMP(7, (u8*)&(mac_addr->sa_data[0]), 14, "");
 >> +
 >> +	if (!is_valid_ether_addr(mac_addr->sa_data)) {
 >> +		ret = -EADDRNOTAVAIL;
 >> +		goto invalid_mac;
 >> +	}
 >> +
 >> +	ehea_port_cb_0 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
 >> +
 >> +	if (!ehea_port_cb_0) {
 >> +		EDEB_ERR(4, "No memory for ehea_port control block");
 >> +		ret = -ENOMEM;
 >> +		goto kzalloc_failed;
 >> +	}
 >> +
 >> +	memcpy((u8*)(&(ehea_port_cb_0->port_mac_addr)),
 >> +	       (u8*)&(mac_addr->sa_data[0]), 6);
 >
 > Don't need casts, memcpy takes void *. You should use ETH_ALEN
 > (include/linux/if_ether.h) instead of 6.
 >

done

 >> +
 >> +	ehea_port_cb_0->port_mac_addr = ehea_port_cb_0->port_mac_addr >> 16;
 >> +
 >> +	EDEB(7, "ehea_port_cb_0");
 >> +	EDEB_DMP(7, (u8*)ehea_port_cb_0,
 >> +		 sizeof(struct hcp_query_ehea_port_cb_0), "");
 >> +
 >> +	hret = ehea_h_modify_ehea_port(port->adapter->handle,
 >> +				       port->logical_port_id,
 >> +				       H_PORT_CB0,
 >> +				       EHEA_BMASK_SET(H_PORT_CB0_MAC, 1),
 >> +				       (void*)ehea_port_cb_0);
 >> +	if (hret != H_SUCCESS) {
 >> +		ret = -EOPNOTSUPP;
 >> +		goto hcall_failed;
 >> +	}
 >> +
 >> +	memcpy(dev->dev_addr, mac_addr->sa_data, dev->addr_len);
 >> +
 >> +	/* Deregister old MAC in PHYP */
 >> +	hret = ehea_broadcast_reg_helper(port, H_DEREG_BCMC);
 >> +	if (hret) {
 >> +		ret = -EOPNOTSUPP;
 >> +		goto hcall_failed;
 >
 > What happens here? No mac? Old mac?
 >

good question, we have to think about a reasonable recovery strategy

 >> +	}
 >> +
 >> +	port->mac_addr = ehea_port_cb_0->port_mac_addr << 16;
 >> +
 >> +	/* Register new MAC in PHYP */
 >> +	hret = ehea_broadcast_reg_helper(port, H_REG_BCMC);
 >> +	if (hret) {
 >> +		ret = -EOPNOTSUPP;
 >> +		goto hcall_failed;
 >> +	}
 >> +
 >> +	ret = 0;
 >> +
 >> +hcall_failed:
 >> +	kfree(ehea_port_cb_0);
 >> +
 >> +kzalloc_failed:
 >> +invalid_mac:
 >> +	EDEB_EX(7, "ret=%d", ret);
 >> +	return ret;
 >> +}
 >> +
 >> +static void ehea_promiscuous(struct net_device *dev, int enable)
 >> +{
 >> +	struct ehea_port *port = dev->priv;
 >> +
 >> +	if (!port->promisc) {
 >> +		if (enable) {
 >> +			/* Enable promiscuous mode */
 >> +			EDEB(7, "Enabling IFF_PROMISC");
 >> +			EDEB_ERR(4, "Enable promiscuous mode: "
 >> +				 "not yet implemented");
 >> +			port->promisc = EHEA_ENABLE;
 >> +		}
 >> +	} else {
 >> +		if (!enable) {
 >> +			/* Disable promiscuous mode */
 >> +			EDEB(7, "Disabling IFF_PROMISC");
 >> +			EDEB_ERR(4, "Disable promiscuous mode: "
 >> +				 "not yet implemented");
 >> +			port->promisc = EHEA_DISABLE;
 >> +		}
 >> +	}
 >> +}
 >> +
 >> +static u64 ehea_multicast_reg_helper(struct ehea_port *port,
 >> +				     u64 mc_mac_addr,
 >> +				     u32 hcallid)
 >> +{
 >> +	u64 hret = H_HARDWARE;
 >> +	u8 reg_type = 0;
 >> +
 >> +	reg_type = EHEA_BCMC_SCOPE_ALL | EHEA_BCMC_MULTICAST
 >> +		 | EHEA_BCMC_UNTAGGED;
 >> +
 >> +	hret = ehea_h_reg_dereg_bcmc(port->adapter->handle,
 >> +				     port->logical_port_id,
 >> +				     reg_type, mc_mac_addr, 0, hcallid);
 >> +	if (hret)
 >> +		goto hcall_failed;
 >> +
 >> +	reg_type = EHEA_BCMC_SCOPE_ALL | EHEA_BCMC_MULTICAST
 >> +		 | EHEA_BCMC_VLANID_ALL;
 >> +
 >> +	hret = ehea_h_reg_dereg_bcmc(port->adapter->handle,
 >> +				     port->logical_port_id,
 >> +				     reg_type, mc_mac_addr, 0, hcallid);
 >> +hcall_failed:
 >> +	return hret;
 >> +}
 >> +
 >> +static int ehea_drop_multicast_list(struct net_device *dev)
 >> +{
 >> +	int ret = 0;
 >> +	u64 hret = H_HARDWARE;
 >> +	struct ehea_port *port = dev->priv;
 >> +	struct ehea_mc_list *mc_entry = port->mc_list;
 >> +	struct list_head *pos;
 >> +	struct list_head *temp;
 >> +
 >> +	EDEB_EN(7, "devname=%s", dev->name);
 >> +
 >> +	if (!list_empty(&mc_entry->list)) {
 >> +		list_for_each_safe(pos, temp, &(port->mc_list->list)) {
 >> +			mc_entry = list_entry(pos, struct ehea_mc_list, list);
 >> +
 >> +			EDEB(7, "Deregistering MAC %lx", mc_entry->macaddr);
 >> +
 >> +			hret = ehea_multicast_reg_helper(port,
 >> +							 mc_entry->macaddr,
 >> +							 H_DEREG_BCMC);
 >> +			if (hret) {
 >> +				EDEB_ERR(4, "Failed deregistering mcast MAC");
 >> +				ret = -EINVAL;
 >> +			}
 >> +
 >> +			list_del(pos);
 >> +			kfree(mc_entry);
 >> +		}
 >> +	}
 >
 > You don't need the if.
 >
 >

ok, removed

 > Ok, I'm tired now, I might read the rest later.
 >
 > cheers
 >

^ permalink raw reply

* Re: [PATCH 5/6] ehea: makefile
From: Thomas Klein @ 2006-08-10 15:06 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: netdev, linux-kernel, Thomas Klein, linux-ppc, Christoph Raisch,
	Marcus Eder
In-Reply-To: <20060809095047.GA11555@mars.ravnborg.org>

Sam Ravnborg wrote:
> On Wed, Aug 09, 2006 at 10:40:20AM +0200, Jan-Bernd Themann wrote:
>   
>> Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com>
>>
>>
>>  drivers/net/ehea/Makefile |    7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>>
>>
>> --- linux-2.6.18-rc4-orig/drivers/net/ehea/Makefile	1969-12-31 
>> 16:00:00.000000000 -0800
>> +++ kernel/drivers/net/ehea/Makefile	2006-08-08 23:59:38.083467216 -0700
>> @@ -0,0 +1,7 @@
>> +#
>> +# Makefile for the eHEA ethernet device driver for IBM eServer System p
>> +#
>> +
>> +ehea_mod-objs = ehea_main.o ehea_phyp.o ehea_qmr.o ehea_ethtool.o 
>> ehea_phyp.o
>> +obj-$(CONFIG_EHEA) += ehea_mod.o
>> +
>>     
>
> Using -objs is deprecated, please use ehea_mod-y.
> This needs to be documented and later warned upon which I will do soon.
>
> 	Sam
>   
Done. Will be included in next patch.

^ permalink raw reply

* Re: [PATCH 2/6] ehea: pHYP interface
From: Thomas Klein @ 2006-08-10 15:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Christoph Raisch, linuxppc-dev, Marcus Eder
In-Reply-To: <200608091514.46861.arnd.bergmann@de.ibm.com>

Arnd Bergmann wrote:
> On Wednesday 09 August 2006 10:38, Jan-Bernd Themann wrote:
>> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_hcall.h 1969-12-31 16:00:00.000000000 -0800
>> +++ kernel/drivers/net/ehea/ehea_hcall.h        2006-08-08 23:59:38.111462960 -0700
>> @@ -0,0 +1,52 @@
>
>> +
>> +/**
>> + * This file contains HCALL defines that are to be included in the appropriate
>> + * kernel files later
>> + */
>> +
>> +#define H_ALLOC_HEA_RESOURCE   0x278
>> +#define H_MODIFY_HEA_QP        0x250
>> +#define H_QUERY_HEA_QP         0x254
>> +#define H_QUERY_HEA            0x258
>> +#define H_QUERY_HEA_PORT       0x25C
>> +#define H_MODIFY_HEA_PORT      0x260
>> +#define H_REG_BCMC             0x264
>> +#define H_DEREG_BCMC           0x268
>> +#define H_REGISTER_HEA_RPAGES  0x26C
>> +#define H_DISABLE_AND_GET_HEA  0x270
>> +#define H_GET_HEA_INFO         0x274
>> +#define H_ADD_CONN             0x284
>> +#define H_DEL_CONN             0x288
>
> I  guess these should go to include/asm-powerpc/hvcall.h instead.
>
>       Arnd <><

We posted a separate patch for hvcall.h (http://ozlabs.org/pipermail/linuxppc-dev/2006-August/025000.html).
As soon as this patch is accepted we'll remove the ehea_hcall.h headerfile.

^ permalink raw reply

* Re: [PATCH] ehea: PPC - New hcall opcode defines
From: Stephen Rothwell @ 2006-08-10 15:20 UTC (permalink / raw)
  To: Thomas Klein; +Cc: tklein, linuxppc-dev, raisch, meder, themann
In-Reply-To: <44DB4B38.9030502@de.ibm.com>

On Thu, 10 Aug 2006 17:05:28 +0200 Thomas Klein <osstklei@de.ibm.com> wrote:
>
> diff -Nurp -X dontdiff linux-2.6.18-rc4/include/asm-powerpc/hvcall.h patched_kernel/include/asm-powerpc/hvcall.h
> --- linux-2.6.18-rc4/include/asm-powerpc/hvcall.h	2006-08-06 11:20:11.000000000 -0700
> +++ patched_kernel/include/asm-powerpc/hvcall.h	2006-08-10 06:26:33.018907062 -0700
> @@ -201,6 +201,19 @@
>   #define H_JOIN			0x298
>   #define H_VASI_STATE            0x2A4
>   #define H_ENABLE_CRQ		0x2B0
> +#define H_ALLOC_HEA_RESOURCE	0x278
> +#define H_MODIFY_HEA_QP		0x250
> +#define H_QUERY_HEA_QP		0x254
> +#define H_QUERY_HEA		0x258
> +#define H_QUERY_HEA_PORT	0x25C
> +#define H_MODIFY_HEA_PORT	0x260
> +#define H_REG_BCMC		0x264
> +#define H_DEREG_BCMC		0x268
> +#define H_REGISTER_HEA_RPAGES	0x26C
> +#define H_DISABLE_AND_GET_HEA	0x270
> +#define H_GET_HEA_INFO		0x274
> +#define H_ADD_CONN		0x284
> +#define H_DEL_CONN		0x288

This patch appears to be whitespace damaged and it would be preferable if
the new defines were in there correct places in numerical order.

Thanks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

^ permalink raw reply

* Re: [PATCH 1/6] ehea: interface to network stack
From: Jan-Bernd Themann @ 2006-08-10 14:49 UTC (permalink / raw)
  To: Alexey Dobriyan
  Cc: Thomas Klein, netdev, linux-kernel, linuxppc-dev,
	Christoph Raisch, Marcus Eder
In-Reply-To: <20060809130646.GA6846@martell.zuzino.mipt.ru>

Hi,

thanks for your comments!

We'll post a modified patch very soon.

Jan-Bernd

Alexey Dobriyan wrote:
> On Wed, Aug 09, 2006 at 10:38:20AM +0200, Jan-Bernd Themann wrote:
>> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_main.c
>> +++ kernel/drivers/net/ehea/ehea_main.c
> 
>> +static inline u64 get_swqe_addr(u64 tmp_addr, int addr_seg)
>> +{
>> +	u64 addr;
>> +	addr = tmp_addr;
>> +	return addr;
>> +}
>> +
>> +static inline u64 get_rwqe_addr(u64 tmp_addr)
>> +{
>> +	return tmp_addr;
>> +}
> 
> The point of this exercise?

has been removed

> 
>> +static inline int ehea_refill_rq3_def(struct ehea_port_res *pr, int nr_of_wqes)
> 
> Way too big to be inline function.
> 
>> +{
>> +	int i;
>> +	int ret = 0;
>> +	struct ehea_qp *qp;
>> +	struct ehea_rwqe *rwqe;
>> +	int skb_arr_rq3_len = pr->skb_arr_rq3_len;
>> +	struct sk_buff **skb_arr_rq3 = pr->skb_arr_rq3;
>> +	EDEB_EN(8, "pr=%p, nr_of_wqes=%d", pr, nr_of_wqes);
>> +	if (nr_of_wqes == 0)
>> +		return -EINVAL;
>> +	qp = pr->qp;
>> +	for (i = 0; i < nr_of_wqes; i++) {
>> +		int index = pr->skb_rq3_index++;
>> +		struct sk_buff *skb = dev_alloc_skb(EHEA_MAX_PACKET_SIZE
>> +						    + NET_IP_ALIGN);
>> +
>> +		if (!skb) {
>> +			EDEB_ERR(4, "No memory for skb. Only %d rwqe 
>> filled.",
>> +				 i);
>> +			ret = -ENOMEM;
>> +			break;
>> +		}
>> +		skb_reserve(skb, NET_IP_ALIGN);
>> +
>> +		rwqe = ehea_get_next_rwqe(qp, 3);
>> +		pr->skb_rq3_index %= skb_arr_rq3_len;
>> +		skb_arr_rq3[index] = skb;
>> +		rwqe->wr_id = EHEA_BMASK_SET(EHEA_WR_ID_TYPE, 
>> EHEA_RWQE3_TYPE)
>> +		    | EHEA_BMASK_SET(EHEA_WR_ID_INDEX, index);
>> +		rwqe->sg_list[0].l_key = ehea_get_recv_lkey(pr);
>> +		rwqe->sg_list[0].vaddr = get_rwqe_addr((u64)skb->data);
>> +		rwqe->sg_list[0].len = EHEA_MAX_PACKET_SIZE;
>> +		rwqe->data_segments = 1;
>> +	}
>> +
>> +	/* Ring doorbell */
>> +	iosync();
>> +	ehea_update_rq3a(qp, i);
>> +	EDEB_EX(8, "");
>> +	return ret;
>> +}
>> +
>> +
>> +static inline int ehea_refill_rq3(struct ehea_port_res *pr, int nr_of_wqes)
>> +{
>> +	return ehea_refill_rq3_def(pr, nr_of_wqes);
>> +}
> 
> ehea_refill_rq3[123] appears to be 1:1 wrappers around
> ehea_refill_rq3[123]_def. Any idea behind them?
> 

introduced for near future features

>> +	init_attr = (struct ehea_qp_init_attr*)
>> +	    kzalloc(sizeof(struct ehea_qp_init_attr), GFP_KERNEL);
> 
> Useless cast.
> 

removed

>> +	pr->skb_arr_sq = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
>> +						    * (max_rq_entries + 1));
> 
> Useless cast

removed
> 
>> +	pr->skb_arr_rq1 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
>> +						     * (max_rq_entries + 1));
> 
>> +	pr->skb_arr_rq2 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
>> +						     * (max_rq_entries + 1));
> 
>> +	pr->skb_arr_rq3 = (struct sk_buff**)vmalloc(sizeof(struct sk_buff*)
>> +						     * (max_rq_entries + 1));
> 
>> +static int ehea_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>> +{
>> +	EDEB_ERR(4, "ioctl not supported: dev=%s cmd=%d", dev->name, cmd);
> 
> Then copy NULL into ->do_ioctl!
> 

done

>> +	return -EOPNOTSUPP;
>> +}
> 
>> +	ehea_port_cb_0 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
>> +
>> +	if (!ehea_port_cb_0) {
>> +		EDEB_ERR(4, "No memory for ehea_port control block");
>> +		ret = -ENOMEM;
>> +		goto kzalloc_failed;
>> +	}
>> +
>> +	memcpy((u8*)(&(ehea_port_cb_0->port_mac_addr)),
>> +	       (u8*)&(mac_addr->sa_data[0]), 6);
> 
> No casts on memcpy arguments.

done

> 
>> +	memcpy((u8*)&ehea_mcl_entry->macaddr, mc_mac_addr, ETH_ALEN);
> 
>> +static inline void ehea_xmit2(struct sk_buff *skb,
>> +			      struct net_device *dev, struct ehea_swqe *swqe,
>> +			      u32 lkey)
>> +{
>> +	int nfrags;
>> +	unsigned short skb_protocol = skb->protocol;
> 
> Useless variable. And it should be __be16, FYI.
> 

changed

>> +	nfrags = skb_shinfo(skb)->nr_frags;
>> +	EDEB_EN(7, "skb->nfrags=%d (0x%X)", nfrags, nfrags);
>> +
>> +	if (skb_protocol == ETH_P_IP) {
> 
> ITYM, htons(ETH_P_IP).
> 

good point, thx

>> +static inline void ehea_xmit3(struct sk_buff *skb,
>> +			      struct net_device *dev, struct ehea_swqe *swqe)
>> +{
>> +	int i;
>> +	skb_frag_t *frag;
>> +	int nfrags = skb_shinfo(skb)->nr_frags;
>> +	u8 *imm_data = &swqe->u.immdata_nodesc.immediate_data[0];
>> +	u64 skb_protocol = skb->protocol;
> 
> Useless var.

removed

> 
>> +
>> +	EDEB_EN(7, "");
>> +	if (likely(skb_protocol == ETH_P_IP)) {
> 
> 				   htons(ETH_P_IP)
> 

^ permalink raw reply

* MPC5200B FEC  Fullduplex Performance problem
From: Thomas Schnürer @ 2006-08-10 15:52 UTC (permalink / raw)
  To: linuxppc-dev


Hello everybody,

I experience a Problem with obtaining the maximum FEC Performance.

I use Linux 2.6.15 with bestcomm 2.2 already and use the FEC in =
fullduplex mode. When I use
a test tool, the maximum transfer rates I get are 90 Mbit vs. 40 Mbit.
(the tool is called iperf, can be downloaded).

Also when checking /proc/interrupts after some time of fullduplex data =
transfer I see that there
are much more rx IRQs than tx IRQs.
In the driver linux/drivers/net/fec_mpc52xx.c I made some debug outputs =
which show me that
also TX FIFO underruns occur.

It seems to me that somehow the RX SDMA task is priorized before TX =
task, and this doesnt change
when I alter the priority values used in the driver.

The board is a MEN Mikro EM01, used clocks are XLB 128 MHz, CPU 384 MHz, =
SDRAM 132 MHz.

Does anyone have Performance measurements for FEC made on the 5200B ?

I would be happy already with reaching some 60/60 Mbits/s.

Thanks!

=20

^ 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