From db2f38c22ea3f545be3b5772e5f9dc5861b74536 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Wed, 22 Apr 2009 20:33:40 +0200 Subject: [PATCH 1/7] palm_bk3710: UDMA performance fix Fix UDMA throughput bug: tCYC averages t2CYCTYP/2, but the code previously assumed it was the same as t2CYCTYP. (That is, it was using just one clock edge, not both.) Move the table's type declaration so it's adjacent to the table, making it more clear what those numbers mean. On one system this change increased throughput by almost 4x: UDMA/66 sometimes topped 23 MB/sec (on a drive known to do much better). On another system it was around a 10% win (UDMA/66 up to 7+ MB/sec). The difference might be caused by the ratio between memory and IDE clocks. In the system with large speedup, this was exactly 2 (as a workaround for a rev 1.1 silicon bug). The other system used a more standard ratio of 1.63 (and rev 2.1 silicon) ... clock domain synch might have some issues, they're not unheard-of. Signed-off-by: David Brownell Acked-by: Sergei Shtylyov Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/palm_bk3710.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c index c7acca0b8733..d1513b4a457c 100644 --- a/drivers/ide/palm_bk3710.c +++ b/drivers/ide/palm_bk3710.c @@ -39,14 +39,6 @@ /* Primary Control Offset */ #define IDE_PALM_ATA_PRI_CTL_OFFSET 0x3F6 -/* - * PalmChip 3710 IDE Controller UDMA timing structure Definition - */ -struct palm_bk3710_udmatiming { - unsigned int rptime; /* Ready to pause time */ - unsigned int cycletime; /* Cycle Time */ -}; - #define BK3710_BMICP 0x00 #define BK3710_BMISP 0x02 #define BK3710_BMIDTP 0x04 @@ -75,13 +67,19 @@ struct palm_bk3710_udmatiming { static unsigned ideclk_period; /* in nanoseconds */ +struct palm_bk3710_udmatiming { + unsigned int rptime; /* tRP -- Ready to pause time (nsec) */ + unsigned int cycletime; /* tCYCTYP2/2 -- avg Cycle Time (nsec) */ + /* tENV is always a minimum of 20 nsec */ +}; + static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = { - {160, 240}, /* UDMA Mode 0 */ - {125, 160}, /* UDMA Mode 1 */ - {100, 120}, /* UDMA Mode 2 */ - {100, 90}, /* UDMA Mode 3 */ - {100, 60}, /* UDMA Mode 4 */ - {85, 40}, /* UDMA Mode 5 */ + {160, 240 / 2,}, /* UDMA Mode 0 */ + {125, 160 / 2,}, /* UDMA Mode 1 */ + {100, 120 / 2,}, /* UDMA Mode 2 */ + {100, 90 / 2,}, /* UDMA Mode 3 */ + {100, 60 / 2,}, /* UDMA Mode 4 */ + {85, 40 / 2,}, /* UDMA Mode 5 */ }; static void palm_bk3710_setudmamode(void __iomem *base, unsigned int dev, From a1f9a89c90b4ac143c5b6054c2a157572b272cd2 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Wed, 22 Apr 2009 20:33:40 +0200 Subject: [PATCH 2/7] ide-cd: fix kernel crash on hppa regression With 2.6.30-rc2 I face a kernel crash on the 32bit hppa architecture due to ide-cd when udev creates the device nodes at startup: Kernel Fault: Code=26 regs=8ed34c40 (Addr=00000024) IASQ: 00000000 00000000 IAOQ: 1034b5ac 1034b5b0 IIR: 4ab30048 ISR: 00000000 IOR: 00000024 CPU: 0 CR30: 8ed34000 CR31: ffff55ff ORIG_R28: 00000000 IAOQ[0]: ide_complete_rq+0x2c/0x70 IAOQ[1]: ide_complete_rq+0x30/0x70 RP(r2): cdrom_newpc_intr+0x178/0x46c Backtrace: [<1035c608>] cdrom_newpc_intr+0x178/0x46c [<1034c494>] ide_intr+0x1b0/0x214 [<1016d284>] handle_IRQ_event+0x70/0x150 [<1016d4b0>] __do_IRQ+0x14c/0x1cc [<102f7864>] superio_interrupt+0x88/0xbc [<1016d284>] handle_IRQ_event+0x70/0x150 [<1016d4b0>] __do_IRQ+0x14c/0x1cc [<10112efc>] do_cpu_irq_mask+0x9c/0xd0 [<10116068>] intr_return+0x0/0x4 This crash seems to happen due to an uninitialized variable "rc". The compiler even warns about that: CC drivers/ide/ide-cd.o /mnt/sda4/home/cvs/parisc/git-kernel/linus-linux-2.6/drivers/ide/ide-cd.c: In function `cdrom_newpc_intr': /mnt/sda4/home/cvs/parisc/git-kernel/linus-linux-2.6/drivers/ide/ide-cd.c:612: warning: `rc' might be used uninitialized in this function After applying the trivial patch below, which just initializes the variable to zero, the kernel doesn't crash any longer: Starting the hotplug events dispatcher: udevd. Synthesizing the initial hotplug events... hda: command error: status=0x51 { DriveReady SeekComplete Error } hda: command error: error=0x54 <3>{ AbortedCommand LastFailedSense=0x05 } ide: failed opcode was: unknown done. Signed-off-by: Helge Deller Acked-by: Borislav Petkov Cc: Linus Cc: Kyle McMartin Cc: "Rafael J. Wysocki" Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/ide-cd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 3aec19d1fdfc..3d4e09969763 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -609,7 +609,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive) struct request *rq = hwif->rq; ide_expiry_t *expiry = NULL; int dma_error = 0, dma, thislen, uptodate = 0; - int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc, nsectors; + int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc = 0, nsectors; int sense = blk_sense_request(rq); unsigned int timeout; u16 len; From b0aedb04eae79372fbe101d98513773d6b89935d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Pr=C3=A9mont?= Date: Wed, 22 Apr 2009 20:33:41 +0200 Subject: [PATCH 3/7] ide: Stop disks on reboot for laptop which cuts power MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My laptop (Acer Travelmate 660) always cuts the power when rebooting which causes the disk to emergency-park it's head. Add a dmi check to stop disk as for shutdown on this laptop. Signed-off-by: Bruno Prémont Cc: Jeff Garzik Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/ide-gd.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c index 1aebdf1a4f58..4b6b71e2cdf5 100644 --- a/drivers/ide/ide-gd.c +++ b/drivers/ide/ide-gd.c @@ -7,6 +7,7 @@ #include #include #include +#include #if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT) #define IDE_DISK_MINORS (1 << PARTN_BITS) @@ -99,6 +100,19 @@ static void ide_gd_resume(ide_drive_t *drive) (void)drive->disk_ops->get_capacity(drive); } +static const struct dmi_system_id ide_coldreboot_table[] = { + { + /* Acer TravelMate 66x cuts power during reboot */ + .ident = "Acer TravelMate 660", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), + DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate 660"), + }, + }, + + { } /* terminate list */ +}; + static void ide_gd_shutdown(ide_drive_t *drive) { #ifdef CONFIG_ALPHA @@ -115,7 +129,8 @@ static void ide_gd_shutdown(ide_drive_t *drive) the disk to expire its write cache. */ if (system_state != SYSTEM_POWER_OFF) { #else - if (system_state == SYSTEM_RESTART) { + if (system_state == SYSTEM_RESTART && + !dmi_check_system(ide_coldreboot_table)) { #endif drive->disk_ops->flush(drive); return; From 83cff839268feb2f31ae7667b9b2b7641dc10575 Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Wed, 22 Apr 2009 20:33:41 +0200 Subject: [PATCH 4/7] mediabay: fix build for CONFIG_BLOCK=n MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Tuesday 14 April 2009 20:31:21 Subrata Modak wrote: > Observed the following build error: > --- > CC drivers/macintosh/mediabay.o > In file included from drivers/macintosh/mediabay.c:21: > include/linux/ide.h:605: error: field ‘request_sense_rq’ has incomplete > type > make[2]: *** [drivers/macintosh/mediabay.o] Error 1 > make[1]: *** [drivers/macintosh] Error 2 > make: *** [drivers] Error 2 > --- mediabay shouldn't include unconditionally so remove the superfluous include from mediabay.c ( will pull in for CONFIG_BLK_DEV_IDE_PMAC=y). Reported-by: Subrata Modak Cc: Paul Mackerras Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/macintosh/mediabay.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c index d7e46d345d9e..eca55ef185b5 100644 --- a/drivers/macintosh/mediabay.c +++ b/drivers/macintosh/mediabay.c @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include From 33e86019f77b6358bfe06767e08154be032d8751 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Thu, 23 Apr 2009 22:53:43 +0200 Subject: [PATCH 5/7] palm_bk3710: those registers/bitfields don't exist Bugfixes noted by checking the code against the controller documentation (TI document number SPRUE21): - Remove declarations for eight non-existent registers (!); and remove accesses to two of them. - Remove access to various non-existent bitfields in some of the registers which *do* exist (those fields must-be-zero). - Provide comment to replace bogus reset logic (removed above, it relied on non-existent bitfields). Resets require GPIO help; this driver doesn't currently know about that. With some minor cleanup: relocate a comment, avoid an extra lookup of the PIO timings. Signed-off-by: David Brownell Cc: Sergei Shtylyov Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/palm_bk3710.c | 67 +++++++++++---------------------------- 1 file changed, 18 insertions(+), 49 deletions(-) diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c index d1513b4a457c..13a5449aabf6 100644 --- a/drivers/ide/palm_bk3710.c +++ b/drivers/ide/palm_bk3710.c @@ -42,16 +42,9 @@ #define BK3710_BMICP 0x00 #define BK3710_BMISP 0x02 #define BK3710_BMIDTP 0x04 -#define BK3710_BMICS 0x08 -#define BK3710_BMISS 0x0A -#define BK3710_BMIDTS 0x0C #define BK3710_IDETIMP 0x40 -#define BK3710_IDETIMS 0x42 -#define BK3710_SIDETIM 0x44 -#define BK3710_SLEWCTL 0x45 #define BK3710_IDESTATUS 0x47 #define BK3710_UDMACTL 0x48 -#define BK3710_UDMATIM 0x4A #define BK3710_MISCCTL 0x50 #define BK3710_REGSTB 0x54 #define BK3710_REGRCVR 0x58 @@ -63,7 +56,6 @@ #define BK3710_UDMATRP 0x70 #define BK3710_UDMAENV 0x74 #define BK3710_IORDYTMP 0x78 -#define BK3710_IORDYTMS 0x7C static unsigned ideclk_period; /* in nanoseconds */ @@ -96,11 +88,6 @@ static void palm_bk3710_setudmamode(void __iomem *base, unsigned int dev, trp = DIV_ROUND_UP(palm_bk3710_udmatimings[mode].rptime, ideclk_period) - 1; - /* udmatim Register */ - val16 = readw(base + BK3710_UDMATIM) & (dev ? 0xFF0F : 0xFFF0); - val16 |= (mode << (dev ? 4 : 0)); - writew(val16, base + BK3710_UDMATIM); - /* udmastb Ultra DMA Access Strobe Width */ val32 = readl(base + BK3710_UDMASTB) & (0xFF << (dev ? 0 : 8)); val32 |= (t0 << (dev ? 8 : 0)); @@ -161,10 +148,11 @@ static void palm_bk3710_setpiomode(void __iomem *base, ide_drive_t *mate, u32 val32; struct ide_timing *t; + t = ide_timing_find_mode(XFER_PIO_0 + mode); + /* PIO Data Setup */ t0 = DIV_ROUND_UP(cycletime, ideclk_period); - t2 = DIV_ROUND_UP(ide_timing_find_mode(XFER_PIO_0 + mode)->active, - ideclk_period); + t2 = DIV_ROUND_UP(t->active, ideclk_period); t2i = t0 - t2 - 1; t2 -= 1; @@ -185,7 +173,6 @@ static void palm_bk3710_setpiomode(void __iomem *base, ide_drive_t *mate, } /* TASKFILE Setup */ - t = ide_timing_find_mode(XFER_PIO_0 + mode); t0 = DIV_ROUND_UP(t->cyc8b, ideclk_period); t2 = DIV_ROUND_UP(t->act8b, ideclk_period); @@ -234,42 +221,23 @@ static void palm_bk3710_set_pio_mode(ide_drive_t *drive, u8 pio) static void __devinit palm_bk3710_chipinit(void __iomem *base) { /* - * enable the reset_en of ATA controller so that when ata signals - * are brought out, by writing into device config. at that - * time por_n signal should not be 'Z' and have a stable value. + * REVISIT: the ATA reset signal needs to be managed through a + * GPIO, which means it should come from platform_data. Until + * we get and use such information, we have to trust that things + * have been reset before we get here. */ - writel(0x0300, base + BK3710_MISCCTL); - - /* wait for some time and deassert the reset of ATA Device. */ - mdelay(100); - - /* Deassert the Reset */ - writel(0x0200, base + BK3710_MISCCTL); /* * Program the IDETIMP Register Value based on the following assumptions * * (ATA_IDETIMP_IDEEN , ENABLE ) | - * (ATA_IDETIMP_SLVTIMEN , DISABLE) | - * (ATA_IDETIMP_RDYSMPL , 70NS) | - * (ATA_IDETIMP_RDYRCVRY , 50NS) | - * (ATA_IDETIMP_DMAFTIM1 , PIOCOMP) | * (ATA_IDETIMP_PREPOST1 , DISABLE) | - * (ATA_IDETIMP_RDYSEN1 , DISABLE) | - * (ATA_IDETIMP_PIOFTIM1 , DISABLE) | - * (ATA_IDETIMP_DMAFTIM0 , PIOCOMP) | * (ATA_IDETIMP_PREPOST0 , DISABLE) | - * (ATA_IDETIMP_RDYSEN0 , DISABLE) | - * (ATA_IDETIMP_PIOFTIM0 , DISABLE) + * + * DM6446 silicon rev 2.1 and earlier have no observed net benefit + * from enabling prefetch/postwrite. */ - writew(0xB388, base + BK3710_IDETIMP); - - /* - * Configure SIDETIM Register - * (ATA_SIDETIM_RDYSMPS1 ,120NS ) | - * (ATA_SIDETIM_RDYRCYS1 ,120NS ) - */ - writeb(0, base + BK3710_SIDETIM); + writew(BIT(15), base + BK3710_IDETIMP); /* * UDMACTL Ultra-ATA DMA Control @@ -281,11 +249,11 @@ static void __devinit palm_bk3710_chipinit(void __iomem *base) /* * MISCCTL Miscellaneous Conrol Register - * (ATA_MISCCTL_RSTMODEP , 1) | - * (ATA_MISCCTL_RESETP , 0) | + * (ATA_MISCCTL_HWNHLD1P , 1 cycle) + * (ATA_MISCCTL_HWNHLD0P , 1 cycle) * (ATA_MISCCTL_TIMORIDE , 1) */ - writel(0x201, base + BK3710_MISCCTL); + writel(0x001, base + BK3710_MISCCTL); /* * IORDYTMP IORDY Timer for Primary Register @@ -355,10 +323,9 @@ static int __init palm_bk3710_probe(struct platform_device *pdev) clk_enable(clk); rate = clk_get_rate(clk); - ideclk_period = 1000000000UL / rate; - /* Register the IDE interface with Linux ATA Interface */ - memset(&hw, 0, sizeof(hw)); + /* NOTE: round *down* to meet minimum timings; we count in clocks */ + ideclk_period = 1000000000UL / rate; mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (mem == NULL) { @@ -388,6 +355,7 @@ static int __init palm_bk3710_probe(struct platform_device *pdev) /* Configure the Palm Chip controller */ palm_bk3710_chipinit(base); + memset(&hw, 0, sizeof(hw)); for (i = 0; i < IDE_NR_PORTS - 2; i++) hw.io_ports_array[i] = (unsigned long) (base + IDE_PALM_ATA_PRI_REG_OFFSET + i); @@ -400,6 +368,7 @@ static int __init palm_bk3710_probe(struct platform_device *pdev) palm_bk3710_port_info.udma_mask = rate < 100000000 ? ATA_UDMA4 : ATA_UDMA5; + /* Register the IDE interface with Linux */ rc = ide_host_add(&palm_bk3710_port_info, hws, NULL); if (rc) goto out; From d7f5143522d938ea7c4f117c6fa6b1d3fa5af994 Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Thu, 23 Apr 2009 22:53:45 +0200 Subject: [PATCH 6/7] palm_bk3710: palm_bk3710_udmatimings[] CodingStyle fixup Remove superfluous commas and add missing whitespaces. Noticed-by: Joe Perches Cc: David Brownell Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/palm_bk3710.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c index 13a5449aabf6..09d813d313f4 100644 --- a/drivers/ide/palm_bk3710.c +++ b/drivers/ide/palm_bk3710.c @@ -66,12 +66,12 @@ struct palm_bk3710_udmatiming { }; static const struct palm_bk3710_udmatiming palm_bk3710_udmatimings[6] = { - {160, 240 / 2,}, /* UDMA Mode 0 */ - {125, 160 / 2,}, /* UDMA Mode 1 */ - {100, 120 / 2,}, /* UDMA Mode 2 */ - {100, 90 / 2,}, /* UDMA Mode 3 */ - {100, 60 / 2,}, /* UDMA Mode 4 */ - {85, 40 / 2,}, /* UDMA Mode 5 */ + { 160, 240 / 2 }, /* UDMA Mode 0 */ + { 125, 160 / 2 }, /* UDMA Mode 1 */ + { 100, 120 / 2 }, /* UDMA Mode 2 */ + { 100, 90 / 2 }, /* UDMA Mode 3 */ + { 100, 60 / 2 }, /* UDMA Mode 4 */ + { 85, 40 / 2 }, /* UDMA Mode 5 */ }; static void palm_bk3710_setudmamode(void __iomem *base, unsigned int dev, From b930f964cfe65941c6b1ba61efedfe49da3f6353 Mon Sep 17 00:00:00 2001 From: Bartlomiej Zolnierkiewicz Date: Thu, 23 Apr 2009 22:53:45 +0200 Subject: [PATCH 7/7] MAINTAINERS: update IDE entry By a popular demand quilt tree was replaced by a git one. Signed-off-by: Bartlomiej Zolnierkiewicz --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 0beac8a7f8f2..2431b05c871b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2743,7 +2743,7 @@ IDE SUBSYSTEM P: Bartlomiej Zolnierkiewicz M: bzolnier@gmail.com L: linux-ide@vger.kernel.org -T: quilt kernel.org/pub/linux/kernel/people/bart/pata-2.6/ +T: git git://git.kernel.org/pub/scm/linux/kernel/git/bart/ide-2.6.git S: Maintained F: Documentation/ide/ F: drivers/ide/