Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood scottw...@freescale.com wrote on 10/11/2009 00:00:04: On Mon, Nov 09, 2009 at 03:53:21PM -0600, Scott Wood wrote: On Fri, Nov 06, 2009 at 10:29:44AM +0100, Joakim Tjernlund wrote: With this, the kernel hangs after Mount-cache hash table entries: 512. Somewhat surprising result. I didn't expect you would even hit this condition now as we haven't enabled the use of dcbX insn yet. The only thing I can think of is the you hit the 0x00f0 due to other dcbX insn use and the kernel managed to fixup this in the page fault handler by pure luck before. It's bizarre -- it still happens even if I revert the branch to FixupDAR. However, if I comment out the final b 151b, it stops happening. It also stops happening sometimes depending on where I stick printk()s to debug this. So it may be an unrelated issue that just got perturbed somehow. OK, figured it out. The fixup code pushed things around so that in Cool! syscall_exit_cont, SRR0/SRR1 were being loaded immediately prior to a page boundary, with the rfi after the page boundary. On crossing the boundary, we take an ITLB miss (which goes from possibility to certainty with the CPU15 workaround), and SRR0/SRR1 get clobbered. I am not familiar with CPU15 since we never had that problem. The patch series is OK then, but one need to add some CPU15 love? I suppose we'll need to find all places where we do rfi with the MMU enabled, and ensure acceptable alignment. :-( Ouch, but it can't be that many places though. Either that, or require that the kernel text be pinned, though that does not interact well with CPU15. Why does not pinning interact well with CPU15? If pinned, you never get a TLB miss for kernel text so that should mitigate the CPU15 problem. Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/8] hvc_console: make the ops pointer const.
Am Dienstag 10 November 2009 07:27:30 schrieb Rusty Russell: This is nicer for modern R/O protection. And noone needs it non-const, so constify the callers as well. Signed-off-by: Rusty Russell ru...@rustcorp.com.au To: Christian Borntraeger borntrae...@de.ibm.com Cc: linuxppc-...@ozlabs.org --- drivers/char/hvc_beat.c |2 +- drivers/char/hvc_console.c|7 --- drivers/char/hvc_console.h|7 --- drivers/char/hvc_iseries.c|2 +- drivers/char/hvc_iucv.c |2 +- drivers/char/hvc_rtas.c |2 +- drivers/char/hvc_udbg.c |2 +- drivers/char/hvc_vio.c|2 +- drivers/char/hvc_xen.c|2 +- drivers/char/virtio_console.c |2 +- 10 files changed, 16 insertions(+), 14 deletions(-) Ok, I started with patches 1-3. I like the result. So for the first 3 patches you can add Tested-by: Christian Borntraeger borntrae...@de.ibm.com (on s390) Reviewed-by: Christian Borntraeger borntrae...@de.ibm.com I have not yet looked at the other ones, but I will try to find some time to look at them. Christian ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] zlib: Optimize inffast when copying direct from output
JFFS2 uses lesser compression ratio and inflate always
ends up in copy direct from output case.
This patch tries to optimize the direct copy procedure.
Uses get_unaligned() but only in one place.
The copy loop just above this one can also use this
optimization, but I havn't done so as I have not tested if it
is a win there too.
On my MPC8321 this is about 17% faster on my JFFS2 root FS
than the original.
---
Would like some testing of the PowerPC boot wrapper and
a LE target before sending it upstream.
arch/powerpc/boot/Makefile |4 ++-
lib/zlib_inflate/inffast.c | 48 +--
2 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 9ae7b7e..98e4c4f 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -20,7 +20,7 @@
all: $(obj)/zImage
BOOTCFLAGS:= -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
--fno-strict-aliasing -Os -msoft-float -pipe \
+-fno-strict-aliasing -Os -msoft-float -pipe -D__KERNEL__\
-fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
-isystem $(shell $(CROSS32CC) -print-file-name=include)
BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -traditional -nostdinc
@@ -34,6 +34,8 @@ BOOTCFLAGS+= -fno-stack-protector
endif
BOOTCFLAGS += -I$(obj) -I$(srctree)/$(obj)
+BOOTCFLAGS += -include include/linux/autoconf.h -Iarch/powerpc/include
+BOOTCFLAGS += -Iinclude
DTS_FLAGS ?= -p 1024
diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
index 8550b0c..0c7fa3d 100644
--- a/lib/zlib_inflate/inffast.c
+++ b/lib/zlib_inflate/inffast.c
@@ -4,6 +4,7 @@
*/
#include linux/zutil.h
+#include asm/unaligned.h
#include inftrees.h
#include inflate.h
#include inffast.h
@@ -24,9 +25,11 @@
#ifdef POSTINC
# define OFF 0
# define PUP(a) *(a)++
+# define UP_UNALIGNED(a) get_unaligned((a)++)
#else
# define OFF 1
# define PUP(a) *++(a)
+# define UP_UNALIGNED(a) get_unaligned(++(a))
#endif
/*
@@ -239,18 +242,41 @@ void inflate_fast(z_streamp strm, unsigned start)
}
}
else {
+ unsigned short *sout;
+ unsigned long loops;
+
from = out - dist; /* copy direct from output */
-do {/* minimum length is three */
-PUP(out) = PUP(from);
-PUP(out) = PUP(from);
-PUP(out) = PUP(from);
-len -= 3;
-} while (len 2);
-if (len) {
-PUP(out) = PUP(from);
-if (len 1)
-PUP(out) = PUP(from);
-}
+/* minimum length is three */
+ /* Align out addr */
+ if (!((long)(out - 1 + OFF)) 1) {
+ PUP(out) = PUP(from);
+ len--;
+ }
+ sout = (unsigned short *)(out - OFF);
+ if (dist 2 ) {
+ unsigned short *sfrom;
+
+ sfrom = (unsigned short *)(from - OFF);
+ loops = len 1;
+ do
+ PUP(sout) = UP_UNALIGNED(sfrom);
+ while (--loops);
+ out = (unsigned char *)sout + OFF;
+ from = (unsigned char *)sfrom + OFF;
+ } else { /* dist == 1 or dist == 2 */
+ unsigned short pat16;
+
+ pat16 = *(sout-2+2*OFF);
+ if (dist == 1)
+ pat16 = (pat16 0xff) | ((pat16 0xff ) 8);
+ loops = len 1;
+ do
+ PUP(sout) = pat16;
+ while (--loops);
+ out = (unsigned char *)sout + OFF;
+ }
+ if (len 1)
+ PUP(out) = PUP(from);
}
}
else if ((op 64) == 0) { /* 2nd level distance code */
--
1.6.4.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Patches for mpc52xx_spi
Hi Grant, here are several patches for the dedicated spi controller on mpc5200 SOC. The patchset contains two fixes and an enhancement. I noticed that your original V4 Patch is still pending for mainline. So I made the patches against the latest version in your -next tree. Tested on a mpc5200b machine with 7 spi devices, (cs lines are conntected to gpios) on 2.6.31, should work with latest kernel also. Please review. cheers Luotao Fu ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/3] mpc52xx_spi: add missing mode_bits definition
Signed-off-by: Luotao Fu l...@pengutronix.de --- drivers/spi/mpc52xx_spi.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c index 5b036f2..79ba678 100644 --- a/drivers/spi/mpc52xx_spi.c +++ b/drivers/spi/mpc52xx_spi.c @@ -430,6 +430,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op, master-num_chipselect = 1; master-setup = mpc52xx_spi_setup; master-transfer = mpc52xx_spi_transfer; + master-mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST; + dev_set_drvdata(op-dev, master); ms = spi_master_get_devdata(master); -- 1.6.5.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/3] mpc52xx_spi: fix clearing status register
Before reading status register to check MODF failure, we have to clear it
first since the MODF flag will be set after initializing the spi master,
if the hardware comes up with a low SS. The processor datasheet reads:
Mode Fault flag -- bit sets if SS input goes low while SPI is configured as a
master. Flag is cleared automatically by an SPI status register read (with MODF
set) followed by a SPI control register 1 write.
Hence simply rereading the register is not sufficient to clear the flag. We
redo the write also to make sure to clear the flag.
Signed-off-by: Luotao Fu l...@pengutronix.de
---
drivers/spi/mpc52xx_spi.c |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
index ef8379b..5b036f2 100644
--- a/drivers/spi/mpc52xx_spi.c
+++ b/drivers/spi/mpc52xx_spi.c
@@ -391,6 +391,7 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
struct mpc52xx_spi *ms;
void __iomem *regs;
int rc;
+ int ctrl1;
/* MMIO registers */
dev_dbg(op-dev, probing mpc5200 SPI device\n);
@@ -399,7 +400,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
return -ENODEV;
/* initialize the device */
- out_8(regs+SPI_CTRL1, SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR);
+ ctrl1 = SPI_CTRL1_SPIE | SPI_CTRL1_SPE | SPI_CTRL1_MSTR;
+ out_8(regs + SPI_CTRL1, ctrl1);
out_8(regs + SPI_CTRL2, 0x0);
out_8(regs + SPI_DATADIR, 0xe); /* Set output pins */
out_8(regs + SPI_PORTDATA, 0x8);/* Deassert /SS signal */
@@ -409,6 +411,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
* on the SPI bus. This fault will also occur if the SPI signals
* are not connected to any pins (port_config setting) */
in_8(regs + SPI_STATUS);
+ out_8(regs + SPI_CTRL1, ctrl1);
+
in_8(regs + SPI_DATA);
if (in_8(regs + SPI_STATUS) SPI_STATUS_MODF) {
dev_err(op-dev, mode fault; is port_config correct?\n);
--
1.6.5.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/3] mpc52xx_spi: add gpio chipselect
This one enables the mpc52xx_spi driver for usage of user defined gpio lines
as chipselect. This way we can control some more spi devices than only one
Signed-off-by: Luotao Fu l...@pengutronix.de
---
drivers/spi/mpc52xx_spi.c | 57 +---
1 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/mpc52xx_spi.c b/drivers/spi/mpc52xx_spi.c
index 79ba678..58c2ce5 100644
--- a/drivers/spi/mpc52xx_spi.c
+++ b/drivers/spi/mpc52xx_spi.c
@@ -21,6 +21,7 @@
#include linux/spi/mpc52xx_spi.h
#include linux/of_spi.h
#include linux/io.h
+#include linux/of_gpio.h
#include asm/time.h
#include asm/mpc52xx.h
@@ -79,7 +80,6 @@ struct mpc52xx_spi {
spinlock_t lock;
struct work_struct work;
-
/* Details of current transfer (length, and buffer pointers) */
struct spi_message *message;/* current message */
struct spi_transfer *transfer; /* current transfer */
@@ -89,6 +89,8 @@ struct mpc52xx_spi {
u8 *rx_buf;
const u8 *tx_buf;
int cs_change;
+ int gpio_cs_count;
+ unsigned int *gpio_cs;
};
/*
@@ -96,7 +98,13 @@ struct mpc52xx_spi {
*/
static void mpc52xx_spi_chipsel(struct mpc52xx_spi *ms, int value)
{
- out_8(ms-regs + SPI_PORTDATA, value ? 0 : 0x08);
+ int cs;
+
+ if (ms-gpio_cs_count 0) {
+ cs = ms-message-spi-chip_select;
+ gpio_direction_output(ms-gpio_cs[cs], value);
+ } else
+ out_8(ms-regs + SPI_PORTDATA, value ? 0 : 0x08);
}
/*
@@ -390,8 +398,9 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
struct spi_master *master;
struct mpc52xx_spi *ms;
void __iomem *regs;
- int rc;
int ctrl1;
+ int rc, i = 0;
+ int gpio_cs;
/* MMIO registers */
dev_dbg(op-dev, probing mpc5200 SPI device\n);
@@ -426,8 +435,8 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
rc = -ENOMEM;
goto err_alloc;
}
+
master-bus_num = -1;
- master-num_chipselect = 1;
master-setup = mpc52xx_spi_setup;
master-transfer = mpc52xx_spi_transfer;
master-mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
@@ -441,6 +450,39 @@ static int __devinit mpc52xx_spi_probe(struct of_device
*op,
ms-irq1 = irq_of_parse_and_map(op-node, 1);
ms-state = mpc52xx_spi_fsmstate_idle;
ms-ipb_freq = mpc5xxx_get_bus_frequency(op-node);
+ ms-gpio_cs_count = of_gpio_count(op-node);
+ if (ms-gpio_cs_count 0) {
+ master-num_chipselect = ms-gpio_cs_count;
+ ms-gpio_cs = kmalloc(ms-gpio_cs_count * sizeof(unsigned int),
+ GFP_KERNEL);
+ if (!ms-gpio_cs) {
+ rc = -ENOMEM;
+ goto err_alloc;
+ }
+
+ for (i = 0; i ms-gpio_cs_count; i++) {
+ gpio_cs = of_get_gpio(op-node, i);
+ if (gpio_cs 0) {
+ dev_err(op-dev,
+ could not parse the gpio field
+ in oftree\n);
+ rc = -ENODEV;
+ goto err_alloc;
+ }
+
+ ms-gpio_cs[i] = gpio_cs;
+ rc = gpio_request(ms-gpio_cs[i], dev_name(op-dev));
+ if (rc) {
+ dev_err(op-dev,
+ can't request spi cs gpio #%d
+ on gpio line %d\n,
+ i, gpio_cs);
+ goto err_gpio;
+ }
+ }
+ } else
+ master-num_chipselect = 1;
+
spin_lock_init(ms-lock);
INIT_LIST_HEAD(ms-queue);
INIT_WORK(ms-work, mpc52xx_spi_wq);
@@ -477,6 +519,9 @@ static int __devinit mpc52xx_spi_probe(struct of_device *op,
err_register:
dev_err(ms-master-dev, initialization failed\n);
spi_master_put(master);
+ err_gpio:
+ while (i-- 0)
+ gpio_free(ms-gpio_cs[i]);
err_alloc:
err_init:
iounmap(regs);
@@ -487,10 +532,14 @@ static int __devexit mpc52xx_spi_remove(struct of_device
*op)
{
struct spi_master *master = dev_get_drvdata(op-dev);
struct mpc52xx_spi *ms = spi_master_get_devdata(master);
+ int i;
free_irq(ms-irq0, ms);
free_irq(ms-irq1, ms);
+ for (i = 0; i ms-gpio_cs_count; i++)
+ gpio_free(ms-gpio_cs[i]);
+
spi_unregister_master(master);
spi_master_put(master);
iounmap(ms-regs);
--
1.6.5.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
SPI driver for linux 2.6.29
Hi,
I've a PPC8313e based board running linux2.6.29 on it. Also I've a
FRAM interfaced to it via SPI. This FRAM almost similar to SPI based
EEPROM, so I'm trying to use EEPROM's driver AT25.
Kernel Configuration :
SPI support -- selected
Freescale MPC83xx/QUICC Engine SPI controller -- selected
(Actually I've replaced MPC83xx's controller driver with modified
MPC8XXX's(from 2.6.31) one for it was not getting probed,)
User mode SPI device driver support -- selected
DTS File;
s...@7000 {
cell-index = 0;
compatible = fsl,spi;
reg = 0x7000 0x1000;
interrupts = 16 0x8;
interrupt-parent = ipic;
mode = cpu;
f...@0{
compatible =
ramtron,fm25; --(Modified at25 as fm25 )
spi-max-frequency =
1;
reg = 0;
};
};
and we are forcing following values in AT25 driver
byte_len= 65536;
name= fram-spi
page_size = 65535;
flags = EE_ADDR2;
Problem:
Drivers are getting complied. But I'm not sure how to acces(i.e,
read/write) the actual SPI-FRAM device.
Should I use
1. /dev/spi/X nodes created by spidev driver
When I use this node, open fails with ERRNO 6
or
2. /sys/bus/spi/devices/spi32766.0/eeprom file.
When I use this sysfs file,Im not able to read / write
properly
Queries:
1. What will be device nodes for this custom at25(fm25) driver,
which registers to spi.c driver?
2. Is spidev.c involved in this picture?
Any help/directions in this regard will of great help.
Thanks n Regards
dhina
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
OOPS on MPC8548 board when writing RAID5 array
CPU is MPC8548, kernel version is 2.6.31.5,CONFIG_FSL_DMA and
CONFIG_ASYNC_TX_DMA options are all enabled.
#mdadm -C /dev/md0 --assume-clean -l5 -n3 /dev/sd{a,b,c}
#dd if=/dev/zero of=/dev/md0 bs=1M count=1000
Oops: Exception in kernel mode, sig: 5 [#1]
MPC85xx CDS
Modules linked in:
NIP: c01c45d8 LR: c01c4d48 CTR:
REGS: c2dd5c80 TRAP: 0700 Not tainted (2.6.31.5)
MSR: 00029000 EE,ME,CE CR: 22004028 XER:
TASK = e820a580[3804] 'md0_raid5' THREAD: c2dd4000
GPR00: 0001 c2dd5d30 e820a580 c2fb1088 0001 0002 1000
GPR08: 0001 c0485a20 ef8092f8 22002024 c2d67870 c0282d2c
GPR16: 1000 e8355c00 c2eff964 0019 0140 c2dd5e00
GPR24: c2dd5dfc 0001 c2dd5dc0 c099c420 c2d67838 0002 c2dd5d58
NIP [c01c45d8] async_tx_quiesce+0x28/0x74
LR [c01c4d48] async_xor+0x208/0x350
Call Trace:
[c2dd5d30] [c02a80f8] fsl_dma_alloc_descriptor+0x24/0x70 (unreliable)
[c2dd5d40] [c01c4d48] async_xor+0x208/0x350
[c2dd5db0] [c02839ec] ops_run_postxor+0xfc/0x1c0
[c2dd5df0] [c0284700] handle_stripe5+0xb24/0x15c0
[c2dd5e70] [c02864c8] handle_stripe+0x34/0x12d4
[c2dd5f10] [c02879ac] raid5d+0x244/0x458
[c2dd5f70] [c02938d4] md_thread+0x5c/0x124
[c2dd5fc0] [c004cc9c] kthread+0x78/0x7c
[c2dd5ff0] [c000f50c] kernel_thread+0x4c/0x68
Instruction dump:
7c0803a6 4e800020 9421fff0 7c0802a6 93e1000c 7c7f1b78 90010014 8063
2f83 419e0034 80030004 5400fffe 0f00 480e19b1 2f830002 419e0030
I checked the kernel source code, and find that this OOPS was caused
by the following BUG_ON code:
It is in crypto/async_tx/async_tx.c:
void async_tx_quiesce(struct dma_async_tx_descriptor **tx)
{
if (*tx) {
/* if ack is already set then we cannot be sure
* we are referring to the correct operation
*/
BUG_ON(async_tx_test_ack(*tx));
/* OOPS occured */
if (dma_wait_for_async_tx(*tx) == DMA_ERROR)
panic(DMA_ERROR waiting for transaction\n);
async_tx_ack(*tx);
*tx = NULL;
}
}
--
The simplest is not all best but the best is surely the simplest!
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
MPC5200B FEC_IEVENT_RFIFO_ERROR leading to Link is Down
Hello, When having high traffic on the MPC5200B ATA (MWDMA2), FEC (100MBit) and LPC (MTD FLASH read) I will get the typical FEC_IEVENT_RFIFO_ERROR's. Without the FLASH reads, that we use to stress the probably existing LPC arbiter or MPC5200B silicon bug when running UDMA2+LPC, and which should not happen using MWDMA2+LPC, I could not yet reproduce the FEC_IEVENT_RFIFO_ERROR. Now comes the problem: since the PHY driver rework followed by John Bonesio's fix (rev 37ccd92f55c6c6) to avoid accessing the DIO bus at interrupt context, this now sometimes causes an irrecoverable link down (PHY: f0003000:00 - Link is Down). This patch is of course necessary to avoid a panic. I can reproduce that on the current DENX/master using an own board very close to the Lite5200B. This board runs fine under the old 2.4.25, of course also sometimes showing the FEC FIFO error. Any ideas? Roman -- Roman FietzeTelemotive AG Büro Mühlhausen Breitwiesen 73347 Mühlhausen Tel.: +49(0)7335/18493-45http://www.telemotive.de ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] ASoC/mpc5200: remove duplicate identical IRQ handler
On Mon, Nov 09, 2009 at 09:40:09AM -0700, Grant Likely wrote: The TX and RX irq handlers are identical. Merge them Applied, thanks. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: term_adt746x: Invert bit required on this Powerbook G4
[ Resending with linuxppc-dev list domain fixed ] On Wed, 2009-11-04 at 13:23 +0100, Max Vozeler wrote: I installed Ubuntu 8.10 on this Powerbook G4 (alu I think) for a friend of mine. As soon as therm_adt746x got loaded, the fan turned into a noise steam engine. This was on Ubuntu's 2.6.31 kernel (2.6.31-14-powerpc) which includes 0512a9a8e277a9de2. I could reproduce it with latest mainline as well. The effect was just as Michel described in the changelog; The fan was running while temps were well below any of the limits, and it stopped only when I set ridiculously low limits. [ 1087.141482] adt746x: version 1 (supported) [ 1087.141495] adt746x: Thermostat bus: 1, address: 0x2e, limit_adjust: 0, fan_speed: -1 [ 1087.141503] sensor 0: HDD BOTTOMSIDE [ 1087.141507] sensor 1: CPU TOPSIDE [ 1087.141512] sensor 2: GPU ON DIE [ 1087.161365] adt746x: ADT7460 initializing [ 1087.165245] adt746x: Lowering max temperatures from 73, 80, 109 to 70, 50, 70 [ 1087.165261] adt746x: Setting speed to 0 for CPU TOPSIDE fan. [ 1087.166302] adt746x: Setting speed to 0 for GPU ON DIE fan. (What worked was limit_adjust=-30, fan did turn off, but so did the Powerbook shortly after, despite being cold.) After some poking around, in which everything seemed to be according to plan including write of 0 to both FAN_SPD_SET regs, I noticed that explicitly *setting* the invert bit as in -write_reg(th, MANUAL_MODE[fan], -(manual|MANUAL_MASK) (~INVERT_MASK)); +write_reg(th, MANUAL_MODE[fan], +(manual|MANUAL_MASK|INVERT_MASK)); seems to cure it. The fan appears to behave normally now, it turns on slowly when the temp limits are reached, otherwise it stays off. The temperature is reasonable (ie, no too hot). So, puzzeled, I checked the spec, and it appears very clear on the question of invert: It should be off by default. Right, that's what I based my patch on. However, it sounds like your PowerBook model (mine is a PowerBook5,8) is wired up such that the invert bit needs to be enabled. I can think of two basic approaches for dealing with this offhand: * Set or clear the invert bit depending on the machine model or whatever is relevant. * Save the bit value during initialization and preserve it whenever writing to the register. Or maybe even add proper suspend/resume hooks which save/restore all hardware state, it seems like it may be luck that the current code works more or less for suspend/resume. I'm not too interested in working on this anymore and I definitely won't have time this or next week, anyone feel free to take it on. -- Earthling Michel Dänzer |http://www.vmware.com Libre software enthusiast | Debian, X and DRI developer ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [spi-devel-general] [PATCH v4] xilinx_spi: Splitted into generic, of and platform driver, added support for DS570
On Tue, Nov 10, 2009 at 9:19 AM, Richard Röjfors
richard.rojf...@mocean-labs.com wrote:
Grant Likely wrote:
Oops, I replied to the original version, but missed the subsequent
versions. Looks like some of my comments still apply though.
Overall, the patch changes too many things all at once. You should
look at splitting it up. At the very least the io accessor changes
should be done in a separate patch.
Hi Richard. Please do another spin of this patch. I don't have any
particular problem with the changes, but it needs to be in a more
granular form so I can review it properly.
+struct xilinx_spi {
+ /* bitbang has to be first */
+ struct spi_bitbang bitbang;
+ struct completion done;
+ struct resource mem; /* phys mem */
+ void __iomem *regs; /* virt. address of the control registers */
+ u32 irq;
+ u8 *rx_ptr; /* pointer in the Tx buffer */
+ const u8 *tx_ptr; /* pointer in the Rx buffer */
+ int remaining_bytes; /* the number of bytes left to transfer */
+ /* offset to the XSPI regs, these might vary... */
+ u8 bits_per_word;
+ bool big_endian; /* The device could be accessed big or
little
+ * endian
+ */
+};
+
Why is the definition of xilinx_spi moved?
I liked the idea of heaving the struct defined in the top of the file.
... which is completely unrelated to the patch purpose, and is not
mentioned in the patch header. If you really want to move it then put
it in a completely separate patch and describe the change properly.
As it is right now it is just noise that makes the stated purpose of
the patch hard to review.
Ah, you changed these to functions instead of macros. I like.
However, as you suggested elsewhere in this thread, you could change
these to callbacks and then eliminate the if/else statements. I think
that is the approach you should use. I don't think you need to worry
about it being slower. Any extra cycles for jumping to a callback
will be far dwarfed by the number of cycles it takes to complete an
SPI transfer.
Sure that can be updated. I prefer to do that in an incremental patch, would
be great to get this
big one merged first.
As already commented on, this patch is too big and does too many
unrelated things. Please split into discrete changes so it can be
reviewed properly.
Thanks,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Joakim Tjernlund wrote: Scott Wood scottw...@freescale.com wrote on 10/11/2009 00:00:04: syscall_exit_cont, SRR0/SRR1 were being loaded immediately prior to a page boundary, with the rfi after the page boundary. On crossing the boundary, we take an ITLB miss (which goes from possibility to certainty with the CPU15 workaround), and SRR0/SRR1 get clobbered. I am not familiar with CPU15 since we never had that problem. The patch series is OK then, but one need to add some CPU15 love? It's not CPU15 itself that is causing the problem, but rather the workaround for CPU15 takes something that has a possibility of happening and makes it certain to happen. Either that, or require that the kernel text be pinned, though that does not interact well with CPU15. Why does not pinning interact well with CPU15? If pinned, you never get a TLB miss for kernel text so that should mitigate the CPU15 problem. The nature of the workaround for CPU15 is that we can't keep it pinned -- we have to take an ITLB miss on every page boundary crossing. If you try to pin, it'll just be invalidated by the workaround. There is an alternate workaround, but it requires compiler changes. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood scottw...@freescale.com wrote on 10/11/2009 22:36:32: Joakim Tjernlund wrote: Scott Wood scottw...@freescale.com wrote on 10/11/2009 21:27:05: Joakim Tjernlund wrote: Scott Wood scottw...@freescale.com wrote on 10/11/2009 17:55:28: Except that the invalidation only happens when you take an ITLB miss on an adjacent page, which means we'd likely never get CPU15 protection for kernel code if pinning is enabled. :-( So tlbie invalidates pinned TLBs too? Yes. OK, and this is in no way unique for 8xx? Not sure about others, but 8xx manual explicitly says that it invalidates reserved entries. But who knows when CPU15 will strike... yes, maybe there is a way around that. Perhaps by using one of the pinned entries for loaded modules, i.e avoid ITLB misses for kernel space? Not sure what you mean... loaded modules won't be pinned, and since they shouldn't contain rfi, don't need to be. But CPU15 may invalidate a pinned TLB if you take a TLB Miss? If not there should not be a problem, because the rest of the kernel will never take a ITLB Miss. I don't see how to pin any part of the kernel without introducing some possibility of CPU15, unless we go scanning the last word of every instruction page, creating trampolines, and hoping there's no data embedded in the text segment. :-P Yes, it is not going to be easy. So aligning the srr0/srr1/rfi properly is needed. BTW, you could probably cram the DARFix into the DTLBerror with some luck. Especially if you allow it to spill over to the next trap. Then create a branch insn at 0x1500 to 0x1600. Would that make everything aligned again? Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 09/11] of: merge prom_{add,remove,modify}_property
Grant Likely wrote:
Merge common code between PowerPC and MicroBlaze
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
---
arch/microblaze/kernel/prom.c | 113
arch/powerpc/kernel/prom.c| 114
drivers/of/base.c | 116 +
3 files changed, 116 insertions(+), 227 deletions(-)
Should the prom_{add,remove,update}_property routines belong in base.c or
of_dynamic.c (patches coming soon for of_dynamic.c)?
My original patch set put these routines in of_dynamic.c and renamed
them to of_{add,remove,update}_property since they are part of the of
code.
-Nathan Fontenot
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] mpc52xx/wdt: merge WDT code into the GPT
On Tue, Nov 10, 2009 at 12:41 PM, Albrecht Dreß albrecht.dr...@arcor.de wrote:
Merge the WDT code into the GPT interface.
Signed-off-by: Albrecht Dreß albrecht.dr...@arcor.de
---
Hi Albrecht,
Thanks for this work. Comments below.
Notes:
The maximum timeout for a 5200 GPT @ 33 MHz clock is ~130 seconds. As this
exceeds the range of an int, some api's had to be changed to u64.
The WDT api is exported as to keep the WDT driver separated from the GPT
driver.
If GPT0 is used as WDT, this prevents the use of any GPT0 GPT function (i.e.
they will fail with -EBUSY). IOW, the safety function always has precedence
over the GPT function. If the kernel has been compiled with
CONFIG_WATCHDOG_NOWAYOUT, this means that GPT0 is locked in WDT mode until
the next reboot - this may be a requirement in safety applications.
This description would look great in the header comment block of the
.c file.
Also, as I commented in patch 3; I'd rather see all the WDT
functionality rolled into this driver. As long as you keep the
functional code blocks logically arranged, I think the driver will be
easier to maintain and understand that way.
arch/powerpc/include/asm/mpc52xx.h | 18 ++-
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 281
++---
2 files changed, 270 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index 2c3fa13..8274ebb 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -60,9 +60,13 @@
#include asm/mpc52xx.h
MODULE_DESCRIPTION(Freescale MPC52xx gpt driver);
-MODULE_AUTHOR(Sascha Hauer, Grant Likely);
+MODULE_AUTHOR(Sascha Hauer, Grant Likely, Albrecht Dreß);
MODULE_LICENSE(GPL);
+#if (defined(CONFIG_MPC5200_WDT)) || (defined(CONFIG_MPC5200_WDT_MODULE))
+#define HAVE_MPC5200_WDT
+#endif
+
/**
* struct mpc52xx_gpt - Private data structure for MPC52xx GPT driver
* @dev: pointer to device structure
@@ -70,6 +74,8 @@ MODULE_LICENSE(GPL);
* @lock: spinlock to coordinate between different functions.
* @of_gc: of_gpio_chip instance structure; used when GPIO is enabled
* @irqhost: Pointer to irq_host instance; used when IRQ mode is supported
+ * @wdt_mode: only used for gpt 0: 0 gpt-only timer, 1 can be used as a
+ * wdt, 2 currently used as wdt, cannot be used as gpt
*/
struct mpc52xx_gpt_priv {
struct list_head list; /* List of all GPT devices */
@@ -78,6 +84,9 @@ struct mpc52xx_gpt_priv {
spinlock_t lock;
struct irq_host *irqhost;
u32 ipb_freq;
+#if defined(HAVE_MPC5200_WDT)
+ u8 wdt_mode;
+#endif
I wouldn't bother with the #if/#endif. I'm willing to sacrifice
32bits for the sake of readability of the code.
+#define NS_PER_SEC 10LL
+
+#define MPC52xx_GPT_CAN_WDT (1 0)
+#define MPC52xx_GPT_IS_WDT (1 1)
+
+
/* -
* Cascaded interrupt controller hooks
*/
@@ -375,36 +393,22 @@ struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq)
}
EXPORT_SYMBOL(mpc52xx_gpt_from_irq);
-/**
- * mpc52xx_gpt_start_timer - Set and enable the GPT timer
- * @gpt: Pointer to gpt private data structure
- * @period: period of timer
- * @continuous: set to 1 to make timer continuous free running
- *
- * An interrupt will be generated every time the timer fires
- */
-int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, int period,
- int continuous)
+/* Calculate the timer counter input register MBAR + 0x6n4 from the
+ * period in ns. The maximum period for 33 MHz IPB clock is ~130s. */
+static int mpc52xx_gpt_calc_counter_input(u64 period, u64 ipb_freq,
+ u32 *reg_val)
{
- u32 clear, set;
u64 clocks;
u32 prescale;
- unsigned long flags;
-
- clear = MPC52xx_GPT_MODE_MS_MASK | MPC52xx_GPT_MODE_CONTINUOUS;
- set = MPC52xx_GPT_MODE_MS_GPIO | MPC52xx_GPT_MODE_COUNTER_ENABLE;
- if (continuous)
- set |= MPC52xx_GPT_MODE_CONTINUOUS;
/* Determine the number of clocks in the requested period. 64 bit
* arithmatic is done here to preserve the precision until the value
- * is scaled back down into the u32 range. Period is in 'ns', bus
- * frequency is in Hz. */
- clocks = (u64)period * (u64)gpt-ipb_freq;
- do_div(clocks, 10); /* Scale it down to ns range */
+ * is scaled back down into the u32 range. */
+ clocks = period * ipb_freq;
+ do_div(clocks, NS_PER_SEC); /* Scale it down to ns range */
Nit: I wouldn't bother with the NS_PER_SEC macro personally. ns per s
is such a fundamental property that I'd rather see the actual number.
- /* This device cannot handle a clock count greater
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Joakim Tjernlund wrote: Scott Wood scottw...@freescale.com wrote on 10/11/2009 22:36:32: Joakim Tjernlund wrote: yes, maybe there is a way around that. Perhaps by using one of the pinned entries for loaded modules, i.e avoid ITLB misses for kernel space? Not sure what you mean... loaded modules won't be pinned, and since they shouldn't contain rfi, don't need to be. But CPU15 may invalidate a pinned TLB if you take a TLB Miss? If not there should not be a problem, because the rest of the kernel will never take a ITLB Miss. It wasn't the CPU15 workaround that I was worried about taking down the pinning -- but rather the CPU15 bug itself causing bad code to be executed inside the pinned kernel mapping. However, the erratum says MMU page, not 4K region, so I suppose if we have a pinned 8M page the problem could only occur at the end of the 8M (by which point the text segment should have ended). Unless we have any evidence that this is not what the erratum means, I'd say make pinning mandatory, and avoid placing modules immediately after a pinned entry. BTW, you could probably cram the DARFix into the DTLBerror with some luck. Especially if you allow it to spill over to the next trap. Then create a branch insn at 0x1500 to 0x1600. Would that make everything aligned again? Yes, until some other code change breaks it again. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood scottw...@freescale.com wrote on 10/11/2009 21:27:05: Joakim Tjernlund wrote: Scott Wood scottw...@freescale.com wrote on 10/11/2009 17:55:28: Except that the invalidation only happens when you take an ITLB miss on an adjacent page, which means we'd likely never get CPU15 protection for kernel code if pinning is enabled. :-( So tlbie invalidates pinned TLBs too? Yes. OK, and this is in no way unique for 8xx? It is likely that you won't get ITLB Misses for normal kernel space but for modules you will. Also, since the pinned DI TLBs overlap, how do you make sure that invalidating a kernel DTLB won't spill over to the pinned ITLB? I don't see when you'd ever invalidate the pinned entry, whether for instruction or data purposes, unless you take an ITLB miss for the page immediately following the pinned mapping. tlbie is used by the kernel in other places too, so I assumed it could be on kernel space too. However, it was just a guess, and by the looks of things, a bad one. Does pinned TLBs work for you? Yes -- I turned on CONFIG_PIN_TLB, padded things so that the rfi is still on the beginning of a new page, and it boots fine. If I keep everything the same but replace MD_RSV4I with zero, it fails again. Ah, good. But who knows when CPU15 will strike... yes, maybe there is a way around that. Perhaps by using one of the pinned entries for loaded modules, i.e avoid ITLB misses for kernel space? In any case one should consider fixing entry_32.S and friends to be properly aligned. Then we are no worse off than when we started, right? Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
On Tue, Nov 10, 2009 at 1:26 PM, Albrecht Dreß albrecht.dr...@arcor.de wrote: Hi Grant: Am 10.11.09 20:59 schrieb(en) Grant Likely: On Tue, Nov 10, 2009 at 12:43 PM, Albrecht Dreß albrecht.dr...@arcor.de wrote: Use the MPC5200 GPT api for the WDT which drastically simplifies this file. Signed-off-by: Albrecht Dreß albrecht.dr...@arcor.de --- drivers/watchdog/mpc5200_wdt.c | 246 +++- 1 files changed, 65 insertions(+), 181 deletions(-) Can the WDT functionality just be merged entirely into arch/powerpc/platforms/52xx/mpc52xx_gpt.c, eliminating the need for this file entirely? I think I'd rather have all the GPT built in behaviour handled by a single driver. I also thought about it, as it has IMHO the cleaner code, and it would have the extra benefit that the gpt-wdt api doesn't need to be public. However, the reasons I hesitated to do so are: - I don't want to remove a file someone else wrote (even it doesn't work); Shouldn't be a problem, and I'll handle the fallout if it is. - WDT code is shifted from drivers/watchdog to arch/powerpc/platforms/52xx which might not be the logical place from the directory layout's pov; There is precedence of this in the past, particularly on arch or platform specific hardware drivers and multifunction devices. (Heck, that's almost entirely what arch/powerpc/sysdev is). I'm not concerned. - a file living in arch/powerpc/platforms/52xx depends upon config options set from drivers/watchdog/Kconfig which may be confusing. I'm not concerned about this either. You see these are more political/cosmetical questions, so I would prefer to leave the decision to the maintainers (i.e. you and Wim). Preparing a fully merged driver is actually a matter of minutes! Do it. I'll champion getting it in. Wim, do you have any issues with this? g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 09/11] of: merge prom_{add,remove,modify}_property
On Tue, Nov 10, 2009 at 1:33 PM, Nathan Fontenot nf...@austin.ibm.com wrote:
Grant Likely wrote:
Merge common code between PowerPC and MicroBlaze
Signed-off-by: Grant Likely grant.lik...@secretlab.ca
---
arch/microblaze/kernel/prom.c | 113
arch/powerpc/kernel/prom.c | 114
drivers/of/base.c | 116
+
3 files changed, 116 insertions(+), 227 deletions(-)
Should the prom_{add,remove,update}_property routines belong in base.c or
of_dynamic.c (patches coming soon for of_dynamic.c)?
My original patch set put these routines in of_dynamic.c and renamed
them to of_{add,remove,update}_property since they are part of the of
code.
First stage for me is to go through the rather mechanical process of
merging common code. As much as possible I'm not changing any APIs,
and I don't have enough background on the of_dynamic stuff to make a
call about where it should ultimately live. The goal of the first
stage is simply to eliminate duplicated code. Rename of functions to
of_ is potentially risky, so I'm deferring those changes until after
all the code is merged.
Feel free to make changes on top of what I'm doing. Keep each one
small to reduce the risk of conflict. I'll pick up the ones that make
sense.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/3] mpc52xx/wdt: OF property to enable the WDT on boot
On Tue, Nov 10, 2009 at 12:40 PM, Albrecht Dreß albrecht.dr...@arcor.de wrote: Add the wdt,on-boot OF property as to reserve a GPT as WDT which may be a requirement in safety-related (e.g. ISO 61508) applications. Signed-off-by: Albrecht Dreß albrecht.dr...@arcor.de --- Documentation/powerpc/dts-bindings/fsl/mpc5200.txt | 15 ++- 1 files changed, 14 insertions(+), 1 deletions(-) diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt index 8447fd7..1eecb06 100644 --- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt +++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt @@ -103,7 +103,20 @@ fsl,mpc5200-gpt nodes - On the mpc5200 and 5200b, GPT0 has a watchdog timer function. If the board design supports the internal wdt, then the device node for GPT0 should -include the empty property 'fsl,has-wdt'. +include the empty property 'fsl,has-wdt'. Note that this does not activate +the watchdog. The timer will function as a GPT if the timer api is used, and +it will function as watchdog if the watchdog device is used. The watchdog +mode has priority over the gpt mode, i.e. if the watchdog is activated, any +gpt api call to this timer will fail with -EBUSY. + +If you add the property + wdt,on-boot = n; +GPT0 will be marked as in-use watchdog, i.e. blocking every gpt access to it. +If n0, the watchdog is started with a timeout of n seconds. If n=0, the +configuration of the watchdog is not touched. This is useful in two cases: +- just mark GPT0 as watchdog, blocking gpt accesses, and configure it later; +- do not touch a configuration assigned by the boot loader which supervises + the boot process itself. I'm not *totally* convinced on the usage model, but I just need some time to think about it. Give me a day or so and ping me again if you haven't heard from me. However, until a common WDT binding is defined, this property needs to be named something like fsl,wdt-on-boot. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/3] mpc52xx/wdt: OF property to enable the WDT on boot
Add the wdt,on-boot OF property as to reserve a GPT as WDT which may be a requirement in safety-related (e.g. ISO 61508) applications. Signed-off-by: Albrecht Dreß albrecht.dr...@arcor.de --- Documentation/powerpc/dts-bindings/fsl/mpc5200.txt | 15 ++- 1 files changed, 14 insertions(+), 1 deletions(-) diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt index 8447fd7..1eecb06 100644 --- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt +++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt @@ -103,7 +103,20 @@ fsl,mpc5200-gpt nodes - On the mpc5200 and 5200b, GPT0 has a watchdog timer function. If the board design supports the internal wdt, then the device node for GPT0 should -include the empty property 'fsl,has-wdt'. +include the empty property 'fsl,has-wdt'. Note that this does not activate +the watchdog. The timer will function as a GPT if the timer api is used, and +it will function as watchdog if the watchdog device is used. The watchdog +mode has priority over the gpt mode, i.e. if the watchdog is activated, any +gpt api call to this timer will fail with -EBUSY. + +If you add the property + wdt,on-boot = n; +GPT0 will be marked as in-use watchdog, i.e. blocking every gpt access to it. +If n0, the watchdog is started with a timeout of n seconds. If n=0, the +configuration of the watchdog is not touched. This is useful in two cases: +- just mark GPT0 as watchdog, blocking gpt accesses, and configure it later; +- do not touch a configuration assigned by the boot loader which supervises + the boot process itself. An mpc5200-gpt can be used as a single line GPIO controller. To do so, add the following properties to the gpt node: ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/3] mpc52xx/wdt: merge WDT code into the GPT
Merge the WDT code into the GPT interface.
Signed-off-by: Albrecht Dreß albrecht.dr...@arcor.de
---
Notes:
The maximum timeout for a 5200 GPT @ 33 MHz clock is ~130 seconds. As this
exceeds the range of an int, some api's had to be changed to u64.
The WDT api is exported as to keep the WDT driver separated from the GPT
driver.
If GPT0 is used as WDT, this prevents the use of any GPT0 GPT function (i.e.
they will fail with -EBUSY). IOW, the safety function always has precedence
over the GPT function. If the kernel has been compiled with
CONFIG_WATCHDOG_NOWAYOUT, this means that GPT0 is locked in WDT mode until
the next reboot - this may be a requirement in safety applications.
arch/powerpc/include/asm/mpc52xx.h| 18 ++-
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 281 ++---
2 files changed, 270 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h
b/arch/powerpc/include/asm/mpc52xx.h
index 707ab75..0ece07f 100644
--- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -279,9 +279,21 @@ extern void mpc52xx_restart(char *cmd);
/* mpc52xx_gpt.c */
struct mpc52xx_gpt_priv;
extern struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq);
-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);
+extern int mpc52xx_gpt_start_timer(struct mpc52xx_gpt_priv *gpt, u64 period,
+ int continuous);
+extern int mpc52xx_gpt_stop_timer(struct mpc52xx_gpt_priv *gpt);
+extern u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt);
+#if (defined(CONFIG_MPC5200_WDT)) || (defined(CONFIG_MPC5200_WDT_MODULE))
+extern struct mpc52xx_gpt_priv *mpc52xx_gpt_wdt_probe(void);
+extern int mpc52xx_gpt_wdt_start(struct mpc52xx_gpt_priv *gpt_wdt,
+int wdt_timeout);
+extern int mpc52xx_gpt_wdt_ping(struct mpc52xx_gpt_priv *gpt_wdt);
+extern int mpc52xx_gpt_wdt_release(struct mpc52xx_gpt_priv *gpt_wdt);
+#if !defined(CONFIG_WATCHDOG_NOWAYOUT)
+extern int mpc52xx_gpt_wdt_stop(struct mpc52xx_gpt_priv *gpt_wdt);
+#endif /* CONFIG_WATCHDOG_NOWAYOUT */
+#endif /* CONFIG_MPC5200_WDT */
+
/* mpc52xx_lpbfifo.c */
#define MPC52XX_LPBFIFO_FLAG_READ (0)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index 2c3fa13..8274ebb 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -60,9 +60,13 @@
#include asm/mpc52xx.h
MODULE_DESCRIPTION(Freescale MPC52xx gpt driver);
-MODULE_AUTHOR(Sascha Hauer, Grant Likely);
+MODULE_AUTHOR(Sascha Hauer, Grant Likely, Albrecht Dreß);
MODULE_LICENSE(GPL);
+#if (defined(CONFIG_MPC5200_WDT)) || (defined(CONFIG_MPC5200_WDT_MODULE))
+#define HAVE_MPC5200_WDT
+#endif
+
/**
* struct mpc52xx_gpt - Private data structure for MPC52xx GPT driver
* @dev: pointer to device structure
@@ -70,6 +74,8 @@ MODULE_LICENSE(GPL);
* @lock: spinlock to coordinate between different functions.
* @of_gc: of_gpio_chip instance structure; used when GPIO is enabled
* @irqhost: Pointer to irq_host instance; used when IRQ mode is supported
+ * @wdt_mode: only used for gpt 0: 0 gpt-only timer, 1 can be used as a
+ *wdt, 2 currently used as wdt, cannot be used as gpt
*/
struct mpc52xx_gpt_priv {
struct list_head list; /* List of all GPT devices */
@@ -78,6 +84,9 @@ struct mpc52xx_gpt_priv {
spinlock_t lock;
struct irq_host *irqhost;
u32 ipb_freq;
+#if defined(HAVE_MPC5200_WDT)
+ u8 wdt_mode;
+#endif
#if defined(CONFIG_GPIOLIB)
struct of_gpio_chip of_gc;
@@ -101,14 +110,23 @@ DEFINE_MUTEX(mpc52xx_gpt_list_mutex);
#define MPC52xx_GPT_MODE_CONTINUOUS(0x0400)
#define MPC52xx_GPT_MODE_OPEN_DRAIN(0x0200)
#define MPC52xx_GPT_MODE_IRQ_EN(0x0100)
+#define MPC52xx_GPT_MODE_WDT_EN(0x8000)
#define MPC52xx_GPT_MODE_ICT_MASK (0x03)
#define MPC52xx_GPT_MODE_ICT_RISING(0x01)
#define MPC52xx_GPT_MODE_ICT_FALLING (0x02)
#define MPC52xx_GPT_MODE_ICT_TOGGLE(0x03)
+#define MPC52xx_GPT_MODE_WDT_PING (0xa5)
+
#define MPC52xx_GPT_STATUS_IRQMASK (0x000f)
+#define NS_PER_SEC 10LL
+
+#define MPC52xx_GPT_CAN_WDT(1 0)
+#define MPC52xx_GPT_IS_WDT (1 1)
+
+
/* -
* Cascaded interrupt controller hooks
*/
@@ -375,36 +393,22 @@ struct mpc52xx_gpt_priv *mpc52xx_gpt_from_irq(int irq)
}
EXPORT_SYMBOL(mpc52xx_gpt_from_irq);
-/**
- * mpc52xx_gpt_start_timer - Set and enable the GPT timer
- * @gpt: Pointer to gpt private data structure
- * @period: period of timer
- * @continuous: set to 1 to make timer continuous free running
- *
- * An interrupt will be
[PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
Use the MPC5200 GPT api for the WDT which drastically simplifies this file.
Signed-off-by: Albrecht Dreß albrecht.dr...@arcor.de
---
drivers/watchdog/mpc5200_wdt.c | 246 +++-
1 files changed, 65 insertions(+), 181 deletions(-)
diff --git a/drivers/watchdog/mpc5200_wdt.c b/drivers/watchdog/mpc5200_wdt.c
index fa9c47c..5bb553c 100644
--- a/drivers/watchdog/mpc5200_wdt.c
+++ b/drivers/watchdog/mpc5200_wdt.c
@@ -2,25 +2,16 @@
#include linux/module.h
#include linux/miscdevice.h
#include linux/watchdog.h
-#include linux/io.h
-#include linux/spinlock.h
#include linux/of_platform.h
#include linux/uaccess.h
#include asm/mpc52xx.h
-#define GPT_MODE_WDT (1 15)
-#define GPT_MODE_CE(1 12)
-#define GPT_MODE_MS_TIMER (0x4)
-
+#define WDT_IDENTITYmpc5200 watchdog on GPT0
struct mpc5200_wdt {
- unsigned count; /* timer ticks before watchdog kicks in */
- long ipb_freq;
- struct miscdevice miscdev;
- struct resource mem;
- struct mpc52xx_gpt __iomem *regs;
- spinlock_t io_lock;
+ int timeout;
+ struct mpc52xx_gpt_priv *timer;
};
/* is_active stores wether or not the /dev/watchdog device is opened */
@@ -32,80 +23,33 @@ static unsigned long is_active;
static struct mpc5200_wdt *wdt_global;
-/* helper to calculate timeout in timer counts */
-static void mpc5200_wdt_set_timeout(struct mpc5200_wdt *wdt, int timeout)
-{
- /* use biggest prescaler of 64k */
- wdt-count = (wdt-ipb_freq + 0x) / 0x1 * timeout;
-
- if (wdt-count 0x)
- wdt-count = 0x;
-}
-/* return timeout in seconds (calculated from timer count) */
-static int mpc5200_wdt_get_timeout(struct mpc5200_wdt *wdt)
-{
- return wdt-count * 0x1 / wdt-ipb_freq;
-}
-
-
-/* watchdog operations */
-static int mpc5200_wdt_start(struct mpc5200_wdt *wdt)
-{
- spin_lock(wdt-io_lock);
- /* disable */
- out_be32(wdt-regs-mode, 0);
- /* set timeout, with maximum prescaler */
- out_be32(wdt-regs-count, 0x0 | wdt-count);
- /* enable watchdog */
- out_be32(wdt-regs-mode, GPT_MODE_CE | GPT_MODE_WDT |
- GPT_MODE_MS_TIMER);
- spin_unlock(wdt-io_lock);
-
- return 0;
-}
-static int mpc5200_wdt_ping(struct mpc5200_wdt *wdt)
-{
- spin_lock(wdt-io_lock);
- /* writing A5 to OCPW resets the watchdog */
- out_be32(wdt-regs-mode, 0xA500 |
- (0xff in_be32(wdt-regs-mode)));
- spin_unlock(wdt-io_lock);
- return 0;
-}
-static int mpc5200_wdt_stop(struct mpc5200_wdt *wdt)
-{
- spin_lock(wdt-io_lock);
- /* disable */
- out_be32(wdt-regs-mode, 0);
- spin_unlock(wdt-io_lock);
- return 0;
-}
-
-
/* file operations */
static ssize_t mpc5200_wdt_write(struct file *file, const char __user *data,
- size_t len, loff_t *ppos)
+size_t len, loff_t *ppos)
{
struct mpc5200_wdt *wdt = file-private_data;
- mpc5200_wdt_ping(wdt);
+ mpc52xx_gpt_wdt_ping(wdt-timer);
return 0;
}
+
static struct watchdog_info mpc5200_wdt_info = {
.options= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
- .identity = mpc5200 watchdog on GPT0,
+ .identity = WDT_IDENTITY,
};
+
static long mpc5200_wdt_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
+ unsigned long arg)
{
struct mpc5200_wdt *wdt = file-private_data;
int __user *data = (int __user *)arg;
int timeout;
+ u64 real_timeout;
int ret = 0;
switch (cmd) {
case WDIOC_GETSUPPORT:
ret = copy_to_user(data, mpc5200_wdt_info,
- sizeof(mpc5200_wdt_info));
+ sizeof(mpc5200_wdt_info));
if (ret)
ret = -EFAULT;
break;
@@ -116,19 +60,23 @@ static long mpc5200_wdt_ioctl(struct file *file, unsigned
int cmd,
break;
case WDIOC_KEEPALIVE:
- mpc5200_wdt_ping(wdt);
+ mpc52xx_gpt_wdt_ping(wdt-timer);
break;
case WDIOC_SETTIMEOUT:
ret = get_user(timeout, data);
if (ret)
break;
- mpc5200_wdt_set_timeout(wdt, timeout);
- mpc5200_wdt_start(wdt);
+ ret = mpc52xx_gpt_wdt_start(wdt-timer, timeout);
+ if (ret)
+ break;
+ wdt-timeout = timeout;
/* fall through and return the timeout */
case WDIOC_GETTIMEOUT:
- timeout = mpc5200_wdt_get_timeout(wdt);
+ real_timeout = mpc52xx_gpt_timer_period(wdt-timer);
+
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood scottw...@freescale.com wrote on 10/11/2009 21:00:07: Joakim Tjernlund wrote: I think I have misunderstood, its is not CPU15 or 8xx problem per se, it is a general problem that could happen to any ppc CPU, right? 8xx just happen to be the first CPU that hits this case due to my DAR fixing Is there any chip other than 8xx where the kernel text is not always pinned? No idea, something for the list to comment on. But if so 8xx should do the same(at least for ITLB), there is no other way to guarantee this problem won't happen I think. Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood scottw...@freescale.com wrote on 10/11/2009 00:00:04: On Mon, Nov 09, 2009 at 03:53:21PM -0600, Scott Wood wrote: On Fri, Nov 06, 2009 at 10:29:44AM +0100, Joakim Tjernlund wrote: With this, the kernel hangs after Mount-cache hash table entries: 512. Somewhat surprising result. I didn't expect you would even hit this condition now as we haven't enabled the use of dcbX insn yet. The only thing I can think of is the you hit the 0x00f0 due to other dcbX insn use and the kernel managed to fixup this in the page fault handler by pure luck before. It's bizarre -- it still happens even if I revert the branch to FixupDAR. However, if I comment out the final b 151b, it stops happening. It also stops happening sometimes depending on where I stick printk()s to debug this. So it may be an unrelated issue that just got perturbed somehow. OK, figured it out. The fixup code pushed things around so that in syscall_exit_cont, SRR0/SRR1 were being loaded immediately prior to a page boundary, with the rfi after the page boundary. On crossing the boundary, we take an ITLB miss (which goes from possibility to certainty with the CPU15 workaround), and SRR0/SRR1 get clobbered. I think I have misunderstood, its is not CPU15 or 8xx problem per se, it is a general problem that could happen to any ppc CPU, right? 8xx just happen to be the first CPU that hits this case due to my DAR fixing I suppose we'll need to find all places where we do rfi with the MMU enabled, and ensure acceptable alignment. :-( That seems the right fix, the asm in entry_32.S needs have some alignment here and there to make sure you don't cross a page boundary at the wrong time. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/3] mpc52xx/wdt: re-enable the MPC5200 WDT
This set of patches merges the MPC5200 WDT into the GPT code, making it functional again - currently, the MPC5200 GPT code blocks using the WDT. Additionally, it defines a new OF property as to reserve and/or enable the WDT during the boot process which may be a requirement for safety-related (e.g. ISO/EN 61508) applications. Note that all patches are against Grant Likely's next tree. Cheers, Albrecht. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood scottw...@freescale.com wrote on 10/11/2009 17:55:28: Scott Wood wrote: Joakim Tjernlund wrote: Why does not pinning interact well with CPU15? If pinned, you never get a TLB miss for kernel text so that should mitigate the CPU15 problem. The nature of the workaround for CPU15 is that we can't keep it pinned -- we have to take an ITLB miss on every page boundary crossing. If you try to pin, it'll just be invalidated by the workaround. Except that the invalidation only happens when you take an ITLB miss on an adjacent page, which means we'd likely never get CPU15 protection for kernel code if pinning is enabled. :-( So tlbie invalidates pinned TLBs too? It is likely that you won't get ITLB Misses for normal kernel space but for modules you will. Also, since the pinned DI TLBs overlap, how do you make sure that invalidating a kernel DTLB won't spill over to the pinned ITLB? Does pinned TLBs work for you? Rex, how does it look in your end? Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
Hi Grant: Am 10.11.09 20:59 schrieb(en) Grant Likely: On Tue, Nov 10, 2009 at 12:43 PM, Albrecht Dreß albrecht.dr...@arcor.de wrote: Use the MPC5200 GPT api for the WDT which drastically simplifies this file. Signed-off-by: Albrecht Dreß albrecht.dr...@arcor.de --- drivers/watchdog/mpc5200_wdt.c | 246 +++- 1 files changed, 65 insertions(+), 181 deletions(-) Can the WDT functionality just be merged entirely into arch/powerpc/platforms/52xx/mpc52xx_gpt.c, eliminating the need for this file entirely? I think I'd rather have all the GPT built in behaviour handled by a single driver. I also thought about it, as it has IMHO the cleaner code, and it would have the extra benefit that the gpt-wdt api doesn't need to be public. However, the reasons I hesitated to do so are: - I don't want to remove a file someone else wrote (even it doesn't work); - WDT code is shifted from drivers/watchdog to arch/powerpc/platforms/52xx which might not be the logical place from the directory layout's pov; - a file living in arch/powerpc/platforms/52xx depends upon config options set from drivers/watchdog/Kconfig which may be confusing. You see these are more political/cosmetical questions, so I would prefer to leave the decision to the maintainers (i.e. you and Wim). Preparing a fully merged driver is actually a matter of minutes! Thanks, Albrecht. pgpJ5TyPKqicn.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
On Tue, Nov 10, 2009 at 12:43 PM, Albrecht Dreß albrecht.dr...@arcor.de wrote: Use the MPC5200 GPT api for the WDT which drastically simplifies this file. Signed-off-by: Albrecht Dreß albrecht.dr...@arcor.de --- drivers/watchdog/mpc5200_wdt.c | 246 +++- 1 files changed, 65 insertions(+), 181 deletions(-) Can the WDT functionality just be merged entirely into arch/powerpc/platforms/52xx/mpc52xx_gpt.c, eliminating the need for this file entirely? I think I'd rather have all the GPT built in behaviour handled by a single driver. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [spi-devel-general] [PATCH v4] xilinx_spi: Splitted into generic, of and platform driver, added support for DS570
Grant Likely wrote:
Oops, I replied to the original version, but missed the subsequent
versions. Looks like some of my comments still apply though.
Overall, the patch changes too many things all at once. You should
look at splitting it up. At the very least the io accessor changes
should be done in a separate patch.
On Mon, Sep 28, 2009 at 7:22 AM, Richard Röjfors
richard.rojf...@mocean-labs.com wrote:
@@ -227,6 +227,21 @@ config SPI_XILINX
See the OPB Serial Peripheral Interface (SPI) (v1.00e)
Product Specification document (DS464) for hardware details.
+ Or for the DS570, see XPS Serial Peripheral Interface (SPI)
(v2.00b)
+
+config SPI_XILINX_OF
+ tristate Xilinx SPI controller OF device
+ depends on SPI_XILINX XILINX_VIRTEX
+ help
+ This is the OF driver for the SPI controller IP from the Xilinx
EDK.
+
+config SPI_XILINX_PLTFM
+ tristate Xilinx SPI controller platform device
+ depends on SPI_XILINX
+ help
+ This is the platform driver for the SPI controller IP
+ from the Xilinx EDK.
+
Personally, I don't think it is necessary to present these options to
the user. I think they can be auto-selected depending on CONFIG_OF
and CONFIG_PLATFORM.
Sure that can be changed, I prefer to to that in an incremental patch after
this one.
+struct xilinx_spi {
+ /* bitbang has to be first */
+ struct spi_bitbang bitbang;
+ struct completion done;
+ struct resource mem; /* phys mem */
+ void __iomem*regs; /* virt. address of the control registers */
+ u32 irq;
+ u8 *rx_ptr; /* pointer in the Tx buffer */
+ const u8 *tx_ptr; /* pointer in the Rx buffer */
+ int remaining_bytes;/* the number of bytes left to transfer */
+ /* offset to the XSPI regs, these might vary... */
+ u8 bits_per_word;
+ bool big_endian;/* The device could be accessed big or little
+* endian
+*/
+};
+
Why is the definition of xilinx_spi moved?
I liked the idea of heaving the struct defined in the top of the file.
/* Register definitions as per OPB Serial Peripheral Interface (SPI)
(v1.00e)
* Product Specification, DS464
*/
-#define XSPI_CR_OFFSET 0x62/* 16-bit Control Register */
+#define XSPI_CR_OFFSET 0x60/* Control Register */
#define XSPI_CR_ENABLE 0x02
#define XSPI_CR_MASTER_MODE0x04
@@ -40,8 +53,9 @@
#define XSPI_CR_RXFIFO_RESET 0x40
#define XSPI_CR_MANUAL_SSELECT 0x80
#define XSPI_CR_TRANS_INHIBIT 0x100
+#define XSPI_CR_LSB_FIRST 0x200
-#define XSPI_SR_OFFSET 0x67/* 8-bit Status Register */
+#define XSPI_SR_OFFSET 0x64/* Status Register */
#define XSPI_SR_RX_EMPTY_MASK 0x01/* Receive FIFO is empty */
#define XSPI_SR_RX_FULL_MASK 0x02/* Receive FIFO is full */
@@ -49,8 +63,8 @@
#define XSPI_SR_TX_FULL_MASK 0x08/* Transmit FIFO is full */
#define XSPI_SR_MODE_FAULT_MASK0x10/* Mode fault error */
-#define XSPI_TXD_OFFSET0x6b/* 8-bit Data Transmit
Register */
-#define XSPI_RXD_OFFSET0x6f/* 8-bit Data Receive
Register */
+#define XSPI_TXD_OFFSET0x68/* Data Transmit Register */
+#define XSPI_RXD_OFFSET0x6C/* Data Receive Register */
#define XSPI_SSR_OFFSET0x70/* 32-bit Slave Select
Register */
@@ -70,43 +84,72 @@
#define XSPI_INTR_TX_UNDERRUN 0x08/* TxFIFO was underrun */
#define XSPI_INTR_RX_FULL 0x10/* RxFIFO is full */
#define XSPI_INTR_RX_OVERRUN 0x20/* RxFIFO was overrun */
+#define XSPI_INTR_TX_HALF_EMPTY0x40/* TxFIFO is half
empty */
#define XIPIF_V123B_RESETR_OFFSET 0x40/* IPIF reset register */
#define XIPIF_V123B_RESET_MASK 0x0a/* the value to write */
-struct xilinx_spi {
- /* bitbang has to be first */
- struct spi_bitbang bitbang;
- struct completion done;
+/* to follow are some functions that does little of big endian read and
+ * write depending on the config of the device.
+ */
+static inline void xspi_write8(struct xilinx_spi *xspi, u32 offs, u8 val)
+{
+ iowrite8(val, xspi-regs + offs + ((xspi-big_endian) ? 3 : 0));
+}
- void __iomem*regs; /* virt. address of the control registers */
+static inline void xspi_write16(struct xilinx_spi *xspi, u32 offs, u16 val)
+{
+ if (xspi-big_endian)
+ iowrite16be(val, xspi-regs + offs + 2);
+ else
+ iowrite16(val, xspi-regs + offs);
+}
- u32 irq;
+static inline void xspi_write32(struct xilinx_spi *xspi, u32 offs, u32 val)
+{
+ if (xspi-big_endian)
+ iowrite32be(val, xspi-regs + offs);
+
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood scottw...@freescale.com wrote on 10/11/2009 23:02:10: Joakim Tjernlund wrote: Scott Wood scottw...@freescale.com wrote on 10/11/2009 22:36:32: Joakim Tjernlund wrote: yes, maybe there is a way around that. Perhaps by using one of the pinned entries for loaded modules, i.e avoid ITLB misses for kernel space? Not sure what you mean... loaded modules won't be pinned, and since they shouldn't contain rfi, don't need to be. But CPU15 may invalidate a pinned TLB if you take a TLB Miss? If not there should not be a problem, because the rest of the kernel will never take a ITLB Miss. It wasn't the CPU15 workaround that I was worried about taking down the pinning -- but rather the CPU15 bug itself causing bad code to be executed inside the pinned kernel mapping. Oh, but then one is screwed anyway. However, the erratum says MMU page, not 4K region, so I suppose if we have a pinned 8M page the problem could only occur at the end of the 8M (by which point the text segment should have ended). The wording makes me wonder why not a dcbi would fix the problem too. Invalidate the problem cache lines and let the branch resolve. Unless we have any evidence that this is not what the erratum means, I'd say make pinning mandatory, and avoid placing modules immediately after a pinned entry. It is worth a try. BTW, you could probably cram the DARFix into the DTLBerror with some luck. Especially if you allow it to spill over to the next trap. Then create a branch insn at 0x1500 to 0x1600. Would that make everything aligned again? Yes, until some other code change breaks it again. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Joakim Tjernlund wrote: Scott Wood scottw...@freescale.com wrote on 10/11/2009 23:02:10: Joakim Tjernlund wrote: It wasn't the CPU15 workaround that I was worried about taking down the pinning -- but rather the CPU15 bug itself causing bad code to be executed inside the pinned kernel mapping. Oh, but then one is screwed anyway. Not if there's a workaround... However, the erratum says MMU page, not 4K region, so I suppose if we have a pinned 8M page the problem could only occur at the end of the 8M (by which point the text segment should have ended). The wording makes me wonder why not a dcbi would fix the problem too. Invalidate the problem cache lines and let the branch resolve. Where would you put the dcbi? How do you regain control after that cache line has been refilled, but before code flows back to the bad branch? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [spi-devel-general] [PATCH v4] xilinx_spi: Splitted into generic, of and platform driver, added support for DS570
-Original Message-
From: glik...@secretlab.ca [mailto:glik...@secretlab.ca] On Behalf Of Grant
Likely
Sent: Tuesday, November 10, 2009 9:45 AM
To: Richard Röjfors
Cc: spi-devel-gene...@lists.sourceforge.net; linuxppc-...@ozlabs.org; Andrew
Morton;
dbrown...@users.sourceforge.net; John Linn
Subject: Re: [spi-devel-general] [PATCH v4] xilinx_spi: Splitted into
generic, of and platform
driver, added support for DS570
On Tue, Nov 10, 2009 at 9:19 AM, Richard Röjfors
richard.rojf...@mocean-labs.com wrote:
Grant Likely wrote:
Oops, I replied to the original version, but missed the subsequent
versions. Looks like some of my comments still apply though.
Overall, the patch changes too many things all at once. You should
look at splitting it up. At the very least the io accessor changes
should be done in a separate patch.
Hi Richard. Please do another spin of this patch. I don't have any
particular problem with the changes, but it needs to be in a more
granular form so I can review it properly.
I agree.
We have a functioning driver such that I need to make sure it doesn't regress.
More granular changes will also allow us to better isolate any problems if they
occur.
Thanks,
John
+struct xilinx_spi {
+ /* bitbang has to be first */
+ struct spi_bitbang bitbang;
+ struct completion done;
+ struct resource mem; /* phys mem */
+ void __iomem *regs; /* virt. address of the control registers
*/
+ u32 irq;
+ u8 *rx_ptr; /* pointer in the Tx buffer */
+ const u8 *tx_ptr; /* pointer in the Rx buffer */
+ int remaining_bytes; /* the number of bytes left to transfer */
+ /* offset to the XSPI regs, these might vary... */
+ u8 bits_per_word;
+ bool big_endian; /* The device could be accessed big or
little
+ * endian
+ */
+};
+
Why is the definition of xilinx_spi moved?
I liked the idea of heaving the struct defined in the top of the file.
... which is completely unrelated to the patch purpose, and is not
mentioned in the patch header. If you really want to move it then put
it in a completely separate patch and describe the change properly.
As it is right now it is just noise that makes the stated purpose of
the patch hard to review.
Ah, you changed these to functions instead of macros. I like.
However, as you suggested elsewhere in this thread, you could change
these to callbacks and then eliminate the if/else statements. I think
that is the approach you should use. I don't think you need to worry
about it being slower. Any extra cycles for jumping to a callback
will be far dwarfed by the number of cycles it takes to complete an
SPI transfer.
Sure that can be updated. I prefer to do that in an incremental patch,
would be great to get this
big one merged first.
As already commented on, this patch is too big and does too many
unrelated things. Please split into discrete changes so it can be
reviewed properly.
Thanks,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
This email and any attachments are intended for the sole use of the named
recipient(s) and contain(s) confidential information that may be proprietary,
privileged or copyrighted under applicable law. If you are not the intended
recipient, do not read, copy, or forward this email message or any attachments.
Delete this email message and any attachments immediately.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood scottw...@freescale.com wrote on 11/11/2009 00:21:18: Joakim Tjernlund wrote: Scott Wood scottw...@freescale.com wrote on 10/11/2009 23:02:10: Joakim Tjernlund wrote: It wasn't the CPU15 workaround that I was worried about taking down the pinning -- but rather the CPU15 bug itself causing bad code to be executed inside the pinned kernel mapping. Oh, but then one is screwed anyway. Not if there's a workaround... However, the erratum says MMU page, not 4K region, so I suppose if we have a pinned 8M page the problem could only occur at the end of the 8M (by which point the text segment should have ended). The wording makes me wonder why not a dcbi would fix the problem too. Invalidate the problem cache lines and let the branch resolve. Where would you put the dcbi? How do you regain control after that cache line has been refilled, but before code flows back to the bad branch? The dcbi would replace the current CPU15 tlbie. I would try mitigating bullet 3 (unresolved long enough ...) by invalidating n+1 cache line. Hopefully that will abort the prefetch. If that doesn't work, invalidate n+2(bullet 4) as it says it must be in cache for the problem to manifest. But I see now that I probably misread the errata so the above wont work. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/6] gianfar: Some fixes
Hi all, Here are some fixes for the gianfar driver, patches on the way. Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/6] skbuff: Do not allow skb recycling with disabled IRQs
NAPI drivers try to recycle SKBs in their polling routine, but we
generally don't know the context in which the polling will be called,
and the skb recycling itself may require IRQs to be enabled.
This patch adds irqs_disabled() test to the skb_recycle_check()
routine, so that we'll not let the drivers hit the skb recycling
path with IRQs disabled.
As a side effect, this patch actually disables skb recycling for some
[broken] drivers. E.g. gianfar driver grabs an irqsave spinlock during
TX ring processing, and then tries to recycle an skb, and that caused
the following badness:
nf_conntrack version 0.5.0 (1008 buckets, 4032 max)
[ cut here ]
Badness at kernel/softirq.c:143
NIP: c003e3c4 LR: c423a528 CTR: c003e344
...
NIP [c003e3c4] local_bh_enable+0x80/0xc4
LR [c423a528] destroy_conntrack+0xd4/0x13c [nf_conntrack]
Call Trace:
[c15d1b60] [c003e32c] local_bh_disable+0x1c/0x34 (unreliable)
[c15d1b70] [c423a528] destroy_conntrack+0xd4/0x13c [nf_conntrack]
[c15d1b80] [c02c6370] nf_conntrack_destroy+0x3c/0x70
--- Exception: c428168c at 0xc15d1c50
LR = 0xc15d1c40
[c15d1ba0] [c0286f3c] skb_release_head_state+0x100/0x104 (unreliable)
[c15d1bb0] [c0288340] skb_recycle_check+0x8c/0x10c
[c15d1bc0] [c01e1688] gfar_poll+0x190/0x384
[c15d1c10] [c02935ac] net_rx_action+0xec/0x22c
[c15d1c50] [c003dd8c] __do_softirq+0xe8/0x224
...
Reported-by: Jon Loeliger j...@jdl.com
Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
---
net/core/skbuff.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 80a9616..941bac9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -493,6 +493,9 @@ int skb_recycle_check(struct sk_buff *skb, int skb_size)
{
struct skb_shared_info *shinfo;
+ if (irqs_disabled())
+ return 0;
+
if (skb_is_nonlinear(skb) || skb-fclone != SKB_FCLONE_UNAVAILABLE)
return 0;
--
1.6.3.3
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/6] gianfar: Remove 'Interrupt problem!' warning
It is OK to poll with disabled IRQs, so remove the warning.
Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
---
drivers/net/gianfar.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 197b358..79c28f5 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2504,8 +2504,6 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue,
int rx_work_limit)
skb_put(skb, pkt_len);
dev-stats.rx_bytes += pkt_len;
- if (in_irq() || irqs_disabled())
- printk(Interrupt problem!\n);
gfar_process_frame(dev, skb, amount_pull);
} else {
--
1.6.3.3
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/6] gianfar: Fix build with CONFIG_PM=y
commit fba4ed030cfae7efdb6b79a57b0c5a9d72c9 (gianfar: Add Multiple Queue Support) introduced the following build failure: CC gianfar.o gianfar.c: In function 'gfar_restore': gianfar.c:1249: error: request for member 'napi' in something not a structure or union This patch fixes the issue. Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com --- drivers/net/gianfar.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c index 79c28f5..a5b0038 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -1246,7 +1246,7 @@ static int gfar_restore(struct device *dev) phy_start(priv-phydev); netif_device_attach(ndev); - napi_enable(priv-gfargrp.napi); + enable_napi(priv); return 0; } -- 1.6.3.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 4/6] gianfar: Fix thinko in gfar_set_rx_stash_index()
We obviously want to write a modified 'temp' value back to the register, not the saved IRQ flags. Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com --- drivers/net/gianfar_sysfs.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/gianfar_sysfs.c b/drivers/net/gianfar_sysfs.c index 3724835..b31c9c8 100644 --- a/drivers/net/gianfar_sysfs.c +++ b/drivers/net/gianfar_sysfs.c @@ -186,7 +186,7 @@ static ssize_t gfar_set_rx_stash_index(struct device *dev, temp = gfar_read(regs-attreli); temp = ~ATTRELI_EI_MASK; temp |= ATTRELI_EI(index); - gfar_write(regs-attreli, flags); + gfar_write(regs-attreli, temp); out: unlock_rx_qs(priv); -- 1.6.3.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 5/6] gianfar: Fix race between gfar_error() and gfar_start_xmit()
gfar_error() can arrive at the middle of gfar_start_xmit() processing,
and so it can trigger transfers of BDs that we don't yet expect to
be transmitted.
Fix this by locking the tx queues in gfar_error().
Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
---
drivers/net/gianfar.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index a5b0038..fde430a 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2943,14 +2943,22 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
if (events IEVENT_CRL)
dev-stats.tx_aborted_errors++;
if (events IEVENT_XFUN) {
+ unsigned long flags;
+
if (netif_msg_tx_err(priv))
printk(KERN_DEBUG %s: TX FIFO underrun,
packet dropped.\n, dev-name);
dev-stats.tx_dropped++;
priv-extra_stats.tx_underrun++;
+ local_irq_save(flags);
+ lock_tx_qs(priv);
+
/* Reactivate the Tx Queues */
gfar_write(regs-tstat, gfargrp-tstat);
+
+ unlock_tx_qs(priv);
+ local_irq_restore(flags);
}
if (netif_msg_tx_err(priv))
printk(KERN_DEBUG %s: Transmit Error\n, dev-name);
--
1.6.3.3
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 6/6] gianfar: Revive SKB recycling
Before calling gfar_clean_tx_ring() the driver grabs an irqsave
spinlock, and then tries to recycle skbs. But since
skb_recycle_check() returns 0 with IRQs disabled, we'll never
recycle any skbs.
It appears that gfar_clean_tx_ring() and gfar_start_xmit() are
mostly idependent and can work in parallel, except when they
modify num_txbdfree.
So we can drop the lock from most sections and thus fix the skb
recycling.
Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
---
drivers/net/gianfar.c | 31 +++
1 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index fde430a..16def13 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1928,14 +1928,11 @@ static int gfar_start_xmit(struct sk_buff *skb, struct
net_device *dev)
/* total number of fragments in the SKB */
nr_frags = skb_shinfo(skb)-nr_frags;
- spin_lock_irqsave(tx_queue-txlock, flags);
-
/* check if there is space to queue this packet */
if ((nr_frags+1) tx_queue-num_txbdfree) {
/* no space, stop the queue */
netif_tx_stop_queue(txq);
dev-stats.tx_fifo_errors++;
- spin_unlock_irqrestore(tx_queue-txlock, flags);
return NETDEV_TX_BUSY;
}
@@ -1999,6 +1996,20 @@ static int gfar_start_xmit(struct sk_buff *skb, struct
net_device *dev)
lstatus |= BD_LFLAG(TXBD_CRC | TXBD_READY) | skb_headlen(skb);
/*
+* We can work in parallel with gfar_clean_tx_ring(), except
+* when modifying num_txbdfree. Note that we didn't grab the lock
+* when we were reading the num_txbdfree and checking for available
+* space, that's because outside of this function it can only grow,
+* and once we've got needed space, it cannot suddenly disappear.
+*
+* The lock also protects us from gfar_error(), which can modify
+* regs-tstat and thus retrigger the transfers, which is why we
+* also must grab the lock before setting ready bit for the first
+* to be transmitted BD.
+*/
+ spin_lock_irqsave(tx_queue-txlock, flags);
+
+ /*
* The powerpc-specific eieio() is used, as wmb() has too strong
* semantics (it requires synchronization between cacheable and
* uncacheable mappings, which eieio doesn't provide and which we
@@ -2225,6 +2236,8 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q
*tx_queue)
skb_dirtytx = tx_queue-skb_dirtytx;
while ((skb = tx_queue-tx_skbuff[skb_dirtytx])) {
+ unsigned long flags;
+
frags = skb_shinfo(skb)-nr_frags;
lbdp = skip_txbd(bdp, frags, base, tx_ring_size);
@@ -2269,7 +2282,9 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q
*tx_queue)
TX_RING_MOD_MASK(tx_ring_size);
howmany++;
+ spin_lock_irqsave(tx_queue-txlock, flags);
tx_queue-num_txbdfree += frags + 1;
+ spin_unlock_irqrestore(tx_queue-txlock, flags);
}
/* If we freed a buffer, we can restart transmission, if necessary */
@@ -2548,7 +2563,6 @@ static int gfar_poll(struct napi_struct *napi, int budget)
int tx_cleaned = 0, i, left_over_budget = budget;
unsigned long serviced_queues = 0;
int num_queues = 0;
- unsigned long flags;
num_queues = gfargrp-num_rx_queues;
budget_per_queue = budget/num_queues;
@@ -2568,14 +2582,7 @@ static int gfar_poll(struct napi_struct *napi, int
budget)
rx_queue = priv-rx_queue[i];
tx_queue = priv-tx_queue[rx_queue-qindex];
- /* If we fail to get the lock,
-* don't bother with the TX BDs */
- if (spin_trylock_irqsave(tx_queue-txlock, flags)) {
- tx_cleaned += gfar_clean_tx_ring(tx_queue);
- spin_unlock_irqrestore(tx_queue-txlock,
- flags);
- }
-
+ tx_cleaned += gfar_clean_tx_ring(tx_queue);
rx_cleaned_per_queue = gfar_clean_rx_ring(rx_queue,
budget_per_queue);
rx_cleaned += rx_cleaned_per_queue;
--
1.6.3.3
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Fwd: [PATCH] powerpc: _exception: kill the obsolete code under is_global_init()]
---BeginMessage---
The code under if (is_global_init()) is bogus, and is_global_init()
itself is not right in mt case.
Contrary to what the comment says, nowadays force_sig_info() does kill
init even if the handler is SIG_DFL. Note that force_sig_info() clears
SIGNAL_UNKILLABLE exactly for this case.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
arch/powerpc/kernel/traps.c | 22 --
1 file changed, 22 deletions(-)
--- TH/arch/powerpc/kernel/traps.c~PPC_EXCEPTION_DONT_CK_INIT 2009-11-10
01:03:23.0 +0100
+++ TH/arch/powerpc/kernel/traps.c 2009-11-10 01:06:41.0 +0100
@@ -198,28 +198,6 @@ void _exception(int signr, struct pt_reg
info.si_code = code;
info.si_addr = (void __user *) addr;
force_sig_info(signr, info, current);
-
- /*
-* Init gets no signals that it doesn't have a handler for.
-* That's all very well, but if it has caused a synchronous
-* exception and we ignore the resulting signal, it will just
-* generate the same exception over and over again and we get
-* nowhere. Better to kill it and let the kernel panic.
-*/
- if (is_global_init(current)) {
- __sighandler_t handler;
-
- spin_lock_irq(current-sighand-siglock);
- handler = current-sighand-action[signr-1].sa.sa_handler;
- spin_unlock_irq(current-sighand-siglock);
- if (handler == SIG_DFL) {
- /* init has generated a synchronous exception
- and it doesn't have a handler for the signal */
- printk(KERN_CRIT init has generated signal %d
- but has no handler for it\n, signr);
- do_exit(signr);
- }
- }
}
#ifdef CONFIG_PPC64
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
---End Message---
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Interrupts not Firing on PPC405EX
Jonathan Haws wrote:
All,
I am having some troubles getting interrupts to fire from my kernel module. I
have connected the ISR with a call to request_irq() and have configured my
device to generate interrupts. However, my ISR is called once when I connect
the interrupt for the first time. After that it never is called again. It
seems like that interrupt is getting stuck disabled, but that does not make
sense as to why.
The device is on the PCIE0 bus and works just fine in another OS (namely
Vxworks - that is the driver I am working on porting to Linux).
Here is how I am connecting the ISR and the ISR itself. Am I doing something
stupid?
Thanks for the help!
Jonathan
PS - Our hardware is a custom spun PPC405EX board based on the AMCC Kilauea
board and uses the kilauea.dts with no modifications.
A quick note - I realize that I am not checking if I was the one to interrupt
the CPU. I am not worried about that right now - especially since I know there
is nothing else that will interrupt the CPU on this IRQ right now anyway - it
never fires.
int fpga_open(struct inode *inode, struct file *filp)
{
int err = 0;
/* Make sure we have successfully probed the device */
if (NULL == fpga_drv.pcidev)
{
return -ENODEV;
}
/* Only one process at a time can have access to the FPGA */
if (0 != atomic_read(fpga_drv.openCount))
{
atomic_inc(fpga_drv.openCount);
printk(KERN_WARNING FPGA: Could not open device: already open.
\n);
return -EBUSY;
}
/* If not already in use, state that we are */
atomic_inc(fpga_drv.openCount);
/* Store a pointer to the PCI device structure */
filp-private_data = fpga_drv.pcidev;
/* Attach ISR to IRQ */
if (request_irq(fpga_drv.pcidev-irq, fpga_isr, IRQF_SHARED,
FPGA_MODULE_NAME, fpga_drv.pcidev))
{
printk( KERN_ERR FPGA: Unable to connect FPGA ISR (%d)!\n,
fpga_drv.pcidev-irq);
return -EPERM;
}
return 0;
}
/* Interrupt Service Routine */
irqreturn_t fpga_isr(int irq, void *dev_id)
{
uint32_t status = 0;
status = fpga_drv.cfg_ptr[FPGA_INTER_STATUS];
printk(KERN_NOTICE FPGA: Interrupt fired! (%#08x)\n, status);
if (status FPGA_INTERRUPT_SPI)
{
rt_sem_v(fpga_drv.sarSem);
}
Your ISR registry and handling is fine. you mentioned that the IRQ only
be trigged once.
Are you sure that nothing is needed to reenable the HW IRQ generation
such as clear status bit ...
Is the algorithm in linux same with it in Vxworks?
Tony
/* Return HANDLED */
return (IRQ_HANDLED);}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
--
Tony Liu | Liu Bo
-
WIND RIVER | China Development Center
Tel: 86-10-8477-8542 ext: 8542 | Fax: 86-10-64790367
(M): 86-136-7117-3612
Address: 15/F, Wangjing TowerB, Chaoyang District, Beijing, P.R.China
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 6/6] gianfar: Revive SKB recycling
-Original Message-
From: Anton Vorontsov [mailto:avoront...@ru.mvista.com]
Sent: Wednesday, November 11, 2009 5:41 AM
To: David Miller
Cc: Fleming Andy-AFLEMING; Jon Loeliger; Kumar
Gopalpet-B05799; Lennert Buytenhek; Stephen Hemminger;
net...@vger.kernel.org; linuxppc-...@ozlabs.org
Subject: [PATCH 6/6] gianfar: Revive SKB recycling
Before calling gfar_clean_tx_ring() the driver grabs an
irqsave spinlock, and then tries to recycle skbs. But since
skb_recycle_check() returns 0 with IRQs disabled, we'll never
recycle any skbs.
It appears that gfar_clean_tx_ring() and gfar_start_xmit() are
mostly idependent and can work in parallel, except when they
modify num_txbdfree.
So we can drop the lock from most sections and thus fix the
skb recycling.
Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com
---
drivers/net/gianfar.c | 31 +++
1 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index fde430a..16def13 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1928,14 +1928,11 @@ static int gfar_start_xmit(struct
sk_buff *skb, struct net_device *dev)
/* total number of fragments in the SKB */
nr_frags = skb_shinfo(skb)-nr_frags;
- spin_lock_irqsave(tx_queue-txlock, flags);
-
/* check if there is space to queue this packet */
if ((nr_frags+1) tx_queue-num_txbdfree) {
/* no space, stop the queue */
netif_tx_stop_queue(txq);
dev-stats.tx_fifo_errors++;
- spin_unlock_irqrestore(tx_queue-txlock, flags);
return NETDEV_TX_BUSY;
}
@@ -1999,6 +1996,20 @@ static int gfar_start_xmit(struct
sk_buff *skb, struct net_device *dev)
lstatus |= BD_LFLAG(TXBD_CRC | TXBD_READY) | skb_headlen(skb);
/*
+ * We can work in parallel with gfar_clean_tx_ring(), except
+ * when modifying num_txbdfree. Note that we didn't
grab the lock
+ * when we were reading the num_txbdfree and checking
for available
+ * space, that's because outside of this function it
can only grow,
+ * and once we've got needed space, it cannot suddenly
disappear.
+ *
+ * The lock also protects us from gfar_error(), which can modify
+ * regs-tstat and thus retrigger the transfers, which is why we
+ * also must grab the lock before setting ready bit for
the first
+ * to be transmitted BD.
+ */
+ spin_lock_irqsave(tx_queue-txlock, flags);
+
+ /*
* The powerpc-specific eieio() is used, as wmb() has too strong
* semantics (it requires synchronization between cacheable and
* uncacheable mappings, which eieio doesn't provide
and which we @@ -2225,6 +2236,8 @@ static int
gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
skb_dirtytx = tx_queue-skb_dirtytx;
while ((skb = tx_queue-tx_skbuff[skb_dirtytx])) {
+ unsigned long flags;
+
frags = skb_shinfo(skb)-nr_frags;
lbdp = skip_txbd(bdp, frags, base, tx_ring_size);
@@ -2269,7 +2282,9 @@ static int gfar_clean_tx_ring(struct
gfar_priv_tx_q *tx_queue)
TX_RING_MOD_MASK(tx_ring_size);
howmany++;
+ spin_lock_irqsave(tx_queue-txlock, flags);
tx_queue-num_txbdfree += frags + 1;
+ spin_unlock_irqrestore(tx_queue-txlock, flags);
}
/* If we freed a buffer, we can restart transmission,
if necessary */ @@ -2548,7 +2563,6 @@ static int
gfar_poll(struct napi_struct *napi, int budget)
int tx_cleaned = 0, i, left_over_budget = budget;
unsigned long serviced_queues = 0;
int num_queues = 0;
- unsigned long flags;
num_queues = gfargrp-num_rx_queues;
budget_per_queue = budget/num_queues;
@@ -2568,14 +2582,7 @@ static int gfar_poll(struct napi_struct
*napi, int budget)
rx_queue = priv-rx_queue[i];
tx_queue = priv-tx_queue[rx_queue-qindex];
- /* If we fail to get the lock,
- * don't bother with the TX BDs */
- if
(spin_trylock_irqsave(tx_queue-txlock, flags)) {
- tx_cleaned +=
gfar_clean_tx_ring(tx_queue);
-
spin_unlock_irqrestore(tx_queue-txlock,
- flags);
- }
-
+ tx_cleaned += gfar_clean_tx_ring(tx_queue);
rx_cleaned_per_queue =
gfar_clean_rx_ring(rx_queue,
budget_per_queue);
rx_cleaned += rx_cleaned_per_queue;
--
Anton, we tried some experiments too at our end, and removing the
spinlocks did help improve the performance and recycling was effective
although, I don't have exact numbers to specify.
But overall I
RE: [PATCH 3/6] gianfar: Fix build with CONFIG_PM=y
-Original Message- From: Anton Vorontsov [mailto:avoront...@ru.mvista.com] Sent: Wednesday, November 11, 2009 5:41 AM To: David Miller Cc: Fleming Andy-AFLEMING; Jon Loeliger; Kumar Gopalpet-B05799; Lennert Buytenhek; Stephen Hemminger; net...@vger.kernel.org; linuxppc-...@ozlabs.org Subject: [PATCH 3/6] gianfar: Fix build with CONFIG_PM=y commit fba4ed030cfae7efdb6b79a57b0c5a9d72c9 (gianfar: Add Multiple Queue Support) introduced the following build failure: CC gianfar.o gianfar.c: In function 'gfar_restore': gianfar.c:1249: error: request for member 'napi' in something not a structure or union This patch fixes the issue. Signed-off-by: Anton Vorontsov avoront...@ru.mvista.com --- drivers/net/gianfar.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c index 79c28f5..a5b0038 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -1246,7 +1246,7 @@ static int gfar_restore(struct device *dev) phy_start(priv-phydev); netif_device_attach(ndev); - napi_enable(priv-gfargrp.napi); + enable_napi(priv); return 0; } I am extreemely sorry for introducing this error, I missed it while porting my last set of patches. Thanks for fixing it. -- Thanks Sandeep ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[git pull] Please pull powerpc.git merge branch
Hi Linus !
This contains one code fix, 3 little device-tree fixes and a bunch of
defconfig updates for Freescale platforms for 2.6.32 along with one
defconfig update for PA-Semi.
Overall, a large diffstat but not much actual churn. If you are unhappy
with the defconfig updates at that stage of the process, let us know and
I'll ask my sub-maintainers to do them earlier.
Cheers,
Ben.
The following changes since commit 799dd75b1a8380a967c929a4551895788c374b31:
Linus Torvalds (1):
Merge branch 'i2c-for-linus' of
git://git.kernel.org/.../jdelvare/staging
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
Anton Vorontsov (2):
powerpc/85xx: Fix USB GPIOs for MPC8569E-MDS boards
powerpc/83xx: Fix u-boot partion size for MPC8377E-WLAN boards
Benjamin Herrenschmidt (1):
Merge commit 'kumar/merge' into merge
Kim Phillips (1):
powerpc/8xxx: enable IPsec ESP by default on mpc83xx/mpc85xx
Kumar Gala (1):
powerpc: 2.6.32 update of defconfigs for embedded 6xx/7xxx, 8xx,
8{3,5,6}xxx
Olof Johansson (1):
powerpc: pasemi_defconfig update
Paul Gortmaker (1):
powerpc/85xx: sbc8548 - fixup of PCI-e related DTS fields
Roel Kluin (1):
powerpc/82xx: kmalloc failure ignored in ep8248e_mdio_probe()
arch/powerpc/boot/dts/mpc8377_wlan.dts|2 +-
arch/powerpc/boot/dts/mpc8569mds.dts |4 +-
arch/powerpc/boot/dts/sbc8548.dts | 17 +-
arch/powerpc/configs/83xx/asp8347_defconfig | 60 ++-
arch/powerpc/configs/83xx/kmeter1_defconfig | 46 +-
arch/powerpc/configs/83xx/mpc8313_rdb_defconfig | 73 ++-
arch/powerpc/configs/83xx/mpc8315_rdb_defconfig | 76 ++-
arch/powerpc/configs/83xx/mpc832x_mds_defconfig | 62 ++-
arch/powerpc/configs/83xx/mpc832x_rdb_defconfig | 66 ++-
arch/powerpc/configs/83xx/mpc834x_itx_defconfig | 60 ++-
arch/powerpc/configs/83xx/mpc834x_itxgp_defconfig | 58 ++-
arch/powerpc/configs/83xx/mpc834x_mds_defconfig | 58 ++-
arch/powerpc/configs/83xx/mpc836x_mds_defconfig | 64 ++-
arch/powerpc/configs/83xx/mpc836x_rdk_defconfig | 60 ++-
arch/powerpc/configs/83xx/mpc837x_mds_defconfig | 64 ++-
arch/powerpc/configs/83xx/mpc837x_rdb_defconfig | 70 ++-
arch/powerpc/configs/83xx/sbc834x_defconfig | 61 ++-
arch/powerpc/configs/85xx/ksi8560_defconfig | 61 ++-
arch/powerpc/configs/85xx/mpc8540_ads_defconfig | 57 ++-
arch/powerpc/configs/85xx/mpc8560_ads_defconfig | 63 ++-
arch/powerpc/configs/85xx/mpc85xx_cds_defconfig | 58 ++-
arch/powerpc/configs/85xx/sbc8548_defconfig | 53 ++-
arch/powerpc/configs/85xx/sbc8560_defconfig | 57 ++-
arch/powerpc/configs/85xx/socrates_defconfig | 68 ++-
arch/powerpc/configs/85xx/stx_gp3_defconfig | 77 ++-
arch/powerpc/configs/85xx/tqm8540_defconfig | 59 ++-
arch/powerpc/configs/85xx/tqm8541_defconfig | 64 ++-
arch/powerpc/configs/85xx/tqm8548_defconfig | 64 ++-
arch/powerpc/configs/85xx/tqm8555_defconfig | 64 ++-
arch/powerpc/configs/85xx/tqm8560_defconfig | 64 ++-
arch/powerpc/configs/85xx/xes_mpc85xx_defconfig | 76 ++-
arch/powerpc/configs/86xx/gef_ppc9a_defconfig | 74 ++-
arch/powerpc/configs/86xx/gef_sbc310_defconfig| 72 ++-
arch/powerpc/configs/86xx/gef_sbc610_defconfig| 80 ++-
arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig | 64 ++-
arch/powerpc/configs/86xx/mpc8641_hpcn_defconfig | 65 ++-
arch/powerpc/configs/86xx/sbc8641d_defconfig | 66 ++-
arch/powerpc/configs/adder875_defconfig | 48 +-
arch/powerpc/configs/c2k_defconfig| 77 ++-
arch/powerpc/configs/ep8248e_defconfig| 49 +-
arch/powerpc/configs/ep88xc_defconfig | 47 +-
arch/powerpc/configs/linkstation_defconfig| 72 ++-
arch/powerpc/configs/mgcoge_defconfig | 51 ++-
arch/powerpc/configs/mgsuvd_defconfig | 45 +-
arch/powerpc/configs/mpc7448_hpc2_defconfig | 58 ++-
arch/powerpc/configs/mpc8272_ads_defconfig| 52 ++-
arch/powerpc/configs/mpc83xx_defconfig| 90 ++-
arch/powerpc/configs/mpc85xx_defconfig| 98 ++--
arch/powerpc/configs/mpc85xx_smp_defconfig| 99 ++--
arch/powerpc/configs/mpc866_ads_defconfig | 54 ++-
arch/powerpc/configs/mpc86xx_defconfig| 70 ++-
arch/powerpc/configs/mpc885_ads_defconfig | 47 +-
arch/powerpc/configs/pasemi_defconfig | 628 +++--
arch/powerpc/configs/pq2fads_defconfig| 55 ++-
arch/powerpc/configs/prpmc2800_defconfig | 67 ++-
arch/powerpc/configs/storcenter_defconfig | 54 ++-
arch/powerpc/platforms/82xx/ep8248e.c | 15 +-
57 files changed, 2603 insertions(+), 1380 deletions(-)
___
Linuxppc-dev mailing
Re: [PATCH 0/8] 8xx: Misc fixes for buggy insn
Scott Wood wrote: Joakim Tjernlund wrote: Why does not pinning interact well with CPU15? If pinned, you never get a TLB miss for kernel text so that should mitigate the CPU15 problem. The nature of the workaround for CPU15 is that we can't keep it pinned -- we have to take an ITLB miss on every page boundary crossing. If you try to pin, it'll just be invalidated by the workaround. Except that the invalidation only happens when you take an ITLB miss on an adjacent page, which means we'd likely never get CPU15 protection for kernel code if pinning is enabled. :-( -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
