* Re: Debugging with no serial port
From: Brian Waite @ 2006-08-10 19:13 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <821B2170E9E7F04FA38DF7EC21DE487105FD1342@VCAEXCH01.hq.corp.viasat.com>
On Thursday 10 August 2006 15:00, Martin, Tim wrote:
> >> Second question would be - is there an easier way to accomplish what
> >> I'm trying to do, which is basically emulate a serial port with GDB.
> >
> > Use NetConsole or remotey log it with syslog...
>
> I guess I wasn't clear in the problem statement - this target doesn't
> have a serial port, an ethernet port, or a file system. So I can't log
> over Ethernet or log by writing to a file. I'm fairly restricted to
> getting data out of the system using GDB controlling a hardware JTAG
> debugger (think BDI2000).
I don't know GDB internals so I cannot answer the specific question relating
to synchronous writes via GDB.
But...
How much data are we talking and is there some area of private memory to stash
the logged data in and use the JTAG adapter to fetch the memory blocks and
write to a local file for post processing? I've done this in places where I
am doing kernbel bringup without a serial port. Your other option is use a
few IO pins (if available) and make a bit banging serial interface with some
extrnal components.
Thanks
Brian
^ permalink raw reply
* RE: Debugging with no serial port
From: Martin, Tim @ 2006-08-10 19:00 UTC (permalink / raw)
To: Frank, linuxppc-embedded
=20
>> Second question would be - is there an easier way to accomplish what=20
>> I'm trying to do, which is basically emulate a serial port with GDB.
>=20
> Use NetConsole or remotey log it with syslog...
>=20
I guess I wasn't clear in the problem statement - this target doesn't
have a serial port, an ethernet port, or a file system. So I can't log
over Ethernet or log by writing to a file. I'm fairly restricted to
getting data out of the system using GDB controlling a hardware JTAG
debugger (think BDI2000).
Tim
^ permalink raw reply
* Re: [RFC] consolidated libdt proposal
From: Mark A. Greer @ 2006-08-10 18:55 UTC (permalink / raw)
To: Milton Miller
Cc: hollisb, pantelis, linuxppc-embedded, linuxppc-dev, xen-ppc-devel
In-Reply-To: <11155228690643c98691.1714636915.miltonm@bga.com>
On Thu, Aug 10, 2006 at 11:51:30AM -0500, Milton Miller wrote:
> On Tue Aug 8 2006 01:04:08 PM CDT, Mark A. Greer wrote:
> > If we're going to allow cmdline editing in the bootwrapper, we would
> > need to extend the size of a property. We've never really talked about
> > cmdline editing in the powerpc branch but I assume that its a good
> > thing(tm). I know I would like to have it so, IMHO, I think we should
> > add it (and therefore require extending a property).
>
> We already have replace the command line merged (it's why my patches
> don't apply).
That's different. What you're talking about is the code that supports
sticking a cmdline in a separate section of the zImage and using it.
What I'm talking about is editing the cmdline at runtime (as in what
used to happen in arch/ppc).
Not sure if we need all of these options but that's what I was talking
about.
Mark
^ permalink raw reply
* latest git: compile error: arch/powerpc/kernel/built-in.o
From: Wolfgang Pfeiffer @ 2006-08-10 18:11 UTC (permalink / raw)
To: linux-ppc
Hi All
I'm trying to compile a fresh git kernel, with latest sources pulled today,
Aug. 10. To no avail so far.
OS is Debian/unstable, last time I upgraded the system as far as the
dependencies allowed doing that was yesterday, Aug. 10.
---------------------
$ cat /proc/cpuinfo
processor : 0
cpu : 7447A, altivec supported
clock : 833.333000MHz
revision : 0.5 (pvr 8003 0105)
bogomips : 16.57
timebase : 8320000
platform : PowerMac
machine : PowerBook5,8
motherboard : PowerBook5,8 MacRISC3 Power Macintosh
detected as : 287 (PowerBook G4 15")
pmac flags : 00000019
L2 cache : 512K unified
pmac-generation : NewWorld
-----------------------------
Here's the config I used:
http://wolfgangpfeiffer.com/config.060810.txt
The compile messages:
--------------------------------
time MAKEFLAGS="CC=gcc-4.1" fakeroot make-kpkg --append-to-version=-060810 kernel_image
results in [excerpts]:
CC lib/sha1.o
CC lib/string.o
CC lib/vsprintf.o
AR lib/lib.a
GEN .version
CHK include/linux/compile.h
UPD include/linux/compile.h
CC init/version.o
LD init/built-in.o
LD .tmp_vmlinux1
arch/powerpc/kernel/built-in.o: In function `__restore_cpu_setup':
(.text+0xcd34): undefined reference to `kexec_sr_activated'
arch/powerpc/kernel/built-in.o: In function `__restore_cpu_setup':
(.text+0xcd44): undefined reference to `crash_kexec_secondary'
arch/powerpc/kernel/built-in.o: In function `early_init_devtree':
(.init.text+0x1cdc): undefined reference to `reserve_crashkernel'
arch/powerpc/kernel/built-in.o: In function `early_init_devtree':
(.init.text+0x1e1c): undefined reference to `overlaps_crashkernel'
make[1]: *** [.tmp_vmlinux1] Error 1
make[1]: Leaving directory `/home/shorty/kernel-factory/git.08102006/linux-2.6'
make: *** [debian/stamp-build-kernel] Error 2
real 9m39.089s
user 8m21.904s
sys 0m59.167s
------------------------------------------------------------
and with gcc-4.0 the same:
time MAKEFLAGS="CC=gcc-4.0" fakeroot make-kpkg --append-to-version=-060810 kernel_image
results in [excerpts]:
CC lib/vsprintf.o
AR lib/lib.a
GEN .version
CHK include/linux/compile.h
UPD include/linux/compile.h
CC init/version.o
LD init/built-in.o
LD .tmp_vmlinux1
arch/powerpc/kernel/built-in.o: In function `__restore_cpu_setup':
(.text+0xcccc): undefined reference to `kexec_sr_activated'
arch/powerpc/kernel/built-in.o: In function `__restore_cpu_setup':
(.text+0xccdc): undefined reference to `crash_kexec_secondary'
arch/powerpc/kernel/built-in.o: In function `early_init_devtree':
(.init.text+0x15d0): undefined reference to `reserve_crashkernel'
arch/powerpc/kernel/built-in.o: In function `early_init_devtree':
(.init.text+0x1710): undefined reference to `overlaps_crashkernel'
make[1]: *** [.tmp_vmlinux1] Error 1
make[1]: Leaving directory `/home/shorty/kernel-factory/git.08102006/linux-2.6'
make: *** [debian/stamp-build-kernel] Error 2
--------------------------------------
Please let me know if you need more details.
Thanks in anticipation
Best Regards
Wolfgang Pfeiffer
--
Wolfgang Pfeiffer: /ICQ: 286585973/ + + + /AIM: crashinglinux/
http://profiles.yahoo.com/wolfgangpfeiffer
Key ID: E3037113
http://keyserver.mine.nu/pks/lookup?search=0xE3037113&fingerprint=on
^ permalink raw reply
* Compatibility Problem between linux-2.4.25 and RedBoot
From: wei.li4 @ 2006-08-10 18:02 UTC (permalink / raw)
To: linuxppc-embedded
Hi all,
Since my evaluation board based on MPC875 is pre-installed RedBoot
Loader and Linux with this kit can be loaded and execute, but I want to
try public powerpc linux, which can be loaded, but not execute. Dose
any one know how to change it so that it is compatible with RedBoot? I
checked the definition of struct bd_info{} is different, is it the
shared struct by bootloader and Linux? How dose loader work with linux?
Thanks.
Wei
^ permalink raw reply
* Re: Debugging with no serial port
From: Frank @ 2006-08-10 17:31 UTC (permalink / raw)
To: Martin, Tim, linuxppc-embedded
In-Reply-To: <821B2170E9E7F04FA38DF7EC21DE487105FD10D3@VCAEXCH01.hq.corp.viasat.com>
--- "Martin, Tim" <tim.martin@viasat.com> wrote:
> This may be more of a question for GDB folks...but I'll post
> it here
> because it relates to embedded systems as well.
>
> I'm trying to validate some signal processing software on an
> embedded
> Virtex4 PowerPC 405. Basically, the validation consists of
> calling
> functions and looking at their outputs. Over time, there are
> several
> thousand numbers to look at, so manually looking at each
> number at a
> break point would be very time consuming (to put it mildly).
>
> The normal way I would go about doing this is to print out the
> data
> (e.g. to a serial port) and post-process the data somewhere
> else. But
> on this particular target, I don't have a serial port. So I
> thought
> about using GDB's breakpoint command list feature. When the
> software
> has data it wants to print out, it calls a function
> "log_stuff". I then
> set a breakpoint and command list in the "log_stuff" function,
> which
> writes out the variables I'm interested in checking out to a
> file named
> outputfile.bin. E.g. the following command file does this:
>
> break log_stuff
> commands
> silent
> if (variable_logging_enabled)
> append value outputfile.bin stuff1_variable
> append value outputfile.bin stuff2_variable
> end
> cont
> end
>
> The problem I have with this approach is that GDB doesn't
> finish writing
> out everything before it continues executing the program, so a
> backlog
> develops. So my first question would be, is there some GDB
> "flush" like
> command I could run before the cont statement?
>
> Second question would be - is there an easier way to
> accomplish what I'm
> trying to do, which is basically emulate a serial port with
> GDB.
>
> Tim
Use NetConsole or remotey log it with syslog...
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
^ permalink raw reply
* Which is better for Wireless Network Support, 2.6 or 2.4?
From: wei.li4 @ 2006-08-10 17:22 UTC (permalink / raw)
To: linuxppc-embedded
Hi All,
I am working on wireless data application via MPC875 host USB, is there
any difference to use Linux 2.6.x or Linux 2.4.x? Is it Linux 2.6 more
like RTOS? Thanks.
Wei
^ permalink raw reply
* Debugging with no serial port
From: Martin, Tim @ 2006-08-10 17:14 UTC (permalink / raw)
To: linuxppc-embedded
[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]
This may be more of a question for GDB folks...but I'll post it here
because it relates to embedded systems as well.
I'm trying to validate some signal processing software on an embedded
Virtex4 PowerPC 405. Basically, the validation consists of calling
functions and looking at their outputs. Over time, there are several
thousand numbers to look at, so manually looking at each number at a
break point would be very time consuming (to put it mildly).
The normal way I would go about doing this is to print out the data
(e.g. to a serial port) and post-process the data somewhere else. But
on this particular target, I don't have a serial port. So I thought
about using GDB's breakpoint command list feature. When the software
has data it wants to print out, it calls a function "log_stuff". I then
set a breakpoint and command list in the "log_stuff" function, which
writes out the variables I'm interested in checking out to a file named
outputfile.bin. E.g. the following command file does this:
break log_stuff
commands
silent
if (variable_logging_enabled)
append value outputfile.bin stuff1_variable
append value outputfile.bin stuff2_variable
end
cont
end
The problem I have with this approach is that GDB doesn't finish writing
out everything before it continues executing the program, so a backlog
develops. So my first question would be, is there some GDB "flush" like
command I could run before the cont statement?
Second question would be - is there an easier way to accomplish what I'm
trying to do, which is basically emulate a serial port with GDB.
Tim
[-- Attachment #2: Type: text/html, Size: 3784 bytes --]
^ permalink raw reply
* Re: Reg. Serial and Virtual consoles
From: Milton Miller @ 2006-08-10 16:51 UTC (permalink / raw)
To: v.selvamuthukumar, linuxppc-dev; +Cc: linuxppc-dev
It sounds to me that you want all input and output on the serial line to be echoed to the vga screen both user and kernel. Do you also require keyboard input?
While others have described how to get the kernel messages displayed on both the screen and the serial port, they were missing the userspace requirement.
You will either need to write your own multiplexer program in user space and do the mirroring there, or use something like the screen program. Either way the program will be running on a pty and some agent will be mirroring the text. If you only require serial input it could be as simple as opening tty1, ttyS0, and the pty master and forking tee and cat to handle the master, your getty would run on the slave.
milton
^ permalink raw reply
* Re: [RFC] consolidated libdt proposal
From: Milton Miller @ 2006-08-10 16:51 UTC (permalink / raw)
To: mgreer, hollisb; +Cc: linuxppc-dev, pantelis, linuxppc-embedded, xen-ppc-devel
On Tue Aug 8 2006 01:04:08 PM CDT, Mark A. Greer wrote:
> If we're going to allow cmdline editing in the bootwrapper, we would
> need to extend the size of a property. We've never really talked about
> cmdline editing in the powerpc branch but I assume that its a good
> thing(tm). I know I would like to have it so, IMHO, I think we should
> add it (and therefore require extending a property).
We already have replace the command line merged (it's why my patches
don't apply).
milton
^ 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
* 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
* 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 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 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 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
* [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
* 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
* 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
* 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
* 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
* 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
* 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
* 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
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox