LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [2.6.19 PATCH 6/7] ehea: eHEA Makefile
From: Jan-Bernd Themann @ 2006-08-18 11:36 UTC (permalink / raw)
  To: netdev
  Cc: Thomas Klein, Jan-Bernd Themann, linux-kernel, Thomas Klein,
	linux-ppc, Christoph Raisch, Marcus Eder

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-18 00:01:00.755974823 -0700
@@ -0,0 +1,7 @@
+#
+# Makefile for the eHEA ethernet device driver for IBM eServer System p
+#
+
+ehea-y = ehea_main.o ehea_phyp.o ehea_qmr.o ehea_ethtool.o ehea_phyp.o
+obj-$(CONFIG_EHEA) += ehea.o
+

^ permalink raw reply

* [2.6.19 PATCH 7/7] ehea: Makefile & Kconfig
From: Jan-Bernd Themann @ 2006-08-18 11:37 UTC (permalink / raw)
  To: netdev
  Cc: Thomas Klein, Jan-Bernd Themann, linux-kernel, Thomas Klein,
	linux-ppc, Christoph Raisch, Marcus Eder

Signed-off-by: Jan-Bernd Themann <themann@de.ibm.com> 


 drivers/net/Kconfig  |    6 ++++++
 drivers/net/Makefile |    1 +
 2 files changed, 7 insertions(+)



diff -Nurp -X dontdiff linux-2.6.18-rc4/drivers/net/Kconfig patched_kernel/drivers/net/Kconfig
--- linux-2.6.18-rc4/drivers/net/Kconfig	2006-08-06 11:20:11.000000000 -0700
+++ patched_kernel/drivers/net/Kconfig	2006-08-08 03:00:49.526421944 -0700
@@ -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
+
 config IXGB
 	tristate "Intel(R) PRO/10GbE support"
 	depends on PCI
diff -Nurp -X dontdiff linux-2.6.18-rc4/drivers/net/Makefile patched_kernel/drivers/net/Makefile
--- linux-2.6.18-rc4/drivers/net/Makefile	2006-08-06 11:20:11.000000000 -0700
+++ patched_kernel/drivers/net/Makefile	2006-08-08 03:00:30.061451584 -0700
@@ -10,6 +10,7 @@ obj-$(CONFIG_E1000) += e1000/
 obj-$(CONFIG_IBM_EMAC) += ibm_emac/
 obj-$(CONFIG_IXGB) += ixgb/
 obj-$(CONFIG_CHELSIO_T1) += chelsio/
+obj-$(CONFIG_EHEA) += ehea/
 obj-$(CONFIG_BONDING) += bonding/
 obj-$(CONFIG_GIANFAR) += gianfar_driver.o
 

^ permalink raw reply

* Re: [2.6.19 PATCH 3/7] ehea: queue management
From: Arjan van de Ven @ 2006-08-18 12:17 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>

> +	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()???

^ permalink raw reply

* [RFC] MPC8260 SPI driver
From: Laurent Pinchart @ 2006-08-18 12:52 UTC (permalink / raw)
  To: linuxppc-embedded, spi-devel-general

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

Hi everybody,

here's a preliminary version of the MPC8260 SPI driver I'm working on. The 
driver supports master mode only, and has been tested with an MMC device in 
SPI mode.

The driver doesn't support the following features:

- >1024 byte transfers. Those should be easy to support by splitting data 
buffers in <=1024 bytes chunks.
- transfer->delay_usecs
- MPC8260 SPI controller error reporting (BSY and TXE interrupts)
- probably other (hopefully minor) things.

Please test the code and report bugs (patches are welcome :-)).

Laurent Pinchart

[-- Attachment #2: mpc8260-spi.patch --]
[-- Type: text/x-diff, Size: 22353 bytes --]

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 23334c8..bd7040e 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -75,6 +75,13 @@ config SPI_BUTTERFLY
 	  inexpensive battery powered microcontroller evaluation board.
 	  This same cable can be used to flash new firmware.
 
+config SPI_MPC8260
+	tristate "Freescale MPC8260 SPI controller"
+	depends on SPI_MASTER && 8260 && EXPERIMENTAL
+	help
+	  This enables using the Freescale MPC8260 SPI controller in master
+	  mode.
+
 config SPI_MPC83xx
 	tristate "Freescale MPC83xx SPI controller"
 	depends on SPI_MASTER && PPC_83xx && EXPERIMENTAL
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 8f4cb67..00a7345 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -14,6 +14,7 @@ # SPI master controller drivers (bus)
 obj-$(CONFIG_SPI_BITBANG)		+= spi_bitbang.o
 obj-$(CONFIG_SPI_BUTTERFLY)		+= spi_butterfly.o
 obj-$(CONFIG_SPI_PXA2XX)		+= pxa2xx_spi.o
+obj-$(CONFIG_SPI_MPC8260)		+= spi_mpc8260.o
 obj-$(CONFIG_SPI_MPC83xx)		+= spi_mpc83xx.o
 obj-$(CONFIG_SPI_S3C24XX_GPIO)		+= spi_s3c24xx_gpio.o
 obj-$(CONFIG_SPI_S3C24XX)		+= spi_s3c24xx.o
diff --git a/drivers/spi/spi_mpc8260.c b/drivers/spi/spi_mpc8260.c
new file mode 100644
index 0000000..ee6c55b
--- /dev/null
+++ b/drivers/spi/spi_mpc8260.c
@@ -0,0 +1,725 @@
+/*
+ * MPC8260 SPI controller driver.
+ *
+ * Author: Laurent Pinchart <laurent.pinchart@technotrade.biz>
+ *
+ * Based on spi_mpc83xx.c by Kumar Gala
+ *      and pxa2xx_spi.c  by Stephen Street
+ *
+ * Copyright (C) 2006 Technotrade S.A.
+ *
+ * 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 of the  License, or (at your
+ * option) any later version.
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+#include <linux/dma-mapping.h>
+#include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/device.h>
+#include <linux/spi/spi.h>
+#include <linux/platform_device.h>
+#include <linux/fsl_devices.h>
+
+#include <asm/cpm2.h>
+#include <asm/irq.h>
+#include <asm/io.h>
+
+/* The MPC8260 SPI controller is a CPM-driven, DMA-based SPI master.
+ *
+ * SPI transfers are submitted to the CPM through buffer descriptors. Each CPM
+ * transfer sends and receives the same amount of data, which can span one or
+ * more Tx buffers and one or more Rx buffers.
+ *
+ * Linux supports three kind of transfers:
+ *
+ * IN     Incomming data are stored directly in the user-supplied Rx buffer by
+ *        the CPM. Outgoing data are don't-care, the Tx buffer will use a
+ *        dedicated scratch pad.
+ * OUT    Incomming data are don't-care, the Rx buffer will use a dedicated
+ *        scratch pad. Outgoing data are sent directly from the user-supplied
+ *        Tx buffer by the CPM.
+ * IN-OUT Incomming data are stored directly in the user-supplied Rx buffer by
+ *        the CPM. Outgoing data are sent directly from the user-supplied Tx
+ *        buffer by the CPM.
+ *
+ * The Rx buffers maximum size has been fixed to 1024 bytes which should be
+ * enough for most applications (the maximum MMC transfer size is 515 bytes).
+ * Transfers larger than 1024 bytes must be split. This is currently not
+ * implemented.
+ *
+ * Linux groups SPI transfers in SPI messages. An SPI message contains one or
+ * more transfers with a given peripheral. Transfers can all share the same
+ * set of parameters (bits per word, speed) or use different parameters.
+ * Transfers sharing the same set of parameters are said to be compatible.
+ * When processing an SPI message, the driver will queue as much transfers as
+ * possible in CPM buffers until it either finds an uncompatible transfer or
+ * it rans out of buffers.
+ */
+
+#define MPC8260_SPI_RX_BUFS	4
+#define MPC8260_SPI_TX_BUFS	4
+#define MPC8260_SPI_RX_SIZE	1024
+
+#define MPC8260_SPI_RX_DONE	(1 << 0)
+#define MPC8260_SPI_TX_DONE	(1 << 1)
+
+/* SPI Controller driver's private data. */
+struct mpc8260_spi {
+	struct completion done;
+
+	/* Registers, parameter RAM and IRQ. */
+	volatile __u16 *pram_base;
+	volatile spi_t __iomem *pram;
+	volatile spictl_cpm2_t __iomem *regs;
+	uint dpram;
+	u32 irq;
+
+	/* Buffer descriptors. */
+	cbd_t *rx_base;
+	cbd_t *rx_head;
+	cbd_t *rx_tail;
+	cbd_t *tx_base;
+	cbd_t *tx_head;
+	cbd_t *tx_tail;
+	void *scratch;
+
+	/* Messages and transfers. */
+	struct list_head messages;
+	struct spi_message *cur_message;
+	struct spi_transfer *cur_transfer;
+	unsigned int cur_length;
+	unsigned int cur_status;
+	unsigned int chip_select;
+	spinlock_t lock;
+
+	/* Platform data. */
+	u32 sysclk;
+	void (*activate_cs) (u8 cs, u8 polarity);
+	void (*deactivate_cs) (u8 cs, u8 polarity);
+};
+
+/* FCC access macros */
+
+#define __spi_out32(addr, x)    out_be32((unsigned *)addr, x)
+#define __spi_out16(addr, x)    out_be16((unsigned short *)addr, x)
+#define __spi_out8(addr, x)     out_8((unsigned char *)addr, x)
+#define __spi_in32(addr)        in_be32((unsigned *)addr)
+#define __spi_in16(addr)        in_be16((unsigned short *)addr)
+#define __spi_in8(addr)         in_8((unsigned char *)addr)
+
+/* parameter space */
+
+/* write, read, set bits, clear bits */
+#define W32(_p, _m, _v) __spi_out32(&(_p)->_m, (_v))
+#define R32(_p, _m)     __spi_in32(&(_p)->_m)
+#define S32(_p, _m, _v) W32(_p, _m, R32(_p, _m) | (_v))
+#define C32(_p, _m, _v) W32(_p, _m, R32(_p, _m) & ~(_v))
+
+#define W16(_p, _m, _v) __spi_out16(&(_p)->_m, (_v))
+#define R16(_p, _m)     __spi_in16(&(_p)->_m)
+#define S16(_p, _m, _v) W16(_p, _m, R16(_p, _m) | (_v))
+#define C16(_p, _m, _v) W16(_p, _m, R16(_p, _m) & ~(_v))
+
+#define W8(_p, _m, _v)  __spi_out8(&(_p)->_m, (_v))
+#define R8(_p, _m)      __spi_in8(&(_p)->_m)
+#define S8(_p, _m, _v)  W8(_p, _m, R8(_p, _m) | (_v))
+#define C8(_p, _m, _v)  W8(_p, _m, R8(_p, _m) & ~(_v))
+
+/*
+ * mpc8260_spi_pump - Pump SPI transfers into the buffer descriptors
+ *
+ * This function fills the empty SPI buffer descriptors with SPI frames from
+ * message/transfer queues.
+ *
+ * The caller must hold the lock spinlock and the spi->message queue must
+ * not be empty.
+ */
+static void mpc8260_spi_pump(struct mpc8260_spi *spi)
+{
+	volatile cbd_t *tx_bd, *rx_bd;
+	void *rx_buf, *tx_buf;
+	struct spi_message *message = spi->cur_message;
+	struct spi_transfer *transfer = spi->cur_transfer;
+	struct spi_device *dev;
+	int bpp, speed;
+	__u16 mode;
+	__u8 pm;
+
+/*
+	printk(KERN_INFO "mpc8260_spi_pump: 1. message %p transfer %p\n",
+		message, transfer);
+*/
+	/* Move to the next message if we are done with the current one. */
+	if (spi->cur_message == NULL) {
+		message = list_entry(spi->messages.next,
+				struct spi_message, queue);
+		transfer = list_entry(message->transfers.next,
+				struct spi_transfer, transfer_list);
+
+		spi->cur_message = message;
+		spi->cur_transfer = transfer;
+	}
+
+/*
+	printk(KERN_INFO "mpc8260_spi_pump: 2. message %p transfer %p\n",
+		message, transfer);
+*/
+	rx_bd = spi->rx_head;
+	tx_bd = spi->tx_head;
+
+	if (!(rx_bd->cbd_sc & BD_SC_EMPTY) &&
+		!(tx_bd->cbd_sc & BD_SC_READY)) {
+
+		/* Disable the SPI controller */
+		W16(spi->regs, spi_spmode, 0x0000);
+
+		/* Select SPI mode based on device settings. */
+		dev = message->spi;
+		bpp = transfer->bits_per_word ? transfer->bits_per_word
+		    : dev->bits_per_word;
+		speed = transfer->speed_hz ? transfer->speed_hz
+		      : dev->max_speed_hz;
+
+		mode = ((dev->mode & SPI_CPOL) ? SPMODE_CI : 0)
+		     | ((dev->mode & SPI_CPHA) ? SPMODE_CP : 0)
+		     | ((dev->mode & SPI_LSB_FIRST) ? 0 : SPMODE_REV)
+		     | SPMODE_LEN(bpp) | SPMODE_EN | SPMODE_MSTR;
+
+		if (spi->sysclk / speed >= 64) {
+			pm = spi->sysclk / (speed * 64);
+			mode |= SPMODE_PM(pm) | SPMODE_DIV16;
+		} else {
+			pm = spi->sysclk / (speed * 4);
+			mode |= SPMODE_PM(pm);
+		}
+
+/*
+		printk(KERN_INFO "mpc8260_spi_pump: setting mode to 0x%04x (%u Hz)\n",
+			 mode, speed);
+*/
+		W16(spi->regs, spi_spmode, mode);
+
+		if (spi->activate_cs)
+			spi->activate_cs(dev->chip_select,
+				dev->mode & SPI_CS_HIGH);
+
+		if (message->is_dma_mapped) {
+/*
+			printk(KERN_INFO "message is DMA-mapped - rx_dma: %p, tx_dma: %p, %u bytes\n",
+				(void*)transfer->rx_dma, (void*)transfer->tx_dma, transfer->len);
+*/
+			rx_bd->cbd_bufaddr = transfer->rx_dma;
+			tx_bd->cbd_bufaddr = transfer->tx_dma;
+		}
+		else {
+			/* DMA-map buffers, use the scratch pad if
+			 * necessary.
+			 */
+/*
+				printk(KERN_INFO "rx_buf: %p, tx_buf: %p, %u bytes\n",
+					transfer->rx_buf, transfer->tx_buf, transfer->len);
+*/
+			rx_buf = transfer->rx_buf ? transfer->rx_buf
+			       : spi->scratch;
+			tx_buf = transfer->tx_buf ? (void*)transfer->tx_buf
+			       : spi->scratch;
+
+			rx_bd->cbd_bufaddr = dma_map_single(
+				&message->spi->dev, rx_buf, transfer->len,
+				DMA_FROM_DEVICE);
+			tx_bd->cbd_bufaddr = dma_map_single(
+				&message->spi->dev, tx_buf, transfer->len,
+				DMA_TO_DEVICE);
+/*
+			printk(KERN_INFO "rx_bd: %p(%p), tx_bd: %p(%p)\n",
+				rx_bd, (void*)rx_bd->cbd_bufaddr,
+				tx_bd, (void*)tx_bd->cbd_bufaddr);
+*/
+		}
+
+		tx_bd->cbd_datlen = transfer->len;
+
+		/* Mark buffers as ready and start the transfer. */
+		rx_bd->cbd_sc = BD_SC_EMPTY | (rx_bd->cbd_sc & BD_SC_WRAP)
+			      | BD_SC_INTRPT;
+		tx_bd->cbd_sc = BD_SC_READY | (tx_bd->cbd_sc & BD_SC_WRAP)
+			      | BD_SC_INTRPT | BD_SC_LAST;
+		wmb();
+		W8(spi->regs, spi_spcom, SPCOM_STR);
+
+		rx_bd = (rx_bd->cbd_sc & BD_SC_WRAP)
+		      ? spi->rx_base : (rx_bd + 1);
+		tx_bd = (tx_bd->cbd_sc & BD_SC_WRAP)
+		      ? spi->tx_base : (tx_bd + 1);
+	}
+	else
+		printk(KERN_INFO "mpc8260_spi_pump: no empty buffer ?\n");
+
+	spi->rx_head = (cbd_t*)rx_bd;
+	spi->tx_head = (cbd_t*)tx_bd;
+}
+
+/*
+ * mpc8260_spi_done - Process finished tranfers
+ *
+ * This function walks the receive and transmit buffer descriptors and updates
+ * all finished transfers accordingly.
+ *
+ * Context: interrupt. The caller must not hold the lock spinlock.
+ */
+static void mpc8260_spi_done(struct mpc8260_spi *spi)
+{
+	volatile cbd_t *tx_bd, *rx_bd;
+	struct spi_message *message = spi->cur_message;
+	struct spi_transfer *transfer = spi->cur_transfer;
+	struct spi_device *dev = message->spi;
+
+/*
+	printk(KERN_INFO "mpc8260_spi_done: message %p transfer %p\n",
+		message, transfer);
+*/
+
+	rx_bd = spi->rx_tail;
+	tx_bd = spi->tx_tail;
+
+	if (!(rx_bd->cbd_sc & BD_SC_EMPTY) &&
+		!(tx_bd->cbd_sc & BD_SC_READY)) {
+
+		if (!message->is_dma_mapped) {
+			dma_unmap_single(&message->spi->dev, rx_buf,
+				transfer->len, DMA_FROM_DEVICE);
+			dma_unmap_single(&message->spi->dev, tx_buf,
+				transfer->len, DMA_TO_DEVICE);
+		}
+
+		spi->cur_length += transfer->len;
+
+		rx_bd = (rx_bd->cbd_sc & BD_SC_WRAP)
+		      ? spi->rx_base : (rx_bd + 1);
+		tx_bd = (tx_bd->cbd_sc & BD_SC_WRAP)
+		      ? spi->tx_base : (tx_bd + 1);
+
+		if (transfer->transfer_list.next == &message->transfers) {
+			message->status = 0;
+			message->actual_length = spi->cur_length;
+
+			spi->cur_message = NULL;
+			spi->cur_transfer = NULL;
+			spi->cur_length = 0;
+
+			message->complete(message->context);
+//			spi->deactivate_cs(dev->chip_select,
+//				dev->mode & SPI_CS_HIGH);
+		}
+		else {
+			spi->cur_transfer = list_entry(
+				transfer->transfer_list.next,
+				struct spi_transfer, transfer_list);
+
+			if (transfer->cs_change) {
+				printk(KERN_INFO "cs_change set\n");
+				spi->deactivate_cs(dev->chip_select,
+					dev->mode & SPI_CS_HIGH);
+			}
+		}
+
+		if (transfer->delay_usecs)
+			printk(KERN_INFO "transfer requested %uus delay\n",
+				transfer->delay_usecs);
+	}
+	else
+		printk(KERN_INFO "mpc8260_spi_done: no buffers ?\n");
+
+	spi->rx_tail = (cbd_t*)rx_bd;
+	spi->tx_tail = (cbd_t*)tx_bd;
+}
+
+/*
+ * SPI message/transfer queues behaviour
+ * -------------------------------------
+ *
+ * Messages are stored in the mpc8260_spi->messages list, with new messages
+ * added at the tail.
+ *
+ * Even though the SPI controller has the ability to handle several queued
+ * buffer descriptors, hardware limitations prevents the driver from handling
+ * more than a single transfer at a given time. When a new transfer is pumped
+ * into the buffer descriptors, cur_message and cur_transfer are updated to
+ * point to the current message and current transfer respectively.
+ *
+ * Once a transfer is finished (both the TXB and RXB have been received),
+ * the next transfer is pumped to the hardware. If the transfer was the last
+ * one in the current message, the message completion handler is called
+ * synchronously. The message is then removed from the message queue and the
+ * cur_message and cur_transfer pointers are set to NULL before pumping the
+ * next transfer.
+ */
+static irqreturn_t mpc8260_spi_irq(s32 irq, void *context_data,
+		struct pt_regs *ptregs)
+{
+	struct mpc8260_spi *spi = context_data;
+	unsigned long flags;
+	__u8 events;
+
+	events = R8(spi->regs, spi_spie);
+
+	if (events & SPIM_BSY) {
+		printk(KERN_INFO "-- SPI --: BSY\n");
+	}
+
+	if (events & SPIM_TXE) {
+		printk(KERN_INFO "-- SPI --: TXE\n");
+	}
+
+	if (events & SPIM_RXB) {
+		spi->cur_status |= MPC8260_SPI_RX_DONE;
+	}
+
+	if (events & SPIM_TXB) {
+		spi->cur_status |= MPC8260_SPI_TX_DONE;
+	}
+
+	if (spi->cur_status == (MPC8260_SPI_TX_DONE | MPC8260_SPI_RX_DONE)) {
+		mpc8260_spi_done(spi);
+		spi->cur_status = 0;
+
+		spin_lock_irqsave(&spi->lock, flags);
+		if (spi->cur_message == NULL)
+			list_del(spi->messages.next);
+		if (!list_empty(&spi->messages))
+			mpc8260_spi_pump(spi);
+		spin_unlock_irqrestore(&spi->lock, flags);
+	}
+
+	/* Clear the events */
+	W8(spi->regs, spi_spie, events);
+
+	return (events & (SPIM_TXE | SPIM_BSY | SPIM_TXB | SPIM_RXB))
+		? IRQ_HANDLED : IRQ_NONE;
+}
+
+/* --------------------------------------------------------------------------
+ * SPI master kernel API
+ */
+
+/* Called by spi_new_device() when a device is created. Initialize device
+ * fields, deassert chip select, ...
+ */
+static int mpc8260_spi_setup(struct spi_device *spi)
+{
+//	struct spi_mpc8260_cs *cs = spi->controller_state;
+//	struct mpc8260_spi *mpc8260_spi = spi_master_get_devdata(spi->master);
+//	int retval;
+
+//	printk(KERN_INFO "mpc8260_spi: setup()\n");
+
+	if (!spi->max_speed_hz)
+		return -EINVAL;
+
+	if (!spi->bits_per_word)
+		spi->bits_per_word = 8;
+
+	dev_dbg(&spi->dev, "%s, mode %d, %u bits/w\n",
+		__FUNCTION__, spi->mode & (SPI_CPOL | SPI_CPHA),
+		spi->bits_per_word);
+
+	return 0;
+}
+
+static int mpc8260_spi_transfer(struct spi_device *spi, struct spi_message *msg)
+{
+	struct mpc8260_spi *mpc8260_spi = spi_master_get_devdata(spi->master);
+	unsigned long flags;
+	int trigger;
+
+/*
+	printk(KERN_INFO "mpc8260_spi: transfer(msg = %p)\n", msg);
+*/
+	msg->actual_length = 0;
+	msg->status = -EINPROGRESS;
+
+	spin_lock_irqsave(&mpc8260_spi->lock, flags);
+	trigger = list_empty(&mpc8260_spi->messages);
+	list_add_tail(&msg->queue, &mpc8260_spi->messages);
+	if (trigger)
+		mpc8260_spi_pump(mpc8260_spi);
+	spin_unlock_irqrestore(&mpc8260_spi->lock, flags);
+
+	return 0;
+}
+
+static void mpc8260_spi_cleanup(const struct spi_device *spi)
+{
+	printk(KERN_INFO "mpc8260_spi: cleanup()\n");
+}
+
+/* --------------------------------------------------------------------------
+ * CPM specific initialization and cleanup
+ */
+static int mpc8260_spi_alloc_bufs(struct mpc8260_spi *mpc8260_spi)
+{
+	cbd_t *bd;
+	int i, size, ret;
+
+	/* Allocate memory for discarded incoming data. */
+	mpc8260_spi->scratch = kzalloc(MPC8260_SPI_RX_SIZE, GFP_KERNEL);
+	if (mpc8260_spi->scratch == NULL) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	/* Allocate dpram. */
+	size = sizeof(cbd_t) * (MPC8260_SPI_RX_BUFS + MPC8260_SPI_TX_BUFS);
+	mpc8260_spi->dpram = cpm_dpalloc(size, 8);
+	if (IS_DPERR(mpc8260_spi->dpram)) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	/* Initialize the buffer descriptors. */
+	bd = (cbd_t*)cpm_dpram_addr(mpc8260_spi->dpram);
+	mpc8260_spi->rx_base = bd;
+	mpc8260_spi->rx_head = bd;
+	mpc8260_spi->rx_tail = bd;
+	for (i = 0; i < MPC8260_SPI_RX_BUFS; ++i, ++bd) {
+		bd->cbd_bufaddr = 0;
+		bd->cbd_datlen = 0;
+		bd->cbd_sc = 0;
+	}
+	bd[-1].cbd_sc = BD_SC_WRAP;
+
+	mpc8260_spi->tx_base = bd;
+	mpc8260_spi->tx_head = bd;
+	mpc8260_spi->tx_tail = bd;
+	for (i = 0; i < MPC8260_SPI_TX_BUFS; ++i, ++bd) {
+		bd->cbd_bufaddr = 0;
+		bd->cbd_datlen = 0;
+		bd->cbd_sc = 0;
+	}
+	bd[-1].cbd_sc = BD_SC_WRAP;
+
+	return 0;
+
+error:
+	kfree(mpc8260_spi->scratch);
+	if (!IS_DPERR(mpc8260_spi->dpram))
+		cpm_dpfree(mpc8260_spi->dpram);
+	return ret;
+}
+
+static void mpc8260_spi_free_bufs(struct mpc8260_spi *mpc8260_spi)
+{
+	cpm_dpfree(mpc8260_spi->dpram);
+	kfree(mpc8260_spi->scratch);
+}
+
+static int mpc8260_spi_init_cpm(struct mpc8260_spi *mpc8260_spi)
+{
+	cpm2_map_t *immap;
+	int ret = 0;
+
+	immap = ioremap(CPM_MAP_ADDR, sizeof(cpm2_map_t));
+	if (immap == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* Initialize I/O ports. */
+	clrbits32(&immap->im_ioport.iop_ppard, 0x00001000);
+	setbits32(&immap->im_ioport.iop_ppard, 0x0000e000);
+	setbits32(&immap->im_ioport.iop_psord, 0x0000f000);
+	clrbits32(&immap->im_ioport.iop_pdird, 0x0000f000);
+
+	*(u16 *)(&immap->im_dprambase[PROFF_SPI_BASE]) = PROFF_SPI;
+
+	if ((ret = mpc8260_spi_alloc_bufs(mpc8260_spi)) < 0)
+		goto out;
+
+	W16(mpc8260_spi->pram, spi_rbase, mpc8260_spi->dpram);
+	W16(mpc8260_spi->pram, spi_tbase, mpc8260_spi->dpram + sizeof(cbd_t) * MPC8260_SPI_RX_BUFS);
+	W8(mpc8260_spi->pram, spi_rfcr, CPMFCR_GBL | CPMFCR_EB);
+	W8(mpc8260_spi->pram, spi_tfcr, CPMFCR_GBL | CPMFCR_EB);
+	W16(mpc8260_spi->pram, spi_mrblr, MPC8260_SPI_RX_SIZE);
+
+	W32(&immap->im_cpm, cp_cpcr, mk_cr_cmd(CPM_CR_SPI_PAGE,
+			CPM_CR_SPI_SBLOCK, 0, CPM_CR_INIT_TRX | CPM_CR_FLG));
+        while (R32(&immap->im_cpm, cp_cpcr) & CPM_CR_FLG) {};
+
+	W16(mpc8260_spi->regs, spi_spmode, 0x0000);
+	W8(mpc8260_spi->regs, spi_spie, 0xff);
+	W8(mpc8260_spi->regs, spi_spim, SPIM_TXE | SPIM_BSY | SPIM_TXB | SPIM_RXB);
+
+out:
+	if (immap)
+		iounmap(immap);
+
+	return ret;
+}
+
+/* --------------------------------------------------------------------------
+ * Initialization
+ */
+
+static int __init mpc8260_spi_probe(struct platform_device *pdev)
+{
+	struct spi_master *master;
+	struct mpc8260_spi *mpc8260_spi;
+	struct fsl_spi_platform_data *pdata;
+	struct resource *regs, *pram;
+	int ret = 0;
+
+	pdata = pdev->dev.platform_data;
+	if (pdata == NULL) {
+		ret = -ENODEV;
+		goto error;
+	}
+
+	master = spi_alloc_master(&pdev->dev, sizeof *mpc8260_spi);
+	if (master == NULL) {
+		dev_err(&pdev->dev, "cannot allocate spi master\n");
+		ret = -ENOMEM;
+		goto error;
+	}
+	platform_set_drvdata(pdev, master);
+
+	master->bus_num = pdata->bus_num;
+	master->num_chipselect = pdata->max_chipselect;
+	master->setup = mpc8260_spi_setup;
+	master->transfer = mpc8260_spi_transfer;
+	master->cleanup = mpc8260_spi_cleanup;
+
+	mpc8260_spi = spi_master_get_devdata(master);
+	INIT_LIST_HEAD(&mpc8260_spi->messages);
+	spin_lock_init(&mpc8260_spi->lock);
+	mpc8260_spi->cur_message = NULL;
+	mpc8260_spi->cur_transfer = NULL;
+	mpc8260_spi->cur_length = 0;
+	mpc8260_spi->cur_status = 0;
+	mpc8260_spi->sysclk = pdata->sysclk;
+	mpc8260_spi->activate_cs = pdata->activate_cs;
+	mpc8260_spi->deactivate_cs = pdata->deactivate_cs;
+
+	/* Get resources(memory, IRQ) associated with the device. */
+	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
+	if (regs == NULL) {
+		dev_err(&pdev->dev, "regs resource not defined\n");
+		ret = -ENODEV;
+		goto free_master;
+	}
+	pram = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pram");
+	if (pram == NULL) {
+		dev_err(&pdev->dev, "pram resource not defined\n");
+		ret = -ENODEV;
+		goto free_master;
+	}
+
+	mpc8260_spi->regs = ioremap(regs->start, regs->end - regs->start + 1);
+	if (mpc8260_spi->regs == NULL) {
+		ret = -ENOMEM;
+		goto put_master;
+	}
+	mpc8260_spi->pram = ioremap(pram->start, pram->end - pram->start + 1);
+	if (mpc8260_spi->pram == NULL) {
+		ret = -ENOMEM;
+		goto unmap_regs;
+	}
+
+	mpc8260_spi->irq = platform_get_irq(pdev, 0);
+	if (mpc8260_spi->irq < 0) {
+		dev_err(&pdev->dev, "irq resource not defined\n");
+		ret = -ENXIO;
+		goto unmap_pram;
+	}
+
+	/* Register SPI interrupt */
+	ret = request_irq(mpc8260_spi->irq, mpc8260_spi_irq,
+			  0, "mpc8260_spi", mpc8260_spi);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "cannot get irq\n");
+		goto unmap_pram;
+	}
+
+	/* SPI controller initializations */
+	ret = mpc8260_spi_init_cpm(mpc8260_spi);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "problem initializing the CPM\n");
+		goto free_irq;
+	}
+
+	ret = spi_register_master(master);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "problem registering spi master\n");
+		goto uninit_cpm;
+	}
+
+	printk(KERN_INFO
+	       "%s: MPC8260 SPI Controller driver at 0x%p (irq = %d)\n",
+	       pdev->dev.bus_id, mpc8260_spi->regs, mpc8260_spi->irq);
+
+	return 0;
+
+uninit_cpm:
+	mpc8260_spi_free_bufs(mpc8260_spi);
+free_irq:
+	free_irq(mpc8260_spi->irq, mpc8260_spi);
+unmap_pram:
+	iounmap(mpc8260_spi->pram);
+unmap_regs:
+	iounmap(mpc8260_spi->regs);
+put_master:
+	spi_master_put(master);
+free_master:
+	kfree(master);
+error:
+	return ret;
+}
+
+static int __devexit mpc8260_spi_remove(struct platform_device *dev)
+{
+	struct mpc8260_spi *mpc8260_spi;
+	struct spi_master *master;
+
+	master = platform_get_drvdata(dev);
+	mpc8260_spi = spi_master_get_devdata(master);
+
+	mpc8260_spi_free_bufs(mpc8260_spi);
+
+	free_irq(mpc8260_spi->irq, mpc8260_spi);
+	iounmap(mpc8260_spi->regs);
+	iounmap(mpc8260_spi->pram);
+	spi_master_put(master);
+
+	return 0;
+}
+
+static struct platform_driver mpc8260_spi_driver = {
+	.probe = mpc8260_spi_probe,
+	.remove = __devexit_p(mpc8260_spi_remove),
+	.driver = {
+		   .name = "fsl-cpm-spi",
+	},
+};
+
+static int __init mpc8260_spi_init(void)
+{
+	return platform_driver_register(&mpc8260_spi_driver);
+}
+
+static void __exit mpc8260_spi_exit(void)
+{
+	platform_driver_unregister(&mpc8260_spi_driver);
+}
+
+module_init(mpc8260_spi_init);
+module_exit(mpc8260_spi_exit);
+
+MODULE_AUTHOR("Laurent Pinchart");
+MODULE_DESCRIPTION("MPC8260 SPI Driver");
+MODULE_LICENSE("GPL");

^ permalink raw reply related

* 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


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