* Re: [RFC/PATCH 2/4] Allow for non-Intel MSI implementations
From: Segher Boessenkool @ 2006-08-18 13:03 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20060818081330.9ADC867B55@ozlabs.org>
> -msiobj-y := msi.o msi-apic.o
> -msiobj-$(CONFIG_IA64_GENERIC) += msi-altix.o
> -msiobj-$(CONFIG_IA64_SGI_SN2) += msi-altix.o
> +msiobj-$(CONFIG_X86) := msi.o msi-apic.o
> +msiobj-$(CONFIG_IA64) := msi.o msi-apic.o
> +msiobj-$(CONFIG_IA64_GENERIC) := msi-altix.o
> +msiobj-$(CONFIG_IA64_SGI_SN2) := msi-altix.o
The msi-altix changes are wrong, /me thinks.
Segher
^ permalink raw reply
* Re: [2.6.19 PATCH 3/7] ehea: queue management
From: Thomas Klein @ 2006-08-18 13:25 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Christoph Raisch, linux-ppc, Marcus Eder
In-Reply-To: <1155903451.4494.168.camel@laptopd505.fenrus.org>
Arjan van de Ven wrote:
>> + queue->queue_length = nr_of_pages * pagesize;
>> + queue->queue_pages = vmalloc(nr_of_pages * sizeof(void *));
>
>
> wow... is this really so large that it warrants a vmalloc()???
Agreed: Replaced with kmalloc()
Regards
Thomas
^ permalink raw reply
* Re: [2.6.19 PATCH 1/7] ehea: interface to network stack
From: Alexey Dobriyan @ 2006-08-18 13:47 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181329.02042.ossthema@de.ibm.com>
On Fri, Aug 18, 2006 at 01:29:01PM +0200, Jan-Bernd Themann wrote:
Was there noticeable performance difference when explicit prefetching is
removed? At some (invisible) point CPUs will become smarter about prefetching
than programmers and this code will be slower than possible.
> +static inline struct sk_buff *get_skb_by_index(struct sk_buff **skb_array,
> + int arr_len,
> + struct ehea_cqe *cqe)
> +{
> + struct sk_buff *skb;
> + void *pref;
> + int x;
> + int skb_index = EHEA_BMASK_GET(EHEA_WR_ID_INDEX, cqe->wr_id);
> +
> + x = skb_index + 1;
> + x &= (arr_len - 1);
> +
> + pref = (void*)skb_array[x];
> + prefetchw(pref);
> + prefetchw(pref + EHEA_CACHE_LINE);
> +
> + pref = (void*)(skb_array[x]->data);
> + prefetch(pref);
> + prefetch(pref + EHEA_CACHE_LINE);
> + prefetch(pref + EHEA_CACHE_LINE * 2);
> + prefetch(pref + EHEA_CACHE_LINE * 3);
> + skb = skb_array[skb_index];
> + skb_array[skb_index] = NULL;
> + return skb;
> +}
> +
> +static inline struct sk_buff *get_skb_by_index_ll(struct sk_buff **skb_array,
> + int arr_len, int wqe_index)
> +{
> + struct sk_buff *skb;
> + void *pref;
> + int x;
> +
> + x = wqe_index + 1;
> + x &= (arr_len - 1);
> +
> + pref = (void*)skb_array[x];
> + prefetchw(pref);
> + prefetchw(pref + EHEA_CACHE_LINE);
> +
> + pref = (void*)(skb_array[x]->data);
> + prefetchw(pref);
> + prefetchw(pref + EHEA_CACHE_LINE);
> +
> + skb = skb_array[wqe_index];
> + skb_array[wqe_index] = NULL;
> + return skb;
> +}
^ permalink raw reply
* Re: [2.6.19 PATCH 3/7] ehea: queue management
From: Arnd Bergmann @ 2006-08-18 13:47 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Thomas Klein, Christoph Raisch, Marcus Eder, Arjan van de Ven
In-Reply-To: <44E5BFB7.4000400@de.ibm.com>
On Friday 18 August 2006 15:25, Thomas Klein wrote:
>
> > wow... is this really so large that it warrants a vmalloc()???
>
> Agreed: Replaced with kmalloc()
My understanding from the previous discussion was that it actually
is a multi-page power of two allocation, so the right choice might
be __get_free_pages() instead of kmalloc, but it probably doesn't
make much of a difference.
You should really do some measurements to see what the minimal
queue sizes are that can get you optimal throughput.
Arnd <><
^ permalink raw reply
* Re: [2.6.19 PATCH 1/7] ehea: interface to network stack
From: Arjan van de Ven @ 2006-08-18 13:56 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Christoph Raisch, Thomas Klein, linux-ppc, Marcus Eder
In-Reply-To: <20060818134728.GB5201@martell.zuzino.mipt.ru>
On Fri, 2006-08-18 at 17:47 +0400, Alexey Dobriyan wrote:
> On Fri, Aug 18, 2006 at 01:29:01PM +0200, Jan-Bernd Themann wrote:
>
> Was there noticeable performance difference when explicit prefetching is
> removed? At some (invisible) point CPUs will become smarter about prefetching
> than programmers and this code will be slower than possible.
Hi,
what you say is true in general, however the packet response part of the
kernel is special; the cpu just cannot know where your card just dma'd
the packet to (and thus cleared it from any caches anywhere in the
system) so it's going to be pretty much a 100% cachemiss always...
Greetings,
Arjan van de Ven
^ permalink raw reply
* Re: [PATCH 6/6] bootwrapper: Add support for the sandpoint platform
From: Matt Porter @ 2006-08-18 14:00 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.60.0608180109060.8228@poirot.grange>
On Fri, Aug 18, 2006 at 01:19:22AM +0200, Guennadi Liakhovetski wrote:
> On Fri, 18 Aug 2006, Guennadi Liakhovetski wrote:
>
> > On Thu, 17 Aug 2006, Mark A. Greer wrote:
> >
> > > Depends on what you mean by "__boot__". This is the code needed to
> > > prepare things--the flat dev tree in this case--so that the kernel
> > > can boot when running the DINK firmware on a sandpoint.
> >
> > ...after getting some vitamins on irc and re-reading some emails, I
> > finally realised what the stuff under boot is for. Expect more confusion
> > from me:-)
>
> Ok, I think, now I have an idea what all these wrappers are about:
>
> we want to be able to boot linux from a number of different environments:
>
> OF
>
> various firmwares / boot ROMs / bootloaders with very different
> capabilities of running foreign code and different calling conventions
>
> "native" Linux bootloaders like u-Boot, etc.
>
> kexec
>
> ...
>
> and the chosen approach is to have a kernel proper common to all these
> possibilities, and "wrappers". These wrappers are boot-method and board
> specific, are distributed with the kernel, located under
> arch/powerpc/boot, get linked with the kernel proper, fdt, with various
> compression possibilities...
>
> Have I got it right now? One of the reasons of me having problems to
> understand this is that on ARM we just say "to boot kernel you have to
> have MMU switched off, registers must contain this and that, you have to
> prepare a list of ATAGs, copy the kernel to a certain address, and jump to
> it. If you don't do that you cannot run Linux on your system":-)
Sure, everybody that's familiar with ARM Linux is aware of that.
That's what the new fdt requirement in arch/powerpc/ does for
PPC as well. arch/powerpc/boot/ wrapper code isn't technically
part of the kernel, it's merely distributed with the kernel source
for convenience. It's a boot shim to translate from "legacy"
firmwares (since they aren't easily replaced) to the standard
method. It is independent of the kernel proper.
-Matt
^ permalink raw reply
* Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
From: Alexey Dobriyan @ 2006-08-18 14:05 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181333.23031.ossthema@de.ibm.com>
On Fri, Aug 18, 2006 at 01:33:22PM +0200, Jan-Bernd Themann wrote:
> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_ethtool.c
> +++ kernel/drivers/net/ehea/ehea_ethtool.c
> +static int netdev_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> +{
> + u64 hret = H_HARDWARE;
useless assignment;
> + struct ehea_port *port = netdev_priv(dev);
> + struct ehea_adapter *adapter = port->adapter;
> + struct hcp_query_ehea_port_cb_4 *cb4 = NULL;
> +
> + cb4 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> + if(!cb4) {
> + ehea_error("no mem for cb4");
> + goto get_settings_exit;
> + }
> +
> + hret = ehea_h_query_ehea_port(adapter->handle,
> + port->logical_port_id,
> + H_PORT_CB4,
> + H_PORT_CB4_ALL,
> + cb4);
> +static void netdev_get_drvinfo(struct net_device *dev,
> + struct ethtool_drvinfo *info)
> +{
> + strncpy(info->driver, DRV_NAME, sizeof(info->driver) - 1);
> + strncpy(info->version, DRV_VERSION, sizeof(info->version) - 1);
Use strlcpy() to not forget -1 accidently.
> +static u32 netdev_get_msglevel(struct net_device *dev)
^^^^^^^^
> +{
> + struct ehea_port *port = netdev_priv(dev);
> + return port->msg_enable;
^^^^^^
Something is mis-named here.
> +}
> +
> +static void netdev_set_msglevel(struct net_device *dev, u32 value)
> +{
> + struct ehea_port *port = netdev_priv(dev);
> + port->msg_enable = value;
> +}
And here.
> +static void netdev_get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + int i = 0;
> + u64 hret = H_HARDWARE;
Useless assignment.
> + struct ehea_port *port = netdev_priv(dev);
> + struct ehea_adapter *adapter = port->adapter;
> + struct ehea_port_res *pr = &port->port_res[0];
> + struct port_state *p_state = &pr->p_state;
> + struct hcp_query_ehea_port_cb_6 *cb6 = NULL;
Ditto.
> + cb6 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> + if(!cb6) {
> + ehea_error("no mem for cb6");
> + goto stats_exit;
> + }
> +
> + hret = ehea_h_query_ehea_port(adapter->handle,
> + port->logical_port_id,
> + H_PORT_CB6,
> + H_PORT_CB6_ALL,
> + cb6);
> +struct ethtool_ops ehea_ethtool_ops = {
> + .get_settings = netdev_get_settings,
> + .get_drvinfo = netdev_get_drvinfo,
> + .get_msglevel = netdev_get_msglevel,
> + .set_msglevel = netdev_set_msglevel,
> + .get_link = ethtool_op_get_link,
> + .get_tx_csum = ethtool_op_get_tx_csum,
Whitespace breakage.
> + .set_tx_csum = ethtool_op_set_tx_csum,
> + .get_sg = ethtool_op_get_sg,
> + .set_sg = ethtool_op_set_sg,
> + .get_tso = ethtool_op_get_tso,
> + .set_tso = ethtool_op_set_tso,
> + .get_strings = netdev_get_strings,
> + .get_stats_count = netdev_get_stats_count,
> + .get_ethtool_stats = netdev_get_ethtool_stats,
> + .set_settings = NULL,
> + .nway_reset = NULL,
> + .get_pauseparam = NULL,
> + .set_pauseparam = NULL,
> + .get_rx_csum = NULL,
> + .set_rx_csum = NULL,
> + .phys_id = NULL,
> + .self_test = NULL,
> + .self_test_count = NULL
If you don't use them, don't mention them at all. Compiler will DTRT.
^ permalink raw reply
* Re: Please pull powerpc.git 'merge' branch
From: Matt Porter @ 2006-08-18 14:15 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <17637.8568.40692.21510@cargo.ozlabs.ibm.com>
On Fri, Aug 18, 2006 at 12:10:00PM +1000, Paul Mackerras wrote:
Paul,
Could you please add the following 4xx patches that were previously
posted? Thanks.
[PATCH] Remove flush_dcache_all export
[PATCH] Fix powerpc 44x_mmu build
-Matt
^ permalink raw reply
* Re: [2.6.19 PATCH 5/7] ehea: main header files
From: Alexey Dobriyan @ 2006-08-18 14:16 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181334.57701.ossthema@de.ibm.com>
On Fri, Aug 18, 2006 at 01:34:57PM +0200, Jan-Bernd Themann wrote:
> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea.h
> +++ kernel/drivers/net/ehea/ehea.h
> +#include <linux/version.h>
Not needed.
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
Really?
> +#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>
Nothing is used here. grep kthread
> +#include <linux/ethtool.h>
> +#include <linux/if_vlan.h>
It this because of "struct vlan_group" just add "struct vlan_group;" at
the beginning of the headers.
> +#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>
Please, use only headers you only really need. Full rebuilds are already pretty
enough. I've included half of include/linux disaster must stop.
^ permalink raw reply
* Re: [2.6.19 PATCH 7/7] ehea: Makefile & Kconfig
From: Alexey Dobriyan @ 2006-08-18 14:18 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181337.44153.ossthema@de.ibm.com>
On Fri, Aug 18, 2006 at 01:37:44PM +0200, Jan-Bernd Themann wrote:
> --- linux-2.6.18-rc4/drivers/net/Kconfig
> +++ patched_kernel/drivers/net/Kconfig
> @@ -2277,6 +2277,12 @@ config CHELSIO_T1
> To compile this driver as a module, choose M here: the module
> will be called cxgb.
>
> +config EHEA
> + tristate "eHEA Ethernet support"
> + depends on IBMEBUS
> + ---help---
> + This driver supports the IBM pSeries ethernet adapter
.
People usually add the following boilerplate:
To compile this driver as a module, choose M here: the module
will be called $FOO.
^ permalink raw reply
* Re: [2.6.19 PATCH 3/7] ehea: queue management
From: Christoph Raisch @ 2006-08-18 14:24 UTC (permalink / raw)
To: abergman
Cc: Thomas Q Klein, Jan-Bernd Themann, netdev, linux-kernel,
linuxppc-dev, Marcus Eder, tklein, Arjan van de Ven
In-Reply-To: <200608181547.47081.arnd.bergmann@de.ibm.com>
> You should really do some measurements to see what the minimal
> queue sizes are that can get you optimal throughput.
>
> Arnd <><
we did.
And as always in performance tuning... one size fits all unfortunately is
not the correct answer.
Therefore we'll leave that open to the user as most other new ethernet
driver did as well.
Regards . . . Christoph R
^ permalink raw reply
* Re: Outstanding Patches Ping
From: Jon Loeliger @ 2006-08-18 14:21 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <17637.1235.223857.925995@cargo.ozlabs.ibm.com>
So, like, the other day Paul Mackerras mumbled:
>
> The "Allow MPC8641 HPCN to build with CONFIG_PCI disabled too" patch
> depends on your "Rewrite the PPC 86xx IRQ handling to use Flat Device
> Tree" which isn't in the queue for 2.6.18, and doesn't apply for
> 2.6.18 since it depends on the constifying of get_property.
Argh! That IRQ rewrite patch is the critical patch to get into 2.6.18
as it is currently totally broken without it, and works with it!
Any chance? I mean, it was working in the early -rc versions of .18
before Ben changed the IRQ world. This is just cleanup effort to
have it continue to work in in .18.
jdl
^ permalink raw reply
* Re: [2.6.19 PATCH 3/7] ehea: queue management
From: Jörn Engel @ 2006-08-18 14:33 UTC (permalink / raw)
To: Thomas Klein
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Christoph Raisch, linux-ppc, Marcus Eder, Arjan van de Ven
In-Reply-To: <44E5BFB7.4000400@de.ibm.com>
On Fri, 18 August 2006 15:25:11 +0200, Thomas Klein wrote:
> Arjan van de Ven wrote:
> >>+ queue->queue_length = nr_of_pages * pagesize;
> >>+ queue->queue_pages = vmalloc(nr_of_pages * sizeof(void *));
> >
> >
> >wow... is this really so large that it warrants a vmalloc()???
>
> Agreed: Replaced with kmalloc()
kzalloc() and you can remove the memset() as well.
Jörn
--
Data dominates. If you've chosen the right data structures and organized
things well, the algorithms will almost always be self-evident. Data
structures, not algorithms, are central to programming.
-- Rob Pike
^ permalink raw reply
* Re: [2.6.19 PATCH 3/7] ehea: queue management
From: Jörn Engel @ 2006-08-18 14:40 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181331.19501.ossthema@de.ibm.com>
On Fri, 18 August 2006 13:31:19 +0200, Jan-Bernd Themann wrote:
>
> + if (queue->current_q_offset > queue->queue_length) {
> + queue->current_q_offset -= queue->pagesize;
> + retvalue = NULL;
> + }
> + else if ((((u64) retvalue) & (EHEA_PAGESIZE-1)) != 0) {
} else if (((u64) retvalue) & (EHEA_PAGESIZE-1)) {
> + if (hret < H_SUCCESS) {
Do you have a reason to keep H_SUCCESS?
> + if(qp_attr->rq_count > 1)
> + hw_queue_dtor(&qp->hw_rqueue2);
> + if(qp_attr->rq_count > 2)
Small amount of whitespace damage.
Jörn
--
Write programs that do one thing and do it well. Write programs to work
together. Write programs to handle text streams, because that is a
universal interface.
-- Doug MacIlroy
^ permalink raw reply
* Re: [2.6.19 PATCH 1/7] ehea: interface to network stack
From: Alexey Dobriyan @ 2006-08-18 14:44 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181329.02042.ossthema@de.ibm.com>
On Fri, Aug 18, 2006 at 01:29:01PM +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 struct net_device_stats *ehea_get_stats(struct net_device *dev)
> +{
> + int i;
> + u64 hret = H_HARDWARE;
Useless assignment here and everywhere.
> + u64 rx_packets = 0;
> + struct ehea_port *port = netdev_priv(dev);
> + struct hcp_query_ehea_port_cb_2 *cb2 = NULL;
> + struct net_device_stats *stats = &port->stats;
> +
> + cb2 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> + if (!cb2) {
> + ehea_error("no mem for cb2");
> + goto get_stat_exit;
> + }
> +
> + hret = ehea_h_query_ehea_port(port->adapter->handle,
> + port->logical_port_id,
> + H_PORT_CB2,
> + H_PORT_CB2_ALL,
> + cb2);
> +static inline int ehea_refill_rq1(struct ehea_port_res *pr, int index,
> + int nr_of_wqes)
> +{
> + int i;
> + int ret = 0;
> + struct sk_buff **skb_arr_rq1 = pr->skb_arr_rq1;
> + int max_index_mask = pr->skb_arr_rq1_len - 1;
> +
> + if (!nr_of_wqes)
> + return 0;
> +
> + for (i = 0; i < nr_of_wqes; i++) {
> + if (!skb_arr_rq1[index]) {
> + skb_arr_rq1[index] = dev_alloc_skb(EHEA_LL_PKT_SIZE);
> +
> + if (!skb_arr_rq1[index]) {
> + ehea_error("no mem for skb/%d wqes filled", i);
> + ret = -ENOMEM;
> + break;
> + }
> + }
> + index--;
> + index &= max_index_mask;
> + }
> + /* Ring doorbell */
> + ehea_update_rq1a(pr->qp, i);
> +
> + return ret;
> +}
> +
> +static int ehea_init_fill_rq1(struct ehea_port_res *pr, int nr_rq1a)
> +{
> + int i;
> + int ret = 0;
> + struct sk_buff **skb_arr_rq1 = pr->skb_arr_rq1;
> +
> + for (i = 0; i < pr->skb_arr_rq1_len; i++) {
> + skb_arr_rq1[i] = dev_alloc_skb(EHEA_LL_PKT_SIZE);
> + if (!skb_arr_rq1[i]) {
> + ehea_error("no mem for skb/%d skbs filled.", i);
> + ret = -ENOMEM;
> + goto nomem;
> + }
> + }
> + /* Ring doorbell */
> + ehea_update_rq1a(pr->qp, nr_rq1a);
> +nomem:
> + 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;
> + struct sk_buff **skb_arr_rq2 = pr->skb_arr_rq2;
> + int index, max_index_mask;
> +
> + if (!nr_of_wqes)
> + return 0;
> +
> + qp = pr->qp;
> +
> + index = pr->skb_rq2_index;
> + max_index_mask = pr->skb_arr_rq2_len - 1;
> + for (i = 0; i < nr_of_wqes; i++) {
> + struct sk_buff *skb = dev_alloc_skb(EHEA_RQ2_PKT_SIZE
> + + NET_IP_ALIGN);
> + if (!skb) {
> + ehea_error("no mem for skb/%d wqes filled", i);
> + ret = -ENOMEM;
> + break;
> + }
> + skb_reserve(skb, NET_IP_ALIGN);
> +
> + 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 = pr->recv_mr.lkey;
> + rwqe->sg_list[0].vaddr = (u64)skb->data;
> + rwqe->sg_list[0].len = EHEA_RQ2_PKT_SIZE;
> + rwqe->data_segments = 1;
> +
> + index++;
> + index &= max_index_mask;
> + }
> + pr->skb_rq2_index = index;
> +
> + /* Ring doorbell */
> + iosync();
> + ehea_update_rq2a(qp, i);
> + 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);
> +}
> +
> +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 = pr->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;
> + int index, max_index_mask;
> +
> + if (nr_of_wqes == 0)
> + return -EINVAL;
> +
> + index = pr->skb_rq3_index;
> + max_index_mask = skb_arr_rq3_len - 1;
> + for (i = 0; i < nr_of_wqes; i++) {
> + struct sk_buff *skb = dev_alloc_skb(EHEA_MAX_PACKET_SIZE
> + + NET_IP_ALIGN);
> + if (!skb) {
> + ehea_error("no mem for skb/%d wqes filled", i);
> + ret = -ENOMEM;
> + break;
> + }
> + skb_reserve(skb, NET_IP_ALIGN);
> +
> + rwqe = ehea_get_next_rwqe(qp, 3);
> +
> + 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 = pr->recv_mr.lkey;
> + rwqe->sg_list[0].vaddr = (u64)skb->data;
> + rwqe->sg_list[0].len = EHEA_MAX_PACKET_SIZE;
> + rwqe->data_segments = 1;
> +
> + index++;
> + index &= max_index_mask;
> + }
> + pr->skb_rq3_index = index;
> +
> + /* Ring doorbell */
> + iosync();
> + ehea_update_rq3a(qp, i);
> + return ret;
> +}
This one looks too big to be inlined as well as other similalry named
functions.
> +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;
> + 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;
> + return -EINVAL;
> +}
> +
> +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 */
> + skb_put(skb, length);
> + skb->dev = dev;
> + skb->ip_summed = CHECKSUM_UNNECESSARY;
> + skb->protocol = eth_type_trans(skb, dev);
> +}
> +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 = NULL;
> + struct ehea_adapter *adapter = port->adapter;
> +
> + memset(pr, 0, sizeof(struct ehea_port_res));
> +
> + pr->skb_arr_rq3 = NULL;
> + pr->skb_arr_rq2 = NULL;
> + pr->skb_arr_rq1 = NULL;
> + pr->skb_arr_sq = NULL;
> + pr->qp = NULL;
> + pr->send_cq = NULL;
> + pr->recv_cq = NULL;
> + pr->send_eq = NULL;
> + pr->recv_eq = NULL;
After memset unneeded. ;-)
> +static int ehea_clean_port_res(struct ehea_port *port, struct ehea_port_res *pr)
Freeing/deallocating/cleaning functions usually return void. The fact
that you always return -EINVAL only reaffirmes my belief. ;-)
> +{
> + int i;
> + int ret = -EINVAL;
> +
> + ehea_destroy_qp(pr->qp);
> + ehea_destroy_cq(pr->send_cq);
> + ehea_destroy_cq(pr->recv_cq);
> + ehea_destroy_eq(pr->send_eq);
> + ehea_destroy_eq(pr->recv_eq);
> +
> + 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);
> + return ret;
> +}
> +static inline void write_swqe2_data(struct sk_buff *skb,
> + struct net_device *dev,
> + struct ehea_swqe *swqe,
> + u32 lkey)
> +{
Way too big.
> + 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;
> +
> + 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 = tmp_addr;
> + swqe->descriptors++;
> + }
> + } else
> + ehea_error("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 = tmp_addr;
> + 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 = tmp_addr;
> + 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 = tmp_addr;
> + }
> + }
> +}
Got it?
> +static inline void ehea_xmit2(struct sk_buff *skb,
> + struct net_device *dev, struct ehea_swqe *swqe,
> + u32 lkey)
> +{
> + int nfrags;
> + nfrags = skb_shinfo(skb)->nr_frags;
> +
> + if (skb->protocol == htons(ETH_P_IP)) {
> + /* IPv4 */
> + swqe->tx_control |= EHEA_SWQE_CRC
> + | EHEA_SWQE_IP_CHECKSUM
> + | EHEA_SWQE_TCP_CHECKSUM
> + | EHEA_SWQE_IMM_DATA_PRESENT
> + | EHEA_SWQE_DESCRIPTORS_PRESENT;
> +
> + write_ip_start_end(swqe, skb);
> +
> + if (skb->nh.iph->protocol == IPPROTO_UDP) {
> + if ((skb->nh.iph->frag_off & IP_MF)
> + || (skb->nh.iph->frag_off & IP_OFFSET))
> + /* IP fragment, so don't change cs */
> + swqe->tx_control &= ~EHEA_SWQE_TCP_CHECKSUM;
> + else
> + write_udp_offset_end(swqe, skb);
> +
> + } else if (skb->nh.iph->protocol == IPPROTO_TCP) {
> + write_tcp_offset_end(swqe, skb);
> + }
> +
> + /* icmp (big data) and ip segmentation packets (all other ip
> + packets) do not require any special handling */
> +
> + } else {
> + /* Other Ethernet Protocol */
> + swqe->tx_control |= EHEA_SWQE_CRC
> + | EHEA_SWQE_IMM_DATA_PRESENT
> + | EHEA_SWQE_DESCRIPTORS_PRESENT;
> + }
> +
> + write_swqe2_data(skb, dev, swqe, lkey);
> +}
> +
> +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];
> +
> + if (likely(skb->protocol == htons(ETH_P_IP))) {
> + /* IPv4 */
> + write_ip_start_end(swqe, skb);
> +
> + if (skb->nh.iph->protocol == IPPROTO_TCP) {
> + swqe->tx_control |= EHEA_SWQE_CRC
> + | EHEA_SWQE_IP_CHECKSUM
> + | EHEA_SWQE_TCP_CHECKSUM
> + | EHEA_SWQE_IMM_DATA_PRESENT;
> +
> + write_tcp_offset_end(swqe, skb);
> +
> + } else if (skb->nh.iph->protocol == IPPROTO_UDP) {
> + if ((skb->nh.iph->frag_off & IP_MF)
> + || (skb->nh.iph->frag_off & IP_OFFSET))
> + /* IP fragment, so don't change cs */
> + swqe->tx_control |= EHEA_SWQE_CRC
> + | EHEA_SWQE_IMM_DATA_PRESENT;
> + else {
> + swqe->tx_control |= EHEA_SWQE_CRC
> + | EHEA_SWQE_IP_CHECKSUM
> + | EHEA_SWQE_TCP_CHECKSUM
> + | EHEA_SWQE_IMM_DATA_PRESENT;
> +
> + write_udp_offset_end(swqe, skb);
> + }
> + } else {
> + /* icmp (big data) and
> + ip segmentation packets (all other ip packets) */
> + swqe->tx_control |= EHEA_SWQE_CRC
> + | EHEA_SWQE_IP_CHECKSUM
> + | EHEA_SWQE_IMM_DATA_PRESENT;
> + }
> + } else {
> + /* Other Ethernet Protocol */
> + swqe->tx_control |= EHEA_SWQE_CRC | EHEA_SWQE_IMM_DATA_PRESENT;
> + }
> + /* copy (immediate) data */
> + if (nfrags == 0) {
> + /* data is in a single piece */
> + memcpy(imm_data, skb->data, skb->len);
> + } else {
> + /* first copy data from the skb->data buffer ... */
> + memcpy(imm_data, skb->data, skb->len - skb->data_len);
> + imm_data += skb->len - skb->data_len;
> +
> + /* ... then copy data from the fragments */
> + for (i = 0; i < nfrags; i++) {
> + frag = &skb_shinfo(skb)->frags[i];
> + memcpy(imm_data,
> + page_address(frag->page) + frag->page_offset,
> + frag->size);
> + imm_data += frag->size;
> + }
> + }
> + swqe->immediate_data_length = skb->len;
> + dev_kfree_skb(skb);
> +}
Ditto.
> +static void ehea_vlan_rx_register(struct net_device *dev,
> + struct vlan_group *grp)
> +{
> + u64 hret = H_HARDWARE;
> + struct hcp_query_ehea_port_cb_1 *cb1 = NULL;
> + struct ehea_port *port = netdev_priv(dev);
> + struct ehea_adapter *adapter = port->adapter;
> +
> + port->vgrp = grp;
> +
> + cb1 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> + if (!cb1) {
> + ehea_error("no mem for cb1");
> + goto vlan_reg_exit;
> + }
> +
> + if (grp)
> + memset(cb1->vlan_filter, 0, sizeof(cb1->vlan_filter));
> + else
> + memset(cb1->vlan_filter, 1, sizeof(cb1->vlan_filter));
Just to be sure, this should be 1 not 0xff?
> + hret = ehea_h_modify_ehea_port(adapter->handle,
> + port->logical_port_id,
> + H_PORT_CB1,
> + H_PORT_CB1_ALL,
> + cb1);
> + if (hret != H_SUCCESS)
> + ehea_error("modify_ehea_port failed");
> +
> + kfree(cb1);
> +
> +vlan_reg_exit:
> + return;
> +}
> +void ehea_clean_all_port_res(struct ehea_port *port)
> +{
> + int ret;
> + int i;
> + for(i = 0; i < port->num_def_qps + port->num_tx_qps; i++)
> + ehea_clean_port_res(port, &port->port_res[i]);
> +
> + ret = ehea_destroy_eq(port->qp_eq);
> +}
ret is entirely useless.
> +int __init ehea_module_init(void)
static
> +{
> + int ret = -EINVAL;
> +
> + printk("IBM eHEA Ethernet Device Driver (Release %s)\n", DRV_VERSION);
> +
> + ret = ibmebus_register_driver(&ehea_driver);
> + if (ret) {
> + ehea_error("failed registering eHEA device driver on ebus");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
Pass ret to upper layer. Simplest way is:
static int __init ehea_module_init(void)
{
return ibmebus_register_driver(&ehea_driver);
}
^ permalink raw reply
* Re: [2.6.19 PATCH 2/7] ehea: pHYP interface
From: Alexey Dobriyan @ 2006-08-18 15:06 UTC (permalink / raw)
To: Jan-Bernd Themann
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181330.21507.ossthema@de.ibm.com>
On Fri, Aug 18, 2006 at 01:30:21PM +0200, Jan-Bernd Themann wrote:
> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_phyp.c
> +++ kernel/drivers/net/ehea/ehea_phyp.c
> + hret = ehea_hcall_9arg_9ret(H_QUERY_HEA_QP,
> + hcp_adapter_handle, /* R4 */
> + qp_category, /* R5 */
> + qp_handle, /* R6 */
> + sel_mask, /* R7 */
> + virt_to_abs(cb_addr), /* R8 */
> + 0, 0, 0, 0, /* R9-R12 */
> + &dummy, /* R4 */
> + &dummy, /* R5 */
> + &dummy, /* R6 */
> + &dummy, /* R7 */
> + &dummy, /* R8 */
> + &dummy, /* R9 */
> + &dummy, /* R10 */
> + &dummy, /* R11 */
> + &dummy); /* R12 */
I asked SO to recount arguments and we've come to a conclusion that
there're in fact 19 args not 18 as the name suggests. 19 args is
I-N-S-A-N-E.
> +u64 ehea_h_register_rpage_eq(const u64 hcp_adapter_handle,
> + const u64 eq_handle,
> + const u8 pagesize,
> + const u8 queue_type,
> + const u64 log_pageaddr, const u64 count)
> +{
> + u64 hret = H_ADAPTER_PARM;
> +
> + if (count != 1)
> + return H_PARAMETER;
> +
> + hret = ehea_h_register_rpage(hcp_adapter_handle, pagesize, queue_type,
> + eq_handle, log_pageaddr, count);
> + return hret;
Just
return ehea_h_register_rpage(...);
> +u64 ehea_h_register_rpage_cq(const u64 hcp_adapter_handle,
> + const u64 cq_handle,
> + const u8 pagesize,
> + const u8 queue_type,
> + const u64 log_pageaddr,
> + const u64 count, const struct h_epa epa)
> +{
> + u64 hret = H_ADAPTER_PARM;
> +
> + if (count != 1)
> + return H_PARAMETER;
> +
> + hret = ehea_h_register_rpage(hcp_adapter_handle, pagesize, queue_type,
> + cq_handle, log_pageaddr, count);
> + return hret;
Ditto.
> +u64 ehea_h_register_rpage_qp(const u64 hcp_adapter_handle,
> + const u64 qp_handle,
> + const u8 pagesize,
> + const u8 queue_type,
> + const u64 log_pageaddr,
> + const u64 count, struct h_epa epa)
> +{
> + u64 hret = H_ADAPTER_PARM;
> +
> + if (count != 1)
> + return H_PARAMETER;
> +
> + hret = ehea_h_register_rpage(hcp_adapter_handle, pagesize, queue_type,
> + qp_handle, log_pageaddr, count);
> + return hret;
> +}
Ditto.
> +static inline int hcp_epas_ctor(struct h_epas *epas, u64 paddr_kernel,
> + u64 paddr_user)
> +{
> + epas->kernel.fw_handle = (u64) ioremap(paddr_kernel, PAGE_SIZE);
> + epas->user.fw_handle = paddr_user;
> + return 0;
> +}
> +
> +static inline int hcp_epas_dtor(struct h_epas *epas)
> +{
> +
> + if (epas->kernel.fw_handle)
> + iounmap((void *)epas->kernel.fw_handle);
> + epas->user.fw_handle = epas->kernel.fw_handle = 0;
> + return 0;
> +}
Always returns 0;
^ permalink raw reply
* Re: [2.6.19 PATCH 2/7] ehea: pHYP interface
From: Anton Blanchard @ 2006-08-18 15:13 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Christoph Raisch, Thomas Klein, linux-ppc, Marcus Eder
In-Reply-To: <20060818150600.GG5201@martell.zuzino.mipt.ru>
Hi,
> I asked SO to recount arguments and we've come to a conclusion that
> there're in fact 19 args not 18 as the name suggests. 19 args is
> I-N-S-A-N-E.
It will be partially cleaned up by:
http://ozlabs.org/pipermail/linuxppc-dev/2006-July/024556.html
However it doesnt fix the fact someone has architected such a crazy
interface :(
Anton
^ permalink raw reply
* Re: [PATCH 02/13] IB/ehca: includes
From: Christoph Raisch @ 2006-08-18 15:35 UTC (permalink / raw)
To: abergman
Cc: linux-kernel, openib-general, linuxppc-dev, Hoang-Nam Nguyen,
Marcus Eder
In-Reply-To: <200608180144.19149.arnd.bergmann@de.ibm.com>
abergman
> > +#define EDEB_P_GENERIC(level,idstring,format,args...) \
>
> These macros are responsible for 61% of the object code size of your
module.
> ...Please get rid of that crap entirely and replace
> it with dev_info/dev_dbg/dev_warn calls where appropriate!
>
> Arnd <><
we'll change these EDEBs to a wrapper around dev_err, dev_dbg and dev_warn
as it's done in the mthca driver.
All EDEB_EN and EDEB_EX will be removed, that type of tracing can be done
if needed by kprobes.
There are a few cases where we won't get to a dev, for these few places
we'll use a simple wrapper around printk, as done in ipoib.
Hope that's the "official" way how to implement it in ib drivers.
Gruss / Regards . . . Christoph R
^ permalink raw reply
* Re: [PATCH 13/13] IB/ehca: makefiles/kconfig
From: Hoang-Nam Nguyen @ 2006-08-18 15:43 UTC (permalink / raw)
To: abergman
Cc: linux-kernel, openib-general, linuxppc-dev, Christoph Raisch,
Marcus Eder
In-Reply-To: <200608180134.39050.arnd.bergmann@de.ibm.com>
abergman@de.ltcfwd.linux.ibm.com wrote on 18.08.2006 01:34:37:
> On Thursday 17 August 2006 22:11, Roland Dreier wrote:
> > +
> > +CFLAGS += -DEHCA_USE_HCALL -DEHCA_USE_HCALL_KERNEL
>
> This seems really pointless, since you're always defining these
> macros to the same value.
>
> Just drop the CFLAGS and remove the code that depends on them
> being different.
Yes, that's true. Those defines are unnecessary. We'll throw them out.
Thx
Hoang-Nam Nguyen
^ permalink raw reply
* Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
From: Thomas Klein @ 2006-08-18 15:41 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
Christoph Raisch, linux-ppc, Marcus Eder
In-Reply-To: <20060818140506.GC5201@martell.zuzino.mipt.ru>
Hi Alexey,
first of all thanks a lot for the extensive review.
Alexey Dobriyan wrote:
>> + u64 hret = H_HARDWARE;
>
> Useless assignment here and everywhere.
>
Initializing returncodes to errorstate is a cheap way to prevent
accidentally returning (uninitalized) success returncodes which
can lead to catastrophic misbehaviour.
>> +static void netdev_get_drvinfo(struct net_device *dev,
>> + struct ethtool_drvinfo *info)
>> +{
>> + strncpy(info->driver, DRV_NAME, sizeof(info->driver) - 1);
>> + strncpy(info->version, DRV_VERSION, sizeof(info->version) - 1);
>
> Use strlcpy() to not forget -1 accidently.
I agree.
Kind regards
Thomas
^ permalink raw reply
* Re: [2.6.19 PATCH 2/7] ehea: pHYP interface
From: Christoph Raisch @ 2006-08-18 16:02 UTC (permalink / raw)
To: Anton Blanchard
Cc: Thomas Q Klein, Jan-Bernd Themann, netdev, linux-kernel,
linux-ppc, Marcus Eder, tklein, Alexey Dobriyan
In-Reply-To: <20060818151340.GB27947@krispykreme>
>
> Hi,
>
> > I asked SO to recount arguments and we've come to a conclusion that
> > there're in fact 19 args not 18 as the name suggests. 19 args is
> > I-N-S-A-N-E.
>
> It will be partially cleaned up by:
>
> http://ozlabs.org/pipermail/linuxppc-dev/2006-July/024556.html
>
> However it doesnt fix the fact someone has architected such a crazy
> interface :(
>
> Anton
well, just as background info, this is the wrapper around
a single assembly instruction which calls system firmware and takes
9 CPU registers for input and 9 CPU registers for output parameters.
This definition by platform architecture won't change in the near future,
but the good news is with Antons change the wrapper will look much nicer.
Gruss / Regards . . . Christoph R
^ permalink raw reply
* Re: [2.6.19 PATCH 3/7] ehea: queue management
From: Arnd Bergmann @ 2006-08-18 16:16 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Q Klein, Jan-Bernd Themann, netdev, linux-kernel,
Christoph Raisch, Marcus Eder, tklein, abergman, Arjan van de Ven
In-Reply-To: <OFC184057C.C58732A2-ONC12571CE.004E5EF6-C12571CE.004ED1D4@de.ibm.com>
On Friday 18 August 2006 16:24, Christoph Raisch wrote:
> And as always in performance tuning... one size fits all unfortunately is
> not the correct answer.
Ah, good. What is the maximum sensible value that you came up with?
> Therefore we'll leave that open to the user as most other new ethernet
> driver did as well.
Sure. The interesting question is whether you want to allow users
to set it to a value that is no longer sensible to do with
__get_free_pages() and requires vmalloc().
Arnd <><
^ permalink raw reply
* Re: Please pull powerpc.git 'merge' branch
From: Greg KH @ 2006-08-18 16:20 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <17637.8568.40692.21510@cargo.ozlabs.ibm.com>
On Fri, Aug 18, 2006 at 12:10:00PM +1000, Paul Mackerras wrote:
> Greg,
>
> Please do:
>
> git pull \
> git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git merge
>
> to get the following PowerPC updates for 2.6.18.
Pulled from, and pushed out.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 02/13] IB/ehca: includes
From: Arnd Bergmann @ 2006-08-18 16:21 UTC (permalink / raw)
To: linuxppc-dev
Cc: Christoph Raisch, Hoang-Nam Nguyen, linux-kernel, openib-general,
Marcus Eder
In-Reply-To: <OFED0915E4.3CED6795-ONC12571CE.0053ECB8-C12571CE.0055546A@de.ibm.com>
On Friday 18 August 2006 17:35, Christoph Raisch wrote:
> we'll change these EDEBs to a wrapper around dev_err, dev_dbg and dev_warn
> as it's done in the mthca driver.
>
> ...
>
> Hope that's the "official" way how to implement it in ib drivers.
I guess it would be even better to just use the dev_* macros directly
instead of having your own wrapper. You can do that in both ehca and ehea.
Arnd <><
^ permalink raw reply
* Re: InfiniBand merge plans for 2.6.19
From: Michael S. Tsirkin @ 2006-08-18 16:21 UTC (permalink / raw)
To: Roland Dreier; +Cc: netdev, linux-kernel, openib-general, linuxppc-dev
In-Reply-To: <adawt9786ii.fsf@cisco.com>
Quoting r. Roland Dreier <rdreier@cisco.com>:
> o I also have the following minor changes queued in the
> for-2.6.19 branch of infiniband.git:
Cold you oplease consider IB/mthca: recover from device errors
as well?
--
MST
^ 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