* Re: [PATCH] powerpc: warning: allocated section `.data_nosave' not in segment
From: Segher Boessenkool @ 2009-09-29 20:40 UTC (permalink / raw)
To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20090929120006.78affe3f@lappy.seanm.ca>
> We need to align before the output section. Having the align inside
> the output section causes the linker to put some filler in there,
> which makes it a non-empty section, but this section isn't assigned to
> a segment so you get a warning from the linker.
>
> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
> ---
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S
> b/arch/powerpc/kernel/vmlinux.lds.S index f564293..e853763 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -236,6 +236,7 @@ SECTIONS
> READ_MOSTLY_DATA(L1_CACHE_BYTES)
> }
>
> + . = ALIGN(PAGE_SIZE);
> .data_nosave : AT(ADDR(.data_nosave) - LOAD_OFFSET) {
> NOSAVE_DATA
> }
Acked-by: Segher Boessenkool <segher@kernel.crashing.org>
The patch is good and fixes a real problem. But, I still don't see how
it caused the "warning: allocated section `.data_nosave' not in
segment",
it would be good if we can find out.
Segher
^ permalink raw reply
* [PATCH] powerpc/5200: add LocalPlus bus FIFO device driver
From: John Bonesio @ 2009-09-29 20:43 UTC (permalink / raw)
To: linuxppc-dev
This is a driver for the FIFO device on the LocalPlus bus on an mpc5200 system.
The driver supports programmed I/O through the FIFO as well as setting up DMA
via the BestComm engine through the FIFO.
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: John Bonesio <bones@secretlab.ca>
---
This driver was originally written by Grant Likely. I have updated it so that
transmitting data (as opposed to receiving data) works better. The driver has
been tested for all 6 transfer modes:
PIO mode (rx and tx)
DMA polled mode (rx and tx)
DMA interrupt mode (rx and tx)
- John
arch/powerpc/include/asm/mpc52xx.h | 39 ++
arch/powerpc/platforms/52xx/Kconfig | 5
arch/powerpc/platforms/52xx/Makefile | 1
arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | 560 +++++++++++++++++++++++++
4 files changed, 605 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/mpc52xx.h
index 8273357..819a0be 100644
--- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -282,6 +282,45 @@ extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, int period,
int continuous);
extern void mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
+/* mpc52xx_lpbfifo.c */
+#define MPC52XX_LPBFIFO_FLAG_READ (0)
+#define MPC52XX_LPBFIFO_FLAG_WRITE (1<<0)
+#define MPC52XX_LPBFIFO_FLAG_NO_INCREMENT (1<<1)
+#define MPC52XX_LPBFIFO_FLAG_NO_DMA (1<<2)
+#define MPC52XX_LPBFIFO_FLAG_POLL_DMA (1<<3)
+
+struct mpc52xx_lpbfifo_request {
+ struct list_head list;
+
+ /* localplus bus address */
+ unsigned int cs;
+ size_t offset;
+
+ /* Memory address */
+ void *data;
+ phys_addr_t data_phys;
+
+ /* Details of transfer */
+ size_t size;
+ size_t pos; /* current position of transfer */
+ int flags;
+
+ /* What to do when finished */
+ void (*callback)(struct mpc52xx_lpbfifo_request *);
+
+ void *priv; /* Driver private data */
+
+ /* statistics */
+ int irq_count;
+ int irq_ticks;
+ u8 last_byte;
+ int buffer_not_done_cnt;
+};
+
+extern int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_request *req);
+extern void mpc52xx_lpbfifo_abort(struct mpc52xx_lpbfifo_request *req);
+extern void mpc52xx_lpbfifo_poll(void);
+
/* mpc52xx_pic.c */
extern void mpc52xx_init_irq(void);
extern unsigned int mpc52xx_get_irq(void);
diff --git a/arch/powerpc/platforms/52xx/Kconfig b/arch/powerpc/platforms/52xx/Kconfig
index 696a5ee..ea00b3d 100644
--- a/arch/powerpc/platforms/52xx/Kconfig
+++ b/arch/powerpc/platforms/52xx/Kconfig
@@ -51,3 +51,8 @@ config PPC_MPC5200_GPIO
select GENERIC_GPIO
help
Enable gpiolib support for mpc5200 based boards
+
+config PPC_MPC5200_LPBFIFO
+ tristate "MPC5200 LocalPlus bus FIFO driver"
+ depends on PPC_MPC52xx
+ select PPC_BESTCOMM_GEN_BD
diff --git a/arch/powerpc/platforms/52xx/Makefile b/arch/powerpc/platforms/52xx/Makefile
index d6ade3d..8df23af 100644
--- a/arch/powerpc/platforms/52xx/Makefile
+++ b/arch/powerpc/platforms/52xx/Makefile
@@ -14,3 +14,4 @@ ifeq ($(CONFIG_PPC_LITE5200),y)
endif
obj-$(CONFIG_PPC_MPC5200_GPIO) += mpc52xx_gpio.o
+obj-$(CONFIG_PPC_MPC5200_LPBFIFO) += mpc52xx_lpbfifo.o
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
new file mode 100644
index 0000000..929d017
--- /dev/null
+++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
@@ -0,0 +1,560 @@
+/*
+ * LocalPlus Bus FIFO driver for the Freescale MPC52xx.
+ *
+ * Copyright (C) 2009 Secret Lab Technologies Ltd.
+ *
+ * This file is released under the GPLv2
+ *
+ * Todo:
+ * - Add support for multiple requests to be queued.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/spinlock.h>
+#include <asm/io.h>
+#include <asm/prom.h>
+#include <asm/mpc52xx.h>
+#include <asm/time.h>
+
+#include <sysdev/bestcomm/bestcomm.h>
+#include <sysdev/bestcomm/bestcomm_priv.h>
+#include <sysdev/bestcomm/gen_bd.h>
+
+MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
+MODULE_DESCRIPTION("MPC5200 LocalPlus FIFO device driver");
+MODULE_LICENSE("GPL");
+
+#define LPBFIFO_REG_PACKET_SIZE (0x00)
+#define LPBFIFO_REG_START_ADDRESS (0x04)
+#define LPBFIFO_REG_CONTROL (0x08)
+#define LPBFIFO_REG_ENABLE (0x0C)
+#define LPBFIFO_REG_BYTES_DONE_STATUS (0x14)
+#define LPBFIFO_REG_FIFO_DATA (0x40)
+#define LPBFIFO_REG_FIFO_STATUS (0x44)
+#define LPBFIFO_REG_FIFO_CONTROL (0x48)
+#define LPBFIFO_REG_FIFO_ALARM (0x4C)
+
+struct mpc52xx_lpbfifo {
+ struct device *dev;
+ phys_addr_t regs_phys;
+ void __iomem *regs;
+ int irq;
+ spinlock_t lock;
+
+ struct bcom_task *bcom_tx_task;
+ struct bcom_task *bcom_rx_task;
+ struct bcom_task *bcom_cur_task;
+
+ /* Current state data */
+ struct mpc52xx_lpbfifo_request *req;
+ int dma_irqs_enabled;
+};
+
+/* The MPC5200 has only one fifo, so only need one instance structure */
+static struct mpc52xx_lpbfifo lpbfifo;
+
+/**
+ * mpc52xx_lpbfifo_kick - Trigger the next block of data to be transfered
+ */
+static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo_request *req)
+{
+ size_t transfer_size = req->size - req->pos;
+ struct bcom_bd *bd;
+ void __iomem *reg;
+ u32 *data;
+ int i;
+ int bit_fields;
+ int dma = !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
+ int write = req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
+ int poll_dma = req->flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA;
+
+ /* Set and clear the reset bits; is good practice in User Manual */
+ out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+
+ /* set master enable bit */
+ out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x00000001);
+ if (!dma) {
+ /* While the FIFO can be setup for transfer sizes as large as
+ * 16M-1, the FIFO itself is only 512 bytes deep and it does
+ * not generate interrupts for FIFO full events (only transfer
+ * complete will raise an IRQ). Therefore when not using
+ * Bestcomm to drive the FIFO it needs to either be polled, or
+ * transfers need to constrained to the size of the fifo.
+ *
+ * This driver restricts the size of the transfer
+ */
+ if (transfer_size > 512)
+ transfer_size = 512;
+
+ /* Load the FIFO with data */
+ if (write) {
+ reg = lpbfifo.regs + LPBFIFO_REG_FIFO_DATA;
+ data = req->data + req->pos;
+ for (i = 0; i < transfer_size; i += 4)
+ out_be32(reg, *data++);
+ }
+
+ /* Unmask both error and completion irqs */
+ out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x00000301);
+ } else {
+ /* Choose the correct direction
+ *
+ * Configure the watermarks so DMA will always complete correctly.
+ * It may be worth experimenting with the ALARM value to see if
+ * there is a performance impacit. However, if it is wrong there
+ * is a risk of DMA not transferring the last chunk of data
+ */
+ if (write) {
+ out_be32(lpbfifo.regs + LPBFIFO_REG_FIFO_ALARM, 0x1e4);
+ out_8(lpbfifo.regs + LPBFIFO_REG_FIFO_CONTROL, 7);
+ lpbfifo.bcom_cur_task = lpbfifo.bcom_tx_task;
+ } else {
+ out_be32(lpbfifo.regs + LPBFIFO_REG_FIFO_ALARM, 0x1ff);
+ out_8(lpbfifo.regs + LPBFIFO_REG_FIFO_CONTROL, 0);
+ lpbfifo.bcom_cur_task = lpbfifo.bcom_rx_task;
+
+ if (poll_dma) {
+ if (lpbfifo.dma_irqs_enabled) {
+ disable_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task));
+ lpbfifo.dma_irqs_enabled = 0;
+ }
+ } else {
+ if (!lpbfifo.dma_irqs_enabled) {
+ enable_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task));
+ lpbfifo.dma_irqs_enabled = 1;
+ }
+ }
+ }
+
+ bd = bcom_prepare_next_buffer(lpbfifo.bcom_cur_task);
+ bd->status = transfer_size;
+ if (!write) {
+ /*
+ * In the DMA read case, the DMA doesn't complete,
+ * possibly due to incorrect watermarks in the ALARM
+ * and CONTROL regs. For now instead of trying to
+ * determine the right watermarks that will make this
+ * work, just increase the number of bytes the FIFO is
+ * expecting.
+ *
+ * When submitting another operation, the FIFO will get
+ * reset, so the condition of the FIFO waiting for a
+ * non-existent 4 bytes will get cleared.
+ */
+ transfer_size += 4; /* BLECH! */
+ }
+ bd->data[0] = req->data_phys + req->pos;
+ bcom_submit_next_buffer(lpbfifo.bcom_cur_task, NULL);
+
+ /* error irq & master enabled bit */
+ bit_fields = 0x00000201;
+
+ /* Unmask irqs */
+ if (write && (!poll_dma))
+ bit_fields |= 0x00000100; /* completion irq too */
+ out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, bit_fields);
+ }
+
+ /* Set transfer size, width, chip select and READ mode */
+ out_be32(lpbfifo.regs + LPBFIFO_REG_START_ADDRESS,
+ req->offset + req->pos);
+ out_be32(lpbfifo.regs + LPBFIFO_REG_PACKET_SIZE, transfer_size);
+
+ bit_fields = req->cs << 24 | 0x000008;
+ if (!write)
+ bit_fields |= 0x010000; /* read mode */
+ out_be32(lpbfifo.regs + LPBFIFO_REG_CONTROL, bit_fields);
+
+ /* Kick it off */
+ out_8(lpbfifo.regs + LPBFIFO_REG_PACKET_SIZE, 0x01);
+ if (dma)
+ bcom_enable(lpbfifo.bcom_cur_task);
+}
+
+/**
+ * mpc52xx_lpbfifo_irq - IRQ handler for LPB FIFO
+ *
+ * On transmit, the dma completion irq triggers before the fifo completion
+ * triggers. Handle the dma completion here instead of the LPB FIFO Bestcomm
+ * task completion irq becuase everyting is not really done until the LPB FIFO
+ * completion irq triggers.
+ *
+ * In other words:
+ * For DMA, on receive, the "Fat Lady" is the bestcom completion irq. on
+ * transmit, the fifo completion irq is the "Fat Lady". The opera (or in this
+ * case the DMA/FIFO operation) is not finished until the "Fat Lady" sings.
+ *
+ * Reasons for entering this routine:
+ * 1) PIO mode rx and tx completion irq
+ * 2) DMA interrupt mode tx completion irq
+ * 3) DMA polled mode tx
+ *
+ * Exit conditions:
+ * 1) Transfer aborted
+ * 2) FIFO complete without DMA; more data to do
+ * 3) FIFO complete without DMA; all data transfered
+ * 4) FIFO complete using DMA
+ *
+ * Condition 1 can occur regardless of whether or not DMA is used.
+ * It requires executing the callback to report the error and exiting
+ * immediately.
+ *
+ * Condition 2 requires programming the FIFO with the next block of data
+ *
+ * Condition 3 requires executing the callback to report completion
+ *
+ * Condition 4 means the same as 3, except that we also retrieve the bcom
+ * buffer so DMA doesn't get clogged up.
+ *
+ * To make things trickier, the spinlock must be dropped before
+ * executing the callback, otherwise we could end up with a deadlock
+ * or nested spinlock condition. The out path is non-trivial, so
+ * extra fiddling is done to make sure all paths lead to the same
+ * outbound code.
+ */
+static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void *dev_id)
+{
+ struct mpc52xx_lpbfifo_request *req;
+ u32 status = in_8(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STATUS);
+ void __iomem *reg;
+ u32 *data;
+ int count, i;
+ int do_callback = 0;
+ u32 ts;
+ unsigned long flags;
+ int dma, write, poll_dma;
+
+ spin_lock_irqsave(&lpbfifo.lock, flags);
+ ts = get_tbl();
+
+ req = lpbfifo.req;
+ if (!req) {
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ pr_err("bogus LPBFIFO IRQ\n");
+ return IRQ_HANDLED;
+ }
+
+ dma = !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
+ write = req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
+ poll_dma = req->flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA;
+
+ if (dma && !write) {
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ pr_err("bogus LPBFIFO IRQ (dma and not writting)\n");
+ return IRQ_HANDLED;
+ }
+
+ if ((status & 0x01) == 0) {
+ goto out;
+ }
+
+ /* check abort bit */
+ if (status & 0x10) {
+ out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+ do_callback = 1;
+ goto out;
+ }
+
+ /* Read result from hardware */
+ count = in_be32(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STATUS);
+ count &= 0x00ffffff;
+
+ if (!dma && !write) {
+ /* copy the data out of the FIFO */
+ reg = lpbfifo.regs + LPBFIFO_REG_FIFO_DATA;
+ data = req->data + req->pos;
+ for (i = 0; i < count; i += 4)
+ *data++ = in_be32(reg);
+ }
+
+ /* Update transfer position and count */
+ req->pos += count;
+
+ /* Decide what to do next */
+ if (req->size - req->pos)
+ mpc52xx_lpbfifo_kick(req); /* more work to do */
+ else
+ do_callback = 1;
+
+ out:
+ /* Clear the IRQ */
+ out_8(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STATUS, 0x01);
+
+ if (dma && (status & 0x11)) {
+ /*
+ * Count the DMA as complete only when the FIFO completion
+ * status or abort bits are set.
+ *
+ * (status & 0x01) should always be the case except sometimes
+ * when using polled DMA.
+ *
+ * (status & 0x10) {transfer aborted}: This case needs more
+ * testing.
+ */
+ bcom_retrieve_buffer(lpbfifo.bcom_cur_task, &status, NULL);
+ }
+ req->last_byte = ((u8 *)req->data)[req->size - 1];
+
+ /* When the do_callback flag is set; it means the transfer is finished
+ * so set the FIFO as idle */
+ if (do_callback)
+ lpbfifo.req = NULL;
+
+ if (irq != 0) /* don't increment on polled case */
+ req->irq_count++;
+
+ req->irq_ticks += get_tbl() - ts;
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+
+ /* Spinlock is released; it is now safe to call the callback */
+ if (do_callback && req->callback)
+ req->callback(req);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * mpc52xx_lpbfifo_bcom_irq - IRQ handler for LPB FIFO Bestcomm task
+ *
+ * Only used when receiving data.
+ */
+static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, void *dev_id)
+{
+ struct mpc52xx_lpbfifo_request *req;
+ unsigned long flags;
+ u32 status;
+ u32 ts;
+
+ spin_lock_irqsave(&lpbfifo.lock, flags);
+ ts = get_tbl();
+
+ req = lpbfifo.req;
+ if (!req || (req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA)) {
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ return IRQ_HANDLED;
+ }
+
+ if (irq != 0) /* don't increment on polled case */
+ req->irq_count++;
+
+ if (!bcom_buffer_done(lpbfifo.bcom_cur_task)) {
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+
+ req->buffer_not_done_cnt++;
+ if ((req->buffer_not_done_cnt % 1000) == 0)
+ pr_err("transfer stalled\n");
+
+ return IRQ_HANDLED;
+ }
+
+ bcom_retrieve_buffer(lpbfifo.bcom_cur_task, &status, NULL);
+
+ req->last_byte = ((u8 *)req->data)[req->size - 1];
+
+ req->pos = status & 0x00ffffff;
+
+ /* Mark the FIFO as idle */
+ lpbfifo.req = NULL;
+
+ /* Release the lock before calling out to the callback. */
+ req->irq_ticks += get_tbl() - ts;
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+
+ if (req->callback)
+ req->callback(req);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * mpc52xx_lpbfifo_bcom_poll - Poll for DMA completion
+ */
+void mpc52xx_lpbfifo_poll(void)
+{
+ struct mpc52xx_lpbfifo_request *req = lpbfifo.req;
+ int dma = !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
+ int write = req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
+
+ /*
+ * For more information, see comments on the "Fat Lady"
+ */
+ if (dma && write)
+ mpc52xx_lpbfifo_irq(0, NULL);
+ else
+ mpc52xx_lpbfifo_bcom_irq(0, NULL);
+}
+EXPORT_SYMBOL(mpc52xx_lpbfifo_poll);
+
+/**
+ * mpc52xx_lpbfifo_submit - Submit an LPB FIFO transfer request.
+ * @req: Pointer to request structure
+ */
+int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_request *req)
+{
+ unsigned long flags;
+
+ if (!lpbfifo.regs)
+ return -ENODEV;
+
+ spin_lock_irqsave(&lpbfifo.lock, flags);
+
+ /* If the req pointer is already set, then a transfer is in progress */
+ if (lpbfifo.req) {
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ return -EBUSY;
+ }
+
+ /* Setup the transfer */
+ lpbfifo.req = req;
+ req->irq_count = 0;
+ req->irq_ticks = 0;
+ req->buffer_not_done_cnt = 0;
+ req->pos = 0;
+
+ mpc52xx_lpbfifo_kick(req);
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+ return 0;
+}
+EXPORT_SYMBOL(mpc52xx_lpbfifo_submit);
+
+void mpc52xx_lpbfifo_abort(struct mpc52xx_lpbfifo_request *req)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&lpbfifo.lock, flags);
+ if (lpbfifo.req == req) {
+ /* Put it into reset and clear the state */
+ bcom_gen_bd_rx_reset(lpbfifo.bcom_rx_task);
+ bcom_gen_bd_tx_reset(lpbfifo.bcom_tx_task);
+ out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+ lpbfifo.req = NULL;
+ }
+ spin_unlock_irqrestore(&lpbfifo.lock, flags);
+}
+EXPORT_SYMBOL(mpc52xx_lpbfifo_abort);
+
+static int __devinit
+mpc52xx_lpbfifo_probe(struct of_device *op, const struct of_device_id *match)
+{
+ struct resource res;
+ int rc = -ENOMEM;
+
+ if (lpbfifo.dev != NULL)
+ return -ENOSPC;
+
+ lpbfifo.irq = irq_of_parse_and_map(op->node, 0);
+ if (!lpbfifo.irq)
+ return -ENODEV;
+
+ if (of_address_to_resource(op->node, 0, &res))
+ return -ENODEV;
+ lpbfifo.regs_phys = res.start;
+ lpbfifo.regs = of_iomap(op->node, 0);
+ if (!lpbfifo.regs)
+ return -ENOMEM;
+
+ spin_lock_init(&lpbfifo.lock);
+
+ /* Put FIFO into reset */
+ out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+
+ /* Register the interrupt handler */
+ rc = request_irq(lpbfifo.irq, mpc52xx_lpbfifo_irq, 0,
+ "mpc52xx-lpbfifo", &lpbfifo);
+ if (rc)
+ goto err_irq;
+
+ /* Request the Bestcomm receive (fifo --> memory) task and IRQ */
+ lpbfifo.bcom_rx_task =
+ bcom_gen_bd_rx_init(2, res.start + LPBFIFO_REG_FIFO_DATA,
+ BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
+ 16*1024*1024);
+ if (!lpbfifo.bcom_rx_task)
+ goto err_bcom_rx;
+
+ rc = request_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task),
+ mpc52xx_lpbfifo_bcom_irq, 0,
+ "mpc52xx-lpbfifo-rx", &lpbfifo);
+ if (rc)
+ goto err_bcom_rx_irq;
+
+ /* Request the Bestcomm transmit (memory --> fifo) task and IRQ */
+ lpbfifo.bcom_tx_task =
+ bcom_gen_bd_tx_init(2, res.start + LPBFIFO_REG_FIFO_DATA,
+ BCOM_INITIATOR_SCLPC, BCOM_IPR_SCLPC);
+ if (!lpbfifo.bcom_tx_task)
+ goto err_bcom_tx;
+
+ lpbfifo.dev = &op->dev;
+ return 0;
+
+ err_bcom_tx:
+ free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo);
+ err_bcom_rx_irq:
+ bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
+ err_bcom_rx:
+ err_irq:
+ iounmap(lpbfifo.regs);
+ lpbfifo.regs = NULL;
+
+ dev_err(&op->dev, "mpc52xx_lpbfifo_probe() failed\n");
+ return -ENODEV;
+}
+
+
+static int __devexit mpc52xx_lpbfifo_remove(struct of_device *op)
+{
+ if (lpbfifo.dev != &op->dev)
+ return 0;
+
+ /* Put FIFO in reset */
+ out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
+
+ /* Release the bestcomm transmit task */
+ free_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task), &lpbfifo);
+ bcom_gen_bd_tx_release(lpbfifo.bcom_tx_task);
+
+ /* Release the bestcomm receive task */
+ free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo);
+ bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
+
+ free_irq(lpbfifo.irq, &lpbfifo);
+ iounmap(lpbfifo.regs);
+ lpbfifo.regs = NULL;
+ lpbfifo.dev = NULL;
+
+ return 0;
+}
+
+static struct of_device_id mpc52xx_lpbfifo_match[] __devinitconst = {
+ { .compatible = "fsl,mpc5200-lpbfifo", },
+ {},
+};
+
+static struct of_platform_driver mpc52xx_lpbfifo_driver = {
+ .owner = THIS_MODULE,
+ .name = "mpc52xx-lpbfifo",
+ .match_table = mpc52xx_lpbfifo_match,
+ .probe = mpc52xx_lpbfifo_probe,
+ .remove = __devexit_p(mpc52xx_lpbfifo_remove),
+};
+
+/***********************************************************************
+ * Module init/exit
+ */
+static int __init mpc52xx_lpbfifo_init(void)
+{
+ pr_debug("Registering LocalPlus bus FIFO driver\n");
+ return of_register_platform_driver(&mpc52xx_lpbfifo_driver);
+}
+module_init(mpc52xx_lpbfifo_init);
+
+static void __exit mpc52xx_lpbfifo_exit(void)
+{
+ pr_debug("Unregistering LocalPlus bus FIFO driver\n");
+ of_unregister_platform_driver(&mpc52xx_lpbfifo_driver);
+}
+module_exit(mpc52xx_lpbfifo_exit);
^ permalink raw reply related
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Rex Feany @ 2009-09-29 21:03 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <OF94793A12.243E8E02-ONC1257640.0040DE05-C1257640.00419DBD@transmode.se>
Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 29/09/2009 10:16:38:
> >
> >
> > > hmm, yes. You do get this and mysterious SEGV if you hit the but so does
> > > other bugs too so this is probably due to missing invalidation.
> > >
> > > I suspect that something like below will fix the problem and
> > > is the "correct" fix(untested, not even compiled):
> >
> > Ok but do we also still have to worry about the "unpopulated" TLB
> > entries and invalidate them somehow when populating ?
>
> Since I am probably the only one that knows about DAR problem I figured
> I should take a stab at it. This is not tested, but I hope Rex and the list
> can do that. Once this works as it should, we can remove all special handling
> for 8xx in copy_tofrom_user() and friends.
> No sign-off yet, want some confirmation first.
It doesn't make a difference. I applied it to the top of the tree,
it got to userspace but it is stuck. Using break I am able to dump the
registers, I'm not sure if this is useful. Oh, well, I finally got to a shell
but it is unusably slow. Is there some test code that would be better to run?
I tried looking through the mailing list archives but I couldn't find anything.
thanks!
/rex.
SysRq : Show Regs
NIP: c00588d0 LR: c000e3c4 CTR: 00001fde
REGS: c3459d90 TRAP: 0501 Not tainted (2.6.32-rc2-00013-g2d222d9-dirty)
MSR: 00009032 <EE,ME,IR,DR> CR: 44008422 XER: 20006a02
TASK = c3438050[13] 'rc.sysinit' THREAD: c3458000
GPR00: c000e3c4 c3459e40 c3438050 c21a2a60 c345bdf4 0fd81032 00000000 3001e920
GPR08: 00000000 00000005 c21a2a60 c0210000 03438260
NIP [c00588d0] handle_mm_fault+0x10/0xacc
LR [c000e3c4] do_page_fault+0x2f0/0x474
Call Trace:
[c3459e40] [c0059328] handle_mm_fault+0xa68/0xacc (unreliable)
[c3459e90] [c000e418] do_page_fault+0x344/0x474
[c3459f40] [c000d520] handle_page_fault+0xc/0x80
Instruction dump:
4bff15f1 39600000 80010014 7d635b78 7c0803a6 bbc10008 38210010 4e800020
7c0802a6 9421ffb0 3d60c021 be810020 <90010054> 38000000 90020000 396b4bec
SysRq : Show Regs
NIP: c000e0e0 LR: c000d520 CTR: 00001fde
REGS: c21adde0 TRAP: 0501 Not tainted (2.6.32-rc2-00013-g2d222d9-dirty)
MSR: 00009032 <EE,ME,IR,DR> CR: 48008424 XER: 00006a02
TASK = c3438460[18] 'sh' THREAD: c21ac000
GPR00: c000d520 c21ade90 c3438460 c21adf50 0fd76cb8 c0000000 00000004 0fee8c44
GPR08: 00009f6c c000d788 00009032 c000d514 03438670
NIP [c000e0e0] do_page_fault+0xc/0x474
LR [c000d520] handle_page_fault+0xc/0x80
Call Trace:
[c21ade90] [c000e418] do_page_fault+0x344/0x474 (unreliable)
[c21adf40] [c000d520] handle_page_fault+0xc/0x80
Instruction dump:
3863f604 7fe4fb78 7fc5f378 4bffcd39 80010014 bbc10008 7c0803a6 38210010
4e800020 7c0802a6 9421ff50 bf010090 <900100b4> 7c7e1b78 800300a0 7c9d2378
SysRq : Show Regs
NIP: c005930c LR: c000e3c4 CTR: 00001fde
REGS: c21add90 TRAP: 0501 Not tainted (2.6.32-rc2-00013-g2d222d9-dirty)
MSR: 00009032 <EE,ME,IR,DR> CR: 24002422 XER: 00006a02
TASK = c3438460[18] 'sh' THREAD: c21ac000
GPR00: c0236000 c21ade40 c3438460 c21a2a60 c3444df4 c3456000 00000000 feff0000
GPR08: c21ac000 03456000 c0220000 00000001 03438670
NIP [c005930c] handle_mm_fault+0xa4c/0xacc
LR [c000e3c4] do_page_fault+0x2f0/0x474
Call Trace:
[c21ade40] [c0059328] handle_mm_fault+0xa68/0xacc (unreliable)
[c21ade90] [c000e3c4] do_page_fault+0x2f0/0x474
[c21adf40] [c000d520] handle_page_fault+0xc/0x80
Instruction dump:
4802192d 48000094 2f9f0000 3ba00001 419e0088 7fe3fb78 4bff46a1 48000078
7fe3fb78 4bff4695 48000070 63390020 <633f0080> 7f45d378 7f63db78 7f04c378
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Rex Feany @ 2009-09-29 21:09 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <1254208031.5771.6.camel@pasglop>
Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org):
> On Mon, 2009-09-28 at 18:21 -0700, Rex Feany wrote:
> > > It's going to be hard for me to get that "right" since I don't really
> > > know what's going on with the core here, but I suppose if we get it
> > > moving along with extra tlb invalidations, that should be "good enough"
> > > until somebody who really knows what's going on comes up with possibly
> > > a better fix.
> >
> > I've tried sticking tlbil_va() in those places, nothing seems to help.
> > In some cases userspace is slow, in other cases userspace is faster and
> > unstable: sometimes commands hang, sometimes I am able to ctrl-c and
> > and kill it, sometimes I get other strange crashes or falures (so far no
> > kernel oopses though).
>
> And you are positive that with 2.6.31 and your other patch, it works
> both fast and stable ? This is strange... the code should be mostly
> identical. I'll have a second look and see if I can get you a patch that
> reproduce -exactly- the behaviour of 2.6.31 plus your patch.
The difference is night and day - with 2.6.31 I can boot into single
user mode, I can run our custom software (I left it running over night
with very simple test script - no crashes).
With the top of the tree I can sometimes boot into a shell, and if it
isn't crashing or hanging on boot it runs very slow (10+ seconds to do
anything, if I am lucky).
thanks!
/rex
^ permalink raw reply
* Re: linux-next: tree build failure
From: Hollis Blanchard @ 2009-09-29 23:39 UTC (permalink / raw)
To: Jan Beulich; +Cc: sfr, linux-kernel, kvm-ppc, linux-next, akpm, linuxppc-dev
In-Reply-To: <4AC1E15502000078000516B5@vpn.id2.novell.com>
On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
> >>> Hollis Blanchard 09/29/09 2:00 AM >>>
> >First, I think there is a real bug here, and the code should read like
> >this (to match the comment):
> > /* type has to be known at build time for optimization */
> >- BUILD_BUG_ON(__builtin_constant_p(type));
> >+ BUILD_BUG_ON(!__builtin_constant_p(type));
> >
> >However, I get the same build error *both* ways, i.e.
> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
> >the new BUILD_BUG_ON() macro isn't working...
>
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).
I think I see what you're saying. Do you have a fix to suggest?
--
Hollis Blanchard
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] powerpc/5200: add LocalPlus bus FIFO device driver
From: Grant Likely @ 2009-09-30 2:25 UTC (permalink / raw)
To: John Bonesio; +Cc: linuxppc-dev
In-Reply-To: <20090929204248.16225.22818.stgit@riker>
On Tue, Sep 29, 2009 at 2:43 PM, John Bonesio <bones@secretlab.ca> wrote:
> This is a driver for the FIFO device on the LocalPlus bus on an mpc5200 s=
ystem.
> The driver supports programmed I/O through the FIFO as well as setting up=
DMA
> via the BestComm engine through the FIFO.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: John Bonesio <bones@secretlab.ca>
Let me know if anyone has comments on this. Since it is a new driver,
I'll probably try to merge it for 2.6.32 in the next week or so even
though the merge window has closed.
Cheers,
g.
> ---
> This driver was originally written by Grant Likely. I have updated it so =
that
> transmitting data (as opposed to receiving data) works better. The driver=
has
> been tested for all 6 transfer modes:
> =A0 =A0 =A0 =A0PIO mode (rx and tx)
> =A0 =A0 =A0 =A0DMA polled mode (rx and tx)
> =A0 =A0 =A0 =A0DMA interrupt mode (rx and tx)
>
> - John
>
> =A0arch/powerpc/include/asm/mpc52xx.h =A0 =A0 =A0 =A0 =A0 =A0| =A0 39 ++
> =A0arch/powerpc/platforms/52xx/Kconfig =A0 =A0 =A0 =A0 =A0 | =A0 =A05
> =A0arch/powerpc/platforms/52xx/Makefile =A0 =A0 =A0 =A0 =A0| =A0 =A01
> =A0arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c | =A0560 +++++++++++++++=
++++++++++
> =A04 files changed, 605 insertions(+), 0 deletions(-)
> =A0create mode 100644 arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
>
> diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/as=
m/mpc52xx.h
> index 8273357..819a0be 100644
> --- a/arch/powerpc/include/asm/mpc52xx.h
> +++ b/arch/powerpc/include/asm/mpc52xx.h
> @@ -282,6 +282,45 @@ extern int mpc52xx_gpt_start_timer(struct mpc52xx_gp=
t_priv *gpt, int period,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int continuous);
> =A0extern void mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
>
> +/* mpc52xx_lpbfifo.c */
> +#define MPC52XX_LPBFIFO_FLAG_READ =A0 =A0 =A0 =A0 =A0 =A0 =A0(0)
> +#define MPC52XX_LPBFIFO_FLAG_WRITE =A0 =A0 =A0 =A0 =A0 =A0 (1<<0)
> +#define MPC52XX_LPBFIFO_FLAG_NO_INCREMENT =A0 =A0 =A0(1<<1)
> +#define MPC52XX_LPBFIFO_FLAG_NO_DMA =A0 =A0 =A0 =A0 =A0 =A0(1<<2)
> +#define MPC52XX_LPBFIFO_FLAG_POLL_DMA =A0 =A0 =A0 =A0 =A0(1<<3)
> +
> +struct mpc52xx_lpbfifo_request {
> + =A0 =A0 =A0 struct list_head list;
> +
> + =A0 =A0 =A0 /* localplus bus address */
> + =A0 =A0 =A0 unsigned int cs;
> + =A0 =A0 =A0 size_t offset;
> +
> + =A0 =A0 =A0 /* Memory address */
> + =A0 =A0 =A0 void *data;
> + =A0 =A0 =A0 phys_addr_t data_phys;
> +
> + =A0 =A0 =A0 /* Details of transfer */
> + =A0 =A0 =A0 size_t size;
> + =A0 =A0 =A0 size_t pos; =A0 =A0 /* current position of transfer */
> + =A0 =A0 =A0 int flags;
> +
> + =A0 =A0 =A0 /* What to do when finished */
> + =A0 =A0 =A0 void (*callback)(struct mpc52xx_lpbfifo_request *);
> +
> + =A0 =A0 =A0 void *priv; =A0 =A0 =A0 =A0 =A0 =A0 /* Driver private data =
*/
> +
> + =A0 =A0 =A0 /* statistics */
> + =A0 =A0 =A0 int irq_count;
> + =A0 =A0 =A0 int irq_ticks;
> + =A0 =A0 =A0 u8 last_byte;
> + =A0 =A0 =A0 int buffer_not_done_cnt;
> +};
> +
> +extern int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_request *req);
> +extern void mpc52xx_lpbfifo_abort(struct mpc52xx_lpbfifo_request *req);
> +extern void mpc52xx_lpbfifo_poll(void);
> +
> =A0/* mpc52xx_pic.c */
> =A0extern void mpc52xx_init_irq(void);
> =A0extern unsigned int mpc52xx_get_irq(void);
> diff --git a/arch/powerpc/platforms/52xx/Kconfig b/arch/powerpc/platforms=
/52xx/Kconfig
> index 696a5ee..ea00b3d 100644
> --- a/arch/powerpc/platforms/52xx/Kconfig
> +++ b/arch/powerpc/platforms/52xx/Kconfig
> @@ -51,3 +51,8 @@ config PPC_MPC5200_GPIO
> =A0 =A0 =A0 =A0select GENERIC_GPIO
> =A0 =A0 =A0 =A0help
> =A0 =A0 =A0 =A0 =A0Enable gpiolib support for mpc5200 based boards
> +
> +config PPC_MPC5200_LPBFIFO
> + =A0 =A0 =A0 tristate "MPC5200 LocalPlus bus FIFO driver"
> + =A0 =A0 =A0 depends on PPC_MPC52xx
> + =A0 =A0 =A0 select PPC_BESTCOMM_GEN_BD
> diff --git a/arch/powerpc/platforms/52xx/Makefile b/arch/powerpc/platform=
s/52xx/Makefile
> index d6ade3d..8df23af 100644
> --- a/arch/powerpc/platforms/52xx/Makefile
> +++ b/arch/powerpc/platforms/52xx/Makefile
> @@ -14,3 +14,4 @@ ifeq ($(CONFIG_PPC_LITE5200),y)
> =A0endif
>
> =A0obj-$(CONFIG_PPC_MPC5200_GPIO) +=3D mpc52xx_gpio.o
> +obj-$(CONFIG_PPC_MPC5200_LPBFIFO) =A0 =A0 =A0+=3D mpc52xx_lpbfifo.o
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c b/arch/powerpc=
/platforms/52xx/mpc52xx_lpbfifo.c
> new file mode 100644
> index 0000000..929d017
> --- /dev/null
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c
> @@ -0,0 +1,560 @@
> +/*
> + * LocalPlus Bus FIFO driver for the Freescale MPC52xx.
> + *
> + * Copyright (C) 2009 Secret Lab Technologies Ltd.
> + *
> + * This file is released under the GPLv2
> + *
> + * Todo:
> + * - Add support for multiple requests to be queued.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/spinlock.h>
> +#include <asm/io.h>
> +#include <asm/prom.h>
> +#include <asm/mpc52xx.h>
> +#include <asm/time.h>
> +
> +#include <sysdev/bestcomm/bestcomm.h>
> +#include <sysdev/bestcomm/bestcomm_priv.h>
> +#include <sysdev/bestcomm/gen_bd.h>
> +
> +MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
> +MODULE_DESCRIPTION("MPC5200 LocalPlus FIFO device driver");
> +MODULE_LICENSE("GPL");
> +
> +#define LPBFIFO_REG_PACKET_SIZE =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(0x00)
> +#define LPBFIFO_REG_START_ADDRESS =A0 =A0 =A0(0x04)
> +#define LPBFIFO_REG_CONTROL =A0 =A0 =A0 =A0 =A0 =A0(0x08)
> +#define LPBFIFO_REG_ENABLE =A0 =A0 =A0 =A0 =A0 =A0 (0x0C)
> +#define LPBFIFO_REG_BYTES_DONE_STATUS =A0(0x14)
> +#define LPBFIFO_REG_FIFO_DATA =A0 =A0 =A0 =A0 =A0(0x40)
> +#define LPBFIFO_REG_FIFO_STATUS =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(0x44)
> +#define LPBFIFO_REG_FIFO_CONTROL =A0 =A0 =A0 (0x48)
> +#define LPBFIFO_REG_FIFO_ALARM =A0 =A0 =A0 =A0 (0x4C)
> +
> +struct mpc52xx_lpbfifo {
> + =A0 =A0 =A0 struct device *dev;
> + =A0 =A0 =A0 phys_addr_t regs_phys;
> + =A0 =A0 =A0 void __iomem *regs;
> + =A0 =A0 =A0 int irq;
> + =A0 =A0 =A0 spinlock_t lock;
> +
> + =A0 =A0 =A0 struct bcom_task *bcom_tx_task;
> + =A0 =A0 =A0 struct bcom_task *bcom_rx_task;
> + =A0 =A0 =A0 struct bcom_task *bcom_cur_task;
> +
> + =A0 =A0 =A0 /* Current state data */
> + =A0 =A0 =A0 struct mpc52xx_lpbfifo_request *req;
> + =A0 =A0 =A0 int dma_irqs_enabled;
> +};
> +
> +/* The MPC5200 has only one fifo, so only need one instance structure */
> +static struct mpc52xx_lpbfifo lpbfifo;
> +
> +/**
> + * mpc52xx_lpbfifo_kick - Trigger the next block of data to be transfere=
d
> + */
> +static void mpc52xx_lpbfifo_kick(struct mpc52xx_lpbfifo_request *req)
> +{
> + =A0 =A0 =A0 size_t transfer_size =3D req->size - req->pos;
> + =A0 =A0 =A0 struct bcom_bd *bd;
> + =A0 =A0 =A0 void __iomem *reg;
> + =A0 =A0 =A0 u32 *data;
> + =A0 =A0 =A0 int i;
> + =A0 =A0 =A0 int bit_fields;
> + =A0 =A0 =A0 int dma =3D !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
> + =A0 =A0 =A0 int write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
> + =A0 =A0 =A0 int poll_dma =3D req->flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA=
;
> +
> + =A0 =A0 =A0 /* Set and clear the reset bits; is good practice in User M=
anual */
> + =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
> +
> + =A0 =A0 =A0 /* set master enable bit */
> + =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x00000001);
> + =A0 =A0 =A0 if (!dma) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* While the FIFO can be setup for transfer=
sizes as large as
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* 16M-1, the FIFO itself is only 512 byt=
es deep and it does
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* not generate interrupts for FIFO full =
events (only transfer
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* complete will raise an IRQ). =A0Theref=
ore when not using
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* Bestcomm to drive the FIFO it needs to=
either be polled, or
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* transfers need to constrained to the s=
ize of the fifo.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* This driver restricts the size of the =
transfer
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (transfer_size > 512)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 transfer_size =3D 512;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Load the FIFO with data */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (write) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D lpbfifo.regs + LPBF=
IFO_REG_FIFO_DATA;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D req->data + req->p=
os;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (i =3D 0; i < transfer_=
size; i +=3D 4)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(re=
g, *data++);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Unmask both error and completion irqs */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE,=
0x00000301);
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Choose the correct direction
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* Configure the watermarks so DMA will a=
lways complete correctly.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* It may be worth experimenting with the=
ALARM value to see if
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* there is a performance impacit. =A0How=
ever, if it is wrong there
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* is a risk of DMA not transferring the =
last chunk of data
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (write) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(lpbfifo.regs + LPB=
FIFO_REG_FIFO_ALARM, 0x1e4);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_8(lpbfifo.regs + LPBFIF=
O_REG_FIFO_CONTROL, 7);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.bcom_cur_task =3D l=
pbfifo.bcom_tx_task;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(lpbfifo.regs + LPB=
FIFO_REG_FIFO_ALARM, 0x1ff);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_8(lpbfifo.regs + LPBFIF=
O_REG_FIFO_CONTROL, 0);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.bcom_cur_task =3D l=
pbfifo.bcom_rx_task;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (poll_dma) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (lpbfifo=
.dma_irqs_enabled) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 disable_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task));
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 lpbfifo.dma_irqs_enabled =3D 0;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!lpbfif=
o.dma_irqs_enabled) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 enable_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task));
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 lpbfifo.dma_irqs_enabled =3D 1;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd =3D bcom_prepare_next_buffer(lpbfifo.bco=
m_cur_task);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->status =3D transfer_size;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!write) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* In the DMA read case, =
the DMA doesn't complete,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* possibly due to incorr=
ect watermarks in the ALARM
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* and CONTROL regs. For =
now instead of trying to
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* determine the right wa=
termarks that will make this
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* work, just increase th=
e number of bytes the FIFO is
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* expecting.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* When submitting anothe=
r operation, the FIFO will get
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* reset, so the conditio=
n of the FIFO waiting for a
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* non-existent 4 bytes w=
ill get cleared.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 transfer_size +=3D 4; /* BL=
ECH! */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bd->data[0] =3D req->data_phys + req->pos;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_submit_next_buffer(lpbfifo.bcom_cur_ta=
sk, NULL);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* error irq & master enabled bit */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields =3D 0x00000201;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Unmask irqs */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (write && (!poll_dma))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D 0x00000100;=
/* completion irq too */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE,=
bit_fields);
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 /* Set transfer size, width, chip select and READ mode */
> + =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_START_ADDRESS,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0req->offset + req->pos);
> + =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_PACKET_SIZE, transfer_s=
ize);
> +
> + =A0 =A0 =A0 bit_fields =3D req->cs << 24 | 0x000008;
> + =A0 =A0 =A0 if (!write)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bit_fields |=3D 0x010000; /* read mode */
> + =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_CONTROL, bit_fields);
> +
> + =A0 =A0 =A0 /* Kick it off */
> + =A0 =A0 =A0 out_8(lpbfifo.regs + LPBFIFO_REG_PACKET_SIZE, 0x01);
> + =A0 =A0 =A0 if (dma)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_enable(lpbfifo.bcom_cur_task);
> +}
> +
> +/**
> + * mpc52xx_lpbfifo_irq - IRQ handler for LPB FIFO
> + *
> + * On transmit, the dma completion irq triggers before the fifo completi=
on
> + * triggers. =A0Handle the dma completion here instead of the LPB FIFO B=
estcomm
> + * task completion irq becuase everyting is not really done until the LP=
B FIFO
> + * completion irq triggers.
> + *
> + * In other words:
> + * For DMA, on receive, the "Fat Lady" is the bestcom completion irq. on
> + * transmit, the fifo completion irq is the "Fat Lady". The opera (or in=
this
> + * case the DMA/FIFO operation) is not finished until the "Fat Lady" sin=
gs.
> + *
> + * Reasons for entering this routine:
> + * 1) PIO mode rx and tx completion irq
> + * 2) DMA interrupt mode tx completion irq
> + * 3) DMA polled mode tx
> + *
> + * Exit conditions:
> + * 1) Transfer aborted
> + * 2) FIFO complete without DMA; more data to do
> + * 3) FIFO complete without DMA; all data transfered
> + * 4) FIFO complete using DMA
> + *
> + * Condition 1 can occur regardless of whether or not DMA is used.
> + * It requires executing the callback to report the error and exiting
> + * immediately.
> + *
> + * Condition 2 requires programming the FIFO with the next block of data
> + *
> + * Condition 3 requires executing the callback to report completion
> + *
> + * Condition 4 means the same as 3, except that we also retrieve the bco=
m
> + * buffer so DMA doesn't get clogged up.
> + *
> + * To make things trickier, the spinlock must be dropped before
> + * executing the callback, otherwise we could end up with a deadlock
> + * or nested spinlock condition. =A0The out path is non-trivial, so
> + * extra fiddling is done to make sure all paths lead to the same
> + * outbound code.
> + */
> +static irqreturn_t mpc52xx_lpbfifo_irq(int irq, void *dev_id)
> +{
> + =A0 =A0 =A0 struct mpc52xx_lpbfifo_request *req;
> + =A0 =A0 =A0 u32 status =3D in_8(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_S=
TATUS);
> + =A0 =A0 =A0 void __iomem *reg;
> + =A0 =A0 =A0 u32 *data;
> + =A0 =A0 =A0 int count, i;
> + =A0 =A0 =A0 int do_callback =3D 0;
> + =A0 =A0 =A0 u32 ts;
> + =A0 =A0 =A0 unsigned long flags;
> + =A0 =A0 =A0 int dma, write, poll_dma;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&lpbfifo.lock, flags);
> + =A0 =A0 =A0 ts =3D get_tbl();
> +
> + =A0 =A0 =A0 req =3D lpbfifo.req;
> + =A0 =A0 =A0 if (!req) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("bogus LPBFIFO IRQ\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return IRQ_HANDLED;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 dma =3D !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
> + =A0 =A0 =A0 write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
> + =A0 =A0 =A0 poll_dma =3D req->flags & MPC52XX_LPBFIFO_FLAG_POLL_DMA;
> +
> + =A0 =A0 =A0 if (dma && !write) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("bogus LPBFIFO IRQ (dma and not writ=
ting)\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return IRQ_HANDLED;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 if ((status & 0x01) =3D=3D 0) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 /* check abort bit */
> + =A0 =A0 =A0 if (status & 0x10) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE,=
0x01010000);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 do_callback =3D 1;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 /* Read result from hardware */
> + =A0 =A0 =A0 count =3D in_be32(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STA=
TUS);
> + =A0 =A0 =A0 count &=3D 0x00ffffff;
> +
> + =A0 =A0 =A0 if (!dma && !write) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* copy the data out of the FIFO */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D lpbfifo.regs + LPBFIFO_REG_FIFO_DAT=
A;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 data =3D req->data + req->pos;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (i =3D 0; i < count; i +=3D 4)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *data++ =3D in_be32(reg);
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 /* Update transfer position and count */
> + =A0 =A0 =A0 req->pos +=3D count;
> +
> + =A0 =A0 =A0 /* Decide what to do next */
> + =A0 =A0 =A0 if (req->size - req->pos)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_lpbfifo_kick(req); /* more work to =
do */
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 do_callback =3D 1;
> +
> + out:
> + =A0 =A0 =A0 /* Clear the IRQ */
> + =A0 =A0 =A0 out_8(lpbfifo.regs + LPBFIFO_REG_BYTES_DONE_STATUS, 0x01);
> +
> + =A0 =A0 =A0 if (dma && (status & 0x11)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* Count the DMA as complete only when th=
e FIFO completion
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* status or abort bits are set.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* (status & 0x01) should always be the c=
ase except sometimes
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* when using polled DMA.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* (status & 0x10) {transfer aborted}: Th=
is case needs more
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* testing.
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_retrieve_buffer(lpbfifo.bcom_cur_task,=
&status, NULL);
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 req->last_byte =3D ((u8 *)req->data)[req->size - 1];
> +
> + =A0 =A0 =A0 /* When the do_callback flag is set; it means the transfer =
is finished
> + =A0 =A0 =A0 =A0* so set the FIFO as idle */
> + =A0 =A0 =A0 if (do_callback)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.req =3D NULL;
> +
> + =A0 =A0 =A0 if (irq !=3D 0) /* don't increment on polled case */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->irq_count++;
> +
> + =A0 =A0 =A0 req->irq_ticks +=3D get_tbl() - ts;
> + =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags);
> +
> + =A0 =A0 =A0 /* Spinlock is released; it is now safe to call the callbac=
k */
> + =A0 =A0 =A0 if (do_callback && req->callback)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->callback(req);
> +
> + =A0 =A0 =A0 return IRQ_HANDLED;
> +}
> +
> +/**
> + * mpc52xx_lpbfifo_bcom_irq - IRQ handler for LPB FIFO Bestcomm task
> + *
> + * Only used when receiving data.
> + */
> +static irqreturn_t mpc52xx_lpbfifo_bcom_irq(int irq, void *dev_id)
> +{
> + =A0 =A0 =A0 struct mpc52xx_lpbfifo_request *req;
> + =A0 =A0 =A0 unsigned long flags;
> + =A0 =A0 =A0 u32 status;
> + =A0 =A0 =A0 u32 ts;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&lpbfifo.lock, flags);
> + =A0 =A0 =A0 ts =3D get_tbl();
> +
> + =A0 =A0 =A0 req =3D lpbfifo.req;
> + =A0 =A0 =A0 if (!req || (req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return IRQ_HANDLED;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 if (irq !=3D 0) /* don't increment on polled case */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->irq_count++;
> +
> + =A0 =A0 =A0 if (!bcom_buffer_done(lpbfifo.bcom_cur_task)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags=
);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->buffer_not_done_cnt++;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((req->buffer_not_done_cnt % 1000) =3D=
=3D 0)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("transfer stalled\n"=
);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return IRQ_HANDLED;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 bcom_retrieve_buffer(lpbfifo.bcom_cur_task, &status, NULL);
> +
> + =A0 =A0 =A0 req->last_byte =3D ((u8 *)req->data)[req->size - 1];
> +
> + =A0 =A0 =A0 req->pos =3D status & 0x00ffffff;
> +
> + =A0 =A0 =A0 /* Mark the FIFO as idle */
> + =A0 =A0 =A0 lpbfifo.req =3D NULL;
> +
> + =A0 =A0 =A0 /* Release the lock before calling out to the callback. */
> + =A0 =A0 =A0 req->irq_ticks +=3D get_tbl() - ts;
> + =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags);
> +
> + =A0 =A0 =A0 if (req->callback)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 req->callback(req);
> +
> + =A0 =A0 =A0 return IRQ_HANDLED;
> +}
> +
> +/**
> + * mpc52xx_lpbfifo_bcom_poll - Poll for DMA completion
> + */
> +void mpc52xx_lpbfifo_poll(void)
> +{
> + =A0 =A0 =A0 struct mpc52xx_lpbfifo_request *req =3D lpbfifo.req;
> + =A0 =A0 =A0 int dma =3D !(req->flags & MPC52XX_LPBFIFO_FLAG_NO_DMA);
> + =A0 =A0 =A0 int write =3D req->flags & MPC52XX_LPBFIFO_FLAG_WRITE;
> +
> + =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0* For more information, see comments on the "Fat Lady"
> + =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 if (dma && write)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_lpbfifo_irq(0, NULL);
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_lpbfifo_bcom_irq(0, NULL);
> +}
> +EXPORT_SYMBOL(mpc52xx_lpbfifo_poll);
> +
> +/**
> + * mpc52xx_lpbfifo_submit - Submit an LPB FIFO transfer request.
> + * @req: Pointer to request structure
> + */
> +int mpc52xx_lpbfifo_submit(struct mpc52xx_lpbfifo_request *req)
> +{
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 if (!lpbfifo.regs)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&lpbfifo.lock, flags);
> +
> + =A0 =A0 =A0 /* If the req pointer is already set, then a transfer is in=
progress */
> + =A0 =A0 =A0 if (lpbfifo.req) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 /* Setup the transfer */
> + =A0 =A0 =A0 lpbfifo.req =3D req;
> + =A0 =A0 =A0 req->irq_count =3D 0;
> + =A0 =A0 =A0 req->irq_ticks =3D 0;
> + =A0 =A0 =A0 req->buffer_not_done_cnt =3D 0;
> + =A0 =A0 =A0 req->pos =3D 0;
> +
> + =A0 =A0 =A0 mpc52xx_lpbfifo_kick(req);
> + =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags);
> + =A0 =A0 =A0 return 0;
> +}
> +EXPORT_SYMBOL(mpc52xx_lpbfifo_submit);
> +
> +void mpc52xx_lpbfifo_abort(struct mpc52xx_lpbfifo_request *req)
> +{
> + =A0 =A0 =A0 unsigned long flags;
> +
> + =A0 =A0 =A0 spin_lock_irqsave(&lpbfifo.lock, flags);
> + =A0 =A0 =A0 if (lpbfifo.req =3D=3D req) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Put it into reset and clear the state */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_gen_bd_rx_reset(lpbfifo.bcom_rx_task);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_gen_bd_tx_reset(lpbfifo.bcom_tx_task);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE,=
0x01010000);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 lpbfifo.req =3D NULL;
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 spin_unlock_irqrestore(&lpbfifo.lock, flags);
> +}
> +EXPORT_SYMBOL(mpc52xx_lpbfifo_abort);
> +
> +static int __devinit
> +mpc52xx_lpbfifo_probe(struct of_device *op, const struct of_device_id *m=
atch)
> +{
> + =A0 =A0 =A0 struct resource res;
> + =A0 =A0 =A0 int rc =3D -ENOMEM;
> +
> + =A0 =A0 =A0 if (lpbfifo.dev !=3D NULL)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENOSPC;
> +
> + =A0 =A0 =A0 lpbfifo.irq =3D irq_of_parse_and_map(op->node, 0);
> + =A0 =A0 =A0 if (!lpbfifo.irq)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
> +
> + =A0 =A0 =A0 if (of_address_to_resource(op->node, 0, &res))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
> + =A0 =A0 =A0 lpbfifo.regs_phys =3D res.start;
> + =A0 =A0 =A0 lpbfifo.regs =3D of_iomap(op->node, 0);
> + =A0 =A0 =A0 if (!lpbfifo.regs)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
> +
> + =A0 =A0 =A0 spin_lock_init(&lpbfifo.lock);
> +
> + =A0 =A0 =A0 /* Put FIFO into reset */
> + =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
> +
> + =A0 =A0 =A0 /* Register the interrupt handler */
> + =A0 =A0 =A0 rc =3D request_irq(lpbfifo.irq, mpc52xx_lpbfifo_irq, 0,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"mpc52xx-lpbfifo", &lpbf=
ifo);
> + =A0 =A0 =A0 if (rc)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto err_irq;
> +
> + =A0 =A0 =A0 /* Request the Bestcomm receive (fifo --> memory) task and =
IRQ */
> + =A0 =A0 =A0 lpbfifo.bcom_rx_task =3D
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_gen_bd_rx_init(2, res.start + LPBFIFO_=
REG_FIFO_DATA,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 BCO=
M_INITIATOR_SCLPC, BCOM_IPR_SCLPC,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 16*=
1024*1024);
> + =A0 =A0 =A0 if (!lpbfifo.bcom_rx_task)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto err_bcom_rx;
> +
> + =A0 =A0 =A0 rc =3D request_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task),
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mpc52xx_lpbfifo_bcom_irq=
, 0,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"mpc52xx-lpbfifo-rx", &l=
pbfifo);
> + =A0 =A0 =A0 if (rc)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto err_bcom_rx_irq;
> +
> + =A0 =A0 =A0 /* Request the Bestcomm transmit (memory --> fifo) task and=
IRQ */
> + =A0 =A0 =A0 lpbfifo.bcom_tx_task =3D
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bcom_gen_bd_tx_init(2, res.start + LPBFIFO_=
REG_FIFO_DATA,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 BCO=
M_INITIATOR_SCLPC, BCOM_IPR_SCLPC);
> + =A0 =A0 =A0 if (!lpbfifo.bcom_tx_task)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto err_bcom_tx;
> +
> + =A0 =A0 =A0 lpbfifo.dev =3D &op->dev;
> + =A0 =A0 =A0 return 0;
> +
> + err_bcom_tx:
> + =A0 =A0 =A0 free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo)=
;
> + err_bcom_rx_irq:
> + =A0 =A0 =A0 bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
> + err_bcom_rx:
> + err_irq:
> + =A0 =A0 =A0 iounmap(lpbfifo.regs);
> + =A0 =A0 =A0 lpbfifo.regs =3D NULL;
> +
> + =A0 =A0 =A0 dev_err(&op->dev, "mpc52xx_lpbfifo_probe() failed\n");
> + =A0 =A0 =A0 return -ENODEV;
> +}
> +
> +
> +static int __devexit mpc52xx_lpbfifo_remove(struct of_device *op)
> +{
> + =A0 =A0 =A0 if (lpbfifo.dev !=3D &op->dev)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> +
> + =A0 =A0 =A0 /* Put FIFO in reset */
> + =A0 =A0 =A0 out_be32(lpbfifo.regs + LPBFIFO_REG_ENABLE, 0x01010000);
> +
> + =A0 =A0 =A0 /* Release the bestcomm transmit task */
> + =A0 =A0 =A0 free_irq(bcom_get_task_irq(lpbfifo.bcom_tx_task), &lpbfifo)=
;
> + =A0 =A0 =A0 bcom_gen_bd_tx_release(lpbfifo.bcom_tx_task);
> +
> + =A0 =A0 =A0 /* Release the bestcomm receive task */
> + =A0 =A0 =A0 free_irq(bcom_get_task_irq(lpbfifo.bcom_rx_task), &lpbfifo)=
;
> + =A0 =A0 =A0 bcom_gen_bd_rx_release(lpbfifo.bcom_rx_task);
> +
> + =A0 =A0 =A0 free_irq(lpbfifo.irq, &lpbfifo);
> + =A0 =A0 =A0 iounmap(lpbfifo.regs);
> + =A0 =A0 =A0 lpbfifo.regs =3D NULL;
> + =A0 =A0 =A0 lpbfifo.dev =3D NULL;
> +
> + =A0 =A0 =A0 return 0;
> +}
> +
> +static struct of_device_id mpc52xx_lpbfifo_match[] __devinitconst =3D {
> + =A0 =A0 =A0 { .compatible =3D "fsl,mpc5200-lpbfifo", },
> + =A0 =A0 =A0 {},
> +};
> +
> +static struct of_platform_driver mpc52xx_lpbfifo_driver =3D {
> + =A0 =A0 =A0 .owner =3D THIS_MODULE,
> + =A0 =A0 =A0 .name =3D "mpc52xx-lpbfifo",
> + =A0 =A0 =A0 .match_table =3D mpc52xx_lpbfifo_match,
> + =A0 =A0 =A0 .probe =3D mpc52xx_lpbfifo_probe,
> + =A0 =A0 =A0 .remove =3D __devexit_p(mpc52xx_lpbfifo_remove),
> +};
> +
> +/***********************************************************************
> + * Module init/exit
> + */
> +static int __init mpc52xx_lpbfifo_init(void)
> +{
> + =A0 =A0 =A0 pr_debug("Registering LocalPlus bus FIFO driver\n");
> + =A0 =A0 =A0 return of_register_platform_driver(&mpc52xx_lpbfifo_driver)=
;
> +}
> +module_init(mpc52xx_lpbfifo_init);
> +
> +static void __exit mpc52xx_lpbfifo_exit(void)
> +{
> + =A0 =A0 =A0 pr_debug("Unregistering LocalPlus bus FIFO driver\n");
> + =A0 =A0 =A0 of_unregister_platform_driver(&mpc52xx_lpbfifo_driver);
> +}
> +module_exit(mpc52xx_lpbfifo_exit);
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: 64bit kernel is huge
From: Michael Neuling @ 2009-09-30 6:01 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linuxppc-dev
In-Reply-To: <20090928074503.GB16073@kryten>
> # size vmlinux
> text data bss dec hex filename
> 9812942 1982496 1105228 12900666 c4d93a vmlinux
... over kernel releases with pseries_defconfig:
text data bss dec hex
2.6.32-rc1 9872090 1971184 1105236 12948510 c5941e
2.6.31 9730495 1708796 1054236 12493527 bea2d7
2.6.30 9221595 1620728 1052812 11895135 b5815f
2.6.29 9104807 1569840 785292 11459939 aedd63
2.6.28 8647080 1699460 780472 11127012 a9c8e4
2.6.27 7461663 1505796 774400 9741859 94a623
2.6.26 7192966 1470432 631024 9294422 8dd256
2.6.25 7163913 1558800 568048 9290761 8dc409
We are getting bigger pretty quickly :-(
Mikey
^ permalink raw reply
* Re: linux-next: tree build failure
From: Jan Beulich @ 2009-09-30 6:29 UTC (permalink / raw)
To: roel kluin
Cc: sfr, hollisb, linux-kernel, kvm-ppc, linux-next, akpm,
linuxppc-dev
In-Reply-To: <25e057c00909290251h55c0dc25o4ab1f2e84c920dca@mail.gmail.com>
>>> roel kluin <roel.kluin@gmail.com> 29.09.09 11:51 >>>
>On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich <jbeulich@novell.com> =
wrote:
>>>>> Hollis Blanchard 09/29/09 2:00 AM >>>
>>>First, I think there is a real bug here, and the code should read like
>>>this (to match the comment):
>>> /* type has to be known at build time for optimization */
>>>- BUILD_BUG_ON(__builtin_constant_p(type));
>>>+ BUILD_BUG_ON(!__builtin_constant_p(type));
>>>
>>>However, I get the same build error *both* ways, i.e.
>>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>>the new BUILD_BUG_ON() macro isn't working...
>>
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>>
>> Jan
>
>then maybe
>
>if(__builtin_constant_p(type))
> BUILD_BUG_ON(1);
>
>would work?
Definitely not - this would result in the compiler *always* generating an
error.
Jan
^ permalink raw reply
* Re: linux-next: tree build failure
From: Jan Beulich @ 2009-09-30 6:35 UTC (permalink / raw)
To: Hollis Blanchard
Cc: sfr, linux-kernel, kvm-ppc, linux-next, akpm, linuxppc-dev
In-Reply-To: <1254267572.15622.1621.camel@slab.beaverton.ibm.com>
>>> Hollis Blanchard <hollisb@us.ibm.com> 30.09.09 01:39 >>>
>On Tue, 2009-09-29 at 10:28 +0100, Jan Beulich wrote:
>> >>> Hollis Blanchard 09/29/09 2:00 AM >>>
>> >First, I think there is a real bug here, and the code should read like
>> >this (to match the comment):
>> > /* type has to be known at build time for optimization */
>> >- BUILD_BUG_ON(__builtin_constant_p(type));
>> >+ BUILD_BUG_ON(!__builtin_constant_p(type));
>> >
>> >However, I get the same build error *both* ways, i.e.
>> >__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>> >the new BUILD_BUG_ON() macro isn't working...
>>=20
>> No, at this point of the compilation process it's neither zero nor one,
>> it's simply considered non-constant by the compiler at that stage
>> (this builtin is used for optimization, not during parsing, and the
>> error gets generated when the body of the function gets parsed,
>> not when code gets generated from it).
>
>I think I see what you're saying. Do you have a fix to suggest?
The one Rusty suggested the other day may help here. I don't like it
as a drop-in replacement for BUILD_BUG_ON() though (due to it
deferring the error generated to the linking stage), I'd rather view
this as an improvement to MAYBE_BUILD_BUG_ON() (which should
then be used here).
Jan
^ permalink raw reply
* Re: MPC8536 PCI rescan to discover FPGA
From: Felix Radensky @ 2009-09-30 7:06 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev
In-Reply-To: <1253604183.7103.213.camel@pasglop>
Hi, Benjamin
Benjamin Herrenschmidt wrote:
> Right. However, in case it's a bit too much work to get
> hotswap implemented on the machine, you may still be able
> to do something simpler from your platform code, after you've
> finished loading the FPGA. I assume the FPGA doesn't contain a
> P2P bridge that would require probing further below the FPGA
> itself.
>
> The basic idea is to call pci_scan_slot() on the devfn where
> the FPGA is supposed to respond.
>
> Then you need to also do some fixup. First you need to call
> pcibios_setup_bus_devices(). This will wire up the device
> to an OF node if you have one, setup some default DMA ops,
> etc...
>
> Note that this function will walk over all devices on that bus
> which is interesting since some of those may have already been
> fully setup initially. Hopefully that isn't a problem. If it
> was to become one, we would have to figure out a way to skip
> devices that have already been "setup".
>
> And finally you call pcibios_finish_adding_to_bus() which will
> do the resource allocation pass on all new devices on the bus
> and register them with the core device layer.
>
> Cheers,
> Ben.
>
>
Thanks for the advice. This approach worked well for me, with some minor
modifications. First I had to clear PPC_INDIRECT_TYPE_NO_PCIE_LINK
in hose->indirect_type, otherwise reads from configuration space fail. And
second, I had to call pci_assign_resource() to complete the assignment
of FPGA
memory. The memory range is 0x0 - 0x7fffff, and PCI code doesn't like
resources
starting at 0.
Again, thanks a lot for your help, much appreciated.
Felix.
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-09-30 7:59 UTC (permalink / raw)
To: Rex Feany; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <20090929210331.GA25779@laura.chatsunix.int.mrv.com>
Rex Feany <RFeany@mrv.com> wrote on 29/09/2009 23:03:31:
>
> Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
>
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 29/09/2009 10:16:38:
> > >
> > >
> > > > hmm, yes. You do get this and mysterious SEGV if you hit the but so does
> > > > other bugs too so this is probably due to missing invalidation.
> > > >
> > > > I suspect that something like below will fix the problem and
> > > > is the "correct" fix(untested, not even compiled):
> > >
> > > Ok but do we also still have to worry about the "unpopulated" TLB
> > > entries and invalidate them somehow when populating ?
> >
> > Since I am probably the only one that knows about DAR problem I figured
> > I should take a stab at it. This is not tested, but I hope Rex and the list
> > can do that. Once this works as it should, we can remove all special handling
> > for 8xx in copy_tofrom_user() and friends.
> > No sign-off yet, want some confirmation first.
>
> It doesn't make a difference. I applied it to the top of the tree,
> it got to userspace but it is stuck. Using break I am able to dump the
> registers, I'm not sure if this is useful. Oh, well, I finally got to a shell
> but it is unusably slow. Is there some test code that would be better to run?
> I tried looking through the mailing list archives but I couldn't find anything.
>
> thanks!
> /rex.
Ok, I have made some minor tweaks and added debug code in
do_page_fault(). Would be great if you could try on both
.31 and top of tree.
Jocke
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 4dd38f1..b3f6687 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -709,6 +709,14 @@ ret_from_except:
SYNC /* Some chip revs have problems here... */
MTMSRD(r10) /* disable interrupts */
+#ifdef CONFIG_8xx
+ /* Tag DAR with a well know value.
+ * This needs to match head_8xx.S and
+ * do_page_fault()
+ */
+ li r3, 0x00f0
+ mtspr SPRN_DAR, r3
+#endif
lwz r3,_MSR(r1) /* Returning to user mode? */
andi. r0,r3,MSR_PR
beq resume_kernel
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 52ff8c5..f9e2363 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -39,6 +39,15 @@
#else
#define DO_8xx_CPU6(val, reg)
#endif
+
+/* DAR needs to be tagged with a known value so that the
+ * DataTLB Miss/Error and do_page_fault() can recognize a
+ * buggy dcbx instruction and workaround the problem.
+ * dcbf, dcbi, dcbst, dcbz instructions do not update DAR
+ * when trapping into a Data TLB Miss/Error. See
+ * DataStoreTLBMiss and DataTLBError for details
+ */
+
__HEAD
_ENTRY(_stext);
_ENTRY(_start);
@@ -352,7 +361,7 @@ InstructionTLBMiss:
* set. All other Linux PTE bits control the behavior
* of the MMU.
*/
-2: li r11, 0x00f0
+ li r11, 0x00f0
rlwimi r10, r11, 0, 24, 28 /* Set 24-27, clear 28 */
DO_8xx_CPU6(0x2d80, r3)
mtspr SPRN_MI_RPN, r10 /* Update TLB entry */
@@ -365,6 +374,19 @@ InstructionTLBMiss:
lwz r3, 8(r0)
#endif
rfi
+2:
+ mfspr r10, SPRN_SRR1
+ rlwinm r10,r10,0,5,3 /* clear bit 4(0x08000000) */
+ mtspr SPRN_SRR1, r10
+
+ mfspr r10, SPRN_M_TW /* Restore registers */
+ lwz r11, 0(r0)
+ mtcr r11
+ lwz r11, 4(r0)
+#ifdef CONFIG_8xx_CPU6
+ lwz r3, 8(r0)
+#endif
+ b InstructionAccess
. = 0x1200
DataStoreTLBMiss:
@@ -428,10 +450,11 @@ DataStoreTLBMiss:
* set. All other Linux PTE bits control the behavior
* of the MMU.
*/
-2: li r11, 0x00f0
+ li r11, 0x00f0
rlwimi r10, r11, 0, 24, 28 /* Set 24-27, clear 28 */
DO_8xx_CPU6(0x3d80, r3)
mtspr SPRN_MD_RPN, r10 /* Update TLB entry */
+ mtspr SPRN_DAR, r11 /* Tag DAR */
mfspr r10, SPRN_M_TW /* Restore registers */
lwz r11, 0(r0)
@@ -441,7 +464,15 @@ DataStoreTLBMiss:
lwz r3, 8(r0)
#endif
rfi
-
+2:
+ mfspr r10, SPRN_M_TW /* Restore registers */
+ lwz r11, 0(r0)
+ mtcr r11
+ lwz r11, 4(r0)
+#ifdef CONFIG_8xx_CPU6
+ lwz r3, 8(r0)
+#endif
+ b DataAccess
/* This is an instruction TLB error on the MPC8xx. This could be due
* to many reasons, such as executing guarded memory or illegal instruction
* addresses. There is nothing to do but handle a big time error fault.
@@ -492,6 +523,8 @@ DataTLBError:
* assuming we only use the dcbi instruction on kernel addresses.
*/
mfspr r10, SPRN_DAR
+ cmpwi cr0, r10, 0x00f0 /* check if DAR holds a tag */
+ beq- 2f
rlwinm r11, r10, 0, 0, 19
ori r11, r11, MD_EVALID
mfspr r10, SPRN_M_CASID
@@ -550,6 +583,7 @@ DataTLBError:
rlwimi r10, r11, 0, 24, 28 /* Set 24-27, clear 28 */
DO_8xx_CPU6(0x3d80, r3)
mtspr SPRN_MD_RPN, r10 /* Update TLB entry */
+ mtspr SPRN_DAR, r11 /* Tag DAR */
mfspr r10, SPRN_M_TW /* Restore registers */
lwz r11, 0(r0)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 7699394..4fefd01 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -125,6 +125,61 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
int trap = TRAP(regs);
int is_exec = trap == 0x400;
+#if 1 /* defined(CONFIG_8xx)*/
+/*
+ Work around DTLB Miss/Error, as these do not update
+ DAR for dcbf, dcbi, dcbst, dcbz instructions
+ This relies on every exception tagging DAR with 0xf0
+ before returning (rfi)
+ DAR is passed as 'address' to this function.
+ */
+ {
+ unsigned long ra, rb, dar, insn;
+ char *istr = NULL;
+
+ insn = *((unsigned long *)regs->nip);
+ if ((insn >> (31-5)) == 31) {
+ if ((insn >> 1) == 1014) /* dcbz ? */
+ istr = "dcbz";
+ if ((insn >> 1) == 86) /* dcbf ? */
+ istr = "dcbf";
+ if ((insn >> 1) == 470) /* dcbi ? */
+ istr = "dcbi";
+ if ((insn >> 1) == 54) /* dcbst ? */
+ istr = "dcbst";
+ }
+ if (istr) {
+ ra = (insn >> (31-15)) & 0x1f; /* Reg RA */
+ rb = (insn >> (31-20)) & 0x1f; /* Reg RB */
+ printk(KERN_CRIT "%s r%ld,r%ld found at NIP:%lx\n",
+ istr, regs->nip, ra, rb);
+ printk(KERN_CRIT "DAR is:%lx (should be 0xf0)\n", regs->dar);
+
+ }
+ if (regs->dar == 0xf0 && !istr)
+ printk(KERN_CRIT "DAR is 0xf0 but insn at NIP is: %lx is"
+ "not a cache insn:%lx!\n", regs->nip, insn);
+ if (trap == 0x300 && address != regs->dar)
+ printk(KERN_CRIT "DAR:%lx != address:%lx!\n", address, regs->dar);
+
+ if (istr || (trap == 0x300 && address == 0xf0)) {
+ insn = *((unsigned long *)regs->nip);
+ /* Check if it is a
+ * dcbf, dcbi, dcbst, dcbz insn ?
+ if ((insn >> (31-5)) != 31)
+ break; // Not a dcbx instruction
+ */
+
+ ra = (insn >> (31-15)) & 0x1f; /* Reg RA */
+ rb = (insn >> (31-20)) & 0x1f; /* Reg RB */
+ dar = regs->gpr[rb];
+ if (ra)
+ dar += regs->gpr[ra];
+ regs->dar = dar;
+ address = dar;
+ }
+ }
+#endif
#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
/*
* Fortunately the bit assignments in SRR1 for an instruction
^ permalink raw reply related
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-09-30 8:19 UTC (permalink / raw)
Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OF5D06EC36.28A07DB5-ONC1257641.00299A2D-C1257641.002BE7B1@transmode.se>
>
> Rex Feany <RFeany@mrv.com> wrote on 29/09/2009 23:03:31:
> >
> > Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
> >
> > > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 29/09/2009 10:16:38:
> > > >
> > > >
> > > > > hmm, yes. You do get this and mysterious SEGV if you hit the but so does
> > > > > other bugs too so this is probably due to missing invalidation.
> > > > >
> > > > > I suspect that something like below will fix the problem and
> > > > > is the "correct" fix(untested, not even compiled):
> > > >
> > > > Ok but do we also still have to worry about the "unpopulated" TLB
> > > > entries and invalidate them somehow when populating ?
> > >
> > > Since I am probably the only one that knows about DAR problem I figured
> > > I should take a stab at it. This is not tested, but I hope Rex and the list
> > > can do that. Once this works as it should, we can remove all special handling
> > > for 8xx in copy_tofrom_user() and friends.
> > > No sign-off yet, want some confirmation first.
> >
> > It doesn't make a difference. I applied it to the top of the tree,
> > it got to userspace but it is stuck. Using break I am able to dump the
> > registers, I'm not sure if this is useful. Oh, well, I finally got to a shell
> > but it is unusably slow. Is there some test code that would be better to run?
> > I tried looking through the mailing list archives but I couldn't find anything.
> >
> > thanks!
> > /rex.
>
> Ok, I have made some minor tweaks and added debug code in
> do_page_fault(). Would be great if you could try on both
> .31 and top of tree.
>
> Jocke
OOPS, found a bug. Use this one instead:
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 4dd38f1..b3f6687 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -709,6 +709,14 @@ ret_from_except:
SYNC /* Some chip revs have problems here... */
MTMSRD(r10) /* disable interrupts */
+#ifdef CONFIG_8xx
+ /* Tag DAR with a well know value.
+ * This needs to match head_8xx.S and
+ * do_page_fault()
+ */
+ li r3, 0x00f0
+ mtspr SPRN_DAR, r3
+#endif
lwz r3,_MSR(r1) /* Returning to user mode? */
andi. r0,r3,MSR_PR
beq resume_kernel
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 52ff8c5..f9e2363 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -39,6 +39,15 @@
#else
#define DO_8xx_CPU6(val, reg)
#endif
+
+/* DAR needs to be tagged with a known value so that the
+ * DataTLB Miss/Error and do_page_fault() can recognize a
+ * buggy dcbx instruction and workaround the problem.
+ * dcbf, dcbi, dcbst, dcbz instructions do not update DAR
+ * when trapping into a Data TLB Miss/Error. See
+ * DataStoreTLBMiss and DataTLBError for details
+ */
+
__HEAD
_ENTRY(_stext);
_ENTRY(_start);
@@ -352,7 +361,7 @@ InstructionTLBMiss:
* set. All other Linux PTE bits control the behavior
* of the MMU.
*/
-2: li r11, 0x00f0
+ li r11, 0x00f0
rlwimi r10, r11, 0, 24, 28 /* Set 24-27, clear 28 */
DO_8xx_CPU6(0x2d80, r3)
mtspr SPRN_MI_RPN, r10 /* Update TLB entry */
@@ -365,6 +374,19 @@ InstructionTLBMiss:
lwz r3, 8(r0)
#endif
rfi
+2:
+ mfspr r10, SPRN_SRR1
+ rlwinm r10,r10,0,5,3 /* clear bit 4(0x08000000) */
+ mtspr SPRN_SRR1, r10
+
+ mfspr r10, SPRN_M_TW /* Restore registers */
+ lwz r11, 0(r0)
+ mtcr r11
+ lwz r11, 4(r0)
+#ifdef CONFIG_8xx_CPU6
+ lwz r3, 8(r0)
+#endif
+ b InstructionAccess
. = 0x1200
DataStoreTLBMiss:
@@ -428,10 +450,11 @@ DataStoreTLBMiss:
* set. All other Linux PTE bits control the behavior
* of the MMU.
*/
-2: li r11, 0x00f0
+ li r11, 0x00f0
rlwimi r10, r11, 0, 24, 28 /* Set 24-27, clear 28 */
DO_8xx_CPU6(0x3d80, r3)
mtspr SPRN_MD_RPN, r10 /* Update TLB entry */
+ mtspr SPRN_DAR, r11 /* Tag DAR */
mfspr r10, SPRN_M_TW /* Restore registers */
lwz r11, 0(r0)
@@ -441,7 +464,15 @@ DataStoreTLBMiss:
lwz r3, 8(r0)
#endif
rfi
-
+2:
+ mfspr r10, SPRN_M_TW /* Restore registers */
+ lwz r11, 0(r0)
+ mtcr r11
+ lwz r11, 4(r0)
+#ifdef CONFIG_8xx_CPU6
+ lwz r3, 8(r0)
+#endif
+ b DataAccess
/* This is an instruction TLB error on the MPC8xx. This could be due
* to many reasons, such as executing guarded memory or illegal instruction
* addresses. There is nothing to do but handle a big time error fault.
@@ -492,6 +523,8 @@ DataTLBError:
* assuming we only use the dcbi instruction on kernel addresses.
*/
mfspr r10, SPRN_DAR
+ cmpwi cr0, r10, 0x00f0 /* check if DAR holds a tag */
+ beq- 2f
rlwinm r11, r10, 0, 0, 19
ori r11, r11, MD_EVALID
mfspr r10, SPRN_M_CASID
@@ -550,6 +583,7 @@ DataTLBError:
rlwimi r10, r11, 0, 24, 28 /* Set 24-27, clear 28 */
DO_8xx_CPU6(0x3d80, r3)
mtspr SPRN_MD_RPN, r10 /* Update TLB entry */
+ mtspr SPRN_DAR, r11 /* Tag DAR */
mfspr r10, SPRN_M_TW /* Restore registers */
lwz r11, 0(r0)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 7699394..789b16b 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -125,6 +125,61 @@ int __kprobes do_page_fault(struct pt_regs *regs, unsigned long address,
int trap = TRAP(regs);
int is_exec = trap == 0x400;
+#if 1 /* defined(CONFIG_8xx)*/
+/*
+ Work around DTLB Miss/Error, as these do not update
+ DAR for dcbf, dcbi, dcbst, dcbz instructions
+ This relies on every exception tagging DAR with 0xf0
+ before returning (rfi)
+ DAR is passed as 'address' to this function.
+ */
+ {
+ unsigned long ra, rb, dar, insn;
+ char *istr = NULL;
+
+ insn = *((unsigned long *)regs->nip);
+ if (((insn >> (31-5)) & 0x3f) == 31) {
+ if (((insn >> 1) & 0x3ff) == 1014) /* dcbz ? 0x3f6 */
+ istr = "dcbz";
+ if (((insn >> 1) & 0x3ff) == 86) /* dcbf ? 0x56 */
+ istr = "dcbf";
+ if (((insn >> 1) & 0x3ff) == 470) /* dcbi ? 0x1d6 */
+ istr = "dcbi";
+ if (((insn >> 1) & 0x3ff) == 54) /* dcbst ? 0x36 */
+ istr = "dcbst";
+ }
+ if (istr) {
+ ra = (insn >> (31-15)) & 0x1f; /* Reg RA */
+ rb = (insn >> (31-20)) & 0x1f; /* Reg RB */
+ printk(KERN_CRIT "%s r%ld,r%ld found at NIP:%lx\n",
+ istr, regs->nip, ra, rb);
+ printk(KERN_CRIT "DAR is:%lx (should be 0xf0)\n", regs->dar);
+
+ }
+ if (regs->dar == 0x00f0 && !istr)
+ printk(KERN_CRIT "DAR is 0x00f0 but insn at NIP is: %lx is"
+ "not a cache insn:%lx!\n", regs->nip, insn);
+ if (trap == 0x300 && address != regs->dar)
+ printk(KERN_CRIT "DAR:%lx != address:%lx!\n", address, regs->dar);
+
+ if (istr || (trap == 0x300 && address == 0x00f0)) {
+ insn = *((unsigned long *)regs->nip);
+ /* Check if it is a
+ * dcbf, dcbi, dcbst, dcbz insn ?
+ if ((insn >> (31-5)) != 31)
+ break; // Not a dcbx instruction
+ */
+
+ ra = (insn >> (31-15)) & 0x1f; /* Reg RA */
+ rb = (insn >> (31-20)) & 0x1f; /* Reg RB */
+ dar = regs->gpr[rb];
+ if (ra)
+ dar += regs->gpr[ra];
+ regs->dar = dar;
+ address = dar;
+ }
+ }
+#endif
#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
/*
* Fortunately the bit assignments in SRR1 for an instruction
^ permalink raw reply related
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Rex Feany @ 2009-09-30 9:00 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <OFFEC1BDFB.17183440-ONC1257641.002D81B9-C1257641.002DC24F@transmode.se>
Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
> > Ok, I have made some minor tweaks and added debug code in
> > do_page_fault(). Would be great if you could try on both
> > .31 and top of tree.
> >
> > Jocke
>
> OOPS, found a bug. Use this one instead:
.31 - no change, it worked before your patch and it works after.
None of your debugging printks show up. I tried removing the tlbil_va()
from do_dcache_icache_coherency() and userspace goes back to be being
slow/non functional.
top of tree - userspace is slow and unstable, random segfaults,
and none of your debugging printks show up :(
take care!
/rex
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-09-30 9:58 UTC (permalink / raw)
To: Rex Feany; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <20090930090002.GA2928@compile2.chatsunix.int.mrv.com>
Rex Feany <RFeany@mrv.com> wrote on 30/09/2009 11:00:02:
>
> Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
>
> > > Ok, I have made some minor tweaks and added debug code in
> > > do_page_fault(). Would be great if you could try on both
> > > .31 and top of tree.
> > >
> > > Jocke
> >
> > OOPS, found a bug. Use this one instead:
>
> .31 - no change, it worked before your patch and it works after.
> None of your debugging printks show up. I tried removing the tlbil_va()
> from do_dcache_icache_coherency() and userspace goes back to be being
> slow/non functional.
>
> top of tree - userspace is slow and unstable, random segfaults,
> and none of your debugging printks show up :(
OK, something strange is going on. I am starting to suspect that there
is some other problem here.
If my patch is any good you should be able to use dcbx insn in copy_tofrom_user().
You could try changing all CONFIG_8xx to CONFIG_8xx_deleted arch/powerpc/lib/copy_32.S
and see if that works and if you see any debug prints.
Jocke
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: fix regression introduced by cache coherency rewrite
From: Joakim Tjernlund @ 2009-09-30 11:18 UTC (permalink / raw)
To: Rex Feany; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <20090930090002.GA2928@compile2.chatsunix.int.mrv.com>
Rex Feany <RFeany@mrv.com> wrote on 30/09/2009 11:00:02:
>
> Thus spake Joakim Tjernlund (joakim.tjernlund@transmode.se):
>
> > > Ok, I have made some minor tweaks and added debug code in
> > > do_page_fault(). Would be great if you could try on both
> > > .31 and top of tree.
> > >
> > > Jocke
> >
> > OOPS, found a bug. Use this one instead:
>
> .31 - no change, it worked before your patch and it works after.
> None of your debugging printks show up. I tried removing the tlbil_va()
> from do_dcache_icache_coherency() and userspace goes back to be being
> slow/non functional.
Had a look at linus tree and there is something I don't understand.
Your fix, e0908085fc2391c85b85fb814ae1df377c8e0dcb, fixes a problem
that was introduced by 8d30c14cab30d405a05f2aaceda1e9ad57800f36 but
8d30c14cab30d405a05f2aaceda1e9ad57800f36 was included in .31 and .31
works and top of tree does not, how can that be?
To me it seems more likely that some mm change introduced between .31 and
top of tree is the culprit.
My patch addresses the problem described in the comment:
/* On 8xx, cache control instructions (particularly
* "dcbst" from flush_dcache_icache) fault as write
* operation if there is an unpopulated TLB entry
* for the address in question. To workaround that,
* we invalidate the TLB here, thus avoiding dcbst
* misbehaviour.
*/
Now you are using this old fix to paper over some other bug or so I think.
Jocke
^ permalink raw reply
* [PATCH] macintosh: Don't assume i2c device probing always succeeds
From: Jean Delvare @ 2009-09-30 13:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras
Cc: Colin Leroy, linuxppc-dev, Tim Shepard
If i2c device probing fails, then there is no driver to dereference
after calling i2c_new_device(). Stop assuming that probing will always
succeed, to avoid NULL pointer dereferences. We have an easier access
to the driver anyway.
Signed-off-by: Jean Delvare <khali@linux-fr.org>
Tested-by: Tim Shepard <shep@alum.mit.edu>
Cc: Colin Leroy <colin@colino.net>
---
This fixes a real-world oops and should be pushed to Linus ASAP, thanks.
drivers/macintosh/therm_adt746x.c | 4 +++-
drivers/macintosh/therm_pm72.c | 4 +++-
drivers/macintosh/windfarm_lm75_sensor.c | 4 +++-
drivers/macintosh/windfarm_max6690_sensor.c | 4 +++-
drivers/macintosh/windfarm_smu_sat.c | 4 +++-
5 files changed, 15 insertions(+), 5 deletions(-)
--- linux-2.6.32-rc1.orig/drivers/macintosh/therm_adt746x.c 2009-09-30 15:12:12.000000000 +0200
+++ linux-2.6.32-rc1/drivers/macintosh/therm_adt746x.c 2009-09-30 15:13:04.000000000 +0200
@@ -124,6 +124,8 @@ read_reg(struct thermostat* th, int reg)
return data;
}
+static struct i2c_driver thermostat_driver;
+
static int
attach_thermostat(struct i2c_adapter *adapter)
{
@@ -148,7 +150,7 @@ attach_thermostat(struct i2c_adapter *ad
* Let i2c-core delete that device on driver removal.
* This is safe because i2c-core holds the core_lock mutex for us.
*/
- list_add_tail(&client->detected, &client->driver->clients);
+ list_add_tail(&client->detected, &thermostat_driver.clients);
return 0;
}
--- linux-2.6.32-rc1.orig/drivers/macintosh/therm_pm72.c 2009-09-30 15:12:12.000000000 +0200
+++ linux-2.6.32-rc1/drivers/macintosh/therm_pm72.c 2009-09-30 15:13:04.000000000 +0200
@@ -286,6 +286,8 @@ struct fcu_fan_table fcu_fans[] = {
},
};
+static struct i2c_driver therm_pm72_driver;
+
/*
* Utility function to create an i2c_client structure and
* attach it to one of u3 adapters
@@ -318,7 +320,7 @@ static struct i2c_client *attach_i2c_chi
* Let i2c-core delete that device on driver removal.
* This is safe because i2c-core holds the core_lock mutex for us.
*/
- list_add_tail(&clt->detected, &clt->driver->clients);
+ list_add_tail(&clt->detected, &therm_pm72_driver.clients);
return clt;
}
--- linux-2.6.32-rc1.orig/drivers/macintosh/windfarm_lm75_sensor.c 2009-09-30 15:12:12.000000000 +0200
+++ linux-2.6.32-rc1/drivers/macintosh/windfarm_lm75_sensor.c 2009-09-30 15:13:04.000000000 +0200
@@ -115,6 +115,8 @@ static int wf_lm75_probe(struct i2c_clie
return rc;
}
+static struct i2c_driver wf_lm75_driver;
+
static struct i2c_client *wf_lm75_create(struct i2c_adapter *adapter,
u8 addr, int ds1775,
const char *loc)
@@ -157,7 +159,7 @@ static struct i2c_client *wf_lm75_create
* Let i2c-core delete that device on driver removal.
* This is safe because i2c-core holds the core_lock mutex for us.
*/
- list_add_tail(&client->detected, &client->driver->clients);
+ list_add_tail(&client->detected, &wf_lm75_driver.clients);
return client;
fail:
return NULL;
--- linux-2.6.32-rc1.orig/drivers/macintosh/windfarm_max6690_sensor.c 2009-09-30 15:12:12.000000000 +0200
+++ linux-2.6.32-rc1/drivers/macintosh/windfarm_max6690_sensor.c 2009-09-30 15:13:04.000000000 +0200
@@ -88,6 +88,8 @@ static int wf_max6690_probe(struct i2c_c
return rc;
}
+static struct i2c_driver wf_max6690_driver;
+
static struct i2c_client *wf_max6690_create(struct i2c_adapter *adapter,
u8 addr, const char *loc)
{
@@ -119,7 +121,7 @@ static struct i2c_client *wf_max6690_cre
* Let i2c-core delete that device on driver removal.
* This is safe because i2c-core holds the core_lock mutex for us.
*/
- list_add_tail(&client->detected, &client->driver->clients);
+ list_add_tail(&client->detected, &wf_max6690_driver.clients);
return client;
fail:
--- linux-2.6.32-rc1.orig/drivers/macintosh/windfarm_smu_sat.c 2009-09-30 15:12:12.000000000 +0200
+++ linux-2.6.32-rc1/drivers/macintosh/windfarm_smu_sat.c 2009-09-30 15:13:04.000000000 +0200
@@ -194,6 +194,8 @@ static struct wf_sensor_ops wf_sat_ops =
.owner = THIS_MODULE,
};
+static struct i2c_driver wf_sat_driver;
+
static void wf_sat_create(struct i2c_adapter *adapter, struct device_node *dev)
{
struct i2c_board_info info;
@@ -222,7 +224,7 @@ static void wf_sat_create(struct i2c_ada
* Let i2c-core delete that device on driver removal.
* This is safe because i2c-core holds the core_lock mutex for us.
*/
- list_add_tail(&client->detected, &client->driver->clients);
+ list_add_tail(&client->detected, &wf_sat_driver.clients);
}
static int wf_sat_probe(struct i2c_client *client,
--
Jean Delvare
^ permalink raw reply
* [PATCH] sound: Don't assume i2c device probing always succeeds
From: Jean Delvare @ 2009-09-30 13:25 UTC (permalink / raw)
To: Johannes Berg, Jaroslav Kysela, Takashi Iwai
Cc: linuxppc-dev, alsa-devel, Tim Shepard
If i2c device probing fails, then there is no driver to dereference
after calling i2c_new_device(). Stop assuming that probing will always
succeed, to avoid NULL pointer dereferences. We have an easier access
to the driver anyway.
Reported-by: Tim Shepard <shep@alum.mit.edu>
Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
---
The code is similar to the one in therm_adt746x, for which Tim reported
a real-world oops, so it should be fixed ASAP.
sound/aoa/codecs/onyx.c | 4 +++-
sound/aoa/codecs/tas.c | 4 +++-
sound/ppc/keywest.c | 4 +++-
3 files changed, 9 insertions(+), 3 deletions(-)
--- linux-2.6.32-rc1.orig/sound/aoa/codecs/onyx.c 2009-09-30 15:13:12.000000000 +0200
+++ linux-2.6.32-rc1/sound/aoa/codecs/onyx.c 2009-09-30 15:13:58.000000000 +0200
@@ -996,6 +996,8 @@ static void onyx_exit_codec(struct aoa_c
onyx->codec.soundbus_dev->detach_codec(onyx->codec.soundbus_dev, onyx);
}
+static struct i2c_driver onyx_driver;
+
static int onyx_create(struct i2c_adapter *adapter,
struct device_node *node,
int addr)
@@ -1027,7 +1029,7 @@ static int onyx_create(struct i2c_adapte
* Let i2c-core delete that device on driver removal.
* This is safe because i2c-core holds the core_lock mutex for us.
*/
- list_add_tail(&client->detected, &client->driver->clients);
+ list_add_tail(&client->detected, &onyx_driver.clients);
return 0;
}
--- linux-2.6.32-rc1.orig/sound/aoa/codecs/tas.c 2009-09-30 15:13:12.000000000 +0200
+++ linux-2.6.32-rc1/sound/aoa/codecs/tas.c 2009-09-30 15:13:58.000000000 +0200
@@ -882,6 +882,8 @@ static void tas_exit_codec(struct aoa_co
}
+static struct i2c_driver tas_driver;
+
static int tas_create(struct i2c_adapter *adapter,
struct device_node *node,
int addr)
@@ -902,7 +904,7 @@ static int tas_create(struct i2c_adapter
* Let i2c-core delete that device on driver removal.
* This is safe because i2c-core holds the core_lock mutex for us.
*/
- list_add_tail(&client->detected, &client->driver->clients);
+ list_add_tail(&client->detected, &tas_driver.clients);
return 0;
}
--- linux-2.6.32-rc1.orig/sound/ppc/keywest.c 2009-09-30 15:13:12.000000000 +0200
+++ linux-2.6.32-rc1/sound/ppc/keywest.c 2009-09-30 15:13:58.000000000 +0200
@@ -40,6 +40,8 @@ static int keywest_probe(struct i2c_clie
return 0;
}
+struct i2c_driver keywest_driver;
+
/*
* This is kind of a hack, best would be to turn powermac to fixed i2c
* bus numbers and declare the sound device as part of platform
@@ -65,7 +67,7 @@ static int keywest_attach_adapter(struct
* This is safe because i2c-core holds the core_lock mutex for us.
*/
list_add_tail(&keywest_ctx->client->detected,
- &keywest_ctx->client->driver->clients);
+ &keywest_driver.clients);
return 0;
}
--
Jean Delvare
^ permalink raw reply
* Re: [patch] powerpc: build modules outside the kernel tree fails, if it was built using O=
From: Yuri Frolov @ 2009-09-30 13:50 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: rep.dot.nop, linuxppc-dev, linux-kbuild
In-Reply-To: <4ABE0CFF.5040001@ru.mvista.com>
Hello,
Any news yet?
Any sort of correction needed?
Thank you,
Yuri
On 09/26/2009 04:45 PM, Yuri Frolov wrote:
> Hello, here is a fixed version.
>
> Compile and export arch/powerpc/lib/crtsavres.o in order to fix the
> "arch/powerpc/lib/crtsavres.o not found" error when "O=" option
> is employed for external module compilation.
> crtsavres.o is a support file, containing save/restore code from gcc,
> simplified down for powerpc architecture needs.
> This file needs to be linked against every module and thus to be built
> before any module.
>
> Documentation/kbuild/kbuild.txt | 8 ++++++++
> Makefile | 2 +-
> arch/powerpc/Makefile | 2 +-
> scripts/Makefile.modpost | 14 ++++++++++++--
> 4 files changed, 22 insertions(+), 4 deletions(-)
>
> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/arch/powerpc/Makefile linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile
> --- linux-2.6/arch/powerpc/Makefile 2009-09-17 20:04:31.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile 2009-09-26 13:35:32.000000000 +0400
> @@ -93,7 +93,7 @@ else
> KBUILD_CFLAGS += $(call cc-option,-mtune=power4)
> endif
> else
> -LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> +KBUILD_MODULE_LINK_SOURCE += arch/powerpc/lib/crtsavres.o
> endif
>
> ifeq ($(CONFIG_TUNE_CELL),y)
> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/Documentation/kbuild/kbuild.txt linux-2.6-powerpc-crtsavres/Documentation/kbuild/kbuild.txt
> --- linux-2.6/Documentation/kbuild/kbuild.txt 2009-09-17 20:04:30.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/Documentation/kbuild/kbuild.txt 2009-09-26 16:03:50.000000000 +0400
> @@ -132,3 +132,11 @@ For tags/TAGS/cscope targets, you can sp
> to be included in the databases, separated by blank space. E.g.:
>
> $ make ALLSOURCE_ARCHS="x86 mips arm" tags
> +
> +KBUILD_MODULE_LINK_SOURCE
> +--------------------------------------------------
> +Compile and export arch/powerpc/lib/crtsavres.o
> +when "O=" option is employed for powerpc external module compilation.
> +This file needs to be linked against every module and thus to be built
> +before any module.
> +
> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/Makefile linux-2.6-powerpc-crtsavres/Makefile
> --- linux-2.6/Makefile 2009-09-17 20:04:30.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/Makefile 2009-09-26 14:23:27.000000000 +0400
> @@ -354,7 +354,7 @@ KERNELVERSION = $(VERSION).$(PATCHLEVEL)
> export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
> export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
> export CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE
> -export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
> +export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS KBUILD_MODULE_LINK_SOURCE
>
> export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
> export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE CFLAGS_GCOV
> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/scripts/Makefile.modpost linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost
> --- linux-2.6/scripts/Makefile.modpost 2009-09-17 20:04:42.000000000 +0400
> +++ linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost 2009-09-26 14:34:28.000000000 +0400
> @@ -122,14 +122,24 @@ quiet_cmd_cc_o_c = CC $@
> cmd_cc_o_c = $(CC) $(c_flags) $(CFLAGS_MODULE) \
> -c -o $@ $<
>
> -$(modules:.ko=.mod.o): %.mod.o: %.mod.c FORCE
> +quiet_cmd_as_o_S = AS $(quiet_modtag) $@
> + cmd_as_o_S = $(CC) $(a_flags) $(AFLAGS_MODULE) -c -o $@ $<
> +
> +ifdef KBUILD_MODULE_LINK_SOURCE
> +$(KBUILD_MODULE_LINK_SOURCE): %.o: %.S FORCE
> + $(Q)mkdir -p $(dir $@)
> + $(call if_changed_dep,as_o_S)
> +endif
> +
> +$(modules:.ko=.mod.o): %.mod.o: %.mod.c $(KBUILD_MODULE_LINK_SOURCE) FORCE
> $(call if_changed_dep,cc_o_c)
>
> targets += $(modules:.ko=.mod.o)
>
> # Step 6), final link of the modules
> quiet_cmd_ld_ko_o = LD [M] $@
> - cmd_ld_ko_o = $(LD) -r $(LDFLAGS) $(LDFLAGS_MODULE) -o $@ \
> + cmd_ld_ko_o = $(LD) -r $(LDFLAGS) $(KBUILD_MODULE_LINK_SOURCE) \
> + $(LDFLAGS_MODULE) -o $@ \
> $(filter-out FORCE,$^)
>
> $(modules): %.ko :%.o %.mod.o FORCE
>
> On 09/25/2009 11:45 PM, Sam Ravnborg wrote:
>> On Thu, Sep 24, 2009 at 03:28:11PM +0400, Yuri Frolov wrote:
>>> Hello,
>>>
>>> here is a corresponding bug: http://bugzilla.kernel.org/show_bug.cgi?id=11143
>>> This patch should correctly export crtsavres.o in order to make O= option working.
>>> Please, consider to apply.
>> Hi Yuri.
>>
>> I like the way you do the extra link in Makefile.modpost.
>> But you need to redo some parts as per comments below.
>>
>>> Fix linking modules against crtsavres.o
>> Please elaborate more on what this commit does.
>>
>>> Previously we got
>>> CC drivers/char/hw_random/rng-core.mod.o
>>> LD [M] drivers/char/hw_random/rng-core.ko
>>> /there/src/buildroot.git.ppc/build_powerpc_nofpu/staging_dir/usr/bin/powerpc-linux-uclibc-ld: arch/powerpc/lib/crtsavres.o: No such file: No such file or directory
>> Always good to include error messages.
>>
>>> * Makefile (LDFLAGS_MODULE_PREREQ): New variable to hold prerequisite
>>> files for modules.
>>> * arch/powerpc/Makefile: add crtsavres.o to LDFLAGS_MODULE_PREREQ.
>>> * scripts/Makefile.modpost (cmd_as_o_S): Copy from Makefile.build.
>>> (cmd_ld_ko_o): Also link LDFLAGS_MODULE_PREREQ.
>>> Provide rule to build objects from assembler.
>> But this GNUism can go - we do not use it in the kernel.
>>
>>> Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
>>> Signed-off by: Yuri Frolov <yfrolov@ru.mvista.com>
>>>
>>> Makefile | 2 ++
>>> arch/powerpc/Makefile | 2 +-
>>> scripts/Makefile.modpost | 12 ++++++++++--
>>> 3 files changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff -urpN -X linux-2.6/Documentation/dontdiff linux-2.6/arch/powerpc/Makefile linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile
>>> --- linux-2.6/arch/powerpc/Makefile 2009-09-17 20:04:31.000000000 +0400
>>> +++ linux-2.6-powerpc-crtsavres/arch/powerpc/Makefile 2009-09-23 22:08:03.000000000 +0400
>>> @@ -93,7 +93,7 @@ else
>>> KBUILD_CFLAGS += $(call cc-option,-mtune=power4)
>>> endif
>>> else
>>> -LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
>>> +LDFLAGS_MODULE_PREREQ += arch/powerpc/lib/crtsavres.o
>>> endif
>> The naming sucks.
>> How about:
>>
>> KBUILD_MODULE_LINK_SOURCE
>>
>> This would tell the reader that this is source to be linked on a module.
>>
>> And this is an arch specific thing so no need to preset it in top-level
>> Makefile.
>> But it is mandatory to include a description in Documentation/kbuild/kbuild.txt
>>
>>
>>> --- linux-2.6/scripts/Makefile.modpost 2009-09-17 20:04:42.000000000 +0400
>>> +++ linux-2.6-powerpc-crtsavres/scripts/Makefile.modpost 2009-09-23 22:15:00.000000000 +0400
>>> @@ -122,14 +122,22 @@ quiet_cmd_cc_o_c = CC $@
>>> cmd_cc_o_c = $(CC) $(c_flags) $(CFLAGS_MODULE) \
>>> -c -o $@ $<
>>>
>>> -$(modules:.ko=.mod.o): %.mod.o: %.mod.c FORCE
>>> +quiet_cmd_as_o_S = AS $(quiet_modtag) $@
>>> +cmd_as_o_S = $(CC) $(a_flags) $(AFLAGS_MODULE) -c -o $@ $<
>> Align this so cmd_as_o_S is under each other - as we do for cmd_cc_o_c
>>
>>
>>> +
>>> +$(LDFLAGS_MODULE_PREREQ): %.o: %.S FORCE
>>> + $(Q)mkdir -p $(dir $@)
>>> + $(call if_changed_dep,as_o_S)
>> Good catch with the mkdir - needed for O= builds.
>> I think we shall wrap this in
>> ifdef KBUILD_MODULE_LINK_SOURCE
>> ...
>> endif
>>
>> So we do not have an empty rule when it is not defined.
>>
>> Please fix up these things and resubmit.
>>
>> Thanks,
>> Sam
>
^ permalink raw reply
* Re: 64bit kernel is huge
From: Arnd Bergmann @ 2009-09-30 13:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard
In-Reply-To: <20090928074503.GB16073@kryten>
On Monday 28 September 2009, Anton Blanchard wrote:
> 262144 kstat_irqs_all
> 131072 irq_desc
> 16384 irq_stat
>
> Could we dynamically allocate our irq structures?
There were patches floating around for that a few years ago,
but I haven't seen anyone working on it since.
> 131072 lppaca
> 65536 paca
>
> I think we've attacked these before, not sure if there is anything left
> we can trim.
well, it essentially comes down to CONFIG_NR_CPUS. If you build with NR_CPUS=4,
this is really small.
> 131072 __log_buf
>
> Looks like we can dynamically allocate a large log buf at runtime. Perhaps
> we should default to a small log_buf and grow it at boot based on machine size
> (eg max cpus).
I like the idea of being able to very easily find the log buffer in
dump files or in hw debuggers for bringup, but it's probably not
worth it for production systems. A config option might be nice.
> 101160 powerpc_opcodes
>
> xmon instruction disassembly. I'd probably prefer to cut off my right hand and
> debug one handed before losing this though.
This should be very compressible though, which will help binary size, but not
runtime memory footprint.
> 77452 _fw_acenic_tg2_bin_bin
>
> We could probably change acenic to be a module,
CONFIG_FIRMWARE_IN_KERNEL ?
> 17784 ioctl_start
I have a patch for this one that I need to submit some day.
Arnd <><
^ permalink raw reply
* Re: [PATCH] sound: Don't assume i2c device probing always succeeds
From: Takashi Iwai @ 2009-09-30 14:13 UTC (permalink / raw)
To: Jean Delvare
Cc: linuxppc-dev, Johannes Berg, Tim Shepard, alsa-devel,
Jaroslav Kysela
In-Reply-To: <20090930152542.3ef753ee@hyperion.delvare>
At Wed, 30 Sep 2009 15:25:42 +0200,
Jean Delvare wrote:
>
> If i2c device probing fails, then there is no driver to dereference
> after calling i2c_new_device(). Stop assuming that probing will always
> succeed, to avoid NULL pointer dereferences. We have an easier access
> to the driver anyway.
>
> Reported-by: Tim Shepard <shep@alum.mit.edu>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> ---
> The code is similar to the one in therm_adt746x, for which Tim reported
> a real-world oops, so it should be fixed ASAP.
Jean, thanks for the patch.
I'm just wondering whether the additional NULL check of client->driver
would be enough? If yes, sound/aoa/onyx.c has it, at least, and we
can add the similar checks to the rest, too.
Takashi
>
> sound/aoa/codecs/onyx.c | 4 +++-
> sound/aoa/codecs/tas.c | 4 +++-
> sound/ppc/keywest.c | 4 +++-
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> --- linux-2.6.32-rc1.orig/sound/aoa/codecs/onyx.c 2009-09-30 15:13:12.000000000 +0200
> +++ linux-2.6.32-rc1/sound/aoa/codecs/onyx.c 2009-09-30 15:13:58.000000000 +0200
> @@ -996,6 +996,8 @@ static void onyx_exit_codec(struct aoa_c
> onyx->codec.soundbus_dev->detach_codec(onyx->codec.soundbus_dev, onyx);
> }
>
> +static struct i2c_driver onyx_driver;
> +
> static int onyx_create(struct i2c_adapter *adapter,
> struct device_node *node,
> int addr)
> @@ -1027,7 +1029,7 @@ static int onyx_create(struct i2c_adapte
> * Let i2c-core delete that device on driver removal.
> * This is safe because i2c-core holds the core_lock mutex for us.
> */
> - list_add_tail(&client->detected, &client->driver->clients);
> + list_add_tail(&client->detected, &onyx_driver.clients);
> return 0;
> }
>
> --- linux-2.6.32-rc1.orig/sound/aoa/codecs/tas.c 2009-09-30 15:13:12.000000000 +0200
> +++ linux-2.6.32-rc1/sound/aoa/codecs/tas.c 2009-09-30 15:13:58.000000000 +0200
> @@ -882,6 +882,8 @@ static void tas_exit_codec(struct aoa_co
> }
>
>
> +static struct i2c_driver tas_driver;
> +
> static int tas_create(struct i2c_adapter *adapter,
> struct device_node *node,
> int addr)
> @@ -902,7 +904,7 @@ static int tas_create(struct i2c_adapter
> * Let i2c-core delete that device on driver removal.
> * This is safe because i2c-core holds the core_lock mutex for us.
> */
> - list_add_tail(&client->detected, &client->driver->clients);
> + list_add_tail(&client->detected, &tas_driver.clients);
> return 0;
> }
>
> --- linux-2.6.32-rc1.orig/sound/ppc/keywest.c 2009-09-30 15:13:12.000000000 +0200
> +++ linux-2.6.32-rc1/sound/ppc/keywest.c 2009-09-30 15:13:58.000000000 +0200
> @@ -40,6 +40,8 @@ static int keywest_probe(struct i2c_clie
> return 0;
> }
>
> +struct i2c_driver keywest_driver;
> +
> /*
> * This is kind of a hack, best would be to turn powermac to fixed i2c
> * bus numbers and declare the sound device as part of platform
> @@ -65,7 +67,7 @@ static int keywest_attach_adapter(struct
> * This is safe because i2c-core holds the core_lock mutex for us.
> */
> list_add_tail(&keywest_ctx->client->detected,
> - &keywest_ctx->client->driver->clients);
> + &keywest_driver.clients);
> return 0;
> }
>
>
>
> --
> Jean Delvare
>
^ permalink raw reply
* Re: [PATCH] sound: Don't assume i2c device probing always succeeds
From: Jean Delvare @ 2009-09-30 15:00 UTC (permalink / raw)
To: Takashi Iwai
Cc: linuxppc-dev, Johannes Berg, Tim Shepard, alsa-devel,
Jaroslav Kysela
In-Reply-To: <s5hbpksy0yq.wl%tiwai@suse.de>
Hi Takashi,
Thanks for the swift reply.
On Wed, 30 Sep 2009 16:13:49 +0200, Takashi Iwai wrote:
> At Wed, 30 Sep 2009 15:25:42 +0200,
> Jean Delvare wrote:
> >
> > If i2c device probing fails, then there is no driver to dereference
> > after calling i2c_new_device(). Stop assuming that probing will always
> > succeed, to avoid NULL pointer dereferences. We have an easier access
> > to the driver anyway.
> >
> > Reported-by: Tim Shepard <shep@alum.mit.edu>
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Johannes Berg <johannes@sipsolutions.net>
> > ---
> > The code is similar to the one in therm_adt746x, for which Tim reported
> > a real-world oops, so it should be fixed ASAP.
>
> Jean, thanks for the patch.
>
> I'm just wondering whether the additional NULL check of client->driver
> would be enough? If yes, sound/aoa/onyx.c has it, at least, and we
> can add the similar checks to the rest, too.
The NULL check of client->driver, if followed by a call to
i2c_unregister_device(), would indeed be enough. But unlike the onyx
driver which we know we sometimes load erroneously, the other drivers
should never fail. I am reluctant to add code to handle error cases
which are not supposed to happen, which is why I prefer my proposed
fix: it does not add code.
That being said, I will be happy with any solution that fixes the
problem, so if you prefer client->driver NULL checks, I can send a
patch doing this.
Thanks,
--
Jean Delvare
^ permalink raw reply
* Re: [PATCH] sound: Don't assume i2c device probing always succeeds
From: Takashi Iwai @ 2009-09-30 15:15 UTC (permalink / raw)
To: Jean Delvare
Cc: linuxppc-dev, Johannes Berg, Tim Shepard, alsa-devel,
Jaroslav Kysela
In-Reply-To: <20090930170006.76e145ca@hyperion.delvare>
At Wed, 30 Sep 2009 17:00:06 +0200,
Jean Delvare wrote:
>
> Hi Takashi,
>
> Thanks for the swift reply.
>
> On Wed, 30 Sep 2009 16:13:49 +0200, Takashi Iwai wrote:
> > At Wed, 30 Sep 2009 15:25:42 +0200,
> > Jean Delvare wrote:
> > >
> > > If i2c device probing fails, then there is no driver to dereference
> > > after calling i2c_new_device(). Stop assuming that probing will always
> > > succeed, to avoid NULL pointer dereferences. We have an easier access
> > > to the driver anyway.
> > >
> > > Reported-by: Tim Shepard <shep@alum.mit.edu>
> > > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > > Cc: Johannes Berg <johannes@sipsolutions.net>
> > > ---
> > > The code is similar to the one in therm_adt746x, for which Tim reported
> > > a real-world oops, so it should be fixed ASAP.
> >
> > Jean, thanks for the patch.
> >
> > I'm just wondering whether the additional NULL check of client->driver
> > would be enough? If yes, sound/aoa/onyx.c has it, at least, and we
> > can add the similar checks to the rest, too.
>
> The NULL check of client->driver, if followed by a call to
> i2c_unregister_device(), would indeed be enough. But unlike the onyx
> driver which we know we sometimes load erroneously, the other drivers
> should never fail. I am reluctant to add code to handle error cases
> which are not supposed to happen, which is why I prefer my proposed
> fix: it does not add code.
>
> That being said, I will be happy with any solution that fixes the
> problem, so if you prefer client->driver NULL checks, I can send a
> patch doing this.
Yes, indeed I prefer NULL check because the user can know the error
at the right place. I share your concern about the code addition,
though :)
I already made a patch below, but it's totally untested.
It'd be helpful if someone can do review and build-test it.
thanks,
Takashi
---
diff --git a/sound/aoa/codecs/tas.c b/sound/aoa/codecs/tas.c
index f0ebc97..0f810c8 100644
--- a/sound/aoa/codecs/tas.c
+++ b/sound/aoa/codecs/tas.c
@@ -897,6 +897,10 @@ static int tas_create(struct i2c_adapter *adapter,
client = i2c_new_device(adapter, &info);
if (!client)
return -ENODEV;
+ if (!client->driver) {
+ i2c_unregister_device(client);
+ return -ENODEV;
+ }
/*
* Let i2c-core delete that device on driver removal.
diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
index 835fa19..473c5a6 100644
--- a/sound/ppc/keywest.c
+++ b/sound/ppc/keywest.c
@@ -59,6 +59,13 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
strlcpy(info.type, "keywest", I2C_NAME_SIZE);
info.addr = keywest_ctx->addr;
keywest_ctx->client = i2c_new_device(adapter, &info);
+ if (!keywest_ctx->client)
+ return -ENODEV;
+ if (!keywest_ctx->client->driver) {
+ i2c_unregister_device(keywest_ctx->client);
+ keywest_ctx->client = NULL;
+ return -ENODEV;
+ }
/*
* Let i2c-core delete that device on driver removal.
^ permalink raw reply related
* Re: [PATCH] sound: Don't assume i2c device probing always succeeds
From: Johannes Berg @ 2009-09-30 15:05 UTC (permalink / raw)
To: Jean Delvare
Cc: Takashi Iwai, linuxppc-dev, alsa-devel, Tim Shepard,
Jaroslav Kysela
In-Reply-To: <20090930170006.76e145ca@hyperion.delvare>
[-- Attachment #1: Type: text/plain, Size: 405 bytes --]
On Wed, 2009-09-30 at 17:00 +0200, Jean Delvare wrote:
> The NULL check of client->driver, if followed by a call to
> i2c_unregister_device(), would indeed be enough. But unlike the onyx
> driver which we know we sometimes load erroneously, the other drivers
> should never fail.
All of these drivers can be loaded manually and then fail though, or am
I misunderstanding something?
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH] sound: Don't assume i2c device probing always succeeds
From: Jean Delvare @ 2009-09-30 15:57 UTC (permalink / raw)
To: Johannes Berg
Cc: Takashi Iwai, linuxppc-dev, alsa-devel, Tim Shepard,
Jaroslav Kysela
In-Reply-To: <1254323111.3959.6.camel@johannes.local>
On Wed, 30 Sep 2009 17:05:11 +0200, Johannes Berg wrote:
> On Wed, 2009-09-30 at 17:00 +0200, Jean Delvare wrote:
>
> > The NULL check of client->driver, if followed by a call to
> > i2c_unregister_device(), would indeed be enough. But unlike the onyx
> > driver which we know we sometimes load erroneously, the other drivers
> > should never fail.
>
> All of these drivers can be loaded manually and then fail though, or am
> I misunderstanding something?
I don't think so. At least tas and keywest have checks before they
attempt to instantiate an i2c client, which I think are reliable. The
onyx case is different because apparently some machines have it but are
difficult to detect:
/* if that didn't work, try desperate mode for older
* machines that have stuff missing from the device tree */
And then we have to attempt to instantiate i2c devices at a not
completely known address, and that may fail. I think this is the reason
why onyx has this extra client->driver NULL check and the other two
drivers do not.
I would really love if someone with good knowledge of the device tree
on mac would convert all these hacks to proper i2c device declarations.
All the infrastructure is available already, but I don't know enough
about open firmware and mac the device tree to do it myself.
--
Jean Delvare
^ permalink raw reply
* Re: [PATCH] sound: Don't assume i2c device probing always succeeds
From: Jean Delvare @ 2009-09-30 16:55 UTC (permalink / raw)
To: Takashi Iwai
Cc: linuxppc-dev, Johannes Berg, Tim Shepard, alsa-devel,
Jaroslav Kysela
In-Reply-To: <s5h4oqkxy3e.wl%tiwai@suse.de>
On Wed, 30 Sep 2009 17:15:49 +0200, Takashi Iwai wrote:
> Yes, indeed I prefer NULL check because the user can know the error
> at the right place. I share your concern about the code addition,
> though :)
>
> I already made a patch below, but it's totally untested.
> It'd be helpful if someone can do review and build-test it.
>
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/sound/aoa/codecs/tas.c b/sound/aoa/codecs/tas.c
> index f0ebc97..0f810c8 100644
> --- a/sound/aoa/codecs/tas.c
> +++ b/sound/aoa/codecs/tas.c
> @@ -897,6 +897,10 @@ static int tas_create(struct i2c_adapter *adapter,
> client = i2c_new_device(adapter, &info);
> if (!client)
> return -ENODEV;
> + if (!client->driver) {
> + i2c_unregister_device(client);
> + return -ENODEV;
> + }
>
> /*
> * Let i2c-core delete that device on driver removal.
> diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
> index 835fa19..473c5a6 100644
> --- a/sound/ppc/keywest.c
> +++ b/sound/ppc/keywest.c
> @@ -59,6 +59,13 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter)
> strlcpy(info.type, "keywest", I2C_NAME_SIZE);
> info.addr = keywest_ctx->addr;
> keywest_ctx->client = i2c_new_device(adapter, &info);
> + if (!keywest_ctx->client)
> + return -ENODEV;
> + if (!keywest_ctx->client->driver) {
> + i2c_unregister_device(keywest_ctx->client);
> + keywest_ctx->client = NULL;
> + return -ENODEV;
> + }
>
> /*
> * Let i2c-core delete that device on driver removal.
This looks good to me. Please add the following comment before the
client->driver check in both drivers:
/*
* We know the driver is already loaded, so the device should be
* already bound. If not it means binding failed, and then there
* is no point in keeping the device instantiated.
*/
Otherwise it's a little difficult to understand why the check is there.
--
Jean Delvare
^ 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