Re: Scary Intel SATA problem: "frozen"
Hello. Mark Lord wrote: Bit #4, when actually implemented, is a rotational seek indicator, which can be used for timing purposes. Hm, I thought it was DSC (drive seek complete) set by the SEEK command completion, and it's always implemented. Didn't you mean IDX (bit 1, IIRC)? But when BUSY (bit #7) is set, the rest are generally nonsense. Indeed... WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Scary Intel SATA problem: "frozen"
Hello. Eric D. Mudama wrote: > Bit #4, when actually implemented, is a rotational seek indicator, > which can be used for timing purposes. Hm, I thought it was DSC (drive seek complete) set by the SEEK command completion, and it's always implemented. Didn't you mean IDX (bit 1, IIRC)? 0x50 is the standard, non queueing "device is ready" status. It used to have those special meanings, but they're pretty obsolete today as I understand it. Erm, some status bits maybe obsolete but I've never heard that the status *values* were specified to mean anything special anywhere... 0x40 is used for queueing, because bit 4 was the service bit for PATA TCQ. I know. This meaning (SERVICE) actualy came from ATAPI > But when BUSY (bit #7) is set, the rest are generally nonsense. Indeed... WBR, Sergei Typically, 0x80 as the busy state indicates the device is in POR reset. Once the firmware is up and running in the device, it often switches from 0x80 to 0xD0 during POR. Oh, I guess it's completely up to the disk makers what other status to show with BSY=1. 0xD0 is the busy state you'd get to if you were 0x50 and received a command, so this is reported typically after the device is up and running. 0x7F usually is hardware indicating nothing is attached to the port, and isn't supposed to infer a non-busy state. Ha, *never* seen that one. It's has always been 0xFF since PC people didn't ever bother themselves with silly pulldowns. :-) You're right, while not meaningful according to spec, you can derive some information from the reported status even when you're only supposed to look at one bit. Well, to some extent... WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: [PATCH] (2.6.19-rc6-mm2) pdc202xx_new: fix PIO mode setup
Shoot! CCed Alan Cox twice, instead of CCing linux-ide...
-- Forwarded Message --
Subject: [PATCH] (2.6.19-rc6-mm2) pdc202xx_new: fix PIO mode setup
Date: Thursday 30 November 2006 20:54
From: Sergei Shtylyov <[EMAIL PROTECTED]>
To: [EMAIL PROTECTED], [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED], [EMAIL PROTECTED]
Fix pdcnew_tune_drive() to always set the PIO mode requested, not pick the
best possible one, change pdcnew_config_drive_xfer_rate() accordingly, and
get rid of the duplicate tuneproc() call in config_chipset_for_dma().
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/pdc202xx_new.c | 15 +--
1 files changed, 5 insertions(+), 10 deletions(-)
Index: linux-2.6/drivers/ide/pci/pdc202xx_new.c
===
--- linux-2.6.orig/drivers/ide/pci/pdc202xx_new.c
+++ linux-2.6/drivers/ide/pci/pdc202xx_new.c
@@ -9,6 +9,7 @@
* Split from:
* linux/drivers/ide/pdc202xx.c Version 0.35Mar. 30, 2002
* Copyright (C) 1998-2002Andre Hedrick <[EMAIL PROTECTED]>
+ * Copyright (C) 2005-2006MontaVista Software, Inc.
* Portions Copyright (C) 1999 Promise Technology, Inc.
* Author: Frank Tiernan ([EMAIL PROTECTED])
* Released under terms of General Public License
@@ -168,12 +169,8 @@ static int pdcnew_new_tune_chipset (ide_
*/
static void pdcnew_tune_drive(ide_drive_t *drive, u8 pio)
{
- u8 speed;
-
- if (pio == 5) pio = 4;
- speed = XFER_PIO_0 + ide_get_best_pio_mode(drive, 255, pio, NULL);
-
- (void)pdcnew_new_tune_chipset(drive, speed);
+ pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
+ (void)pdcnew_new_tune_chipset(drive, XFER_PIO_0 + pio);
}
static u8 pdcnew_new_cable_detect (ide_hwif_t *hwif)
@@ -207,10 +204,8 @@ static int config_chipset_for_dma (ide_d
speed = ide_dma_speed(drive, pdcnew_ratemask(drive));
- if (!(speed)) {
- hwif->tuneproc(drive, 5);
+ if (!speed)
return 0;
- }
(void) hwif->speedproc(drive, speed);
return ide_dma_enable(drive);
@@ -234,7 +229,7 @@ static int pdcnew_config_drive_xfer_rate
} else if ((id->capability & 8) || (id->field_valid & 2)) {
fast_ata_pio:
- hwif->tuneproc(drive, 5);
+ hwif->tuneproc(drive, 255);
return hwif->ide_dma_off_quietly(drive);
}
/* IORDY not supported */
---
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] winbond IDE depends on IDEDMA
Hello. Olaf Hering wrote: winbond ide depends on idedma. [...] Index: linux-2.6.19/drivers/ide/Kconfig === --- linux-2.6.19.orig/drivers/ide/Kconfig +++ linux-2.6.19/drivers/ide/Kconfig @@ -391,7 +391,7 @@ config BLK_DEV_RZ1000 config BLK_DEV_SL82C105 tristate "Winbond SL82c105 support" - depends on PCI && (PPC || ARM) && BLK_DEV_IDEPCI + depends on PCI && (PPC || ARM) && BLK_DEV_IDEPCI && BLK_DEV_IDEDMA_PCI help If you have a Winbond SL82c105 IDE controller, say Y here to enable special configuration for this chip. This is common on various CHRP This whole item should be moved below, under ifdef BLK_DEV_IDEDMA_PCI then, for consistency. Please recast. WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.19-rc6-mm2) pdc202xx_new cleanup
Clean up the driver in preparation of the coming major fixes:
- replace the stupid pdcnew_new_ prefixes with mere pdcnew_;
- get rid of useless parens and type casts, clean up some printk's;
- incorporte Albert Lee's former style cleanup patch from removing spaces
from the function definitions and adding some new lines here and there
(http://marc.theaimsgroup.com/?l=linux-ide&m=110992442032300&w=2);
- use a better criterion for determining higher UltraDMA modes, and add
a comment concerning the doubtful value of the IORDY/prefetch enabling
in config_chipset_for_dma().
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/pdc202xx_new.c | 60 +++--
1 files changed, 35 insertions(+), 25 deletions(-)
Index: linux-2.6/drivers/ide/pci/pdc202xx_new.c
===
--- linux-2.6.orig/drivers/ide/pci/pdc202xx_new.c
+++ linux-2.6/drivers/ide/pci/pdc202xx_new.c
@@ -77,7 +77,7 @@ static const char *pdc_quirk_drives[] =
set_2regs(0x13,(c));\
} while(0)
-static u8 pdcnew_ratemask (ide_drive_t *drive)
+static u8 pdcnew_ratemask(ide_drive_t *drive)
{
u8 mode;
@@ -96,12 +96,14 @@ static u8 pdcnew_ratemask (ide_drive_t *
default:
return 0;
}
+
if (!eighty_ninty_three(drive))
mode = min(mode, (u8)1);
+
return mode;
}
-static int check_in_drive_lists (ide_drive_t *drive, const char **list)
+static int check_in_drive_lists(ide_drive_t *drive, const char **list)
{
struct hd_driveid *id = drive->id;
@@ -121,7 +123,7 @@ static int check_in_drive_lists (ide_dri
return 0;
}
-static int pdcnew_new_tune_chipset (ide_drive_t *drive, u8 xferspeed)
+static int pdcnew_tune_chipset(ide_drive_t *drive, u8 xferspeed)
{
ide_hwif_t *hwif= HWIF(drive);
unsigned long indexreg = hwif->dma_vendor1;
@@ -157,7 +159,7 @@ static int pdcnew_new_tune_chipset (ide_
;
}
- return (ide_config_drive_speed(drive, speed));
+ return ide_config_drive_speed(drive, speed);
}
/* 0123456 7 8
@@ -170,34 +172,39 @@ static int pdcnew_new_tune_chipset (ide_
static void pdcnew_tune_drive(ide_drive_t *drive, u8 pio)
{
pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
- (void)pdcnew_new_tune_chipset(drive, XFER_PIO_0 + pio);
+ (void)pdcnew_tune_chipset(drive, XFER_PIO_0 + pio);
}
-static u8 pdcnew_new_cable_detect (ide_hwif_t *hwif)
+static u8 pdcnew_cable_detect(ide_hwif_t *hwif)
{
hwif->OUTB(0x0b, hwif->dma_vendor1);
- return ((u8)((hwif->INB(hwif->dma_vendor3) & 0x04)));
+ return hwif->INB(hwif->dma_vendor3) & 0x04;
}
-static int config_chipset_for_dma (ide_drive_t *drive)
+
+static int config_chipset_for_dma(ide_drive_t *drive)
{
struct hd_driveid *id = drive->id;
ide_hwif_t *hwif= HWIF(drive);
- u8 speed= -1;
- u8 cable;
-
- u8 ultra_66 = ((id->dma_ultra & 0x0010) ||
- (id->dma_ultra & 0x0008)) ? 1 : 0;
-
- cable = pdcnew_new_cable_detect(hwif);
+ u8 ultra_66 = (id->dma_ultra & 0x0078) ? 1 : 0;
+ u8 cable= pdcnew_cable_detect(hwif);
+ u8 speed;
if (ultra_66 && cable) {
- printk(KERN_WARNING "Warning: %s channel requires an 80-pin
cable for operation.\n", hwif->channel ? "Secondary":"Primary");
+ printk(KERN_WARNING "Warning: %s channel "
+ "requires an 80-pin cable for operation.\n",
+ hwif->channel ? "Secondary" : "Primary");
printk(KERN_WARNING "%s reduced to Ultra33 mode.\n",
drive->name);
}
if (drive->media != ide_disk)
return 0;
- if (id->capability & 4) { /* IORDY_EN & PREFETCH_EN */
+
+ if (id->capability & 4) {
+ /*
+* Set IORDY_EN & PREFETCH_EN (this seems to have
+* NO real effect since this register is reloaded
+* by hardware when the transfer mode is selected)
+*/
hwif->OUTB((0x13 + ((drive->dn%2) ? 0x08 : 0x00)),
hwif->dma_vendor1);
hwif->OUTB((hwif->INB(hwif->dma_vendor3)|0x03),
hwif->dma_vendor3);
}
@@ -211,7 +218,7 @@ static int config_chipset_for_dma (ide_d
return ide_dma_enable(drive);
}
-static int pdcnew_config_drive_xfer_rate (ide_drive_t *drive)
+static int pdcnew_config_drive_xfer_rate(ide_drive_t *drive)
{
ide_hwif_t *hwif= HWIF(drive);
Re: [PATCH] (2.6.19-rc6-mm2) pdc202xx_new cleanup
Hello. Alan wrote: I believe this is completely the wrong thing to do. Adding a ton of changes to the existing (and stable) life expired drivers/ide driver rather than keeping new and risky stuff in the new libata code is bad. The new and risky stuff is long agon in there. The existing code *works*, its been rock solid since the reset drain fix Don't make me laugh. pdc202xx_new certainly doesn't deserve these compliments. It has known PLL problems even on x86 if you have more than 2 contorollers, and on non-x86 this turns into its complete inability to support anything above UltraDMA/33. and it is the code everyone who is conservative relies on not to eat their data. We have somewhere to do new and cool stuff its libata. I'm not interested in libata development currently, and interested in fixing the age-old crap in drivers/ide/. I don't see the point in risking destabilising a good solid driver. I can just about see justification for !X86 implementation of the PLL handling but that is about it. All in a good time. Alan WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] (2.6.19-rc6-mm2) pdc202xx_new cleanup
Hello. Sergei Shtylyov wrote: I believe this is completely the wrong thing to do. Adding a ton of changes to the existing (and stable) life expired drivers/ide driver rather than keeping new and risky stuff in the new libata code is bad. The new and risky stuff is long agon in there. The existing code *works*, its been rock solid since the reset drain fix Don't make me laugh. pdc202xx_new certainly doesn't deserve these compliments. It has known PLL problems even on x86 if you have more than 2 contorollers Oh, and I forgot to add that Ultra133 chips don't get the proper timings even on x86 -- they get overclocked b/c BIOS programs 133 MHz DPLL clock and the chip auto-loads the 100 MHz timings (overriding the driver's override). I don't see the point in risking destabilising a good solid driver. I can just about see justification for !X86 implementation of the PLL handling but that is about it. All in a good time. The main patches are gonna appear in a few days (at last). Alan WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pata_sl82c105 can not reserve IO region
Olaf Hering wrote: The printk in pci_request_region has 'bar + 1', so 6 should be possible if i becomes 5. Does the IO region of the last bar look correct? I'd say it looks suspicious since it's not adjacent to all the other regions... In fact, after looking at your /proc/ioports/ I can say that the BAR is actually unassigned and its *actual* value is 0 which the driver may not like (the ones that lspci show are the physical memory addresses not the actual I/O space addresses in this case). That's why the reservation fails. 00:03.1 IDE interface: Symphony Labs SL82c105 (rev 05) (prog-if 8f [Master SecP SecO PriP PriO]) Control: I/O+ Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Well, BAR5 is indeed 0. 30: 00 00 00 00 00 00 00 00 00 00 00 00 56 01 02 28 40: b3 08 ff 00 09 09 00 00 09 09 00 00 09 09 00 00 50: 09 09 00 00 00 00 00 00 00 00 00 00 00 00 00 00 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 70: 00 00 00 00 00 00 00 00 00 00 00 00 ff 00 00 00 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 name "ide" linux,phandle00d5cdc0 (14011840) assigned-addresses 81001910 f000 0008 81001914 f010 0004 81001918 f020 0008 8100191c f030 0004 81001920 f040 0010 81001924 0010 Yeah, the device tree has 0 for BAR5 too... WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] (2.6.19-rc6-mm2) pdc202xx_new cleanup
Hello. Sergei Shtylyov wrote: I believe this is completely the wrong thing to do. Adding a ton of changes to the existing (and stable) life expired drivers/ide driver rather than keeping new and risky stuff in the new libata code is bad. The new and risky stuff is long agon in there. So, they're neither that new nor risky... The question is why I have to push the drivers/ide/pcu/pdc202xx_new changes which are now 1.5 years old already into the -mm tree again. The existing code *works*, its been rock solid since the reset drain fix Don't make me laugh. pdc202xx_new certainly doesn't deserve these compliments. It has known PLL problems even on x86 if you have more than 2 contorollers Oh, and I forgot to add that Ultra133 chips don't get the proper timings even on x86 -- they get overclocked b/c BIOS programs 133 MHz DPLL clock and the chip auto-loads the 100 MHz timings (overriding the driver's override). I must admit that I've gona a bit too far here, having forgotten how Promise BIOSes setup the chips. The overclocking only happens if you have at least one UltraDMA/133 capable drive plugged in -- in this cases, PIO modes will be overclocked for this drives and any modes for the non-UltraDMA/133 drives if you also have those plugged in... I don't see the point in risking destabilising a good solid driver. I can Risk of destablilizing the driver in the experimental tree? Isn't that why it came into being at all? Also, can you point me out the parts of the cleaup patch which you consider risky? just about see justification for !X86 implementation of the PLL handling but that is about it. All in a good time. The main patches are gonna appear in a few days (at last). Alan WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mark PCI resource with start 0 as unassigned
Hello.
Olaf Hering wrote:
mark pci resources with start 0 as unassigned
libata calls pci_request_regions to claim bar 0 - 5
bar 5 has base 0.
Tested on a p630 in SMP mode with pata_sl82c105
[...]
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -1234,6 +1234,14 @@ static void __devinit fixup_resource(str
struct pci_controller *hose = pci_bus_to_host(dev->bus);
unsigned long start, end, mask, offset;
+ /*
+* tell the core code that this ressource is unassigned
+* fixes p630 winbond IDE with libata
+*/
+ if (res->start == 0) {
+ res->flags = 0;
Wouldn't it be better to set flags to IORESOURCE_UNSET to get it reassigned?
+ return;
+ }
if (res->flags & IORESOURCE_IO) {
offset = (unsigned long)hose->io_base_virt - pci_io_base;
WBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mark PCI resource with start 0 as unassigned
Hello.
Segher Boessenkool wrote:
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -1234,6 +1234,14 @@ static void __devinit fixup_resource(str
struct pci_controller *hose = pci_bus_to_host(dev->bus);
unsigned long start, end, mask, offset;
+ /*
+* tell the core code that this ressource is unassigned
+* fixes p630 winbond IDE with libata
+*/
+ if (res->start == 0) {
+ res->flags = 0;
+ return;
+ }
if (res->flags & IORESOURCE_IO) {
offset = (unsigned long)hose->io_base_virt - pci_io_base;
Please make this run on pSeries only; on a PowerMac for
example, it's totally normal that the first PCI legacy I/O
BAR in the system gets assigned 0.
What do you mean by legacy I/O BAR? If you mean IDE controller, that would
drive IDE core mad like this:
W82C105_IDE: inconsistent baseregs (BIOS) for port 0, skipping
Segher
WBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mark PCI resource with start 0 as unassigned
Hello. Segher Boessenkool wrote: Please make this run on pSeries only; on a PowerMac for example, it's totally normal that the first PCI legacy I/O BAR in the system gets assigned 0. What do you mean by legacy I/O BAR? Any PCI BAR with bits 1:0 == 0b01. If you mean IDE controller, that would drive IDE core mad like this: W82C105_IDE: inconsistent baseregs (BIOS) for port 0, skipping So that needs fixing too, then. I'd agree here, that check in the IDE code seems like being too x86 specific. I'm having issues with it as well on MPC85xx/U-Boot... Segher WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mark PCI resource with start 0 as unassigned
Hello. Alan wrote: W82C105_IDE: inconsistent baseregs (BIOS) for port 0, skipping So that needs fixing too, then. Both PCI core and IDE interpret a zero length resource as unassigned. This is not about 0-length resource, this is about 0-address. Look at ide_hwif_confiure() in drivers/ide/setup-pci.c... That is probably better than clearing the flags in retrospect. I'd agree here, that check in the IDE code seems like being too x86 specific. I'm having issues with it as well on MPC85xx/U-Boot... setup-pci is for SFF8038i devices. It therefore knows that for assigned resources they must be I/O. It also assumes that zero is not a valid I/O port just like zero is not a valid IRQ. You should know that the IRQ assumption is *not* true even for x86 since IRQ0 is and has always been a perfectly valid IRQ (used by PIT). Stick a real IDE resource at zero > and drivers/ide can't cope. Yeah, I've noticed. Unfortunately, a lot of PPC platforms (at least) are doing exactly this... Alan WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the correct way to indicate an unassigned PCI resource ?
Hello. Alan wrote: Both PCI core and IDE interpret a zero length resource as unassigned. This is not about 0-length resource, this is about 0-address. Look at ide_hwif_confiure() in drivers/ide/setup-pci.c... The discussion I was having was about sl82cxx and handling unassigned resources. The zero address isn't relevant to that. You were following up to the particular error message emitted by the IDE core (which you've now deleted), so I corrected you on its reason, that's all. You should know that the IRQ assumption is *not* true even for x86 since IRQ0 is and has always been a perfectly valid IRQ (used by PIT). Please see previous million recyclings of that discussion and Linus answer. When Linus remaps IRQ0 on x86, I'll follow that code as a testament. Until this happens, I consider is just an opinion. Forcing every arch but x86 to remap IRQ0 is an example of the double standards. Stick a real IDE resource at zero > and drivers/ide can't cope. Yeah, I've noticed. Unfortunately, a lot of PPC platforms (at least) are doing exactly this... The checks need pushing upwards and removing from their current place - the pci layer should check the resource length, the isa pnp should I believe check for zero address etc. So, it's OK to remove the base *address* check in ide_hwif_confiure() altogether? libata makes a similar assumption in ata_resources_present() as someone (GregKH ???) needs to define what the proper way to encode "resource not allocated" into the PCI resources should be. If someone on the PCI list (cc'd) or Greg can give a definitive answer then we can go fix the offenders now. Well, I thought that was IORESOURCE_UNSET... Alan WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the correct way to indicate an unassigned PCI resource ?
Hello. Alan wrote: When Linus remaps IRQ0 on x86, I'll follow that code as a testament. Until this happens, I consider is just an opinion. Forcing every arch but x86 to remap IRQ0 is an example of the double standards. Yawn.. x86 does not expose IRQ 0 outside of arch specific code. Can you believe, some non-x86 platofrms also don't -- for example, IRQ0 may be internal to SOC, not shareable or routable outside of it, BUT the SOC device is driven by the standard driver (I'm minding 8250.c here). Yet we're told that we should remap it, period... The checks need pushing upwards and removing from their current place - the pci layer should check the resource length, the isa pnp should I believe check for zero address etc. So, it's OK to remove the base *address* check in ide_hwif_confiure() altogether? IFF you move all the other checks, verify their correctness and then get them tested for a while yes All other checks aren't hindering (at least me ATM). What's probably worth doing is to check the result of ide_pci_check_iomem() some lines above the discussed check (it's ignored now). WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the correct way to indicate an unassigned PCI resource ?
Hello. Sergei Shtylyov wrote: When Linus remaps IRQ0 on x86, I'll follow that code as a testament. Until this happens, I consider is just an opinion. Forcing every arch but x86 to remap IRQ0 is an example of the double standards. Yawn.. x86 does not expose IRQ 0 outside of arch specific code. Can you believe, some non-x86 platofrms also don't -- for example, IRQ0 may be internal to SOC, not shareable or routable outside of it, BUT the SOC device is driven by the standard driver (I'm minding 8250.c here). Yet we're told that we should remap it, period... The checks need pushing upwards and removing from their current place - the pci layer should check the resource length, the isa pnp should I believe check for zero address etc. Although, I'm getting the point -- PCI is likely to return 0 for unassigned the interrupt line register (this isn't always true though). So, some mixup is possible in that regard. Well, then we're unlucky, and indeed remapping IRQ0 has sense... WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the correct way to indicate an unassigned PCI resource ?
Hello. Gabriel Paubert wrote: On Mon, 2006-12-04 at 14:22 +, Alan wrote: The discussion I was having was about sl82cxx and handling unassigned resources. The zero address isn't relevant to that. Well, actually, it's unclear to me wether the resource is unassigned or has been assigned to 0 :-) And in the later case, why claim'ing it fails. Well, I don't have the PCI specification, but I have a device with the Try googling for pdf21.pdf, pdf22.pdf if you need it. :-) following gem in its errata (name edited and changed to ): ""The contains two PCI base registers to allow for both greater flexibility in tightly constrained I/O space as well as the "on the fly" option to access the registers from either I/O or memory space. Both PCI base registers contained in the will accept the value of "zero" as a valid and decodable address. This differs from the PCI 2.1 specification, where a zero value being written to the PCI base register should disable the register space. I haven't found such words in PCI 2.1 -- it only said that 0 is not a valid address (those words are gone from 2.2). will continue to decode for register accesses using the value "zero" written to the PCI_BS register as the base address for decoding."" I'd say it's absolutely valid bahavior. which makes me suspect that a base address of zero really should mean unassigned and is a way to disable base registers on a region by region basis. AFAIR, there's never been a provision to enable/disable BARs on an individual basis in PCI (except the expansion ROM BAR). The decoders are only controlled via 2 command register bits for I/O and memory space. Now the fun with this device is that the I/O region occupies 4kB, so That's a crappy device indeed, I/O alloction limit is 256 bytes (that's due to PC-specific limitation actually). Such regions may (and should) be left unassigned by f/w (and I/O decoder disabled). it leads to serious conflicts since there is also an ISA bridge in the system (no 8259 anymore, serial console dead, etc...). The best way to make this bug harmless is to write all ones to said base register (it has 32 bit). This moves it to an address which cannot be generated by the host bridge (unless you program it in a really weird way). You may also completely disable the I/O decoder for this device. Whatever a specification says, you'll always find some device that has a bug in this area. Yeah. I've already encountered such one... WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.19-rc6-mm2) sl82c105: straighten up IDE control/status register caching
Straighten up the IDE control/status register caching -- you *really* can't
cache the shared register per-channel and hope that it won't get out ouf sync.
Set the PIO fallback mode to PIO0 for the slave drive as well as master --
there was no point in having them different (most probably a resutl of typo).
Do a bit of reformat and cleanup while at it...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/sl82c105.c | 31 +--
1 files changed, 13 insertions(+), 18 deletions(-)
Index: linux-2.6/drivers/ide/pci/sl82c105.c
===
--- linux-2.6.orig/drivers/ide/pci/sl82c105.c
+++ linux-2.6/drivers/ide/pci/sl82c105.c
@@ -299,14 +299,14 @@ static void sl82c105_selectproc(ide_driv
//DBG(("sl82c105_selectproc(drive:%s)\n", drive->name));
mask = hwif->channel ? CTRL_P1F16 : CTRL_P0F16;
- old = val = *((u32 *)&hwif->hwif_data);
+ old = val = (u32)pci_get_drvdata(dev);
if (drive->using_dma)
val &= ~mask;
else
val |= mask;
if (old != val) {
pci_write_config_dword(dev, 0x40, val);
- *((u32 *)&hwif->hwif_data) = val;
+ pci_set_drvdata(dev, (void *)val);
}
}
@@ -316,14 +316,13 @@ static void sl82c105_selectproc(ide_driv
*/
static void sl82c105_resetproc(ide_drive_t *drive)
{
- ide_hwif_t *hwif = HWIF(drive);
- struct pci_dev *dev = hwif->pci_dev;
+ struct pci_dev *dev = HWIF(drive)->pci_dev;
u32 val;
DBG(("sl82c105_resetproc(drive:%s)\n", drive->name));
pci_read_config_dword(dev, 0x40, &val);
- *((u32 *)&hwif->hwif_data) = val;
+ pci_set_drvdata(dev, (void *)val);
}
/*
@@ -394,6 +393,7 @@ static unsigned int __devinit init_chips
pci_read_config_dword(dev, 0x40, &val);
val |= CTRL_P0EN | CTRL_P0F16 | CTRL_P1F16;
pci_write_config_dword(dev, 0x40, val);
+ pci_set_drvdata(dev, (void *)val);
return dev->irq;
}
@@ -404,30 +404,25 @@ static unsigned int __devinit init_chips
static void __devinit init_hwif_sl82c105(ide_hwif_t *hwif)
{
- struct pci_dev *dev = hwif->pci_dev;
unsigned int rev;
u8 dma_state;
- u32 val;
-
+
DBG(("init_hwif_sl82c105(hwif: ide%d)\n", hwif->index));
hwif->tuneproc = tune_sl82c105;
hwif->selectproc = sl82c105_selectproc;
hwif->resetproc = sl82c105_resetproc;
-
- /* Default to PIO 0 for fallback unless tuned otherwise,
-* we always autotune PIO, this is done before DMA is
-* checked, so there is no risk of accidentally disabling
-* DMA
- */
+
+ /*
+* Default to PIO 0 for fallback unless tuned otherwise.
+* We always autotune PIO, this is done before DMA is checked,
+* so there's no risk of accidentally disabling DMA
+*/
hwif->drives[0].pio_speed = XFER_PIO_0;
hwif->drives[0].autotune = 1;
- hwif->drives[1].pio_speed = XFER_PIO_1;
+ hwif->drives[1].pio_speed = XFER_PIO_0;
hwif->drives[1].autotune = 1;
- pci_read_config_dword(dev, 0x40, &val);
- *((u32 *)&hwif->hwif_data) = val;
-
hwif->atapi_dma = 0;
hwif->mwdma_mask = 0;
hwif->swdma_mask = 0;
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the correct way to indicate an unassigned PCI resource ?
Hello. Grant Grundler wrote: Well, I don't have the PCI specification, but I have a device with the Try googling for pdf21.pdf, pdf22.pdf if you need it. :-) I think you meant pci21.pdf/pci22.pdf/pci23.pdf. And if you find them, trust me when I say whoever is hosting those files can expect a cease-and-desist letter in the mail shortly there after. Better to just ask someone with proper access to lookup the parts you need to know (i.e. ask here). Member companies are listed at: http://www.pcisig.com/membership/about_us/membership_roster/ if you want to approach someone offlist. That's not fun. :-) following gem in its errata (name edited and changed to ): ""The contains two PCI base registers to allow for both greater flexibility in tightly constrained I/O space as well as the "on the fly" option to access the registers from either I/O or memory space. Both PCI base registers contained in the will accept the value of "zero" as a valid and decodable address. This differs from the PCI 2.1 specification, where a zero value being written to the PCI base register should disable the register space. I haven't found such words in PCI 2.1 -- it only said that 0 is not a valid address (those words are gone from 2.2). AFAIK, zero is a valid address for IO Port space on several architectures. But PCI generic code should never use it. See usage of PCIBIOS_MIN_IO and PCIBIOS_MIN_MEM in the various asm-*/pci.h files. That'd be all good if bootloaders also folllowed this rule... Not all of them do, hence the thread. :-/ which makes me suspect that a base address of zero really should mean unassigned and is a way to disable base registers on a region by region basis. AFAIR, there's never been a provision to enable/disable BARs on an individual basis in PCI (except the expansion ROM BAR). The decoders are only controlled via 2 command register bits for I/O and memory space. One can "disable" a BAR by pointing it at an address that is NOT routed by the upstream bridge. Ie CPU physical addresses that can never reach that secondary bus. But I'm not aware of any code to do that currently Yeah, that's the trick that Gabriel has already described... and it certainly won't work with all "PCI-like" (think integrated south bridges) devices. Well, those are using subtractive decoders, so will only get the R/W cycles that nobody else cared about. If using I/O ports higher than 0x the trick will still work on x86 even in presence of ISA bridges... hth, grant WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the correct way to indicate an unassigned PCI resource ?
Hello. Benjamin Herrenschmidt wrote: On Tue, Dec 05, Benjamin Herrenschmidt wrote: Olaf, can you give me a dump of /proc/ioports ? What is sitting at 0 on that PCI bus ? with IDE=y ==> /proc/ioports <== -001f : dma1 So it's indeed colliding with the cruft above. I reckon it's a bug in the firmware of this machine. Add to pseries/pci.c a quirk for that chipset (don't forget to test for machine_is(pseries) in the quirk as they get called for all platforms in a combo kernel. The quirk shall check if resource 6 has a 0 base and clear the size as Alan suggested (possibly setting the UNSET flag as well). Erm, I suspect it's either one or another -- you probably need to keep the size intact for resource marked IORESOURCE_UNSET. At least that's what the other code does... Ben. WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.19-rc6-mm2) pdc202xx_new: fix PLL/timing issues
Fix the CRC errors in the higher UltraDMA modes with the Promise PDC20268 and
newer chips that always occur on non-x86 machines and when there are more than
2 adapters on x86 machines. Fix the overclocking issue for PDC20269 and newer
chips that occurs when an UltraDMA/133 capable drive is connected. Here's the
summary of changes:
- add code to detect the PLL input clock detection and setup it output clock,
remove the PowerMac hacks;
- replace the macros accessing the indexed regiters with functions, switch to
using them where appropriate, gather the PIO/MWDMA/UDMA timings into tables;
- rewrite the speedproc() handler to set the drive's transfer mode first, and
then override the timing registers set by hardware on UltraDMA/133 chips;
- use better criterion for determining higher UltraDMA modes, and add comment
concerning the doubtful value of the code enabling IORDY/prefetch;
- replace the stupid 'pdcnew_new_' prefixes with mere 'pdcnew_';
- get rid of unneded spaces, parens and type casts, clean up some printk's,
add some new lines here and there...
This work is loosely based on these former patches by Albert Lee:
[1] http://marc.theaimsgroup.com/?l=linux-ide&m=110992442032300
[2] http://marc.theaimsgroup.com/?l=linux-ide&m=110992457729382
[3] http://marc.theaimsgroup.com/?l=linux-ide&m=11099247420
[4] http://marc.theaimsgroup.com/?l=linux-ide&m=111019224802939
Some PLL clock detection code was backported from his pata_pdc2027x driver...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
---
This code has been successfully tested by me on PDC2026[89] chips.
I tried to keep this rework as several patches but it made no sense: [2] was
largely a modification of the non-working timing override code, [3] by itself
extended the overclocking issue to the case of non-UltraDMA/133 drives, and
finally, the cleanup patch based on [1] ended up rejected...
drivers/ide/pci/pdc202xx_new.c | 485 +++--
1 files changed, 376 insertions(+), 109 deletions(-)
Index: linux-2.6/drivers/ide/pci/pdc202xx_new.c
===
--- linux-2.6.orig/drivers/ide/pci/pdc202xx_new.c
+++ linux-2.6/drivers/ide/pci/pdc202xx_new.c
@@ -39,6 +39,14 @@
#define PDC202_DEBUG_CABLE 0
+#undef DEBUG
+
+#ifdef DEBUG
+#define DBG(fmt, args...) printk("%s: " fmt, __FUNCTION__, ## args)
+#else
+#define DBG(fmt, args...)
+#endif
+
static const char *pdc_quirk_drives[] = {
"QUANTUM FIREBALLlct08 08",
"QUANTUM FIREBALLP KA6.4",
@@ -51,37 +59,11 @@ static const char *pdc_quirk_drives[] =
NULL
};
-#define set_2regs(a, b)\
- do {\
- hwif->OUTB((a + adj), indexreg);\
- hwif->OUTB(b, datareg); \
- } while(0)
-
-#define set_ultra(a, b, c) \
- do {\
- set_2regs(0x10,(a));\
- set_2regs(0x11,(b));\
- set_2regs(0x12,(c));\
- } while(0)
-
-#define set_ata2(a, b) \
- do {\
- set_2regs(0x0e,(a));\
- set_2regs(0x0f,(b));\
- } while(0)
-
-#define set_pio(a, b, c) \
- do {\
- set_2regs(0x0c,(a));\
- set_2regs(0x0d,(b));\
- set_2regs(0x13,(c));\
- } while(0)
-
-static u8 pdcnew_ratemask (ide_drive_t *drive)
+static u8 max_dma_rate(struct pci_dev *pdev)
{
u8 mode;
- switch(HWIF(drive)->pci_dev->device) {
+ switch(pdev->device) {
case PCI_DEVICE_ID_PROMISE_20277:
case PCI_DEVICE_ID_PROMISE_20276:
case PCI_DEVICE_ID_PROMISE_20275:
@@ -96,12 +78,21 @@ static u8 pdcnew_ratemask (ide_drive_t *
default:
return 0;
}
- if (!eighty_ninty_three(drive))
- mode = min(mode, (u8)1);
+
return mode;
}
-static int check_in_drive_lists (ide_drive_t *drive, const char **list)
+static u8 pdcnew_ratemask(ide_drive_t *drive)
+{
+ u8 mode = max_dma_rate(HWIF(drive)->pci_dev);
+
+ if (!eighty_ninty_three(drive))
+ mode = min_t(u8, mode, 1);
+
+ return mode;
+}
+
+static int check_in_drive_lists(ide_drive_t *drive, const char **list)
{
struct hd_driveid *id = drive->id;
@@ -121,43 +112,141 @@ static int check_in_drive_lists (ide_dri
return 0;
}
-static int pdcnew_new_tu
[PATCH 2.6.19-rc1] Toshiba TC86C001 IDE driver
Behold! This is the driver for the Toshiba TC86C001 GOKU-S IDE controller,
completely reworked from the original brain-damaged Toshiba's 2.4 version.
This single channel UltraDMA/66 controller is very simple in programming, yet
Toshiba managed to plant many interesting bugs in it. The particularly nasty
"limitation 5" (as they call the errata) caused me to abuse the IDE core in a
possibly most interesting way so far. However, this is still better than the
#ifdef mess in drivers/ide/ide-io.c that the original version included (well,
it had much more mess)...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/Kconfig|5
drivers/ide/pci/Makefile |1
drivers/ide/pci/tc86c001.c | 304 +
drivers/pci/quirks.c | 18 ++
include/linux/pci_ids.h|1
5 files changed, 329 insertions(+)
Index: linux-2.6/drivers/ide/Kconfig
===
--- linux-2.6.orig/drivers/ide/Kconfig
+++ linux-2.6/drivers/ide/Kconfig
@@ -742,6 +742,11 @@ config BLK_DEV_VIA82CXXX
This allows the kernel to change PIO, DMA and UDMA speeds and to
configure the chip to optimum performance.
+config BLK_DEV_TC86C001
+ tristate "Toshiba TC86C001 support"
+ help
+ This driver adds support for Toshiba TC86C001 GOKU-S chip.
+
endif
config BLK_DEV_IDE_PMAC
Index: linux-2.6/drivers/ide/pci/Makefile
===
--- linux-2.6.orig/drivers/ide/pci/Makefile
+++ linux-2.6/drivers/ide/pci/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_BLK_DEV_SIIMAGE) += siimag
obj-$(CONFIG_BLK_DEV_SIS5513) += sis5513.o
obj-$(CONFIG_BLK_DEV_SL82C105) += sl82c105.o
obj-$(CONFIG_BLK_DEV_SLC90E66) += slc90e66.o
+obj-$(CONFIG_BLK_DEV_TC86C001) += tc86c001.o
obj-$(CONFIG_BLK_DEV_TRIFLEX) += triflex.o
obj-$(CONFIG_BLK_DEV_TRM290) += trm290.o
obj-$(CONFIG_BLK_DEV_VIA82CXXX)+= via82cxxx.o
Index: linux-2.6/drivers/ide/pci/tc86c001.c
===
--- /dev/null
+++ linux-2.6/drivers/ide/pci/tc86c001.c
@@ -0,0 +1,304 @@
+/*
+ * drivers/ide/pci/tc86c001.c Version 1.00Dec 12, 2006
+ *
+ * Copyright (C) 2002 Toshiba Corporation
+ * Copyright (C) 2005-2006 MontaVista Software, Inc. <[EMAIL PROTECTED]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+
+static inline u8 tc86c001_ratemask(ide_drive_t *drive)
+{
+ return eighty_ninty_three(drive) ? 2 : 1;
+}
+
+static int tc86c001_tune_chipset(ide_drive_t *drive, u8 speed)
+{
+ ide_hwif_t *hwif= HWIF(drive);
+ unsigned long scr_port = hwif->config_data + (drive->dn ? 0x02 : 0x00);
+ u16 mode, scr = hwif->INW(scr_port);
+
+ speed = ide_rate_filter(tc86c001_ratemask(drive), speed);
+
+ switch (speed) {
+ case XFER_UDMA_4: mode = 0x00c0; break;
+ case XFER_UDMA_3: mode = 0x00b0; break;
+ case XFER_UDMA_2: mode = 0x00a0; break;
+ case XFER_UDMA_1: mode = 0x0090; break;
+ case XFER_UDMA_0: mode = 0x0080; break;
+ case XFER_MW_DMA_2: mode = 0x0070; break;
+ case XFER_MW_DMA_1: mode = 0x0060; break;
+ case XFER_MW_DMA_0: mode = 0x0050; break;
+ case XFER_PIO_4:mode = 0x0400; break;
+ case XFER_PIO_3:mode = 0x0300; break;
+ case XFER_PIO_2:mode = 0x0200; break;
+ case XFER_PIO_1:mode = 0x0100; break;
+ case XFER_PIO_0:
+ default:mode = 0x; break;
+ }
+
+ scr &= (speed < XFER_MW_DMA_0) ? 0xf8ff : 0xff0f;
+ scr |= mode;
+ hwif->OUTW(scr, scr_port);
+
+ return ide_config_drive_speed(drive, speed);
+}
+
+static void tc86c001_tune_drive(ide_drive_t *drive, u8 pio)
+{
+ pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
+ (void) tc86c001_tune_chipset(drive, XFER_PIO_0 + pio);
+}
+
+/*
+ * HACKITY HACK
+ *
+ * This is a workaround for the limitation 5 of the TC86C001 IDE controller:
+ * if a DMA transfer terminates prematurely, the controller leaves the device's
+ * interrupt request (INTRQ) pending and does not generate a PCI interrupt (or
+ * set the interrupt bit in the DMA status register), thus no PCI interrupt
+ * will occur until a DMA transfer has been successfully completed.
+ *
+ * We work around this by initiating dummy, zero-length DMA transfer on
+ * a DMA timeout expiration. I found no better way to do this with the current
+ * ID
Re: [PATCH 2.6.19-rc1] Toshiba TC86C001 IDE driver
Hello. On Wednesday 13 December 2006 01:48, Sergei Shtylyov wrote: > Behold! This is the driver for the Toshiba TC86C001 GOKU-S IDE controller, > completely reworked from the original brain-damaged Toshiba's 2.4 version. Shoot, the patch is actually against the most recent Linus' tree, so it's 2.6.20-rc1... Andrew, 2.6.19-rc6-mm2 series keeps failing to completely apply to 2.6.19-rc6 head in my git repo -- don't know why, maybe that's my git... Hence the patch was againt Linus' tree. WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.19-rc1] Toshiba TC86C001 IDE driver
Hello. Randy Dunlap wrote: Behold! This is the driver for the Toshiba TC86C001 GOKU-S IDE controller, completely reworked from the original brain-damaged Toshiba's 2.4 version. This single channel UltraDMA/66 controller is very simple in programming, yet Toshiba managed to plant many interesting bugs in it. The particularly nasty "limitation 5" (as they call the errata) caused me to abuse the IDE core in a possibly most interesting way so far. However, this is still better than the #ifdef mess in drivers/ide/ide-io.c that the original version included (well, it had much more mess)... Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]> drivers/ide/Kconfig|5 drivers/ide/pci/Makefile |1 drivers/ide/pci/tc86c001.c | 304 + drivers/pci/quirks.c | 18 ++ include/linux/pci_ids.h|1 5 files changed, 329 insertions(+) Index: linux-2.6/drivers/ide/Kconfig === --- linux-2.6.orig/drivers/ide/Kconfig +++ linux-2.6/drivers/ide/Kconfig @@ -742,6 +742,11 @@ config BLK_DEV_VIA82CXXX This allows the kernel to change PIO, DMA and UDMA speeds and to configure the chip to optimum performance. +config BLK_DEV_TC86C001 + tristate "Toshiba TC86C001 support" Needs something here like lots of other IDE PCI drivers have: depends on PCI && BLK_DEV_IDEPCI or at least: depends on PCI No, it's already under if BLK_DEV_IDEPCI. And if you really look into Kconfig you'll see hwo it's done there... + help + This driver adds support for Toshiba TC86C001 GOKU-S chip. + endif Here's that endif. WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.19-rc1] Toshiba TC86C001 IDE driver
Hello.
Alan wrote:
+ * We work around this by initiating dummy, zero-length DMA transfer on
+ * a DMA timeout expiration. I found no better way to do this with the current
Novel workaround and probably better than resetting the chip as the
winbong does.
I didn't try resetting however the datasheet suggests it just won't do.
+static int tc86c001_busproc(ide_drive_t *drive, int state)
+{
Waste of space having a busproc routine. The maintainer removed all the
usable hotplug support from old IDE so this might as well be dropped.
Don't know what you mean, ioctl is still there...
@@ -1407,6 +1407,24 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_IN
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,0x260a, quirk_intel_pcie_pm);
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,0x260b, quirk_intel_pcie_pm);
+/*
+ * Toshiba TC86C001 IDE controller reports the standard 8-byte BAR0 size
+ * but PIO transfer won't work if BAR0 falls at the odd 8 bytes.
+ * Re-allocate the region if needed.
+ */
NAK. I think this fixup should be testing if the device port 0 is in
native mode before doing the fixup. In comaptibility mode bar 0 is
The chip is native mode only.
"Close but no cookie": please fix the PCI quirk to match the current -mm
behaviour with the ATA resource tree. Otherwise - nice driver.
Ugh, I should've expected some backstab from -mm tree...
Alan
WBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.20-rc1] Toshiba TC86C001 IDE driver (take 2)
Behold! This is the driver for the Toshiba TC86C001 GOKU-S PCI IDE controller,
completely reworked from the original brain-damaged Toshiba's 2.4 version.
This single channel UltraDMA/66 controller is very simple in programming, yet
Toshiba managed to plant many interesting bugs in it. The particularly nasty
"limitation 5" (as they call the errata) caused me to abuse the IDE core in a
possibly most interesting way so far. However, this is still better than the
#ifdef mess in drivers/ide/ide-io.c that the original version included (well,
it had much more mess)...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
---
I knew I'll miss something due to this haste... :-/
Here's a respun version which fixes the PIO fallback code to not always try to
set PIO5 and makes the driver complain about being to unable reserve the regs
at BAR5, otherwise it's only some wording changes in the comments...
drivers/ide/Kconfig|5
drivers/ide/pci/Makefile |1
drivers/ide/pci/tc86c001.c | 308 +
drivers/pci/quirks.c | 18 ++
include/linux/pci_ids.h|1
5 files changed, 333 insertions(+)
Index: linux-2.6/drivers/ide/Kconfig
===
--- linux-2.6.orig/drivers/ide/Kconfig
+++ linux-2.6/drivers/ide/Kconfig
@@ -742,6 +742,11 @@ config BLK_DEV_VIA82CXXX
This allows the kernel to change PIO, DMA and UDMA speeds and to
configure the chip to optimum performance.
+config BLK_DEV_TC86C001
+ tristate "Toshiba TC86C001 support"
+ help
+ This driver adds support for Toshiba TC86C001 GOKU-S chip.
+
endif
config BLK_DEV_IDE_PMAC
Index: linux-2.6/drivers/ide/pci/Makefile
===
--- linux-2.6.orig/drivers/ide/pci/Makefile
+++ linux-2.6/drivers/ide/pci/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_BLK_DEV_SIIMAGE) += siimag
obj-$(CONFIG_BLK_DEV_SIS5513) += sis5513.o
obj-$(CONFIG_BLK_DEV_SL82C105) += sl82c105.o
obj-$(CONFIG_BLK_DEV_SLC90E66) += slc90e66.o
+obj-$(CONFIG_BLK_DEV_TC86C001) += tc86c001.o
obj-$(CONFIG_BLK_DEV_TRIFLEX) += triflex.o
obj-$(CONFIG_BLK_DEV_TRM290) += trm290.o
obj-$(CONFIG_BLK_DEV_VIA82CXXX)+= via82cxxx.o
Index: linux-2.6/drivers/ide/pci/tc86c001.c
===
--- /dev/null
+++ linux-2.6/drivers/ide/pci/tc86c001.c
@@ -0,0 +1,308 @@
+/*
+ * drivers/ide/pci/tc86c001.c Version 1.00Dec 12, 2006
+ *
+ * Copyright (C) 2002 Toshiba Corporation
+ * Copyright (C) 2005-2006 MontaVista Software, Inc. <[EMAIL PROTECTED]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+
+static inline u8 tc86c001_ratemask(ide_drive_t *drive)
+{
+ return eighty_ninty_three(drive) ? 2 : 1;
+}
+
+static int tc86c001_tune_chipset(ide_drive_t *drive, u8 speed)
+{
+ ide_hwif_t *hwif= HWIF(drive);
+ unsigned long scr_port = hwif->config_data + (drive->dn ? 0x02 : 0x00);
+ u16 mode, scr = hwif->INW(scr_port);
+
+ speed = ide_rate_filter(tc86c001_ratemask(drive), speed);
+
+ switch (speed) {
+ case XFER_UDMA_4: mode = 0x00c0; break;
+ case XFER_UDMA_3: mode = 0x00b0; break;
+ case XFER_UDMA_2: mode = 0x00a0; break;
+ case XFER_UDMA_1: mode = 0x0090; break;
+ case XFER_UDMA_0: mode = 0x0080; break;
+ case XFER_MW_DMA_2: mode = 0x0070; break;
+ case XFER_MW_DMA_1: mode = 0x0060; break;
+ case XFER_MW_DMA_0: mode = 0x0050; break;
+ case XFER_PIO_4:mode = 0x0400; break;
+ case XFER_PIO_3:mode = 0x0300; break;
+ case XFER_PIO_2:mode = 0x0200; break;
+ case XFER_PIO_1:mode = 0x0100; break;
+ case XFER_PIO_0:
+ default:mode = 0x; break;
+ }
+
+ scr &= (speed < XFER_MW_DMA_0) ? 0xf8ff : 0xff0f;
+ scr |= mode;
+ hwif->OUTW(scr, scr_port);
+
+ return ide_config_drive_speed(drive, speed);
+}
+
+static void tc86c001_tune_drive(ide_drive_t *drive, u8 pio)
+{
+ pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
+ (void) tc86c001_tune_chipset(drive, XFER_PIO_0 + pio);
+}
+
+/*
+ * HACKITY HACK
+ *
+ * This is a workaround for the limitation 5 of the TC86C001 IDE controller:
+ * if a DMA transfer terminates prematurely, the controller leaves the device's
+ * interrupt request (INTRQ) pending and does not generate a PCI interrupt (or
+ * s
Re: [-mm patch] drivers/ide/pci/tc86c001.c: make a function static
Hello. Adrian Bunk wrote: +toshiba-tc86c001-ide-driver-take-2.patch This patch makes the needlessly global init_hwif_tc86c001() static. Duh, I hoped tha this driver may get into 2.6.20-rc1 and finally overlooked this. Sigh, uou won't believe how much time this driver rewrite spent in an unfinished state in my internal tree... :-/ Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> --- BTW: I'm not sure whether it'd be a good idea to include such a driver for the legacy IDE subsystem without a libata based driver for the same hardware. Well, I'd agree with Alan here. Don't expect me to convert this to libata in the foreseeable future... I'd like to join the folks hacking on libata but this certainly won't happen soon (if at all)... WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [-mm patch] drivers/ide/pci/tc86c001.c: make a function static
Hello. Adrian Bunk wrote: This patch makes the needlessly global init_hwif_tc86c001() static. Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> If this patch hasn't been accepted by Andrew yet, could you add another fixlet: init_chipset_tc86c001() should've been __devinit. If not or it's already accepted, I'll post the patchlet myself later... WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.20-rc2-mm1] PIIX tuneproc() fixes/cleanups
Fix/cleanup the driver's tuneproc() and ratemask() methods:
- PPE, IE, and TIME bits need to be cleared beforehand for the slave drive as
well as master (Alan probably just forgot about it);
- this driver only supports PIO modes up to 4, so must pass the correct limit
to ide_get_best_pio_mode();
- use min_t() macro instead of min();
- simplify slave vs master drive evaluation;
- do come coding and formatting cleanups...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/piix.c | 21 ++---
1 files changed, 10 insertions(+), 11 deletions(-)
Index: linux-2.6/drivers/ide/pci/piix.c
===
--- linux-2.6.orig/drivers/ide/pci/piix.c
+++ linux-2.6/drivers/ide/pci/piix.c
@@ -1,5 +1,5 @@
/*
- * linux/drivers/ide/pci/piix.c Version 0.45May 12, 2006
+ * linux/drivers/ide/pci/piix.c Version 0.46December 3, 2006
*
* Copyright (C) 1998-1999 Andrzej Krzysztofowicz, Author and Maintainer
* Copyright (C) 1998-2000 Andre Hedrick <[EMAIL PROTECTED]>
@@ -163,7 +163,7 @@ static u8 piix_ratemask (ide_drive_t *dr
* if the drive cannot see an 80pin cable.
*/
if (!eighty_ninty_three(drive))
- mode = min(mode, (u8)1);
+ mode = min_t(u8, mode, 1);
return mode;
}
@@ -216,7 +216,7 @@ static void piix_tune_drive (ide_drive_t
{
ide_hwif_t *hwif= HWIF(drive);
struct pci_dev *dev = hwif->pci_dev;
- int is_slave= (&hwif->drives[1] == drive);
+ int is_slave= drive->dn & 1;
int master_port = hwif->channel ? 0x42 : 0x40;
int slave_port = 0x44;
unsigned long flags;
@@ -225,7 +225,7 @@ static void piix_tune_drive (ide_drive_t
static DEFINE_SPINLOCK(tune_lock);
int control = 0;
-/* ISP RTC */
+/* ISP RTC */
static const u8 timings[][2]= {
{ 0, 0 },
{ 0, 0 },
@@ -233,7 +233,7 @@ static void piix_tune_drive (ide_drive_t
{ 2, 1 },
{ 2, 3 }, };
- pio = ide_get_best_pio_mode(drive, pio, 5, NULL);
+ pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
/*
* Master vs slave is synchronized above us but the slave register is
@@ -243,25 +243,24 @@ static void piix_tune_drive (ide_drive_t
spin_lock_irqsave(&tune_lock, flags);
pci_read_config_word(dev, master_port, &master_data);
- if (pio >= 2)
+ if (pio > 1)
control |= 1; /* Programmable timing on */
if (drive->media == ide_disk)
control |= 4; /* Prefetch, post write */
- if (pio >= 3)
+ if (pio > 2)
control |= 2; /* IORDY */
if (is_slave) {
- master_data = master_data | 0x4000;
+ master_data |= 0x4000;
+ master_data &= ~0x0070;
if (pio > 1) {
/* enable PPE, IE and TIME */
master_data = master_data | (control << 4);
- } else {
- master_data &= ~0x0070;
}
pci_read_config_byte(dev, slave_port, &slave_data);
slave_data = slave_data & (hwif->channel ? 0x0f : 0xf0);
slave_data = slave_data | (((timings[pio][0] << 2) |
timings[pio][1]) << (hwif->channel ? 4 : 0));
} else {
- master_data = master_data & 0xccf8;
+ master_data &= ~0x3307;
if (pio > 1) {
/* enable PPE, IE and TIME */
master_data = master_data | control;
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.20-rc2-mm1] slc90e66: carry over fixes from piix driver
Synchronize with version 0.46 of the Intel PIIX/ICH driver:
- carry over Alan's and my own fixes in the tuneproc() method and my cleanups
both there and in the ratemask() method;
- SLC90E66 only supports MW DMA modes 1/2 and SW DMA mode 2 (just like Intel
chips), so don't claim support for other MW/SW DMA modes;
- don't check dor non-NULL drive->id in the ide_dma_check() method -- this is
assumed to be true in all other drivers;
- do some coding/formatting cleanups while at it...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/slc90e66.c | 55 +++--
1 files changed, 34 insertions(+), 21 deletions(-)
Index: linux-2.6/drivers/ide/pci/slc90e66.c
===
--- linux-2.6.orig/drivers/ide/pci/slc90e66.c
+++ linux-2.6/drivers/ide/pci/slc90e66.c
@@ -1,5 +1,5 @@
/*
- * linux/drivers/ide/pci/slc90e66.c Version 0.12May 12, 2006
+ * linux/drivers/ide/pci/slc90e66.c Version 0.13December 30, 2006
*
* Copyright (C) 2000-2002 Andre Hedrick <[EMAIL PROTECTED]>
* Copyright (C) 2006 MontaVista Software, Inc. <[EMAIL PROTECTED]>
@@ -26,7 +26,7 @@ static u8 slc90e66_ratemask (ide_drive_t
u8 mode = 2;
if (!eighty_ninty_three(drive))
- mode = min(mode, (u8)1);
+ mode = min_t(u8, mode, 1);
return mode;
}
@@ -65,36 +65,47 @@ static void slc90e66_tune_drive (ide_dri
{
ide_hwif_t *hwif= HWIF(drive);
struct pci_dev *dev = hwif->pci_dev;
- int is_slave= (&hwif->drives[1] == drive);
+ int is_slave= drive->dn & 1;
int master_port = hwif->channel ? 0x42 : 0x40;
int slave_port = 0x44;
unsigned long flags;
u16 master_data;
u8 slave_data;
-/* ISP RTC */
+ int control = 0;
+/* ISP RTC */
static const u8 timings[][2]= {
- { 0, 0 },
- { 0, 0 },
- { 1, 0 },
- { 2, 1 },
- { 2, 3 }, };
+ { 0, 0 },
+ { 0, 0 },
+ { 1, 0 },
+ { 2, 1 },
+ { 2, 3 }, };
- pio = ide_get_best_pio_mode(drive, pio, 5, NULL);
+ pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
spin_lock_irqsave(&ide_lock, flags);
pci_read_config_word(dev, master_port, &master_data);
+
+ if (pio > 1)
+ control |= 1; /* Programmable timing on */
+ if (drive->media == ide_disk)
+ control |= 4; /* Prefetch, post write */
+ if (pio > 2)
+ control |= 2; /* IORDY */
if (is_slave) {
- master_data = master_data | 0x4000;
- if (pio > 1)
+ master_data |= 0x4000;
+ master_data &= ~0x0070;
+ if (pio > 1) {
/* enable PPE, IE and TIME */
- master_data = master_data | 0x0070;
+ master_data = master_data | (control << 4);
+ }
pci_read_config_byte(dev, slave_port, &slave_data);
slave_data = slave_data & (hwif->channel ? 0x0f : 0xf0);
slave_data = slave_data | (((timings[pio][0] << 2) |
timings[pio][1]) << (hwif->channel ? 4 : 0));
} else {
- master_data = master_data & 0xccf8;
- if (pio > 1)
+ master_data &= ~0x3307;
+ if (pio > 1) {
/* enable PPE, IE and TIME */
- master_data = master_data | 0x0007;
+ master_data = master_data | control;
+ }
master_data = master_data | (timings[pio][0] << 12) |
(timings[pio][1] << 8);
}
pci_write_config_word(dev, master_port, master_data);
@@ -173,7 +184,7 @@ static int slc90e66_config_drive_xfer_ra
drive->init_speed = 0;
- if (id && (id->capability & 1) && drive->autodma) {
+ if ((id->capability & 1) && drive->autodma) {
if (ide_use_dma(drive) && slc90e66_config_drive_for_dma(drive))
return hwif->ide_dma_on(drive);
@@ -201,7 +212,7 @@ static void __devinit init_hwif_slc90e66
hwif->irq = hwif->channel ? 15 : 14;
hwif->speedproc = &slc90e66_tune_chipset;
- hwif->tuneproc = &slc90e66_tune_drive;
+ hwif->tuneproc =
[PATCH 2.6.20-rc2-mm1] HPT36x PCI clock detection fix
Fix minor coding mistake in the HPT36x PCI clock detection code noticed by
Bartlomiej Zolnierkiewicz -- it always reported 33 MHz due to the missing
'break' statements. This, however, most probably never mattered -- in fact,
I was thinking of removing the 25/40 MHz cases completely since HPT36x BIOSes
didn't seem to set any other value than 7 into the 'cmd_high_time' field, i.e.
supported only 33 MHz PCI.
Note that in the original driver there was another bug: 25 and 40 MHz cases
were interchanged. Since the 'cmd_high_time' field is in units of PCI clocks,
a lower clock count just *cannot* correspond to a higher frequency, i. e. it
should be 5 for 25 MHz PCI and 9 for 40 MHz PCI, not the other way around.
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/hpt366.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/ide/pci/hpt366.c
===
--- linux-2.6.orig/drivers/ide/pci/hpt366.c
+++ linux-2.6/drivers/ide/pci/hpt366.c
@@ -1,5 +1,5 @@
/*
- * linux/drivers/ide/pci/hpt366.c Version 1.00Jun 25, 2006
+ * linux/drivers/ide/pci/hpt366.c Version 1.01Dec 23, 2006
*
* Copyright (C) 1999-2003 Andre Hedrick <[EMAIL PROTECTED]>
* Portions Copyright (C) 2001 Sun Microsystems, Inc.
@@ -107,7 +107,8 @@
* frequency
* - switch to using the DPLL clock and enable UltraATA/133 mode by default on
* anything newer than HPT370/A
- * - fold PCI clock detection and DPLL setup code into init_chipset_hpt366();
+ * - fold PCI clock detection and DPLL setup code into init_chipset_hpt366(),
+ * also fixing the interchanged 25/40 MHz PCI clock cases for HPT36x chips;
* unify HPT36x/37x timing setup code and the speedproc handlers by joining
* the register setting lists into the table indexed by the clock selected
* Sergei Shtylyov, <[EMAIL PROTECTED]> or <[EMAIL PROTECTED]>
@@ -1125,11 +1126,14 @@ static unsigned int __devinit init_chips
switch((itr1 >> 8) & 0x07) {
case 0x09:
pci_clk = 40;
+ break;
case 0x05:
pci_clk = 25;
+ break;
case 0x07:
default:
pci_clk = 33;
+ break;
}
}
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: DMA errors with HTP370 IDE in <= 2.6.18.2
Michael Bueker wrote: > In the controller's setup utility, I can choose access modes. If i switch it from UDMA5 to PIO0, the only difference during boot is: PIO0 is outrageously slow, use PIO4 (very slow as well but at least somewhat faster). Anyway, this won't help with Linux... ide2: BM-DMA at 0xa000-0xa007, BIOS settings: hde:pio, hdf:pio instead of ide2: BM-DMA at 0xa000-0xa007, BIOS settings: hde:DMA, hdf:DMA You should specify the option ide=nodma to the kernel to disable IDE core from using DMA. Please help me getting DMA to work with this controller. For starters, please try the driver from 2.6.20-rc1 and (if that doesn't help) from the -mm tree. I rewrote it and it's working OK on at least my HPT370... Thanks in advance, ~Mik WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: No DMA with HTP370 IDE controller
Hello. Michael Bueker wrote: Have you looked at changes to drivers/ide/pci/hpt366.c? The code is not the same, there are a lot of fixes in -mm: hpt3xx-rework-rate-filtering.patch hpt3xx-rework-rate-filtering-tidy.patch hpt3xx-print-the-real-chip-name-at-startup.patch hpt3xx-switch-to-using-pci_get_slot.patch hpt3xx-cache-channels-mcr-address.patch hpt3x7-merge-speedproc-handlers.patch hpt370-clean-up-dma-timeout-handling.patch hpt3xx-init-code-rewrite.patch I applied all of these patches, but the errors remains just the same. I actually did it twice to be sure, but to no avail. Then it's probably the same issue as here: http://bugzilla.kernel.org/show_bug.cgi?id=7703 What's really strange is that my HPT370 is working fine. However, the chip marking says it's HPT370A despite the revision ID is 3 (which should correspond to HPT370)... ~Mik WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
hwif->rw_disk() method
Hello. Alan/Bart/anybody, could you enlighten me why hwif->rw_disk() method came into being at all? IIUC, hwif->dma_setup() method could have been used instead which is called from the ATAPI drivers and the ide-taskfile.c, while hwif->rw_disk() is only called from ide-disk.c. I'm thinking about replacing this hook in hpt366.c (the only its user) an getting rid of it altogether... MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hwif->rw_disk() method
Hello. Alan wrote: Alan/Bart/anybody, could you enlighten me why hwif->rw_disk() method came into being at all? When you needed to wrap entire disk operations. The ->dma_ methods only wrap DMA commands. Yeah, good point. But it's not likely that the HighPoint driver actually needs this wrapper for PIO or even MW DMA commands... For the current support hardware it can probably go, no idea if anyone embedded uses it any more and can't be bothered to check 8) Nobody does, as far I can see, except this wretched driver... :-) MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: No DMA with HTP370 IDE controller
Hello. Michael Bueker wrote: Then it's probably the same issue as here: http://bugzilla.kernel.org/show_bug.cgi?id=7703 What's really strange is that my HPT370 is working fine. However, the chip marking says it's HPT370A despite the revision ID is 3 (which should correspond to HPT370)... Unfortunately, I'm unable to compile a 2.4.18 kernel (my gcc is probably too new) for testing. But anyways - do you think there's hope for fixing this? If I can provide you with any more helpful information, please tell me. Hope dies last. :-) Unfortunately, I don't have enough time to work on this, and the fact that it's not locally reproducible also doesn't help. I'll try to come up with something to test but not very soon... My controller is: http://www.michael-bueker.de/files/hpt370/ 01:09.0 Mass storage controller: Triones Technologies, Inc. HPT366/368/370/370A/372/372N (rev 03) Subsystem: Triones Technologies, Inc. HPT370 UDMA100 Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium TAbort- SERR- Latency: 120 (2000ns min, 2000ns max) Interrupt: pin A routed to IRQ 11 Region 0: I/O ports at 9000 [size=8] Region 1: I/O ports at 9400 [size=4] Region 2: I/O ports at 9800 [size=8] Region 3: I/O ports at 9c00 [size=4] Region 4: I/O ports at a000 [size=256] Expansion ROM at da02 [disabled] [size=128K] Exactly what I have here but the chip marking is HPT370A. A bit puzzling... ~Mik WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hwif->rw_disk() method
Hello.
Alan wrote:
Alan/Bart/anybody, could you enlighten me why hwif->rw_disk() method came
into being at all?
When you needed to wrap entire disk operations. The ->dma_ methods only
wrap DMA commands.
BTW, I've looked at the clock turnaround code both in drivers/ide/hpt366.c
and drivers/ata/pata_hpt3x2n.c and compared it to the HighPoint driver and I'm
dazed and confused (again?): contrary to what HighPoint does (I'm not sure
they're still doing it at all due to confused nature of their drivers :-),
Linux uses DPLL on reads and PCI clock on writes. The libata driver seem to
have an explicit contradiction:
static int hpt3x2n_use_dpll(struct ata_port *ap, int reading)
{
long flags = (long)ap->host->private_data;
/* See if we should use the DPLL */
if (reading == 0)
return USE_DPLL;/* Needed for write */
if (flags & PCI66)
return USE_DPLL;/* Needed at 66Mhz */
return 0;
}
static unsigned int hpt3x2n_qc_issue_prot(struct ata_queued_cmd *qc)
{
struct ata_taskfile *tf = &qc->tf;
struct ata_port *ap = qc->ap;
int flags = (long)ap->host->private_data;
if (hpt3x2n_pair_idle(ap)) {
int dpll = hpt3x2n_use_dpll(ap, (tf->flags & ATA_TFLAG_WRITE));
if ((flags & USE_DPLL) != dpll) {
if (dpll == 1)
hpt3x2n_set_clock(ap, 0x21);
else
hpt3x2n_set_clock(ap, 0x23);
}
}
return ata_qc_issue_prot(qc);
}
ATA_TFLAG_WRITE indicates a write transfer (host-to-device) while
hpt3x2n_use_dpll() contrarywise expects the read transfer indication. Comments?
WBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/19] trm290: remove redundant CONFIG_BLK_DEV_IDEDMA
Hello. Bartlomiej Zolnierkiewicz wrote: [PATCH] trm290: remove redundant CONFIG_BLK_DEV_IDEDMA #ifdef-s Nice to see you riding again. :-) In drivers/ide/Kconfig BLK_DEV_TRM290 depends on BLK_DEV_IDEDMA_PCI (on which is BLK_DEV_IDEDMA dependant on). I also remember you urging me to get rid of ide_setup_dma() call there. ;-) MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/19] ide: update MAINTAINERS entry
Hello. Bartlomiej Zolnierkiewicz wrote: [PATCH] ide: update MAINTAINERS entry Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> --- MAINTAINERS |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) Index: my-2.6/MAINTAINERS === --- my-2.6.orig/MAINTAINERS +++ my-2.6/MAINTAINERS @@ -1585,12 +1585,11 @@ M: [EMAIL PROTECTED] W: http://www.developer.ibm.com/welcome/netfinity/serveraid.html S: Supported -IDE DRIVER [GENERAL] +IDE SUBSYSTEM P: Bartlomiej Zolnierkiewicz -M: [EMAIL PROTECTED] -L: linux-kernel@vger.kernel.org +M: [EMAIL PROTECTED] L: linux-ide@vger.kernel.org -T: git kernel.org:/pub/scm/linux/kernel/git/bart/ide-2.6.git +T: quilt kernel.org/pub/linux/kernel/people/bart/pata-2.6/ S: Maintained BTW, if I'm (or somebody else) sending patches with IDE driver *fixes*, should these be against that quilt tree (containing largely reworked IDE code), against fresh Linus' tree (or some recent -rc), or against recent -mm tree? WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/15] ide: disable DMA in ->ide_dma_check for "no IORDY" case
Hello.
Bartlomiej Zolnierkiewicz wrote:
[PATCH] ide: disable DMA in ->ide_dma_check for "no IORDY" case
I've looked thru the code, and found more issues with the PIO fallback
there. Will try to cook up patches for at least some drivers...
Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
Index: b/drivers/ide/pci/aec62xx.c
===
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -214,12 +214,10 @@ static int aec62xx_config_drive_xfer_rat
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
aec62xx_tune_drive(drive, 5);
This function looks like it's working (thouugh having the wrong limit of
PIO5 on auto-tuning) but is unnecassary complex.
Heh, the driver is certainly a rip-off form hpt366.c. What a doubtful
example they have chosen... :-)
Index: b/drivers/ide/pci/atiixp.c
===
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -264,10 +264,9 @@ static int atiixp_dma_check(ide_drive_t
tspeed = ide_get_best_pio_mode(drive, 255, 5, NULL);
speed = atiixp_dma_2_pio(XFER_PIO_0 + tspeed) + XFER_PIO_0;
It's simply stupid to convert PIO mode to PIO mode. The whole idea is
doubtful as well..
hwif->speedproc(drive, speed);
Well, well, the tuneproc() method can't ahndle auto-tunuing here (255)...
And it also doesn't set up drive's own speed. The code seem to be another
rip-off from piix.c, repeating all its mistakes... :-)
Index: b/drivers/ide/pci/cmd64x.c
===
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -479,12 +479,10 @@ static int cmd64x_config_drive_for_dma (
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
config_chipset_for_pio(drive, 1);
This function will always set PIO mode 4. Mess.
Index: b/drivers/ide/pci/cs5535.c
===
--- a/drivers/ide/pci/cs5535.c
+++ b/drivers/ide/pci/cs5535.c
@@ -206,10 +206,9 @@ static int cs5535_dma_check(ide_drive_t
if (ide_use_fast_pio(drive)) {
speed = ide_get_best_pio_mode(drive, 255, 4, NULL);
cs5535_set_drive(drive, speed);
Could be folded into tuneproc() method call.
Index: b/drivers/ide/pci/pdc202xx_old.c
===
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -339,12 +339,10 @@ static int pdc202xx_config_drive_xfer_ra
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
hwif->tuneproc(drive, 5);
This driver's tuneproc() method always auto-tunes here instead of setting
what it's told too -- I'll send a patch...
Index: b/drivers/ide/pci/siimage.c
===
--- a/drivers/ide/pci/siimage.c
+++ b/drivers/ide/pci/siimage.c
@@ -420,12 +420,10 @@ static int siimage_config_drive_for_dma
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
config_chipset_for_pio(drive, 1);
This function will also always set PIO mode 4. More mess.
Index: b/drivers/ide/pci/sis5513.c
===
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -678,12 +678,10 @@ static int sis5513_config_xfer_rate(ide_
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
sis5513_tune_drive(drive, 5);
Ugh, PIO fallback effectively always tries to set mode 4 here (thanks
it's not 5). Mess.
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.20-rc4-mm1) pdc202xx_old: fix PIO mode setup
Fix the driver's tuneproc() method to always set the PIO mode requested and not
pick the best possible one, rename it to pdc202xx_tune_drive(), and change the
calls to it accordingly; remove the preceding comment which has nothing to do
with the code.
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/pdc202xx_old.c | 24 +++-
1 files changed, 7 insertions(+), 17 deletions(-)
Index: linux-2.6/drivers/ide/pci/pdc202xx_old.c
===
--- linux-2.6.orig/drivers/ide/pci/pdc202xx_old.c
+++ linux-2.6/drivers/ide/pci/pdc202xx_old.c
@@ -2,6 +2,7 @@
* linux/drivers/ide/pci/pdc202xx_old.c Version 0.36Sept 11, 2002
*
* Copyright (C) 1998-2002Andre Hedrick <[EMAIL PROTECTED]>
+ * Copyright (C) 2006-2007MontaVista Software, Inc.
*
* Promise Ultra33 cards with BIOS v1.20 through 1.28 will need this
* compiled into the kernel if you have more than one card installed.
@@ -216,21 +217,10 @@ static int pdc202xx_tune_chipset (ide_dr
}
-/* 0123456 7 8
- * 960, 480, 390, 300, 240, 180, 120, 90, 60
- * 180, 150, 120, 90, 60
- * DMA_Speed
- * 180, 120, 90, 90, 90, 60, 30
- * 11, 5, 4, 3, 2, 1, 0
- */
-static void config_chipset_for_pio (ide_drive_t *drive, u8 pio)
+static void pdc202xx_tune_drive(ide_drive_t *drive, u8 pio)
{
- u8 speed = 0;
-
- if (pio == 5) pio = 4;
- speed = XFER_PIO_0 + ide_get_best_pio_mode(drive, 255, pio, NULL);
-
- pdc202xx_tune_chipset(drive, speed);
+ pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
+ pdc202xx_tune_chipset(drive, XFER_PIO_0 + pio);
}
static u8 pdc202xx_old_cable_detect (ide_hwif_t *hwif)
@@ -348,7 +338,7 @@ static int pdc202xx_config_drive_xfer_ra
} else if ((id->capability & 8) || (id->field_valid & 2)) {
fast_ata_pio:
- hwif->tuneproc(drive, 5);
+ pdc202xx_tune_drive(drive, 255);
return hwif->ide_dma_off_quietly(drive);
}
/* IORDY not supported */
@@ -463,7 +453,7 @@ static void pdc202xx_reset (ide_drive_t
pdc202xx_reset_host(hwif);
pdc202xx_reset_host(mate);
- hwif->tuneproc(drive, 5);
+ pdc202xx_tune_drive(drive, 255);
}
static unsigned int __devinit init_chipset_pdc202xx(struct pci_dev *dev,
@@ -490,7 +480,7 @@ static void __devinit init_hwif_pdc202xx
hwif->rqsize = 256;
hwif->autodma = 0;
- hwif->tuneproc = &config_chipset_for_pio;
+ hwif->tuneproc = &pdc202xx_tune_drive;
hwif->quirkproc = &pdc202xx_quirkproc;
if (hwif->pci_dev->device != PCI_DEVICE_ID_PROMISE_20246)
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/15] ide: disable DMA in ->ide_dma_check for "no IORDY" case
Hello.
Bartlomiej Zolnierkiewicz wrote:
[PATCH] ide: disable DMA in ->ide_dma_check for "no IORDY" case
I've looked thru the code, and found more issues with the PIO fallback
there. Will try to cook up patches for at least some drivers...
Great, if possible please base them on top of the IDE tree...
Erm, I had doubts about it (having in mind that all that code is more of a
cleanups than fixes). Maybe it'd be a good idea to separate the fix and
cleanup series somehow...
Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
Index: b/drivers/ide/pci/aec62xx.c
===
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -214,12 +214,10 @@ static int aec62xx_config_drive_xfer_rat
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
aec62xx_tune_drive(drive, 5);
This function looks like it's working (thouugh having the wrong limit of
PIO5 on auto-tuning) but is unnecassary complex.
Yes, it seems that there are actually two bugs here:
* the maximum allowed PIO mode should be PIO4 not PIO5
* for auto-tuning ("pio" == 255) it incorrectly sets PIO0
(255 fails to the default case in the switch statement)
Yeah, you if you pass 255, it won't work (so, drive->autotune must be
broken). But the driver itself have the wrong idea of 5 meaning auto-tune, so
fallback should still work.
Heh, the driver is certainly a rip-off form hpt366.c. What a doubtful
example they have chosen... :-)
hehe
The driver's authorship explains it all. :-)
Index: b/drivers/ide/pci/atiixp.c
===
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -264,10 +264,9 @@ static int atiixp_dma_check(ide_drive_t
tspeed = ide_get_best_pio_mode(drive, 255, 5, NULL);
speed = atiixp_dma_2_pio(XFER_PIO_0 + tspeed) + XFER_PIO_0;
It's simply stupid to convert PIO mode to PIO mode. The whole idea is
doubtful as well..
It is side-effect of basing atiixp on piix driver. Fixing it will allow PIO1
to be used (good) because atiixp_dma_2_pio() always downgrades PIO1 to PIO0
(leftover from piix - on Intel chipsets same timings are used for PIO0/1).
hwif->speedproc(drive, speed);
Well, well, the tuneproc() method can't ahndle auto-tunuing here
(255)...
Yes, definitely a bug.
Ugh... don't expect patches form me soon though. My first priority is the
drivers that we support here...
And it also doesn't set up drive's own speed. The code seem to be another
rip-off from piix.c, repeating all its mistakes... :-)
:)
Index: b/drivers/ide/pci/cmd64x.c
===
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -479,12 +479,10 @@ static int cmd64x_config_drive_for_dma (
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
config_chipset_for_pio(drive, 1);
This function will always set PIO mode 4. Mess.
Yep.
I'm going to send the patch for both this and siimage.c...
Index: b/drivers/ide/pci/cs5535.c
===
--- a/drivers/ide/pci/cs5535.c
+++ b/drivers/ide/pci/cs5535.c
@@ -206,10 +206,9 @@ static int cs5535_dma_check(ide_drive_t
if (ide_use_fast_pio(drive)) {
speed = ide_get_best_pio_mode(drive, 255, 4, NULL);
cs5535_set_drive(drive, speed);
Could be folded into tuneproc() method call.
Using ->tuneproc() will also set the PIO mode on the drive
which is not done currently...
Hm, ide_config_drive_speed() is called by both tuneproc() method and
cs5535_set_drive(), so I saw no issue there...
Index: b/drivers/ide/pci/sis5513.c
===
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -678,12 +678,10 @@ static int sis5513_config_xfer_rate(ide_
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
sis5513_tune_drive(drive, 5);
Ugh, PIO fallback effectively always tries to set mode 4 here (thanks
it's not 5). Mess.
Yep, but it seems to be even more complicated since config_art_rwp_pio()
is a mess^2 - chipset is programmed to the best PIO mode while the
device is set to PIO4... *sigh*...
Sorry, this one is low prio for me... :-)
Thanks,
Bart
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org
Re: [PATCH 8/15] ide: disable DMA in ->ide_dma_check for "no IORDY" case
Hello. Bartlomiej Zolnierkiewicz wrote: I've looked thru the code, and found more issues with the PIO fallback there. Will try to cook up patches for at least some drivers... Great, if possible please base them on top of the IDE tree... Erm, I had doubts about it (having in mind that all that code is more of a cleanups than fixes). Maybe it'd be a good idea to separate the fix and cleanup series somehow... I generally tend do cleanups as a groundwork for the real fixes and separate cleanups and fixes to have good base for dealing with regressions. Often all changes (cleanups/fixes) could be included in one patch but then I would have had harsh times when debugging the regressions. It matters a lot if you hit an unknown (or known but the documentation is covered by NDA) hardware bug - you can concentrate on a small patch changing the way in which hardware is accessed instead of that big patch moving code around etc. Also the thing is that the same bugs are propagated over many drivers so doing cleanups which merge code before fixing the bug makes sense. We can then fix the damn bug once and for all and not worry about somebody copy-n-pasting the bug from the yet-to-be-fixed driver (i.e. in the next patch IDE update there will be patch to check return value of ->speedproc in ide_tune_dma(), without ide-fix-dma-mask/ide-max-dma-mode/ide-tune-dma-helper patches I would have to go over all drivers to fix this bug and still there won't be a guarantee that same bug wouldn't be introduced in some new driver). The other advantage of doing cleanups is that code becomes cleaner/simpler which matters a lot for this codebase, i.e. ide-dma-off-void.patch exposed (yet to be fixed) bug in set_using_dma() (->ide_dma_off_quietly always returns 0 which is passed by ->ide_dma_check to set_using_dma() which incorrectly then calls ->ide_dma_on). Well, this seems a newly intruduced bug. It's all fine but goes somewhat against Linus' policy as far as I understnad it: fixes are merged all the time while cleanups (along with new code) are merged mostly duting the merge window. Moreover I don't find the current tree to be more of cleanups than fixes, here is the analysis of current series file: Maybe I slightly exaggerated, being impressed by the volume of your recent changes. :-) But still... # # IDE patches from 2.6.20-rc3-mm1 # toshiba-tc86c001-ide-driver-take-2.patch toshiba-tc86c001-ide-driver-take-2-fix.patch toshiba-tc86c001-ide-driver-take-2-fix-2.patch -- new driver I'd count that as cleanup, since it's definitely not fix. ;-) hpt3xx-rework-rate-filtering.patch hpt3xx-rework-rate-filtering-tidy.patch hpt3xx-print-the-real-chip-name-at-startup.patch hpt3xx-switch-to-using-pci_get_slot.patch hpt3xx-cache-channels-mcr-address.patch hpt3x7-merge-speedproc-handlers.patch hpt370-clean-up-dma-timeout-handling.patch hpt3xx-init-code-rewrite.patch piix-fix-82371mx-enablebits.patch piix-tuneproc-fixes-cleanups.patch slc90e66-carry-over-fixes-from-piix-driver.patch hpt36x-pci-clock-detection-fix.patch jmicron-warning-fix.patch -- fixes (but most have cleanups mixed in) Yeah, but not that those came in from the -mm tree. pdc202xx_new-remove-useless-code.patch pdc202xx_-remove-check_in_drive_lists-abomination.patch -- cleanups # # IDE patches applied by Andrew (2.6.20-rc4-mm1) # atiixpc-remove-unused-code.patch -- cleanup atiixpc-sb600-ide-only-has-one-channel.patch atiixpc-add-cable-detection-support-for-ati-ide.patch ide-generic-jmicron-has-its-own-drivers-now.patch -- fixes Same about these 3. ide-maintainers-entry.patch -- n/a # # IT8213 # it8213-ide-driver.patch it8213-ide-driver-update.patch -- new driver # # patches posted on Jan 11 2007 # ia64-pci_get_legacy_ide_irq.patch ide-pci-init-tags.patch -- fixes pdc202xx_old-dead-code.patch au1xxx-dead-code.patch ide-pio-blacklisted.patch ide-no-dsc-flag.patch trm290-dma-ifdefs.patch ide-pci-device-tables.patch ide-dev-openers.patch hpt366-init-dma.patch cs5530-cleanup.patch svwks-cleanup.patch sis5513-config-xfer-rate.patch ide-set-xfer-rate.patch ide-use-fast-pio-v2.patch ide-io-cleanup.patch -- cleanups # # Delkin CardBus CF driver (Mark Lord <[EMAIL PROTECTED]>) # delkin_cb-ide-driver.patch -- new driver # # IDE ACPI support (Hannes Reinecke <[EMAIL PROTECTED]>) # ide-acpi-support.patch -- new functionality (fixes PM on some machines) # # ide-pnp exit fix (Tejun Heo <[EMAIL PROTECTED]>) # ide-pnp-exit-fix.patch -- fix # # VIA IDE update (Josepch Chan <[EMAIL PROTECTED]>) # via-ide-update.patch -- fix I'd put fixes before the rewrites and new code... # # patches posted on 18 Jan 2007 # it8213-ide-driver-update-fixes.patch -- fix Well, this is a fix to the newly added driver, so may go anywhere after it... ide-mmio-flag.patch -- cleanup hpt34x-tune-chipset-fix.patch -- fix id
Re: [PATCH 8/15] ide: disable DMA in ->ide_dma_check for "no IORDY" case
Hello.
Bartlomiej Zolnierkiewicz wrote:
The other advantage of doing cleanups is that code becomes cleaner/simpler
which matters a lot for this codebase, i.e. ide-dma-off-void.patch exposed
(yet to be fixed) bug in set_using_dma() (->ide_dma_off_quietly always returns
0 which is passed by ->ide_dma_check to set_using_dma() which incorrectly
then calls ->ide_dma_on).
Well, this seems a newly intruduced bug.
The old code is so convulted that it is hard to see it w/o cleanup. :)
->ide_dma_check implementations often do
return hwif->ide_dma_off_quietly(drive);
so the return value of ide_dma_off_quietly() (which is always 0) is passed to
if (HWIF(drive)->ide_dma_check(drive)) return -EIO;
in ide.c:set_using_dma() -> as a result the next line is executed
if (HWIF(drive)->ide_dma_on(drive)) return -EIO;
Ah, indeed! Nice. :-)
It's all fine but goes somewhat against Linus' policy as far as I
understnad it: fixes are merged all the time while cleanups (along with new
code) are merged mostly duting the merge window.
Moreover I don't find the current tree to be more of cleanups than fixes,
here is the analysis of current series file:
Maybe I slightly exaggerated, being impressed by the volume of your recent
changes. :-)
But still...
#
# IDE patches from 2.6.20-rc3-mm1
#
toshiba-tc86c001-ide-driver-take-2.patch
toshiba-tc86c001-ide-driver-take-2-fix.patch
toshiba-tc86c001-ide-driver-take-2-fix-2.patch
-- new driver
I'd count that as cleanup, since it's definitely not fix. ;-)
hpt3xx-rework-rate-filtering.patch
hpt3xx-rework-rate-filtering-tidy.patch
hpt3xx-print-the-real-chip-name-at-startup.patch
hpt3xx-switch-to-using-pci_get_slot.patch
hpt3xx-cache-channels-mcr-address.patch
hpt3x7-merge-speedproc-handlers.patch
hpt370-clean-up-dma-timeout-handling.patch
hpt3xx-init-code-rewrite.patch
piix-fix-82371mx-enablebits.patch
piix-tuneproc-fixes-cleanups.patch
slc90e66-carry-over-fixes-from-piix-driver.patch
hpt36x-pci-clock-detection-fix.patch
jmicron-warning-fix.patch
-- fixes (but most have cleanups mixed in)
Yeah, but not that those came in from the -mm tree.
Oops, "not that" didn't make sense here :-)
ide-mmio-flag.patch
-- cleanup
hpt34x-tune-chipset-fix.patch
-- fix
ide-fix-pio-fallback.patch
-- fix
Those 2 are seem more of a cleanup to me...
They fix real but quite hard to hit bugs.
I'll put them at the end of the fixes series.
Well, most of the recent fixes were for such issues. Nobody had screamed
about them, it took a code review to find them. :-)
ide-set-dma-helper.patch
ide-dma-off-void.patch
ide-dma-host-on-void.patch
ide-fix-dma-masks.patch
ide-max-dma-mode.patch
ide-tune-dma-helper.patch
-- cleanups
Would make sense to keep those last in the tail of queue because of the
amount of changes they introduce. Possibly even IDE subsystem wide cleanups
They are at the end already - no problem here. :)
I meant "in the future"...
and if you would like me to shuffle ordering of the patches (but without
need of rewritting them) it also OK
Erm, no talking about the rewrite but that way you may have to rebase
cleanups on top of fixes. This seems unavoidble though due to the way the
kernel patch acceptance process is working, as far as I understand it...
I'll change the ordering of the patches based on your suggestions
and generally try to keep such order of fixes first and cleanups later.
Thanks. :-)
Index: b/drivers/ide/pci/cmd64x.c
===
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -479,12 +479,10 @@ static int cmd64x_config_drive_for_dma (
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
return hwif->ide_dma_on(drive);
- if (ide_use_fast_pio(drive)) {
+ if (ide_use_fast_pio(drive))
config_chipset_for_pio(drive, 1);
This function will always set PIO mode 4. Mess.
Yep.
I'm going to send the patch for both this and siimage.c...
OK
Not sure if I'll be able to find a card to test it soon though (I prefer
to test my stuff before submitting, even the simple changes :-).
Please send it anyway.
Ugh, this one is more tough than pdc202xx_old.c -- since tuneproc() is
also borken (doesn't set drive's own transfer mode).
And... I looked into speedproc() handler, then into PCI0646U datasheet for
reference and was really terrified: the code for SW/DW DMA setup us utter
nonsense! It writes to some reserved bits of BMIDE status reg. instead of
doinf the real setup, and twiddles the drive 0/1 DMA capable bit which nobody
asks it to do... Really messy mess. :-(
Thanks,
Bart
WBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.h
Re: [PATCH 10/15] ide: add ide_set_dma() helper
Hello again. :-)
Bartlomiej Zolnierkiewicz wrote:
[PATCH] ide: add ide_set_dma() helper
* add ide_set_dma() helper and make ide_hwif_t.ide_dma_check return
-1 when DMA needs to be disabled (== need to call ->ide_dma_off_quietly)
0 when DMA needs to be enabled (== need to call ->ide_dma_on)
1 when DMA setting shouldn't be changed
* fix IDE code to use ide_set_dma() instead if using ->ide_dma_check directly
Here are a few comments related to the code being patched:
Index: b/drivers/ide/pci/alim15x3.c
===
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -507,17 +507,15 @@ static int config_chipset_for_dma (ide_d
*
* Configure a drive for DMA operation. If DMA is not possible we
* drop the drive into PIO mode instead.
- *
- * FIXME: exactly what are we trying to return here
*/
-
+
static int ali15x3_config_drive_for_dma(ide_drive_t *drive)
{
ide_hwif_t *hwif= HWIF(drive);
struct hd_driveid *id = drive->id;
if ((m5229_revision<=0x20) && (drive->media!=ide_disk))
- return hwif->ide_dma_off_quietly(drive);
+ goto no_dma_set;
Isn't it better to just return -1?
@@ -552,9 +550,10 @@ try_dma_modes:
ata_pio:
hwif->tuneproc(drive, 255);
no_dma_set:
- return hwif->ide_dma_off_quietly(drive);
+ return -1;
}
- return hwif->ide_dma_on(drive);
+
+ return 0;
}
Ugh, this code looks like it's asking to be converted into calling
ide_use_dma(). instead all of that...
Index: b/drivers/ide/pci/cs5520.c
===
--- a/drivers/ide/pci/cs5520.c
+++ b/drivers/ide/pci/cs5520.c
@@ -132,12 +132,11 @@ static void cs5520_tune_drive(ide_drive_
static int cs5520_config_drive_xfer_rate(ide_drive_t *drive)
{
- ide_hwif_t *hwif = HWIF(drive);
-
/* Tune the drive for PIO modes up to PIO 4 */
cs5520_tune_drive(drive, 4);
Ugh. Why not ask drive? :-/
/* Then tell the core to use DMA operations */
- return hwif->ide_dma_on(drive);
+ return 0;
That must be the famous VDMA thing... :-)
I wonder if it *actually* works on HPT36x/37x which seem to have support
for it...
Index: b/drivers/ide/pci/jmicron.c
===
--- a/drivers/ide/pci/jmicron.c
+++ b/drivers/ide/pci/jmicron.c
@@ -164,14 +164,12 @@ static int config_chipset_for_dma (ide_d
static int jmicron_config_drive_for_dma (ide_drive_t *drive)
{
- ide_hwif_t *hwif= drive->hwif;
+ if (ide_use_dma(drive) && config_chipset_for_dma(drive))
+ return 0;
- if (ide_use_dma(drive)) {
- if (config_chipset_for_dma(drive))
- return hwif->ide_dma_on(drive);
- }
config_jmicron_chipset_for_pio(drive, 1);
The 2nd argument of that funtion is useless -- it basically does nothing
if 0 is passed. Another case of mindless copy-paste. :-)
Index: b/drivers/ide/pci/pdc202xx_old.c
===
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -332,17 +332,15 @@ chipset_is_set:
static int pdc202xx_config_drive_xfer_rate (ide_drive_t *drive)
{
- ide_hwif_t *hwif= HWIF(drive);
-
drive->init_speed = 0;
if (ide_use_dma(drive) && config_chipset_for_dma(drive))
- return hwif->ide_dma_on(drive);
+ return 0;
if (ide_use_fast_pio(drive))
- hwif->tuneproc(drive, 5);
+ config_chipset_for_pio(drive, 5);
That part is obsoleted by my recent fix...
Index: b/drivers/ide/pci/piix.c
===
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -386,20 +386,18 @@ static int piix_config_drive_for_dma (id
static int piix_config_drive_xfer_rate (ide_drive_t *drive)
{
- ide_hwif_t *hwif= HWIF(drive);
-
drive->init_speed = 0;
if (ide_use_dma(drive) && piix_config_drive_for_dma(drive))
- return hwif->ide_dma_on(drive);
+ return 0;
if (ide_use_fast_pio(drive)) {
/* Find best PIO mode. */
- (void) hwif->speedproc(drive, XFER_PIO_0 +
- ide_get_best_pio_mode(drive, 255, 4,
NULL));
+ u8 pio = ide_get_best_pio_mode(drive, 255, 4, NULL);
+ (void)piix_tune_chipset(drive, XFER_PIO_0 + pio);
}
Will try to fix the tuneproc() nuisance RSN. :-)
Index: b/drivers/ide/pci/serverworks.c
===
--- a/drivers/ide/pci/serverworks.c
+++ b/drivers/ide/pci/serverworks.c
@@ -315,17 +315,15 @@ static int config_chipset_for_dma (ide_d
static int s
Re: [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void
Hello again. :-)
Bartlomiej Zolnierkiewicz wrote:
[PATCH] ide: make ide_hwif_t.ide_dma_host_on void
* since ide_hwif_t.ide_dma_host_on is called either when drive->using_dma == 1
or when return value is discarded make it void, also drop "ide_" prefix
* make __ide_dma_host_on() void and drop "__" prefix
Below are some nits which also apply to the previous patch...
Index: b/drivers/ide/pci/atiixp.c
===
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -101,7 +101,7 @@ static u8 atiixp_dma_2_pio(u8 xfer_rate)
}
}
-static int atiixp_ide_dma_host_on(ide_drive_t *drive)
+static void atiixp_ide_dma_host_on(ide_drive_t *drive)
{
Would seem logical to get rid of ide_ in this function's name also...
struct pci_dev *dev = drive->hwif->pci_dev;
unsigned long flags;
[...]
Index: b/drivers/ide/pci/sgiioc4.c
===
--- a/drivers/ide/pci/sgiioc4.c
+++ b/drivers/ide/pci/sgiioc4.c
[...]
@@ -307,13 +307,8 @@ sgiioc4_ide_dma_test_irq(ide_drive_t * d
return sgiioc4_checkirq(HWIF(drive));
}
-static int
-sgiioc4_ide_dma_host_on(ide_drive_t * drive)
+static void sgiioc4_ide_dma_host_on(ide_drive_t * drive)
Same comment here...
{
- if (drive->using_dma)
- return 0;
-
- return 1;
}
static void sgiioc4_ide_dma_host_off(ide_drive_t * drive)
@@ -610,7 +605,7 @@ ide_init_sgiioc4(ide_hwif_t * hwif)
hwif->ide_dma_on = &sgiioc4_ide_dma_on;
hwif->dma_off_quietly = &sgiioc4_ide_dma_off_quietly;
hwif->ide_dma_test_irq = &sgiioc4_ide_dma_test_irq;
- hwif->ide_dma_host_on = &sgiioc4_ide_dma_host_on;
+ hwif->dma_host_on = &sgiioc4_ide_dma_host_on;
hwif->dma_host_off = &sgiioc4_ide_dma_host_off;
hwif->ide_dma_lostirq = &sgiioc4_ide_dma_lostirq;
hwif->ide_dma_timeout = &__ide_dma_timeout;
Unrelated note: not sure why this default value needs explicit assignemnt...
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void
Hello again. :-)
Bartlomiej Zolnierkiewicz wrote:
[PATCH] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void
Below are my nits on the patch itself, and the code it changes.
Index: b/drivers/ide/pci/atiixp.c
===
--- a/drivers/ide/pci/atiixp.c
+++ b/drivers/ide/pci/atiixp.c
@@ -121,7 +121,7 @@ static int atiixp_ide_dma_host_on(ide_dr
return __ide_dma_host_on(drive);
}
-static int atiixp_ide_dma_host_off(ide_drive_t *drive)
+static void atiixp_ide_dma_host_off(ide_drive_t *drive)
{
struct pci_dev *dev = drive->hwif->pci_dev;
unsigned long flags;
[...]
@@ -306,7 +306,7 @@ static void __devinit init_hwif_atiixp(i
hwif->udma_four = 0;
hwif->ide_dma_host_on = &atiixp_ide_dma_host_on;
- hwif->ide_dma_host_off = &atiixp_ide_dma_host_off;
+ hwif->dma_host_off = &atiixp_ide_dma_host_off;
hwif->ide_dma_check = &atiixp_dma_check;
if (!noautodma)
hwif->autodma = 1;
Would seem logical to get rid of ide_ in the function's name also...
Index: b/drivers/ide/pci/sgiioc4.c
===
--- a/drivers/ide/pci/sgiioc4.c
+++ b/drivers/ide/pci/sgiioc4.c
@@ -282,12 +282,11 @@ sgiioc4_ide_dma_on(ide_drive_t * drive)
return HWIF(drive)->ide_dma_host_on(drive);
}
-static int
-sgiioc4_ide_dma_off_quietly(ide_drive_t * drive)
+static void sgiioc4_ide_dma_off_quietly(ide_drive_t *drive)
{
drive->using_dma = 0;
- return HWIF(drive)->ide_dma_host_off(drive);
+ drive->hwif->dma_host_off(drive);
}
static int sgiioc4_ide_dma_check(ide_drive_t *drive)
@@ -317,12 +316,9 @@ sgiioc4_ide_dma_host_on(ide_drive_t * dr
return 1;
}
-static int
-sgiioc4_ide_dma_host_off(ide_drive_t * drive)
+static void sgiioc4_ide_dma_host_off(ide_drive_t * drive)
{
sgiioc4_clearirq(drive);
-
- return 0;
}
static int
@@ -612,10 +608,10 @@ ide_init_sgiioc4(ide_hwif_t * hwif)
hwif->ide_dma_end = &sgiioc4_ide_dma_end;
hwif->ide_dma_check = &sgiioc4_ide_dma_check;
hwif->ide_dma_on = &sgiioc4_ide_dma_on;
- hwif->ide_dma_off_quietly = &sgiioc4_ide_dma_off_quietly;
+ hwif->dma_off_quietly = &sgiioc4_ide_dma_off_quietly;
hwif->ide_dma_test_irq = &sgiioc4_ide_dma_test_irq;
hwif->ide_dma_host_on = &sgiioc4_ide_dma_host_on;
- hwif->ide_dma_host_off = &sgiioc4_ide_dma_host_off;
+ hwif->dma_host_off = &sgiioc4_ide_dma_host_off;
hwif->ide_dma_lostirq = &sgiioc4_ide_dma_lostirq;
hwif->ide_dma_timeout = &__ide_dma_timeout;
The same here...
Index: b/drivers/ide/pci/sl82c105.c
===
--- a/drivers/ide/pci/sl82c105.c
+++ b/drivers/ide/pci/sl82c105.c
@@ -261,26 +261,24 @@ static int sl82c105_ide_dma_on (ide_driv
if (config_for_dma(drive)) {
config_for_pio(drive, 4, 0, 0);
Ugh, this forces PIO4 on fallback... and dma_on() doesn't select any modes
in any other driver but this one. :-/
- return HWIF(drive)->ide_dma_off_quietly(drive);
+ drive->hwif->dma_off_quietly(drive);
+ return 0;
}
printk(KERN_INFO "%s: DMA enabled\n", drive->name);
return __ide_dma_on(drive);
}
-static int sl82c105_ide_dma_off_quietly (ide_drive_t *drive)
+static void sl82c105_ide_dma_off_quietly(ide_drive_t *drive)
Also worth renaming...
{
u8 speed = XFER_PIO_0;
- int rc;
-
+
DBG(("sl82c105_ide_dma_off_quietly(drive:%s)\n", drive->name));
- rc = __ide_dma_off_quietly(drive);
+ ide_dma_off_quietly(drive);
if (drive->pio_speed)
Should always be non-zero since explicitly initialized.
speed = drive->pio_speed - XFER_PIO_0;
config_for_pio(drive, speed, 0, 1);
drive->current_speed = drive->pio_speed;
dma_off() shouldn't be changing current_speed IMHO.
-
- return rc;
}
The patch to fix those two functions is also cooking...
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello.
Bartlomiej Zolnierkiewicz wrote:
[PATCH] ide: fix UDMA/MWDMA/SWDMA masks
* use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
* add udma_mask field to ide_pci_device_t and use it to initialize
->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
* fix UDMA masks to match with chipset specific *_ratemask()
(alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
filtering method - done in the next patch)
More nit picking (-:
Index: b/drivers/ide/pci/cmd64x.c
===
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
hwif->swdma_mask = 0x07;
if (dev->device == PCI_DEVICE_ID_CMD_643)
- hwif->ultra_mask = 0x80;
+ hwif->ultra_mask = 0x00;
if (dev->device == PCI_DEVICE_ID_CMD_646)
- hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
+ hwif->ultra_mask =
+ (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
if (dev->device == PCI_DEVICE_ID_CMD_648)
hwif->ultra_mask = 0x1f;
Hm, well, this doesn't look consistent with the changes in other drivers.
This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...
You'd only have to check for PCI-646 revisions < 5 then...
Index: b/drivers/ide/pci/piix.c
===
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide
case PCI_DEVICE_ID_INTEL_82371FB_0:
case PCI_DEVICE_ID_INTEL_82371FB_1:
case PCI_DEVICE_ID_INTEL_82371SB_1:
- hwif->ultra_mask = 0x80;
+ hwif->ultra_mask = 0x00;
break;
case PCI_DEVICE_ID_INTEL_82371AB:
case PCI_DEVICE_ID_INTEL_82443MX_1:
@@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide
case PCI_DEVICE_ID_INTEL_82801AB_1:
hwif->ultra_mask = 0x07;
break;
+ case PCI_DEVICE_ID_INTEL_82801AA_1:
+ case PCI_DEVICE_ID_INTEL_82372FB_1:
+ hwif->ultra_mask = 0x1f;
+ break;
Alas, I'm afraid this part is wrong!
At least, the cable detection should work for 82801AA the same way as for
the 82801Bx and newer chips, if Intel's datasheet is to be trusted... I think
we should fall thru here.
default:
if (!hwif->udma_four)
hwif->udma_four = piix_cable_detect(hwif);
This one also certainly asks for explicit hwif->cds->ultra_mask
initializers... Thus almost all of this switch statement could go away...
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello.
Bartlomiej Zolnierkiewicz wrote:
[PATCH] ide: fix UDMA/MWDMA/SWDMA masks
* use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
* add udma_mask field to ide_pci_device_t and use it to initialize
->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
* fix UDMA masks to match with chipset specific *_ratemask()
(alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
filtering method - done in the next patch)
More issues with aec62xx.c driver, found after looking at the next patch...
Index: b/drivers/ide/pci/aec62xx.c
===
--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c
@@ -270,11 +270,13 @@ static unsigned int __devinit init_chips
static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif)
{
+ struct pci_dev *dev = hwif->pci_dev;
+
hwif->autodma = 0;
hwif->tuneproc = &aec62xx_tune_drive;
hwif->speedproc = &aec62xx_tune_chipset;
- if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
+ if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
hwif->serialized = hwif->channel;
if (hwif->mate)
@@ -286,7 +288,16 @@ static void __devinit init_hwif_aec62xx(
return;
}
- hwif->ultra_mask = 0x7f;
+ hwif->ultra_mask = hwif->cds->udma_mask;
+
+ /* atp865 and atp865r */
+ if (hwif->ultra_mask == 0x3f) {
+ unsigned long io = pci_resource_start(dev, 4);
+
+ if (inb(io) & 0x10)
+ hwif->ultra_mask = 0x7f; /* udma0-6 */
+ }
+
Looks like another intruduced buglet: you're reading DMA command, but
aec62xx_ratemask() was reading DMA status originally for this bit.
hwif->mwdma_mask = 0x07;
hwif->swdma_mask = 0x07;
Hm, caught another nit: this driver doesn't actually support single-word
DMA modes... :-)
Index: b/drivers/ide/pci/alim15x3.c
===
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a
hwif->atapi_dma = 1;
- if (m5229_revision > 0x20)
- hwif->ultra_mask = 0x7f;
+ if (m5229_revision <= 0x20)
+ hwif->ultra_mask = 0x00; /* no udma */
+ else if (m5229_revision < 0xC2)
+ hwif->ultra_mask = 0x07; /* udma0-2 */
+ else if (m5229_revision == 0xC2 || m5229_revision == 0xC3)
+ hwif->ultra_mask = 0x1f; /* udma0-4 */
+ else if (m5229_revision == 0xC4)
+ hwif->ultra_mask = 0x3f; /* udma0-5 */
+ else
+ hwif->ultra_mask = 0x7f; /* udma0-6 */
+
hwif->mwdma_mask = 0x07;
hwif->swdma_mask = 0x07;
Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver...
And PIO setting via speedproc() method is broken -- it passes to tuneproc()
method mode number not biased by -XFER_PIO_0 beforehand. Will cook up some
patch, maybe... :-/
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode
Hello.
Bartlomiej Zolnierkiewicz wrote:
[PATCH] ide: rework the code for selecting the best DMA transfer mode
Here's another portion of comments...
Depends on the "ide: fix UDMA/MWDMA/SWDMA masks" patch.
* add ide_hwif_t.filter_udma_mask hook for filtering UDMA mask
Erm, maybe a shorter method name like udma_filter would go with the others
better. But well, that's my taste. :-)
(use it in alim15x3, hpt366, siimage and serverworks drivers)
* add ide_max_dma_mode() for finding best DMA mode for the device
(loosely based on some older libata-core.c code)
* convert ide_dma_speed() users to use ide_max_dma_mode()
* make ide_rate_filter() take "ide_drive_t *drive" as an argument instead
of "u8 mode" and teach it to how to use UDMA mask to do filtering
* use ide_rate_filter() in hpt366 driver
* remove no longer needed ide_dma_speed() and *_ratemask()
* unexport eighty_ninty_three()
Index: b/drivers/ide/ide-dma.c
===
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -705,6 +705,80 @@ int ide_use_dma(ide_drive_t *drive)
EXPORT_SYMBOL_GPL(ide_use_dma);
+static const u8 xfer_mode_bases[] = {
+ XFER_UDMA_0,
+ XFER_MW_DMA_0,
+ XFER_SW_DMA_0,
+};
+
+static unsigned int ide_get_mode_mask(ide_drive_t *drive, u8 base)
+{
+ struct hd_driveid *id = drive->id;
+ ide_hwif_t *hwif = drive->hwif;
+ unsigned int mask = 0;
+
+ switch(base) {
+ case XFER_UDMA_0:
+ if ((id->field_valid & 4) == 0)
+ break;
+
+ mask = id->dma_ultra & hwif->ultra_mask;
+
+ if (hwif->filter_udma_mask)
+ mask &= hwif->filter_udma_mask(drive);
+
+ if ((mask & 0x78) && (eighty_ninty_three(drive) == 0))
+ mask &= 0x07;
+ break;
+ case XFER_MW_DMA_0:
+ mask = id->dma_mword & hwif->mwdma_mask;
+ break;
+ case XFER_SW_DMA_0:
+ mask = id->dma_1word & hwif->swdma_mask;
+ break;
+ default:
+ BUG();
+ break;
+ }
+
+ return mask;
+}
+
+/**
+ * ide_max_dma_mode- compute DMA speed
+ * @drive: IDE device
+ *
+ * Checks the drive capabilities and returns the speed to use
+ * for the DMA transfer. Returns 0 if the drive is incapable
+ * of DMA transfers.
+ */
+
+u8 ide_max_dma_mode(ide_drive_t *drive)
+{
+ ide_hwif_t *hwif = drive->hwif;
+ unsigned int mask;
+ int x, i;
+ u8 mode = 0;
+
+ if (drive->media != ide_disk && hwif->atapi_dma == 0)
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(xfer_mode_bases); i++) {
+ mask = ide_get_mode_mask(drive, xfer_mode_bases[i]);
+ x = fls(mask) - 1;
+ if (x >= 0) {
+ mode = xfer_mode_bases[i] + x;
+ break;
+ }
+ }
+
+ printk(KERN_DEBUG "%s: selected mode 0x%x\n", drive->name, mode);
+
+ return mode;
+}
+
+EXPORT_SYMBOL_GPL(ide_max_dma_mode);
+
I didn't quite like the array/loop approach but well, that's my taste (I'd
rather put the mode-from-mask evaluation to the function and call it thrice)...
Index: b/drivers/ide/ide-lib.c
===
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -69,123 +69,34 @@ char *ide_xfer_verbose (u8 xfer_rate)
EXPORT_SYMBOL(ide_xfer_verbose);
/**
- * ide_dma_speed - compute DMA speed
- * @drive: drive
- * @mode: modes available
- *
- * Checks the drive capabilities and returns the speed to use
- * for the DMA transfer. Returns 0 if the drive is incapable
- * of DMA transfers.
- */
-
-u8 ide_dma_speed(ide_drive_t *drive, u8 mode)
[...]
-EXPORT_SYMBOL(ide_dma_speed);
Alas, my ide_dma_speed() fix is going to be oudated rather quickly... :-)
Index: b/drivers/ide/pci/hpt366.c
===
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -513,43 +513,31 @@ static int check_in_drive_list(ide_drive
return 0;
}
-static u8 hpt3xx_ratemask(ide_drive_t *drive)
-{
- struct hpt_info *info = pci_get_drvdata(HWIF(drive)->pci_dev);
- u8 mode = info->max_mode;
-
- if (!eighty_ninty_three(drive) && mode)
- mode = min(mode, (u8)1);
- return mode;
-}
-
/*
* Note for the future; the SATA hpt37x we must set
* either PIO or UDMA modes 0,4,5
*/
-
-static u8 hpt3xx_ratefilter(ide_drive_t *drive, u8 speed)
+
+static u8 hpt3xx_filter_udma_mask(ide_drive_t *drive)
{
struct hpt_info *info = pci_get_drvdata(HWIF(drive)->pci_dev);
u8 chip_type= info->chip_type;
- u8 mode = hpt3xx_ratemask(drive);
-
- if
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Alan wrote: Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... Thats long been broken. Should be correct in the libata driver Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code from the IDE driver. No way it could be working. You may check against the PC64x datasheets (if you have them -- if you don't I think I may share) and see for yourself -- it's abolutely idiotic. Alan WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello.
Alan wrote:
Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver...
Thats long been broken. Should be correct in the libata driver
Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code
from the IDE driver. No way it could be working. You may check against the
PC64x datasheets (if you have them -- if you don't I think I may share) and
see for yourself -- it's abolutely idiotic.
I.e. MWDMA2 should be working due to the way the driver is written (it
sets up PIO4 timings when auto-tuning DMA) but not the other modes since
speedproc() method is brain-damaged in this part.
Not a suprise to be honest. I fixed some of the ALi stuff when I did it
and I think that was pushed back into drivers/ide. The CMD64x hasn't had
much love really.
Another buglet found by random glancing at this driver:
/**
* cmd648_dma_stop - DMA stop callback
* @qc: Command in progress
*
* DMA has completed.
*/
static void cmd648_bmdma_stop(struct ata_queued_cmd *qc)
{
struct ata_port *ap = qc->ap;
struct pci_dev *pdev = to_pci_dev(ap->host->dev);
u8 dma_intr;
int dma_reg = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0;
int dma_mask = ap->port_no ? ARTTIM2 : CFR;
ata_bmdma_stop(qc);
pci_read_config_byte(pdev, dma_reg, &dma_intr);
pci_write_config_byte(pdev, dma_reg, dma_intr | dma_mask);
}
dma_reg and dma_mask initializers must have been swapped since ARTTIM2 and
CFR are regster names. So, the code reads/writes semi-random regs...
Wouldn't mind the older 64x (not 640) data sheets if they are sharable.
Sent what I had on this machine. Will looks for newer revision of
PCJ0646U2 spec elsewhere...
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ide: clear bmdma status in ide_intr()
Hello.
Albert Lee wrote:
patch 2/2:
Do the dma status clearing in ide_intr() and add a new
hwif->ide_dma_clear_irq such that LLDD could override it.
Signed-off-by: Albert Lee <[EMAIL PROTECTED]>
---
Tested ok on ICH4 and pdc20275. Not sure if this would have bad effect for
other adapters.
Patch against 2.6.20-rc5, for your review, thanks.
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-dma.c
02_add_to_ide_intr/drivers/ide/ide-dma.c
--- 01_remove_from_ide_cd/drivers/ide/ide-dma.c 2006-11-30 05:57:37.0
+0800
+++ 02_add_to_ide_intr/drivers/ide/ide-dma.c2007-01-24 11:07:58.0
+0800
@@ -650,6 +650,22 @@ static int __ide_dma_test_irq(ide_drive_
drive->name, __FUNCTION__);
return 0;
}
+
+/* returns 1 on error, 0 otherwise */
Why? Do you check the result?
+int __ide_dma_clear_irq(ide_drive_t *drive)
+{
+ ide_hwif_t *hwif = HWIF(drive);
+ u8 dma_stat;
+
+ /* clear the INTR & ERROR bits */
+ if (hwif->dma_status) {
hwif->dma_status should always be set, else ide-dma.c just won't work. The
check seems superfluous.
+ dma_stat = hwif->INB(hwif->dma_status);
+ /* Should we force the bit as well ? */
Good idea.
+ hwif->OUTB(dma_stat, hwif->dma_status);
I'm afraid this would break __ide_dma_end() function which tests the error
conditions and returns failure if (dma_stat & 7) != 4...
+ }
+
+ return 0;
+}
#endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
int __ide_dma_bad_drive (ide_drive_t *drive)
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-io.c
02_add_to_ide_intr/drivers/ide/ide-io.c
--- 01_remove_from_ide_cd/drivers/ide/ide-io.c 2006-11-30 05:57:37.0
+0800
+++ 02_add_to_ide_intr/drivers/ide/ide-io.c 2007-01-24 11:21:41.0
+0800
@@ -1644,6 +1644,18 @@ irqreturn_t ide_intr (int irq, void *dev
}
hwgroup->handler = NULL;
del_timer(&hwgroup->timer);
+
+ /* Some controllers might set DMA INTR no matter DMA or PIO;
+* bmdma status might need to be cleared even for
+* PIO interrupts to prevent spurious irq or irq lost.
+*/
Either "being lost" or "loss". Sorry for grammar nitpicking. :-)
+ if (hwif->ide_dma_clear_irq && !(hwif->dma))
+ /* ide_dma_end() needs bmdma status for error checking.
+* So, skip clearing bmdma status here and leave it
+* to ide_dma_end() if this is dma interrupt.
+*/
+ hwif->ide_dma_clear_irq(drive);
+
spin_unlock(&ide_lock);
Ah, you're only calling this when hwif->dma == 0... Well, that's probably
fine. Although, once introduced, this method asks for more use...
if (drive->unmask)
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: share PIO limits among devices sharing a channel
Alan wrote:
PIO xfermask limits should be shared by all devices on the same
channel to avoid violating device selection timing. libata used to
This is not a good way to deal with this. Only command block (8-bit)
timings should be the same for both drives on channel, data register (16-bit)
timings may be different.
NAK, this is totally wrong
+ /* PIO xfermask limits are shared by all devices on the same
+* channel to avoid violating device selection timing.
+*/
+ for (i = 0; i < ATA_MAX_DEVICES; i++) {
+ struct ata_device *d = &ap->device[i];
+ unsigned int pio_mask;
+
+ if (ata_dev_absent(d))
+ continue;
+
+ ata_unpack_xfermask(ata_id_xfermask(d->id),
+ &pio_mask, NULL, NULL);
+ pio_mask &= d->pio_mask;
+ xfer_mask &= ata_pack_xfermask(pio_mask, UINT_MAX, UINT_MAX);
+ }
NAK
This "guarantee" was deliberately removed long ago and is completely
bogus.
I'd agree...
The good ATA chipsets do not suffer from selection timing limits of this
form. The less smart ones do and the drivers correctly merge the timing
values, including a whole chunk of functionality in the ata_timing
interface to get it right when doing DMA modes.
I need to have a look at this (when I have time :-)...
Adding this patch is the regression. Even the ancient drivers/ide code
does this properly.
Not really, at least not all drivers. Namely, hpt366.c (still) doesn't
merge 8-bit timings (maybe this is handled in hardware but the datasheets
don't tell about it then) -- I need to look at fixing this... Well, it was
even worse before "the grand rewrite" since even setting UltraDMA modes
changed PIO timings (both 8- and 16-bit) to match PIO4 -- piix/slc90e66 are
still doing this kind of crap (I'm going to fix this at last).
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: share PIO limits among devices sharing a channel
Hello. Alan wrote: Not really, at least not all drivers. Namely, hpt366.c (still) doesn't merge 8-bit timings (maybe this is handled in hardware but the datasheets don't tell about it then) -- I need to look at fixing this... Well, it was I've never been able to find out - but HPT's own drivers don't seem to do it under any OS so I assume its ok. From their drivers/BIOSes/chips I can only assume their engeneers' serious brain damage and incompetence. :-) Well, Linux driver was hardly better. The timing stuff should be pretty much the same for both ide and libata as libata lifted most of the rather nice ide-timing stuff directly from the old IDE layer. Yeah, pata_hpt3*.c still have the same issues as the old hpt366 driver, namely changing PIO timings when setting DMA modes. Alan WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ide: clear bmdma status in ide_intr() (revised)
Albert Lee wrote:
patch 2/2 (revised):
- Do the dma status clearing in ide_intr() and add a new
hwif->ide_dma_clear_irq such that LLDD can override it.
- Fix drive->waiting_for_dma to work with CDB-intr devices.
Signed-off-by: Albert Lee <[EMAIL PROTECTED]>
---
hwif->dma is not reliable: ide_intr() races with dma_start(). Sometimes we got
hwif->dma == 0
in ide_intr() even though it is actually a DMA interrupt. So,
drive->waiting_for_dma is used instead.
Also revised per Sergei's comments and let ide_dma_clear_irq return "void".
The "if (hwif->dma_status)" check in __ide_dma_clear_irq() is kept as is since
I think
it would be safer for the old ISA/VESA IDE devices that has no BMDMA registers.
ISA/VESA drivers shouldn't have this method defined at all since they're
PIO only. But well, it won't hurt...
For your review, thanks.
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-cd.c
02_add_to_ide_intr/drivers/ide/ide-cd.c
--- 01_remove_from_ide_cd/drivers/ide/ide-cd.c 2007-01-24 11:00:03.0
+0800
+++ 02_add_to_ide_intr/drivers/ide/ide-cd.c 2007-01-25 16:52:20.0
+0800
@@ -923,6 +923,10 @@ static ide_startstop_t cdrom_start_packe
HWIF(drive)->OUTB(drive->ctl, IDE_CONTROL_REG);
if (CDROM_CONFIG_FLAGS (drive)->drq_interrupt) {
+ /* waiting for CDB interrupt, not DMA yet. */
+ if (info->dma)
+ drive->waiting_for_dma = 0;
+
/* packet command */
ide_execute_command(drive, WIN_PACKETCMD, handler,
ATAPI_WAIT_PC, cdrom_timer_expiry);
return ide_started;
@@ -965,6 +969,10 @@ static ide_startstop_t cdrom_transfer_pa
/* Check for errors. */
if (cdrom_decode_status(drive, DRQ_STAT, NULL))
return ide_stopped;
+
+ /* Ok, next interrupt will be dma interrupt. */
+ if (info->dma)
+ drive->waiting_for_dma = 1;
} else {
/* Otherwise, we must wait for DRQ to get set. */
if (ide_wait_stat(&startstop, drive, DRQ_STAT,
Erm... shouldn't we set drive->waiting_for_dma in hwif->dma_start() then?
Why it's set in hwif->dma_setup() at all I wonder?
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ide: clear bmdma status in ide_intr() (revised)
Hello.
Sergei Shtylyov wrote:
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-cd.c
02_add_to_ide_intr/drivers/ide/ide-cd.c
--- 01_remove_from_ide_cd/drivers/ide/ide-cd.c2007-01-24 11:00:03.0
+0800
+++ 02_add_to_ide_intr/drivers/ide/ide-cd.c2007-01-25 16:52:20.0
+0800
@@ -923,6 +923,10 @@ static ide_startstop_t cdrom_start_packe
HWIF(drive)->OUTB(drive->ctl, IDE_CONTROL_REG);
if (CDROM_CONFIG_FLAGS (drive)->drq_interrupt) {
+/* waiting for CDB interrupt, not DMA yet. */
+if (info->dma)
+drive->waiting_for_dma = 0;
+
/* packet command */
ide_execute_command(drive, WIN_PACKETCMD, handler, ATAPI_WAIT_PC,
cdrom_timer_expiry);
return ide_started;
@@ -965,6 +969,10 @@ static ide_startstop_t cdrom_transfer_pa
/* Check for errors. */
if (cdrom_decode_status(drive, DRQ_STAT, NULL))
return ide_stopped;
+
+/* Ok, next interrupt will be dma interrupt. */
+if (info->dma)
+drive->waiting_for_dma = 1;
} else {
/* Otherwise, we must wait for DRQ to get set. */
if (ide_wait_stat(&startstop, drive, DRQ_STAT,
Erm... shouldn't we set drive->waiting_for_dma in hwif->dma_start()
then? Why it's set in hwif->dma_setup() at all I wonder?
Well, it'll become raicy as well then... Isn't it ugly. :-/
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello.
Sergei Shtylyov wrote:
Not a suprise to be honest. I fixed some of the ALi stuff when I did it
and I think that was pushed back into drivers/ide. The CMD64x hasn't had
much love really.
Another buglet found by random glancing at this driver:
/**
* cmd648_dma_stop - DMA stop callback
* @qc: Command in progress
*
* DMA has completed.
*/
static void cmd648_bmdma_stop(struct ata_queued_cmd *qc)
{
struct ata_port *ap = qc->ap;
struct pci_dev *pdev = to_pci_dev(ap->host->dev);
u8 dma_intr;
int dma_reg = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0;
int dma_mask = ap->port_no ? ARTTIM2 : CFR;
ata_bmdma_stop(qc);
pci_read_config_byte(pdev, dma_reg, &dma_intr);
pci_write_config_byte(pdev, dma_reg, dma_intr | dma_mask);
}
dma_reg and dma_mask initializers must have been swapped since
ARTTIM2 and CFR are regster names. So, the code reads/writes
semi-random regs...
BTW, on PCI0646U2 and later chips, the interrupt status (it's not really
DMA interrupt status but a latched INTRQ signal not "coupled" with DMA logic,
according to the datasheets) can be read from MRDMODE reg. which is accessible
in I/O space at BMIDE base + 1 which is certainly faster. That's what
drivers/ide/cmd64x.c is doing in its test_dma_irq() method (however, it's
doign this on PCI0643 and early revs of PCI0646 which don't have these bits).
The driver's dma_end() method is acting really strange: it checks if the cjip
is PCI-648/9 and reads the PCI config space to clear those interrupt bits
while these chips have them in I/O mapped MRDMODE; OTOH, it ignores these bits
on earlier chips which have them in oonfig. space only (CFR/ARTTIM23 regs)...
go figure. I'm going to clean this up but don't heve the h/w handy... :-/
Wouldn't mind the older 64x (not 640) data sheets if they are sharable.
Sent what I had on this machine. Will looks for newer revision of
PCJ0646U2 spec elsewhere...
Sent rev. 1.3... Hopefully gkernel.sourceforge.net/specs/ will be updated.
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/15] ide: disable DMA in ->ide_dma_check for "no IORDY" case
Hello. Bartlomiej Zolnierkiewicz wrote: The other advantage of doing cleanups is that code becomes cleaner/simpler which matters a lot for this codebase, i.e. ide-dma-off-void.patch exposed (yet to be fixed) bug in set_using_dma() (->ide_dma_off_quietly always returns 0 which is passed by ->ide_dma_check to set_using_dma() which incorrectly then calls ->ide_dma_on). Well, this seems a newly intruduced bug. The old code is so convulted that it is hard to see it w/o cleanup. :) ->ide_dma_check implementations often do return hwif->ide_dma_off_quietly(drive); so the return value of ide_dma_off_quietly() (which is always 0) is passed to if (HWIF(drive)->ide_dma_check(drive)) return -EIO; in ide.c:set_using_dma() -> as a result the next line is executed if (HWIF(drive)->ide_dma_on(drive)) return -EIO; So, the error seems to call hwif->ide_dma_on() after hwif->ide_dma_check() since hwif->ide_dma_check() must've already called that. MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Updated SiI h/w docs (was Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks)
Hello. Jeff Garzik wrote: Now, the site seems to have 2 versions of the PCI-648/9 specs. :-) I looked into those in specs/sii and noted that PCI-648 spec was newer than mine, but PCI-649 was older. It's probably a good idea to put all the CMD/SiI files in the single directory, so there's no dupes. PS: I certainly used to have a newer spec for PCI0646U2 but it's not at hand for some reason... Oops, this chip is named differently, without 0 -- that's why I didn't notice the file locally. Sending rev 1.3 now... (added linux-ide to CC and SiI folks to BCC) All the docs are now at http://gkernel.sourceforge.net/specs/sii/ including the one you just sent. Well, the purpose of avoiding dupes still not achieved. :-) Though maybe having several revisions of one spec is worth it... Jeff MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pata_sil680: PIO1 taskfile transfers oveclocking fix
Fix PIO mode 1 overclocked taskfile transfers -- probably a typo carried over
from drivers/ide/pci/siimage.c where I've found it by documentation check...
drivers/ata/pata_sil680.c |2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6/drivers/ata/pata_sil680.c
===
--- linux-2.6.orig/drivers/ata/pata_sil680.c
+++ linux-2.6/drivers/ata/pata_sil680.c
@@ -135,7 +135,7 @@ static void sil680_error_handler(struct
static void sil680_set_piomode(struct ata_port *ap, struct ata_device *adev)
{
static u16 speed_p[5] = { 0x328A, 0x2283, 0x1104, 0x10C3, 0x10C1 };
- static u16 speed_t[5] = { 0x328A, 0x1281, 0x1281, 0x10C3, 0x10C1 };
+ static u16 speed_t[5] = { 0x328A, 0x2283, 0x1281, 0x10C3, 0x10C1 };
unsigned long tfaddr = sil680_selreg(ap, 0x02);
unsigned long addr = sil680_seldev(ap, adev, 0x04);
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] libata IORDY handling
Hello. Sergei Shtylyov wrote: Fix PIO mode 1 overclocked taskfile transfers -- probably a typo carried over from drivers/ide/pci/siimage.c where I've found it by documentation check... drivers/ata/pata_sil680.c |2 +- 1 files changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6/drivers/ata/pata_sil680.c === --- linux-2.6.orig/drivers/ata/pata_sil680.c +++ linux-2.6/drivers/ata/pata_sil680.c @@ -135,7 +135,7 @@ static void sil680_error_handler(struct static void sil680_set_piomode(struct ata_port *ap, struct ata_device *adev) It's sad to say but there's another bug in this function (even a regression from drivers/ide/pci/siimage.c) -- the 16-bit IORDY is not enabled when setting PIO mode (there's code that twiddles IORDY enable but that's actually only for *taskfile* accesses, 16-bit IORDY is controlled by the same PCI config. registers 80h/84h that enable DMA/UDMA transfer on SiI 680). I looked into fixing this but had a feeling that the thing wasn't right from the very start, including ata_pio_need_iordy(). In my understanding of the ANSI T13 stadrads, when one issues Set Features subcommand Set Transfer Mode with sector count register of 0x8 thru 0xC this means that IORDY *must* be enabled. That's what the ATA/ATAPI-6 says, for example: Table 45 - Transfer mode values Mode Bits (7:3) Bits (2:0) PIO default mode 0b 000b PIO default mode, disable IORDY 0b 001b PIO *flow control* transfer mode 1b mode Comments? MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pata_sil680: PIO1 taskfile transfers overclocking fix (repost)
Fix PIO mode 1 overclocked taskfile transfers -- probably a typo carried over
from drivers/ide/pci/siimage.c where I've found it by documentation check...
drivers/ata/pata_sil680.c |2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
Index: linux-2.6/drivers/ata/pata_sil680.c
===
--- linux-2.6.orig/drivers/ata/pata_sil680.c
+++ linux-2.6/drivers/ata/pata_sil680.c
@@ -135,7 +135,7 @@ static void sil680_error_handler(struct
static void sil680_set_piomode(struct ata_port *ap, struct ata_device *adev)
{
static u16 speed_p[5] = { 0x328A, 0x2283, 0x1104, 0x10C3, 0x10C1 };
- static u16 speed_t[5] = { 0x328A, 0x1281, 0x1281, 0x10C3, 0x10C1 };
+ static u16 speed_t[5] = { 0x328A, 0x2283, 0x1281, 0x10C3, 0x10C1 };
unsigned long tfaddr = sil680_selreg(ap, 0x02);
unsigned long addr = sil680_seldev(ap, adev, 0x04);
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] libata IORDY handling
Hello. Alan wrote: I looked into fixing this but had a feeling that the thing wasn't right from the very start, including ata_pio_need_iordy(). In my understanding of the ANSI T13 stadrads, when one issues Set Features subcommand Set Transfer Mode with sector count register of 0x8 thru 0xC this means that IORDY *must* be enabled. Yes. It is more complex than the current code handles. That's one reason I added ata_pio_need_iordy(), because it would need to change and I also meant this function: if IORDY *must* be enabled even for PIO mode 0 (being set the way liabata sets it, via the mentioned subcommand), what's the point of checking if we need to enable IORDY? The only reason for this function as it seems is to check if we can do *without* IORDY still... Even if so, we must check if any of these both drives need IORDY to decide whether we need to enable IORDY checking on taskfile accesses or not -- which this function fails to do... hardcoding it would be particularly ugly. Yeah, hardcoding is ugly, no doubt about it. This is still a problem with both pdc202xx_new and pata_pdc2027x, for example... This will matter for supporting some utterly ancient junk. Well, SiI680 seemed to me quite well designed. :-) Alan MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] siimage: PIO1/2 taskfile transfer overclocking fix
Fix two typos found by SiI680A documentation check. They caused the taskfile
transfer overclocking:
- in PIO mode 1 as 0x2283 must be used for both data and taskfile transfers;
- in PIO mode 2 as data and taskfile timings are swapped when writing to the
MMIO regs.
Fix coding style and trailing whitespace in enclosing statements while at it...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/siimage.c | 59 ++
1 files changed, 29 insertions(+), 30 deletions(-)
Index: linux-2.6/drivers/ide/pci/siimage.c
===
--- linux-2.6.orig/drivers/ide/pci/siimage.c
+++ linux-2.6/drivers/ide/pci/siimage.c
@@ -1,8 +1,9 @@
/*
- * linux/drivers/ide/pci/siimage.c Version 1.07Nov 30, 2003
+ * linux/drivers/ide/pci/siimage.c Version 1.11Jan 27, 2007
*
* Copyright (C) 2001-2002 Andre Hedrick <[EMAIL PROTECTED]>
* Copyright (C) 2003 Red Hat <[EMAIL PROTECTED]>
+ * Copyright (C) 2007 MontaVista Software, Inc.
*
* May be copied or modified under the terms of the GNU General Public License
*
@@ -205,41 +206,39 @@ static void siimage_tuneproc (ide_drive_
unsigned long tfaddr= siimage_selreg(hwif, 0x02);
/* cheat for now and use the docs */
- switch(mode_wanted) {
- case 4:
- speedp = 0x10c1;
- speedt = 0x10c1;
- break;
- case 3:
- speedp = 0x10C3;
- speedt = 0x10C3;
- break;
- case 2:
- speedp = 0x1104;
- speedt = 0x1281;
- break;
- case 1:
- speedp = 0x2283;
- speedt = 0x1281;
- break;
- case 0:
- default:
- speedp = 0x328A;
- speedt = 0x328A;
- break;
+ switch (mode_wanted) {
+ case 4:
+ speedp = 0x10c1;
+ speedt = 0x10c1;
+ break;
+ case 3:
+ speedp = 0x10c3;
+ speedt = 0x10c3;
+ break;
+ case 2:
+ speedp = 0x1104;
+ speedt = 0x1281;
+ break;
+ case 1:
+ speedp = 0x2283;
+ speedt = 0x2283;
+ break;
+ case 0:
+ default:
+ speedp = 0x328a;
+ speedt = 0x328a;
+ break;
}
- if (hwif->mmio)
- {
- hwif->OUTW(speedt, addr);
- hwif->OUTW(speedp, tfaddr);
+
+ if (hwif->mmio) {
+ hwif->OUTW(speedp, addr);
+ hwif->OUTW(speedt, tfaddr);
/* Now set up IORDY */
if(mode_wanted == 3 || mode_wanted == 4)
hwif->OUTW(hwif->INW(tfaddr-2)|0x200, tfaddr-2);
else
hwif->OUTW(hwif->INW(tfaddr-2)&~0x200, tfaddr-2);
- }
- else
- {
+ } else {
pci_write_config_word(hwif->pci_dev, addr, speedp);
pci_write_config_word(hwif->pci_dev, tfaddr, speedt);
pci_read_config_word(hwif->pci_dev, tfaddr-2, &speedp);
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] siimage: PIO1/2 taskfile transfer overclocking fix
Hello, I wrote: Fix two typos found by SiI680A documentation check. They caused the taskfile transfer overclocking: - in PIO mode 1 as 0x2283 must be used for both data and taskfile transfers; - in PIO mode 2 as data and taskfile timings are swapped when writing to the MMIO regs. Fix coding style and trailing whitespace in enclosing statements while at it... Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]> Forgot to specify that this patch was against Linus' tree... MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] libata IORDY handling
Hello. Alan wrote: It's sad to say but there's another bug in this function (even a regression from drivers/ide/pci/siimage.c) -- the 16-bit IORDY is not enabled when setting PIO mode (there's code that twiddles IORDY enable but that's actually only for *taskfile* accesses, 16-bit IORDY is controlled by the same PCI config. registers 80h/84h that enable DMA/UDMA transfer on SiI 680). Fixed - also fixed clearing the bits when going into PIO mode from DMA, which fixes a potential DMA changedown bug. Yeah, this is an issue in siimage.c... Also redone the iordy stuff as your emails reminded me that it needed finishing off and sorting out. Atttaching my (already obsolete) patch, just in case... Alan MBR, Sergei Index: linux-2.6/drivers/ata/pata_sil680.c === --- linux-2.6.orig/drivers/ata/pata_sil680.c +++ linux-2.6/drivers/ata/pata_sil680.c @@ -139,9 +139,12 @@ static void sil680_set_piomode(struct at unsigned long tfaddr = sil680_selreg(ap, 0x02); unsigned long addr = sil680_seldev(ap, adev, 0x04); + unsigned long addr_mask = 0x80 + 4 * ap->port_no; struct pci_dev *pdev = to_pci_dev(ap->host->dev); + int port_shift = adev->devno * 4; int pio = adev->pio_mode - XFER_PIO_0; int lowest_pio = pio; + u8 mode; u16 reg; struct ata_device *pair = ata_dev_pair(adev); @@ -152,11 +155,31 @@ static void sil680_set_piomode(struct at pci_write_config_word(pdev, addr, speed_p[pio]); pci_write_config_word(pdev, tfaddr, speed_t[lowest_pio]); - pci_read_config_word(pdev, tfaddr-2, ®); - reg &= ~0x0200; /* Clear IORDY */ + /* + * Let's see if we need to enable IORDY checking on data transfer... + * + * NOTE: Selecting any DMA mode also enables IORDY checkign, + * so we must first check if not already in DMA mode beforehand + * to avoid disabling it... + */ + pci_read_config_byte(pdev, addr_mask, &mode); + if (ata_pio_need_iordy(adev) && ((mode >> port_shift) & 0x03) == 0x00) + mode |= 0x01 << port_shift; + else if (((mode >> port_shift) & 0x03) == 0x01) + mode &= ~(0x03 << port_shift); + pci_write_config_byte(pdev, addr_mask, mode); + + /* + * Let's see if we need to enable IORDY checking on taskfile. + * + * NOTE: We can only disable IORDY if both drives agree to it. + */ + pci_read_config_word(pdev, tfaddr - 2, ®); if (ata_pio_need_iordy(adev)) - reg |= 0x0200; /* Enable IORDY */ - pci_write_config_word(pdev, tfaddr-2, reg); + reg |= 0x0200; + else if (pair == NULL || !ata_pio_need_iordy(pair)) + reg &= ~0x0200; + pci_write_config_word(pdev, tfaddr - 2, reg); } /**
Re: [PATCH 2/2] ide: clear bmdma status in ide_intr() for Intel controllers (revised #3)
Hello.
Albert Lee wrote:
patch 2/2 (revised):
- Fix drive->waiting_for_dma to work with CDB-intr devices.
- Do the dma status clearing in ide_intr() and add a new
hwif->ide_dma_clear_irq for Intel controllers.
Signed-off-by: Albert Lee <[EMAIL PROTECTED]>
---
Calling hwif->ide_dma_clear_irq for all controllers looks risky.
I'm not sure why it's so risky...
Revised to do it only for the Intel controllers.
(If any other adapters need it, we may add it later.)
Patch against 2.6.20-rc6. Tested ok on my ICH4 and pdc20275 adapters.
Please review/apply, thanks.
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-cd.c
02_add_to_ide_intr/drivers/ide/ide-cd.c
--- 01_remove_from_ide_cd/drivers/ide/ide-cd.c 2007-01-29 17:23:34.0
+0800
+++ 02_add_to_ide_intr/drivers/ide/ide-cd.c 2007-01-29 17:23:58.0
+0800
@@ -923,6 +923,10 @@ static ide_startstop_t cdrom_start_packe
HWIF(drive)->OUTB(drive->ctl, IDE_CONTROL_REG);
if (CDROM_CONFIG_FLAGS (drive)->drq_interrupt) {
+ /* waiting for CDB interrupt, not DMA yet. */
+ if (info->dma)
+ drive->waiting_for_dma = 0;
+
/* packet command */
ide_execute_command(drive, WIN_PACKETCMD, handler,
ATAPI_WAIT_PC, cdrom_timer_expiry);
return ide_started;
@@ -965,6 +969,10 @@ static ide_startstop_t cdrom_transfer_pa
/* Check for errors. */
if (cdrom_decode_status(drive, DRQ_STAT, NULL))
return ide_stopped;
+
+ /* Ok, next interrupt will be DMA interrupt. */
+ if (info->dma)
+ drive->waiting_for_dma = 1;
} else {
/* Otherwise, we must wait for DRQ to get set. */
if (ide_wait_stat(&startstop, drive, DRQ_STAT,
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-dma.c
02_add_to_ide_intr/drivers/ide/ide-dma.c
--- 01_remove_from_ide_cd/drivers/ide/ide-dma.c 2006-11-30 05:57:37.0
+0800
+++ 02_add_to_ide_intr/drivers/ide/ide-dma.c2007-01-29 17:23:58.0
+0800
@@ -650,6 +650,20 @@ static int __ide_dma_test_irq(ide_drive_
drive->name, __FUNCTION__);
return 0;
}
+
+void __ide_dma_clear_irq(ide_drive_t *drive)
+{
+ ide_hwif_t *hwif = HWIF(drive);
+ u8 dma_stat;
+
+ /* clear the INTR & ERROR bits */
+ dma_stat = hwif->INB(hwif->dma_status);
+ /* Should we force the bit as well ? */
+ hwif->OUTB(dma_stat, hwif->dma_status);
+}
+
+EXPORT_SYMBOL_GPL(__ide_dma_clear_irq);
+
#endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
int __ide_dma_bad_drive (ide_drive_t *drive)
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-io.c
02_add_to_ide_intr/drivers/ide/ide-io.c
--- 01_remove_from_ide_cd/drivers/ide/ide-io.c 2006-11-30 05:57:37.0
+0800
+++ 02_add_to_ide_intr/drivers/ide/ide-io.c 2007-01-29 17:23:58.0
+0800
@@ -1644,6 +1644,18 @@ irqreturn_t ide_intr (int irq, void *dev
}
hwgroup->handler = NULL;
del_timer(&hwgroup->timer);
+
+ /* Some controllers might set DMA INTR no matter DMA or PIO;
+* bmdma status might need to be cleared even for
+* PIO interrupts to prevent spurious/lost irq.
+*/
+ if (hwif->ide_dma_clear_irq && !(drive->waiting_for_dma))
+ /* ide_dma_end() needs bmdma status for error checking.
+* So, skip clearing bmdma status here and leave it
+* to ide_dma_end() if this is dma interrupt.
+*/
+ hwif->ide_dma_clear_irq(drive);
+
spin_unlock(&ide_lock);
if (drive->unmask)
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide.c
02_add_to_ide_intr/drivers/ide/ide.c
--- 01_remove_from_ide_cd/drivers/ide/ide.c 2007-01-29 17:19:48.0
+0800
+++ 02_add_to_ide_intr/drivers/ide/ide.c2007-01-29 17:23:58.0
+0800
@@ -503,6 +503,7 @@ static void ide_hwif_restore(ide_hwif_t
hwif->ide_dma_on = tmp_hwif->ide_dma_on;
hwif->ide_dma_off_quietly= tmp_hwif->ide_dma_off_quietly;
hwif->ide_dma_test_irq = tmp_hwif->ide_dma_test_irq;
+ hwif->ide_dma_clear_irq = tmp_hwif->ide_dma_clear_irq;
hwif->ide_dma_host_on= tmp_hwif->ide_dma_host_on;
hwif->ide_dma_host_off = tmp_hwif->ide_dma_host_off;
hwif->ide_dma_lostirq= tmp_hwif->ide_dma_lostirq;
diff -Nrup 01_remove_from_ide_cd/drivers/ide/pci/piix.c
02_add_to_ide_intr/drivers/ide/pci/piix.c
--- 01_remove_from_ide_cd/drivers/ide/pci/piix.c2007-01-29
17:23:34.0 +0800
+++ 02_add_to_ide_intr/drivers/ide/pci/piix.c 2007-01-29 17:23:58.0
+0800
@@ -483,6 +483,7 @@ static void __devinit init_hwif_piix(ide
if (!hwif->dma_base)
return;
+ hwif->ide_dma_clear_irq = &__ide_dma_clear_irq;
If this handler is only called by single dri
[PATCH] pata_sl82c105: wrong assumptions about compatible PIO modes
Fix the wrong "compatible" PIO mode choices: MWDMA0 has 480 ns cycle while PIO1
only has 383 ns cycle, and MWDMA2 timings matchs those of PIO4 exactly.
---
Frankly speaking, I'm not sure this function is useful or correct at all --
with the DMA timings being actually programmed in sl82c105_bmdma_start()...
And the issue of the same registers being used for both PIO and DMA timings is
not specific for this driver at all but seems to be addressed only by it...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ata/pata_sl82c105.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/ata/pata_sl82c105.c
===
--- linux-2.6.orig/drivers/ata/pata_sl82c105.c
+++ linux-2.6/drivers/ata/pata_sl82c105.c
@@ -139,13 +139,13 @@ static void sl82c105_set_dmamode(struct
{
switch(adev->dma_mode) {
case XFER_MW_DMA_0:
- sl82c105_configure_piomode(ap, adev, 1);
+ sl82c105_configure_piomode(ap, adev, 0);
break;
case XFER_MW_DMA_1:
sl82c105_configure_piomode(ap, adev, 3);
break;
case XFER_MW_DMA_2:
- sl82c105_configure_piomode(ap, adev, 3);
+ sl82c105_configure_piomode(ap, adev, 4);
break;
default:
BUG();
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HPA and failed opcode was: 0x37 ?
Hello.
Steven Scholz wrote:
Hmm. Don't think so. Since the use of ioremap() I think the MMU treats the
area as none-cacheable, right?
Thats only the first half of the story. If you don't decode byte level
fetches in the FPGA or the code is doing something like
read 16 bit value
shift 8
return half
for 8 bit reads you'll get wrong answers.
I have connected HDD's A[2..0] to CPU's A[3..1] and do something like
for (i = IDE_DATA_OFFSET; i <= IDE_STATUS_OFFSET; i++) {
hw.io_ports[i] = ide_virt_base + (i << 1);
}
thus all HDD registers are accessed on a 16bit aligned address. Thus
ide_inb() should return the correct value.
And btw are things like identify driver use 8bit transfers?
No, 8-bit transfers are used only for taskfile access. The data register
is accessed as 16-bit.
How could one then explain
current capacity is 78140160 sectors would be 0x04A85300
native capacity is 185074430006016 sectors would be0xA852FFA85300
? First three bytes ok, then the other three bytes rubbish?
Note that they're not complete garbage but equal the value of the lower 3
bytes minus 1. What is clear is that Read Native Max Address Ext command must
be mistreating the HOB bit... :-)
Steven
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pata_sl82c105: wrong assumptions about compatible PIO modes
Hello. Alan wrote: Fix the wrong "compatible" PIO mode choices: MWDMA0 has 480 ns cycle while PIO1 only has 383 ns cycle, and MWDMA2 timings matchs those of PIO4 exactly. Thanks for all this review work Oh, I've only started but there's too much to do elsewhere... :-) Frankly speaking, I'm not sure this function is useful or correct at all -- with the DMA timings being actually programmed in sl82c105_bmdma_start()... It ought to be right - bmdma_start loads the real DMA mode - set_dmamode/set_piomode load the right PIO timings You just said set_piomode() is called before set_dmamode() by the libata core anyway -- there seems to be no point in re-writing the modes until the actual DMA is started. - bmdma_stop restores the right PIO timings And the issue of the same registers being used for both PIO and DMA timings is not specific for this driver at all but seems to be addressed only by it... For most drivers (those using the ata_timing interface) the timing merge is done by ata_timing_compute(). Ah... Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]> Acked-by: Alan Cox <[EMAIL PROTECTED]> MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sl82c105: DMA support fixes
Fix a number of issues with the DMA support code:
- driver claims support for all SW/MW DMA modes while supporting only MWDMA2;
- ide_dma_check() method tries to enable DMA on the "known good" drives which
don't support MWDMA2;
- ide_dma_on() method upon failure to set drive to MWDMA2 re-tunes already
tuned PIO mode and calls ide_dma_off() method instead of returning error;
- ide_dma_off() method sets drive->current_speed while it doesn't actually
change (only the PIO timings are re-loaded into the chip's registers);
- init_hwif() method forcibly sets/resets both "drive DMA capable" bits while
this is properly handled by ide_dma_{on,off}() methods being called later...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/sl82c105.c | 18 --
1 files changed, 4 insertions(+), 14 deletions(-)
Index: linux-2.6/drivers/ide/pci/sl82c105.c
===
--- linux-2.6.orig/drivers/ide/pci/sl82c105.c
+++ linux-2.6/drivers/ide/pci/sl82c105.c
@@ -164,7 +164,7 @@ static int sl82c105_check_drive (ide_dri
return hwif->ide_dma_on(drive);
}
- if (__ide_dma_good_drive(drive))
+ if (__ide_dma_good_drive(drive) && id->eide_dma_time < 150)
return hwif->ide_dma_on(drive);
} while (0);
@@ -259,10 +259,8 @@ static int sl82c105_ide_dma_on (ide_driv
{
DBG(("sl82c105_ide_dma_on(drive:%s)\n", drive->name));
- if (config_for_dma(drive)) {
- config_for_pio(drive, 4, 0, 0);
- return HWIF(drive)->ide_dma_off_quietly(drive);
- }
+ if (config_for_dma(drive))
+ return 1;
printk(KERN_INFO "%s: DMA enabled\n", drive->name);
return __ide_dma_on(drive);
}
@@ -278,7 +276,6 @@ static int sl82c105_ide_dma_off_quietly
if (drive->pio_speed)
speed = drive->pio_speed - XFER_PIO_0;
config_for_pio(drive, speed, 0, 1);
- drive->current_speed = drive->pio_speed;
return rc;
}
@@ -401,11 +398,9 @@ static unsigned int __devinit init_chips
/*
* Initialise the chip
*/
-
static void __devinit init_hwif_sl82c105(ide_hwif_t *hwif)
{
unsigned int rev;
- u8 dma_state;
DBG(("init_hwif_sl82c105(hwif: ide%d)\n", hwif->index));
@@ -431,7 +426,6 @@ static void __devinit init_hwif_sl82c105
if (!hwif->dma_base)
return;
- dma_state = hwif->INB(hwif->dma_base + 2) & ~0x60;
rev = sl82c105_bridge_revision(hwif->pci_dev);
if (rev <= 5) {
/*
@@ -441,11 +435,8 @@ static void __devinit init_hwif_sl82c105
printk("%s: Winbond 553 bridge revision %d, BM-DMA
disabled\n",
hwif->name, rev);
} else {
- dma_state |= 0x60;
-
hwif->atapi_dma = 1;
- hwif->mwdma_mask = 0x07;
- hwif->swdma_mask = 0x07;
+ hwif->mwdma_mask = 0x04;
hwif->ide_dma_check = &sl82c105_check_drive;
hwif->ide_dma_on = &sl82c105_ide_dma_on;
@@ -462,7 +453,6 @@ static void __devinit init_hwif_sl82c105
if (hwif->mate)
hwif->serialized = hwif->mate->serialized = 1;
}
- hwif->OUTB(dma_state, hwif->dma_base + 2);
}
static ide_pci_device_t sl82c105_chipset __devinitdata = {
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HPA and failed opcode was: 0x37 ?
Hello. Steven Scholz wrote: How could one then explain current capacity is 78140160 sectors would be 0x04A85300 native capacity is 185074430006016 sectors would be0xA852FFA85300 ? First three bytes ok, then the other three bytes rubbish? Note that they're not complete garbage but equal the value of the lower 3 bytes minus 1. What is clear is that Read Native Max Address Ext command must be mistreating the HOB bit... :-) Well this results from addr++; /* since the return value is (maxlba - 1), we add 1 */ in idedisk_read_native_max_address_ext(). Apparently on my system u32 high = (args.hobRegister[IDE_HCYL_OFFSET] << 16) | (args.hobRegister[IDE_LCYL_OFFSET] << 8) | args.hobRegister[IDE_SECTOR_OFFSET]; u32 low = ((args.tfRegister[IDE_HCYL_OFFSET])<<16) | ((args.tfRegister[IDE_LCYL_OFFSET])<<8) | (args.tfRegister[IDE_SECTOR_OFFSET]); high and low contain the same values! :-( Right. I also have doubts about IDE_CONTROL_REG being properly decoded/handled in your FPGA. I "hardcoded" set_max = capacity, now I get hda: Host Protected Area detected. current capacity is 78140160 sectors (40007 MB) native capacity is 78140160 sectors (40007 MB) hda: Host Protected Area disabled. hda: 4289221376 sectors (2196081 MB) w/8192KiB Cache, CHS=65535/255/63 ;-) But still something must go terribly wrong. Erm, the resulting capacity still seems wrong, so you need to also change idedisk_set_max_address_ext(). smartctl shows 7 Seek_Error_Rate 0x000f 061 060 030 - 1513926 195 Hardware_ECC_Recovered 0x001a 100 066 000 - 104721680 And after just doing a "mke2fs /dev/hda1" I see 7 Seek_Error_Rate 0x000f 061 060 030 - 1516919 195 Hardware_ECC_Recovered 0x001a 095 066 000 - 104791382 :-( I'm not sure smartctl names these attributes corectly -- they're vendor specific, after all... :-) -- Steven MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HPA and failed opcode was: 0x37 ?
Hello. Steven Scholz wrote: Maybe the FPGA is discarding the taskfile writes once the HOB bit is set? It wouldn't affect the READ NATIVE MAX ADDRESS EXT command being issued since the only the command register matters for that, but it would affect SET MAX ADDRESS EXT working properly. Without a device > 137GB to test with, and without errors on the device, you might not see any odd behavior in normal usage. It's *really* unlikely the drive doesn't have a working taskfile. I am sorry. But I don't understand what a "working taskfile" means. I don't have CONFIG_IDE_TASK_IOCTL enabled if that means anything. The FPGA does nothing except decoding adresseses from the ARM cpu, controlling the A[2..0], CS[1..0], IOR, IOW lines of the HDD. I've not seen you quoting the code that sets ofsset for the IDE_CONTROL_REG, BTW... Without this register, LBA48 is completely broken In linux all I do is setting up some hw_regs_t struct (with hw.io_ports[]) according to my memory map and call ide_register_hw() ... So looks like I need to find out more about that taskfile stuff. Taskfile is another (and probably an older one) anme for the IDE command block registers. Steven MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HPA and failed opcode was: 0x37 ?
Hello.
Steven Scholz wrote:
Sergei,
The FPGA does nothing except decoding adresseses from the ARM cpu,
controlling the A[2..0], CS[1..0], IOR, IOW lines of the HDD.
I've not seen you quoting the code that sets ofsset for the
IDE_CONTROL_REG, BTW... Without this register, LBA48 is completely broken
for (i = IDE_DATA_OFFSET; i <= IDE_STATUS_OFFSET; i++) {
hw.io_ports[i] = ide_virt_base + (i << 1);
}
hw.io_ports[IDE_CONTROL_OFFSET] = ide_virt_base + 0x10;
Thus it has an offset 0x10 from the base address.
ide0 at 0xc3856000-0xc3856007,0xc3856010 on irq 27
But I just noticed that A[2..0] should look like "110" when accessing the
"Device Control Register". Thus the offset should be 0x16 instead! Right?
If your FPGA passes A[2..0] untouched to the IDE bus, it's certainly
wrong. It must pass 110.
--
Steven
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Alan wrote: Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... Thats long been broken. Should be correct in the libata driver I've looked thru the specs and it seemed to me that ULi hardware is much broken PIO wise: their max active time is 8 cycles even on taskfile access which gives 240 ns while standard requeires 290 ns for modes 0 thru 2... I've also noted that the tuneproc() method in both cmd64x.c and alim15x3.c seems to misdo recovery calculation, taking address setup into account -- that should be slightly overclocking PIO modes 0/1 (ULi docs don't shed much light on how it should be calculated)... Well, this seems fixed in libata drivers. Alan MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: no DRQ after issuing MULTWRITE_EXT ?
Hello.
Steven Scholz wrote:
I am seeing kernel messages like
[ 1284.48] hda: status timeout: status=0xd0 { Busy }
[ 1284.48] ide: failed opcode was: unknown
[ 1284.48] hda: no DRQ after issuing MULTWRITE_EXT
[ 1284.49] ide0: unexpected interrupt, status=0x80, count=1
[ 1284.83] ide0: reset: success
Looks like a spurious interrupt... Is your IDE IRQ shared with other devices?
with a 2.5" Seagate ST940813AM on my embedded ARM system (linux 2.6.14, no
IDE controller. HDD registers just memory mapped).
HDD is used in PIO4 with MultSect=16.
What exactly does that mean? Only that the HDD is to slow to answer this
request with 100ms (WAIT_DRQ=(HZ/10))?
Doubt it. The old ATA standard specified the maximum DRQ assertion time of
20 ms.
And will the request issued again after the reset of the drive?
It should be.
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: no DRQ after issuing MULTWRITE_EXT ?
Hello.
Steven Scholz wrote:
I am seeing kernel messages like
[ 1284.48] hda: status timeout: status=0xd0 { Busy }
[ 1284.48] ide: failed opcode was: unknown
[ 1284.48] hda: no DRQ after issuing MULTWRITE_EXT
[ 1284.49] ide0: unexpected interrupt, status=0x80, count=1
[ 1284.83] ide0: reset: success
Looks like a spurious interrupt... Is your IDE IRQ shared with other
devices?
No.
But IIUC then first the HDD times out, the ide driver handles this and then
an irq occurs which is probably the irq the driver was waiting for. And
The driver is *not* waiting for any IRQ after issuing MULTWRITE_EXT, that's
why it's "unexpected".
since the driver already handled the timeout that (delayed) irq is of course
"unexpected" ... (Just a guess)
Doubt it. The old ATA standard specified the maximum DRQ assertion
time of 20 ms.
I know.
And will the request issued again after the reset of the drive?
It should be.
Thanks.
Steven
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: no DRQ after issuing MULTWRITE_EXT ?
Hello. Steven Scholz wrote: Looks like a spurious interrupt... Is your IDE IRQ shared with other devices? .. But IIUC then first the HDD times out, the ide driver handles this and then an irq occurs which is probably the irq the driver was waiting for. The driver is *not* waiting for any IRQ after issuing MULTWRITE_EXT, that's why it's "unexpected". Hmm. But how does he "know" when that MULTWRITE_EXT is finished!? It's considered finished in this case as there was no starting DRQ "handshake". The first interrupt should happen only after the first block of data is written. Steven MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply)
Hello.
Tejun Heo wrote:
Okay, here's another try at fixing the detection bug. I went through
intel ich docs and compared with the ide piix driver. This patch
fixes the following problems.
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index c6bf1a3..51f55a0 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -728,8 +728,14 @@ static void piix_set_piomode (struct ata_port *ap, struct
ata_device *adev)
if (adev->class == ATA_DEV_ATA)
control |= 4; /* PPE enable */
+ /* PIO configuration clears DTE unconditionally. It will be
+* programmed in set_dmamode which is guaranteed to be called
+* after set_piomode if any DMA mode is available.
+*/
Actually, I think ata_timing_merge() should just be performed when setting
MWDMA mode... This should be the right thing to do in most cases (however,
this hardware has some complications in the form of only 2-bit wide
active/recovery counts and 2 fast timing bank select bits)...
pci_read_config_word(dev, master_port, &master_data);
if (is_slave) {
+ /* clear TIME1|IE1|PPE1|DTE1 */
+ master_data &= 0xff0f;
Yeah, I've fixed this oversight in piix.c...
/* Enable SITRE (seperate slave timing register) */
master_data |= 0x4000;
/* enable PPE1, IE1 and TIME1 as needed */
@@ -737,12 +743,14 @@ static void piix_set_piomode (struct ata_port *ap, struct
ata_device *adev)
pci_read_config_byte(dev, slave_port, &slave_data);
slave_data &= (ap->port_no ? 0x0f : 0xf0);
/* Load the timing nibble for this slave */
- slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) <<
(ap->port_no ? 4 : 0);
+ slave_data |= ((timings[pio][0] << 2) | timings[pio][1])
+ << (ap->port_no ? 4 : 0);
} else {
- /* Master keeps the bits in a different format */
- master_data &= 0xccf8;
+ /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */
+ master_data &= 0xccf0;
/* Enable PPE, IE and TIME as appropriate */
master_data |= control;
+ /* load ISP and RCT */
master_data |=
(timings[pio][0] << 12) |
(timings[pio][1] << 8);
@@ -859,7 +867,7 @@ static void do_pata_set_dmamode (struct ata_port *ap,
struct ata_device *adev, i
master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY
*/
master_data |= control << 4;
pci_read_config_byte(dev, 0x44, &slave_data);
- slave_data &= (0x0F + 0xE1 * ap->port_no);
+ slave_data &= (ap->port_no ? 0x0f : 0xf0);
/* Load the matching timing */
slave_data |= ((timings[pio][0] << 2) | timings[pio][1])
<< (ap->port_no ? 4 : 0);
pci_write_config_byte(dev, 0x44, slave_data);
@@ -871,8 +879,11 @@ static void do_pata_set_dmamode (struct ata_port *ap,
struct ata_device *adev, i
(timings[pio][0] << 12) |
(timings[pio][1] << 8);
}
- udma_enable &= ~(1 << devid);
- pci_write_config_word(dev, master_port, master_data);
+
+ if (ap->udma_mask) {
+ udma_enable &= ~(1 << devid);
+ pci_write_config_word(dev, master_port, master_data);
+ }
I've also noticed that this is done at the end of piix_set_piomode() and I
see no reason why. Isn't it just a leftover from the piix.c brokenness? This
driver coupled PIO and UDMA timing updates for no conceivable reason?
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply)
Hello. Mark Lord wrote: It's amazing how poorly we have programmed PIIX, for the lifespan of Linux... Actually, we've only been fiddling timings on PIIX since 1998 or so, when AH began taking over Linux IDE. Still, 10 years is an eternity. The most funny thing is that despite being called piix.c the driver seems to have always been broken for the original PIIX which didn't have separate mastre/slave timings. :-) Cheers MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply)
Hello. Alan wrote: Actually, I think ata_timing_merge() should just be performed when setting MWDMA mode... This should be the right thing to do in most cases (however, this hardware has some complications in the form of only 2-bit wide active/recovery counts and 2 fast timing bank select bits)... Yeap, that'll be nice. Dunno whether modifying piix/ata_piix too much would be a good idea tho considering the wide usage. If I remember rightly the 2bits is ok because you can set the bit to say that timings are for DMA only, Yeah, after thinking a bit more, the current logic seems good enough, i. e. it's better to slow down PIO to mode 0 than further slow down DMA to match the current PIO speed -- however, this is already happening with MWDMA1 which is actually faster than PIO3 (150 vs 180 ns). But well, who cares... :-) the device then uses slower than PIO0 for all other cycles. Well, thankfully, compatible mode is PIO0 exactly (except for taskfile accesses which are way slower indeed). In both mwdma and pio cases, they're just turning off UDMA. Don't know whether it's actually necessary but still afraid to change it unless there is a good reason. The tuning manual I have does it, so I do it 8) Do you mean 29860004.pdf? Actually, I'm not seeing anything alike here. It would've been stupid idea to couple UDMA to PIO but well... after looking at the HighPoint datasheets, one becomes hard to surprise. :-) Alan Well, after looking at do_pata_set_dmamode(), I have another question: why IORDY enable is forced here? It has *nothing* to do with the IDE DMA protocol. (This also seems to be an issue with piix.c...) MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.20-rc7) alim15x3: fix PIO mode setup
The driver's tuneproc() method fails to set the drive's own speed -- fix this
by renaming the function to ali15x3_tune_pio() and "wrapping" the new tuneproc()
method around it and making it return the mode set, update the heading comment.
Also, setting PIO mode via the speedproc() method does not work due to passing
to the tuneproc() method's a mode number not biased by XFER_PIO_0 -- fix this
along with a typo in the heading comment...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/alim15x3.c | 35 +++
1 files changed, 27 insertions(+), 8 deletions(-)
Index: linux-2.6/drivers/ide/pci/alim15x3.c
===
--- linux-2.6.orig/drivers/ide/pci/alim15x3.c
+++ linux-2.6/drivers/ide/pci/alim15x3.c
@@ -1,5 +1,5 @@
/*
- * linux/drivers/ide/pci/alim15x3.cVersion 0.172003/01/02
+ * linux/drivers/ide/pci/alim15x3.cVersion 0.212007/02/03
*
* Copyright (C) 1998-2000 Michel Aubry, Maintainer
* Copyright (C) 1998-2000 Andrzej Krzysztofowicz, Maintainer
@@ -9,6 +9,7 @@
* May be copied or modified under the terms of the GNU General Public License
* Copyright (C) 2002 Alan Cox <[EMAIL PROTECTED]>
* ALi (now ULi M5228) support by Clear Zhang <[EMAIL PROTECTED]>
+ * Copyright (C) 2007 MontaVista Software, Inc. <[EMAIL PROTECTED]>
*
* (U)DMA capable version of ali 1533/1543(C), 1535(D)
*
@@ -280,15 +281,17 @@ static int ali_get_info (char *buffer, c
#endif /* defined(DISPLAY_ALI_TIMINGS) && defined(CONFIG_PROC_FS) */
/**
- * ali15x3_tune_drive - set up a drive
+ * ali15x3_tune_pio- set up chipset for PIO mode
* @drive: drive to tune
- * @pio: unused
+ * @pio: desired mode
*
- * Select the best PIO timing for the drive in question. Then
- * program the controller for this drive set up
+ * Select the best PIO mode for the drive in question.
+ * Then program the controller for this mode.
+ *
+ * Returns the PIO mode programmed.
*/
-static void ali15x3_tune_drive (ide_drive_t *drive, u8 pio)
+static u8 ali15x3_tune_pio (ide_drive_t *drive, u8 pio)
{
ide_pio_data_t d;
ide_hwif_t *hwif = HWIF(drive);
@@ -356,6 +359,22 @@ static void ali15x3_tune_drive (ide_driv
* { 20, 50, 30 }PIO Mode 5 with IORDY (nonstandard)
*/
+ return pio;
+}
+
+/**
+ * ali15x3_tune_drive - set up drive for PIO mode
+ * @drive: drive to tune
+ * @pio: desired mode
+ *
+ * Program the controller with the best PIO timing for the given drive.
+ * Then set up the drive itself.
+ */
+
+static void ali15x3_tune_drive (ide_drive_t *drive, u8 pio)
+{
+ pio = ali15x3_tune_pio(drive, pio);
+ (void) ide_config_drive_speed(drive, XFER_PIO_0 + pio);
}
/**
@@ -430,7 +449,7 @@ static u8 ali15x3_ratemask (ide_drive_t
}
/**
- * ali15x3_tune_chipset- set up chiset for new speed
+ * ali15x3_tune_chipset- set up chipset/drive for new speed
* @drive: drive to configure for
* @xferspeed: desired speed
*
@@ -461,7 +480,7 @@ static int ali15x3_tune_chipset (ide_dri
pci_write_config_byte(dev, m5229_udma, tmpbyte);
if (speed < XFER_SW_DMA_0)
- ali15x3_tune_drive(drive, speed);
+ (void) ali15x3_tune_pio(drive, speed - XFER_PIO_0);
} else {
pci_read_config_byte(dev, m5229_udma, &tmpbyte);
tmpbyte &= (0x0f << ((1-unit) << 2));
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup
The driver's tuneproc() method fails to set the drive's own speed -- fix this
by renaming the function to cmd64x_tune_pio(), making it return the mode set,
and "wrapping" the new tuneproc() method around it; while at it, also get rid
of the non-working prefetch control code, remove redundant PIO5 mode limitation,
and make cmdprintk() give more sensible mode info. Also, get rid of the broken
config_chipset_for_pio() which always tried to set PIO4 instead auto-tuning PIO.
Note that removing a call from config_chipset_for_dma() breaks "rudimentary"
MWDMA2 support which can only work because of the timing registers being pre-
setup for PIO4 since the code in the speedproc() method which sets up the chip
for SW/MW DMA is completely bogus (!) and I'm going to remove it for the time
being in the next patch...
Oh, and add the missing PIO5 support to the speedproc() method while at it. :-)
Warning: compile tested only -- getting to the real hardware isn't that easy...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/cmd64x.c | 92 ---
1 files changed, 32 insertions(+), 60 deletions(-)
Index: linux-2.6/drivers/ide/pci/cmd64x.c
===
--- linux-2.6.orig/drivers/ide/pci/cmd64x.c
+++ linux-2.6/drivers/ide/pci/cmd64x.c
@@ -262,43 +262,25 @@ static void program_drive_counts (ide_dr
}
/*
- * Attempts to set the interface PIO mode.
- * The preferred method of selecting PIO modes (e.g. mode 4) is
- * "echo 'piomode:4' > /proc/ide/hdx/settings". Special cases are
- * 8: prefetch off, 9: prefetch on, 255: auto-select best mode.
- * Called with 255 at boot time.
+ * This routine selects drive's best PIO mode, calculates setup/active/recovery
+ * counts, and writes them into the chipset registers.
*/
-
-static void cmd64x_tuneproc (ide_drive_t *drive, u8 mode_wanted)
+static u8 cmd64x_tune_pio (ide_drive_t *drive, u8 mode_wanted)
{
int setup_time, active_time, recovery_time;
int clock_time, pio_mode, cycle_time;
u8 recovery_count2, cycle_count;
int setup_count, active_count, recovery_count;
int bus_speed = system_bus_clock();
- /*byte b;*/
ide_pio_data_t d;
- switch (mode_wanted) {
- case 8: /* set prefetch off */
- case 9: /* set prefetch on */
- mode_wanted &= 1;
- /*set_prefetch_mode(index, mode_wanted);*/
- cmdprintk("%s: %sabled cmd640 prefetch\n",
- drive->name, mode_wanted ? "en" : "dis");
- return;
- }
-
- mode_wanted = ide_get_best_pio_mode (drive, mode_wanted, 5, &d);
- pio_mode = d.pio_mode;
+ pio_mode = ide_get_best_pio_mode(drive, mode_wanted, 5, &d);
cycle_time = d.cycle_time;
/*
* I copied all this complicated stuff from cmd640.c and made a few
* minor changes. For now I am just going to pray that it is correct.
*/
- if (pio_mode > 5)
- pio_mode = 5;
setup_time = ide_pio_timings[pio_mode].setup_time;
active_time = ide_pio_timings[pio_mode].active_time;
recovery_time = cycle_time - (setup_time + active_time);
@@ -320,22 +302,27 @@ static void cmd64x_tuneproc (ide_drive_t
if (active_count > 16)
active_count = 16; /* maximum allowed by cmd646 */
- /*
-* In a perfect world, we might set the drive pio mode here
-* (using WIN_SETFEATURE) before continuing.
-*
-* But we do not, because:
-* 1) this is the wrong place to do it
-* (proper is do_special() in ide.c)
-* 2) in practice this is rarely, if ever, necessary
-*/
program_drive_counts (drive, setup_count, active_count, recovery_count);
- cmdprintk("%s: selected cmd646 PIO mode%d : %d (%dns)%s, "
+ cmdprintk("%s: PIO mode wanted %d, selected %d (%dns)%s, "
"clocks=%d/%d/%d\n",
- drive->name, pio_mode, mode_wanted, cycle_time,
+ drive->name, mode_wanted, pio_mode, cycle_time,
d.overridden ? " (overriding vendor mode)" : "",
setup_count, active_count, recovery_count);
+
+ return pio_mode;
+}
+
+/*
+ * Attempts to set drive's PIO mode.
+ * The preferred method of selecting PIO modes (e.g. mode 4) is
+ * "echo 'piomode:4' > /proc/ide/hdx/settings".
+ * Special case is 255: auto-select best mode (used at boot time).
+ */
+static void cmd64x_tune_drive (ide_drive_t *drive, u8 pio)
+{
+ pio = cmd64x_tune_pio(drive, pio);
+ (void) ide_config_drive_speed(drive, XFER_PIO_0 + pio)
Re: [PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup (take 2)
[Finally forgot to stamp MV's copyright on the driver, so here's take #2...]
The driver's tuneproc() method fails to set the drive's own speed -- fix this
by renaming the function to cmd64x_tune_pio(), making it return the mode set,
and "wrapping" the new tuneproc() method around it; while at it, also get rid
of the non-working prefetch control code, remove redundant PIO5 mode limitation,
and make cmdprintk() give more sensible mode info. Also, get rid of the broken
config_chipset_for_pio() which always tried to set PIO4 instead auto-tuning PIO.
Note that removing a call from config_chipset_for_dma() breaks "rudimentary"
MWDMA2 support which can only work because of the timing registers being pre-
setup for PIO4 since the code in the speedproc() method which sets up the chip
for SW/MW DMA is completely bogus (!) and I'm going to remove it for the time
being in the next patch.
Oh, and add the missing PIO5 support to the speedproc() method while at it. :-)
Warning: compile tested only -- getting to the real hardware isn't that easy...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/cmd64x.c | 95 ---
1 files changed, 34 insertions(+), 61 deletions(-)
Index: linux-2.6/drivers/ide/pci/cmd64x.c
===
--- linux-2.6.orig/drivers/ide/pci/cmd64x.c
+++ linux-2.6/drivers/ide/pci/cmd64x.c
@@ -1,6 +1,6 @@
/* $Id: cmd64x.c,v 1.21 2000/01/30 23:23:16
*
- * linux/drivers/ide/pci/cmd64x.c Version 1.30Sept 10, 2002
+ * linux/drivers/ide/pci/cmd64x.c Version 1.41Feb 3, 2007
*
* cmd64x.c: Enable interrupts at initialization time on Ultra/PCI machines.
* Note, this driver is not used at all on other systems because
@@ -12,6 +12,7 @@
* Copyright (C) 1998 David S. Miller (davem@redhat.com)
*
* Copyright (C) 1999-2002 Andre Hedrick <[EMAIL PROTECTED]>
+ * Copyright (C) 2007 MontaVista Software, Inc. <[EMAIL PROTECTED]>
*/
#include
@@ -262,43 +263,25 @@ static void program_drive_counts (ide_dr
}
/*
- * Attempts to set the interface PIO mode.
- * The preferred method of selecting PIO modes (e.g. mode 4) is
- * "echo 'piomode:4' > /proc/ide/hdx/settings". Special cases are
- * 8: prefetch off, 9: prefetch on, 255: auto-select best mode.
- * Called with 255 at boot time.
+ * This routine selects drive's best PIO mode, calculates setup/active/recovery
+ * counts, and writes them into the chipset registers.
*/
-
-static void cmd64x_tuneproc (ide_drive_t *drive, u8 mode_wanted)
+static u8 cmd64x_tune_pio (ide_drive_t *drive, u8 mode_wanted)
{
int setup_time, active_time, recovery_time;
int clock_time, pio_mode, cycle_time;
u8 recovery_count2, cycle_count;
int setup_count, active_count, recovery_count;
int bus_speed = system_bus_clock();
- /*byte b;*/
ide_pio_data_t d;
- switch (mode_wanted) {
- case 8: /* set prefetch off */
- case 9: /* set prefetch on */
- mode_wanted &= 1;
- /*set_prefetch_mode(index, mode_wanted);*/
- cmdprintk("%s: %sabled cmd640 prefetch\n",
- drive->name, mode_wanted ? "en" : "dis");
- return;
- }
-
- mode_wanted = ide_get_best_pio_mode (drive, mode_wanted, 5, &d);
- pio_mode = d.pio_mode;
+ pio_mode = ide_get_best_pio_mode(drive, mode_wanted, 5, &d);
cycle_time = d.cycle_time;
/*
* I copied all this complicated stuff from cmd640.c and made a few
* minor changes. For now I am just going to pray that it is correct.
*/
- if (pio_mode > 5)
- pio_mode = 5;
setup_time = ide_pio_timings[pio_mode].setup_time;
active_time = ide_pio_timings[pio_mode].active_time;
recovery_time = cycle_time - (setup_time + active_time);
@@ -320,22 +303,27 @@ static void cmd64x_tuneproc (ide_drive_t
if (active_count > 16)
active_count = 16; /* maximum allowed by cmd646 */
- /*
-* In a perfect world, we might set the drive pio mode here
-* (using WIN_SETFEATURE) before continuing.
-*
-* But we do not, because:
-* 1) this is the wrong place to do it
-* (proper is do_special() in ide.c)
-* 2) in practice this is rarely, if ever, necessary
-*/
program_drive_counts (drive, setup_count, active_count, recovery_count);
- cmdprintk("%s: selected cmd646 PIO mode%d : %d (%dns)%s, "
+ cmdprintk("%s: PIO mode wanted %d, selected %d (%dns)%s, "
"clocks=%d/%d/%d\n",
-
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Bartlomiej Zolnierkiewicz wrote: Index: b/drivers/ide/pci/cmd64x.c === --- a/drivers/ide/pci/cmd64x.c +++ b/drivers/ide/pci/cmd64x.c @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i hwif->swdma_mask = 0x07; if (dev->device == PCI_DEVICE_ID_CMD_643) - hwif->ultra_mask = 0x80; + hwif->ultra_mask = 0x00; if (dev->device == PCI_DEVICE_ID_CMD_646) - hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80; + hwif->ultra_mask = + (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00; if (dev->device == PCI_DEVICE_ID_CMD_648) hwif->ultra_mask = 0x1f; Hm, well, this doesn't look consistent with the changes in other drivers. This driver asks for explicit hwif->cds->ultra_mask initializers, IMO... You'd only have to check for PCI-646 revisions < 5 then... reworked Thanks. :-) Index: b/drivers/ide/pci/piix.c === --- a/drivers/ide/pci/piix.c +++ b/drivers/ide/pci/piix.c default: if (!hwif->udma_four) hwif->udma_four = piix_cable_detect(hwif); This one also certainly asks for explicit hwif->cds->ultra_mask initializers... Thus almost all of this switch statement could go away... Alas doing it now would make the nice DECLARE_PIIX_DEV() macro go away Why? Could add another argument to that macro... (=> a lot of duplicated code)... could be done in the future... Yes, of course. Thanks, Bart MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup (take 2)
Hello.
Bartlomiej Zolnierkiewicz wrote:
[Finally forgot to stamp MV's copyright on the driver, so here's take #2...]
The driver's tuneproc() method fails to set the drive's own speed -- fix this
by renaming the function to cmd64x_tune_pio(), making it return the mode set,
and "wrapping" the new tuneproc() method around it; while at it, also get rid
of the non-working prefetch control code, remove redundant PIO5 mode limitation,
and make cmdprintk() give more sensible mode info. Also, get rid of the broken
config_chipset_for_pio() which always tried to set PIO4 instead auto-tuning PIO.
Note that removing a call from config_chipset_for_dma() breaks "rudimentary"
MWDMA2 support which can only work because of the timing registers being pre-
setup for PIO4 since the code in the speedproc() method which sets up the chip
for SW/MW DMA is completely bogus (!) and I'm going to remove it for the time
being in the next patch.
Oh, and add the missing PIO5 support to the speedproc() method while at it. :-)
Generally looks fine, some comments below.
Warning: compile tested only -- getting to the real hardware isn't that easy...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/cmd64x.c | 95 ---
1 files changed, 34 insertions(+), 61 deletions(-)
Index: linux-2.6/drivers/ide/pci/cmd64x.c
===
--- linux-2.6.orig/drivers/ide/pci/cmd64x.c
+++ linux-2.6/drivers/ide/pci/cmd64x.c
@@ -1,6 +1,6 @@
/* $Id: cmd64x.c,v 1.21 2000/01/30 23:23:16
*
- * linux/drivers/ide/pci/cmd64x.c Version 1.30Sept 10, 2002
+ * linux/drivers/ide/pci/cmd64x.c Version 1.41Feb 3, 2007
*
* cmd64x.c: Enable interrupts at initialization time on Ultra/PCI machines.
* Note, this driver is not used at all on other systems because
@@ -12,6 +12,7 @@
* Copyright (C) 1998 David S. Miller (davem@redhat.com)
*
* Copyright (C) 1999-2002 Andre Hedrick <[EMAIL PROTECTED]>
+ * Copyright (C) 2007 MontaVista Software, Inc. <[EMAIL PROTECTED]>
*/
#include
@@ -262,43 +263,25 @@ static void program_drive_counts (ide_dr
}
/*
- * Attempts to set the interface PIO mode.
- * The preferred method of selecting PIO modes (e.g. mode 4) is
- * "echo 'piomode:4' > /proc/ide/hdx/settings". Special cases are
- * 8: prefetch off, 9: prefetch on, 255: auto-select best mode.
- * Called with 255 at boot time.
+ * This routine selects drive's best PIO mode, calculates setup/active/recovery
+ * counts, and writes them into the chipset registers.
*/
-
-static void cmd64x_tuneproc (ide_drive_t *drive, u8 mode_wanted)
+static u8 cmd64x_tune_pio (ide_drive_t *drive, u8 mode_wanted)
{
int setup_time, active_time, recovery_time;
int clock_time, pio_mode, cycle_time;
u8 recovery_count2, cycle_count;
int setup_count, active_count, recovery_count;
int bus_speed = system_bus_clock();
- /*byte b;*/
ide_pio_data_t d;
- switch (mode_wanted) {
- case 8: /* set prefetch off */
- case 9: /* set prefetch on */
- mode_wanted &= 1;
- /*set_prefetch_mode(index, mode_wanted);*/
- cmdprintk("%s: %sabled cmd640 prefetch\n",
- drive->name, mode_wanted ? "en" : "dis");
- return;
- }
-
- mode_wanted = ide_get_best_pio_mode (drive, mode_wanted, 5, &d);
- pio_mode = d.pio_mode;
+ pio_mode = ide_get_best_pio_mode(drive, mode_wanted, 5, &d);
cycle_time = d.cycle_time;
After this change if an unaware user passes "8" or "9" to enable/disable
prefetch (user doesn't know that it has never worked) it will result
in PIO5 being programmed.
Yes. :-)
I don't think that this is a real world issue but for paranoia reasons
please add pio == 8/9 check to cmd64x_tune_drive(), something like:
/*
* letf-over from prefetch setting (pio == 8/9),
* needed to prevent PIO5 from being programmed
*/
if (pio == 8 || pio == 9)
return;
OK, I'll probably just leave the prefetch code where it was.
This will vanish when core code will do filtering of user space
PIO mode change requests...
[ ... ]
+/*
+ * Attempts to set drive's PIO mode.
+ * The preferred method of selecting PIO modes (e.g. mode 4) is
+ * "echo 'piomode:4' > /proc/ide/hdx/settings".
+ * Special case is 255: auto-select best mode (used at boot time).
+ */
The preferred method is to let the driver do the tuning and if for some
reason somebody wants to change the PIO mode, the ioctl interface is
preferred over the deprecated "/pro
Re: [PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup (take 2)
Hello, I wrote: Index: linux-2.6/drivers/ide/pci/cmd64x.c === --- linux-2.6.orig/drivers/ide/pci/cmd64x.c +++ linux-2.6/drivers/ide/pci/cmd64x.c While this was always incorrectly setting PIO4, the PIO4 is "the usual" case and for this driver we need to program PIO explicitly even when using DMA. Hm, why it's *so* special, i.e. why almost all the other drivers can get away without it (the majority seems to have autotune set *only* if hwif->dma_base is seen as 0 in the init_hwif() method? :-/ The core code doesn't program PIO mode unless told to (->autotune flag == 1) so after the above change PIO mode won't be programmed et all. I think that we now need to set ->autotune unconditionally in init_hwif_cmd64x(). Don't think we *need* to. Look at the code at the end of init_chipset() method. Those values it writes to ARTTIM/DRWTIM registers already matches PIO4! That's another question what this code is doing there, being both duplicate and misplaced. :-) No problem. This actually seems the right thing to do in all drivers, just like the libata core does. :-) MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] amd74xx: don't configure udma mode higher than BIOS did
Tejun Heo wrote: CK804 IDE, at least mine, reports 80c in a lot of cases where it shouldn't. I dunno the reason but it also makes drives confused about cable type. Maybe it has the wrong capacitor attached or something. This is A8N-E from ASUS, probably one of the popular ones using nf4. When that happens, libata EH does its job and slows the interface to udma33 after quite a few error messages. On IDE, if this happens, the drive is put into PIO mode making the machine painful to use. Why? The old IDE core should just be downgrading UDMA to a speed where it starts working (i.e. 44 or 33 MB/s usually). MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.20) pata_oldpiix: fix PIO2 underclocking
Fix the PIO mode 2 using mode 0 timings -- this driver should enable the
fast timing bank starting with PIO2, just like the ata_piix driver does.
Also, fix/rephrase some comments while at it.
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
---
drivers/ata/pata_oldpiix.c | 18 ++
1 files changed, 10 insertions(+), 8 deletions(-)
Index: linux-2.6/drivers/ata/pata_oldpiix.c
===
--- linux-2.6.orig/drivers/ata/pata_oldpiix.c
+++ linux-2.6/drivers/ata/pata_oldpiix.c
@@ -25,7 +25,7 @@
#include
#define DRV_NAME "pata_oldpiix"
-#define DRV_VERSION"0.5.2"
+#define DRV_VERSION"0.5.3"
/**
* oldpiix_pre_reset - probe begin
@@ -94,19 +94,21 @@ static void oldpiix_set_piomode (struct
{ 2, 1 },
{ 2, 3 }, };
- if (pio > 2)
- control |= 1; /* TIME1 enable */
+ if (pio > 1)
+ control |= 1; /* TIME */
if (ata_pio_need_iordy(adev))
- control |= 2; /* IE IORDY */
+ control |= 2; /* IE */
- /* Intel specifies that the PPE functionality is for disk only */
+ /* Intel specifies that the prefetch/posting is for disk only */
if (adev->class == ATA_DEV_ATA)
- control |= 4; /* PPE enable */
+ control |= 4; /* PPE */
pci_read_config_word(dev, idetm_port, &idetm_data);
- /* Enable PPE, IE and TIME as appropriate. Clear the other
- drive timing bits */
+ /*
+* Set PPE, IE and TIME as appropriate.
+* Clear the other drive's timing bits.
+*/
if (adev->devno == 0) {
idetm_data &= 0xCCE0;
idetm_data |= control;
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.20) pata_mpiix: fix PIO setup issues
Fix clearing/setting the wrong TIME/IE/PPE bits for a slave drive caused by a
wrong shift count.
Fix the PIO mode 1 being overclocked by wrongly selecting the fast timing bank.
Also, fix/rephrase some comments while at it.
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
---
drivers/ata/pata_mpiix.c | 19 ++-
1 files changed, 10 insertions(+), 9 deletions(-)
Index: linux-2.6/drivers/ata/pata_mpiix.c
===
--- linux-2.6.orig/drivers/ata/pata_mpiix.c
+++ linux-2.6/drivers/ata/pata_mpiix.c
@@ -35,7 +35,7 @@
#include
#define DRV_NAME "pata_mpiix"
-#define DRV_VERSION "0.7.3"
+#define DRV_VERSION "0.7.4"
enum {
IDETIM = 0x6C, /* IDE control register */
@@ -80,8 +80,8 @@ static void mpiix_error_handler(struct a
* @adev: ATA device
*
* Called to do the PIO mode setup. The MPIIX allows us to program the
- * IORDY sample point (2-5 clocks), recovery 1-4 clocks and whether
- * prefetching or iordy are used.
+ * IORDY sample point (2-5 clocks), recovery (1-4 clocks) and whether
+ * prefetching or IORDY are used.
*
* This would get very ugly because we can only program timing for one
* device at a time, the other gets PIO0. Fortunately libata calls
@@ -103,18 +103,19 @@ static void mpiix_set_piomode(struct ata
{ 2, 3 }, };
pci_read_config_word(pdev, IDETIM, &idetim);
- /* Mask the IORDY/TIME/PPE0 bank for this device */
+
+ /* Mask the IORDY/TIME/PPE for this device */
if (adev->class == ATA_DEV_ATA)
- control |= PPE; /* PPE enable for disk */
+ control |= PPE; /* Enable prefetch/posting for disk */
if (ata_pio_need_iordy(adev))
- control |= IORDY; /* IORDY */
- if (pio > 0)
+ control |= IORDY;
+ if (pio > 1)
control |= FTIM;/* This drive is on the fast timing
bank */
/* Mask out timing and clear both TIME bank selects */
idetim &= 0xCCEE;
- idetim &= ~(0x07 << (2 * adev->devno));
- idetim |= (control << (2 * adev->devno));
+ idetim &= ~(0x07 << (4 * adev->devno));
+ idetim |= control << (4 * adev->devno);
idetim |= (timings[pio][0] << 12) | (timings[pio][1] << 8);
pci_write_config_word(pdev, IDETIM, idetim);
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.20) pata_mpiix: probing cleanup
MPIIX has only single channel IDE which can be configured for either primary or
secondary legacy I/O ports and IRQ. So, get rid of the unneeded second probe
entry in mpiix_init_one() and of the invalid (but unused anyway) enable bits in
mpiix_pre_reset().
Warning: this cleanup has only been compile-tested...
drivers/ata/pata_mpiix.c | 63 ---
1 files changed, 27 insertions(+), 36 deletions(-)
Index: linux-2.6/drivers/ata/pata_mpiix.c
===
--- linux-2.6.orig/drivers/ata/pata_mpiix.c
+++ linux-2.6/drivers/ata/pata_mpiix.c
@@ -35,7 +35,7 @@
#include
#define DRV_NAME "pata_mpiix"
-#define DRV_VERSION "0.7.4"
+#define DRV_VERSION "0.7.5"
enum {
IDETIM = 0x6C, /* IDE control register */
@@ -49,12 +49,9 @@ enum {
static int mpiix_pre_reset(struct ata_port *ap)
{
struct pci_dev *pdev = to_pci_dev(ap->host->dev);
- static const struct pci_bits mpiix_enable_bits[] = {
- { 0x6D, 1, 0x80, 0x80 },
- { 0x6F, 1, 0x80, 0x80 }
- };
+ static const struct pci_bits mpiix_enable_bits = { 0x6D, 1, 0x80, 0x80
};
- if (!pci_test_config_bits(pdev, &mpiix_enable_bits[ap->port_no]))
+ if (!pci_test_config_bits(pdev, &mpiix_enable_bits))
return -ENOENT;
ap->cbl = ATA_CBL_PATA40;
return ata_std_prereset(ap);
@@ -202,10 +199,9 @@ static struct ata_port_operations mpiix_
static int mpiix_init_one(struct pci_dev *dev, const struct pci_device_id *id)
{
/* Single threaded by the PCI probe logic */
- static struct ata_probe_ent probe[2];
+ static struct ata_probe_ent probe;
static int printed_version;
u16 idetim;
- int enabled;
if (!printed_version++)
dev_printk(KERN_DEBUG, &dev->dev, "version " DRV_VERSION "\n");
@@ -224,37 +220,32 @@ static int mpiix_init_one(struct pci_dev
without BARs set fools the setup. #2 If you pci_disable_device
the MPIIX your box goes castors up */
- INIT_LIST_HEAD(&probe[0].node);
- probe[0].dev = pci_dev_to_dev(dev);
- probe[0].port_ops = &mpiix_port_ops;
- probe[0].sht = &mpiix_sht;
- probe[0].pio_mask = 0x1F;
- probe[0].irq = 14;
- probe[0].irq_flags = SA_SHIRQ;
- probe[0].port_flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST;
- probe[0].n_ports = 1;
- probe[0].port[0].cmd_addr = 0x1F0;
- probe[0].port[0].ctl_addr = 0x3F6;
- probe[0].port[0].altstatus_addr = 0x3F6;
-
- /* The secondary lurks at different addresses but is otherwise
- the same beastie */
-
- INIT_LIST_HEAD(&probe[1].node);
- probe[1] = probe[0];
- probe[1].irq = 15;
- probe[1].port[0].cmd_addr = 0x170;
- probe[1].port[0].ctl_addr = 0x376;
- probe[1].port[0].altstatus_addr = 0x376;
+ INIT_LIST_HEAD(&probe.node);
+ probe.dev = pci_dev_to_dev(dev);
+ probe.port_ops = &mpiix_port_ops;
+ probe.sht = &mpiix_sht;
+ probe.pio_mask = 0x1F;
+ probe.irq_flags = SA_SHIRQ;
+ probe.port_flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST;
+ probe.n_ports = 1;
+
+ /* See if it's primary or secondary channel... */
+ if (idetim & SECONDARY) {
+ probe.irq = 15;
+ probe.port[0].cmd_addr = 0x170;
+ probe.port[0].ctl_addr = 0x376;
+ probe.port[0].altstatus_addr = 0x376;
+ } else {
+ probe.irq = 14;
+ probe.port[0].cmd_addr = 0x1F0;
+ probe.port[0].ctl_addr = 0x3F6;
+ probe.port[0].altstatus_addr = 0x3F6;
+ }
/* Let libata fill in the port details */
- ata_std_ports(&probe[0].port[0]);
- ata_std_ports(&probe[1].port[0]);
+ ata_std_ports(&probe.port[0]);
- /* Now add the port that is active */
- enabled = (idetim & SECONDARY) ? 1 : 0;
-
- if (ata_device_add(&probe[enabled]))
+ if (ata_device_add(&probe))
return 0;
return -ENODEV;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.20) pata_mpiix: probing cleanup (resend)
[Cr*p, forgot both the sign-off and cut-off! :-)]
MPIIX has only single channel IDE which can be configured for either primary or
secondary legacy I/O ports and IRQ. So, get rid of the unneeded second probe
entry in mpiix_init_one() and of the invalid (but unused anyway) enable bits in
mpiix_pre_reset().
Warning: this cleanup has only been compile-tested...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
---
drivers/ata/pata_mpiix.c | 63 ---
1 files changed, 27 insertions(+), 36 deletions(-)
Index: linux-2.6/drivers/ata/pata_mpiix.c
===
--- linux-2.6.orig/drivers/ata/pata_mpiix.c
+++ linux-2.6/drivers/ata/pata_mpiix.c
@@ -35,7 +35,7 @@
#include
#define DRV_NAME "pata_mpiix"
-#define DRV_VERSION "0.7.4"
+#define DRV_VERSION "0.7.5"
enum {
IDETIM = 0x6C, /* IDE control register */
@@ -49,12 +49,9 @@ enum {
static int mpiix_pre_reset(struct ata_port *ap)
{
struct pci_dev *pdev = to_pci_dev(ap->host->dev);
- static const struct pci_bits mpiix_enable_bits[] = {
- { 0x6D, 1, 0x80, 0x80 },
- { 0x6F, 1, 0x80, 0x80 }
- };
+ static const struct pci_bits mpiix_enable_bits = { 0x6D, 1, 0x80, 0x80
};
- if (!pci_test_config_bits(pdev, &mpiix_enable_bits[ap->port_no]))
+ if (!pci_test_config_bits(pdev, &mpiix_enable_bits))
return -ENOENT;
ap->cbl = ATA_CBL_PATA40;
return ata_std_prereset(ap);
@@ -202,10 +199,9 @@ static struct ata_port_operations mpiix_
static int mpiix_init_one(struct pci_dev *dev, const struct pci_device_id *id)
{
/* Single threaded by the PCI probe logic */
- static struct ata_probe_ent probe[2];
+ static struct ata_probe_ent probe;
static int printed_version;
u16 idetim;
- int enabled;
if (!printed_version++)
dev_printk(KERN_DEBUG, &dev->dev, "version " DRV_VERSION "\n");
@@ -224,37 +220,32 @@ static int mpiix_init_one(struct pci_dev
without BARs set fools the setup. #2 If you pci_disable_device
the MPIIX your box goes castors up */
- INIT_LIST_HEAD(&probe[0].node);
- probe[0].dev = pci_dev_to_dev(dev);
- probe[0].port_ops = &mpiix_port_ops;
- probe[0].sht = &mpiix_sht;
- probe[0].pio_mask = 0x1F;
- probe[0].irq = 14;
- probe[0].irq_flags = SA_SHIRQ;
- probe[0].port_flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST;
- probe[0].n_ports = 1;
- probe[0].port[0].cmd_addr = 0x1F0;
- probe[0].port[0].ctl_addr = 0x3F6;
- probe[0].port[0].altstatus_addr = 0x3F6;
-
- /* The secondary lurks at different addresses but is otherwise
- the same beastie */
-
- INIT_LIST_HEAD(&probe[1].node);
- probe[1] = probe[0];
- probe[1].irq = 15;
- probe[1].port[0].cmd_addr = 0x170;
- probe[1].port[0].ctl_addr = 0x376;
- probe[1].port[0].altstatus_addr = 0x376;
+ INIT_LIST_HEAD(&probe.node);
+ probe.dev = pci_dev_to_dev(dev);
+ probe.port_ops = &mpiix_port_ops;
+ probe.sht = &mpiix_sht;
+ probe.pio_mask = 0x1F;
+ probe.irq_flags = SA_SHIRQ;
+ probe.port_flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST;
+ probe.n_ports = 1;
+
+ /* See if it's primary or secondary channel... */
+ if (idetim & SECONDARY) {
+ probe.irq = 15;
+ probe.port[0].cmd_addr = 0x170;
+ probe.port[0].ctl_addr = 0x376;
+ probe.port[0].altstatus_addr = 0x376;
+ } else {
+ probe.irq = 14;
+ probe.port[0].cmd_addr = 0x1F0;
+ probe.port[0].ctl_addr = 0x3F6;
+ probe.port[0].altstatus_addr = 0x3F6;
+ }
/* Let libata fill in the port details */
- ata_std_ports(&probe[0].port[0]);
- ata_std_ports(&probe[1].port[0]);
+ ata_std_ports(&probe.port[0]);
- /* Now add the port that is active */
- enabled = (idetim & SECONDARY) ? 1 : 0;
-
- if (ata_device_add(&probe[enabled]))
+ if (ata_device_add(&probe))
return 0;
return -ENODEV;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: IORDY handling, quiet handling
Alan wrote:
For further review
Turn on the IORDY handling logic.
If a controller doesnt support IORDY don't try and use it
If a device doesn't support IORDY don't try and use it
Looks like we shouldn't change it default PIO mode at all in this case.
See below why...
Otherwise use it whenever possible.
Always use IORDY for PIO3 and higher (it's mandatory)
Send the correct set_features command if operating without IORDY
Signed-off-by: Alan Cox <[EMAIL PROTECTED]>
diff -u --new-file --recursive --exclude-from /usr/src/exclude
linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-core.c
linux-2.6.20-rc6-mm3/drivers/ata/libata-core.c
--- linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-core.c 2007-01-31
14:20:39.0 +
+++ linux-2.6.20-rc6-mm3/drivers/ata/libata-core.c 2007-02-01
16:14:01.0 +
@@ -1404,30 +1446,44 @@
* Check if the current speed of the device requires IORDY. Used
* by various controllers for chip configuration.
*/
-
+
unsigned int ata_pio_need_iordy(const struct ata_device *adev)
{
- int pio;
- int speed = adev->pio_mode - XFER_PIO_0;
-
- if (speed < 2)
+ /* Controller doesn't support IORDY. Probably a pointless check
+ as the caller should know this */
+ if (adev->ap->flags & ATA_FLAG_NO_IORDY)
return 0;
- if (speed > 2)
+ /* PIO3 and higher it is mandatory */
+ if (adev->pio_mode > XFER_PIO_2)
return 1;
+ /* We turn it on when possible */
+ if (ata_id_has_iordy(adev->id))
+ return 1;
+ return 0;
+}
+/**
+ * ata_pio_mask_no_iordy - Return the non IORDY mask
+ * @adev: ATA device
+ *
+ * Compute the highest mode possible if we are not using iordy. Return
+ * -1 if no iordy mode is available.
+ */
+
+static u32 ata_pio_mask_no_iordy(const struct ata_device *adev)
+{
/* If we have no drive specific rule, then PIO 2 is non IORDY */
-
if (adev->id[ATA_ID_FIELD_VALID] & 2) { /* EIDE */
- pio = adev->id[ATA_ID_EIDE_PIO];
+ u16 pio = adev->id[ATA_ID_EIDE_PIO];
/* Is the speed faster than the drive allows non IORDY ? */
if (pio) {
/* This is cycle times not frequency - watch the logic!
*/
if (pio > 240) /* PIO2 is 240nS per cycle */
- return 1;
- return 0;
+ return 3 << ATA_SHIFT_PIO;
+ return 7 << ATA_SHIFT_PIO;
}
}
- return 0;
+ return 3 << ATA_SHIFT_PIO;
}
Well, after looking into ATA-1, it seems that the logic may have been and
may be still is somewhat wrong here. There was PIO2 already defined but no
EIDE fields yet (and yet optional IORDY line already here). We probably
should always consider PIO2 non-IORDY indeed (unless told not to by the word
67) but go figure... :-)
@@ -3466,8 +3529,18 @@
tf.command = ATA_CMD_SET_FEATURES;
tf.feature = SETFEATURES_XFER;
tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+
+ /* Older CFA may not support this command */
+ if (ata_id_is_cfa(dev->id))
+ tf.flags |= ATA_TFLAG_QUIET;
+
tf.protocol = ATA_PROT_NODATA;
- tf.nsect = dev->xfer_mode;
+
+ /* Ancient devices may need us to avoid IORDY */
+ if (ata_pio_need_iordy(dev))
+ tf.nsect = dev->xfer_mode;
+ else
+ tf.nsect = 0x01;
Note that ATA-1 did *not* yet define this subcode (only 0 for "block
transfer"). I think that we should not change the PIO modes at all on
non-IORDY drives, otherwise it's becoming pointless and messy -- you're not
setting what you're told here and force the drive's default mode instead...
why then change it at all?
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: IORDY handling, quiet handling
Hello, I wrote: @@ -3466,8 +3529,18 @@ tf.command = ATA_CMD_SET_FEATURES; tf.feature = SETFEATURES_XFER; tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + +/* Older CFA may not support this command */ +if (ata_id_is_cfa(dev->id)) +tf.flags |= ATA_TFLAG_QUIET; + tf.protocol = ATA_PROT_NODATA; -tf.nsect = dev->xfer_mode; + +/* Ancient devices may need us to avoid IORDY */ +if (ata_pio_need_iordy(dev)) +tf.nsect = dev->xfer_mode; +else +tf.nsect = 0x01; Note that ATA-1 did *not* yet define this subcode (only 0 for "block transfer"). I think that we should not change the PIO modes at all on non-IORDY drives, otherwise it's becoming pointless and messy -- you're After thinking a bit more: we should not do it on non-EIDE drives (however, this is probably the same thing). If our host by some mischance has no IORDY support and an EIDE drive has it and supports disabling (otherwise there's little we can do), we must issue subcode 0x01 and tune our host controller to the default PIO mode. not setting what you're told here and force the drive's default mode instead... why then change it at all? Anyway, all this logic should be one layer higher than this function... MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] (2.6.20) cmd64x: fix PIO mode setup (take 3)
The driver's tuneproc() method fails to set the drive's own speed -- fix this
by renaming the function to cmd64x_tune_pio(), making it return the mode set,
and "wrapping" the new tuneproc() method around it; while at it, also get rid
of the non-working prefetch control code (filtering out related argument values
in the "wrapper"), remove redundant PIO5 mode limitation, make cmdprintk() give
more sensible mode info, and remove mention about the obsolete /proc/ interface.
Get rid of the broken config_chipset_for_pio() which always tried to set PIO4,
switch to always auto-tuning PIO instead.
Oh, and add the missing PIO5 support to the speedproc() method while at it. :-)
Warning: compile tested only -- getting to the real hardware isn't that easy...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/cmd64x.c | 108 ++-
1 files changed, 43 insertions(+), 65 deletions(-)
Index: linux-2.6/drivers/ide/pci/cmd64x.c
===
--- linux-2.6.orig/drivers/ide/pci/cmd64x.c
+++ linux-2.6/drivers/ide/pci/cmd64x.c
@@ -1,6 +1,6 @@
/* $Id: cmd64x.c,v 1.21 2000/01/30 23:23:16
*
- * linux/drivers/ide/pci/cmd64x.c Version 1.30Sept 10, 2002
+ * linux/drivers/ide/pci/cmd64x.c Version 1.41Feb 3, 2007
*
* cmd64x.c: Enable interrupts at initialization time on Ultra/PCI machines.
* Note, this driver is not used at all on other systems because
@@ -12,6 +12,7 @@
* Copyright (C) 1998 David S. Miller (davem@redhat.com)
*
* Copyright (C) 1999-2002 Andre Hedrick <[EMAIL PROTECTED]>
+ * Copyright (C) 2007 MontaVista Software, Inc. <[EMAIL PROTECTED]>
*/
#include
@@ -262,43 +263,25 @@ static void program_drive_counts (ide_dr
}
/*
- * Attempts to set the interface PIO mode.
- * The preferred method of selecting PIO modes (e.g. mode 4) is
- * "echo 'piomode:4' > /proc/ide/hdx/settings". Special cases are
- * 8: prefetch off, 9: prefetch on, 255: auto-select best mode.
- * Called with 255 at boot time.
+ * This routine selects drive's best PIO mode, calculates setup/active/recovery
+ * counts, and then writes them into the chipset registers.
*/
-
-static void cmd64x_tuneproc (ide_drive_t *drive, u8 mode_wanted)
+static u8 cmd64x_tune_pio (ide_drive_t *drive, u8 mode_wanted)
{
int setup_time, active_time, recovery_time;
int clock_time, pio_mode, cycle_time;
u8 recovery_count2, cycle_count;
int setup_count, active_count, recovery_count;
int bus_speed = system_bus_clock();
- /*byte b;*/
ide_pio_data_t d;
- switch (mode_wanted) {
- case 8: /* set prefetch off */
- case 9: /* set prefetch on */
- mode_wanted &= 1;
- /*set_prefetch_mode(index, mode_wanted);*/
- cmdprintk("%s: %sabled cmd640 prefetch\n",
- drive->name, mode_wanted ? "en" : "dis");
- return;
- }
-
- mode_wanted = ide_get_best_pio_mode (drive, mode_wanted, 5, &d);
- pio_mode = d.pio_mode;
+ pio_mode = ide_get_best_pio_mode(drive, mode_wanted, 5, &d);
cycle_time = d.cycle_time;
/*
* I copied all this complicated stuff from cmd640.c and made a few
* minor changes. For now I am just going to pray that it is correct.
*/
- if (pio_mode > 5)
- pio_mode = 5;
setup_time = ide_pio_timings[pio_mode].setup_time;
active_time = ide_pio_timings[pio_mode].active_time;
recovery_time = cycle_time - (setup_time + active_time);
@@ -320,22 +303,33 @@ static void cmd64x_tuneproc (ide_drive_t
if (active_count > 16)
active_count = 16; /* maximum allowed by cmd646 */
- /*
-* In a perfect world, we might set the drive pio mode here
-* (using WIN_SETFEATURE) before continuing.
-*
-* But we do not, because:
-* 1) this is the wrong place to do it
-* (proper is do_special() in ide.c)
-* 2) in practice this is rarely, if ever, necessary
-*/
program_drive_counts (drive, setup_count, active_count, recovery_count);
- cmdprintk("%s: selected cmd646 PIO mode%d : %d (%dns)%s, "
+ cmdprintk("%s: PIO mode wanted %d, selected %d (%dns)%s, "
"clocks=%d/%d/%d\n",
- drive->name, pio_mode, mode_wanted, cycle_time,
+ drive->name, mode_wanted, pio_mode, cycle_time,
d.overridden ? " (overriding vendor mode)" : "",
setup_count, active_count, recovery_count);
+
+ return pio_mode;
+}
+
+/
[PATCH] (pata-2.6 fix queue) piix/slc90e66: more tuneproc() fixing
The tuneproc() method in both these drivers failed to set the drive's own speed.
Fix this by renaming the function and "wrapping around it" the new tuneproc()
method. While at it, convert the rest of the PIO setup code into proper C. :-)
Warning: this has only been compile tested (but we are in the merge window
already anyway :-).
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
drivers/ide/pci/piix.c | 45 +
drivers/ide/pci/slc90e66.c | 35 +++
2 files changed, 48 insertions(+), 32 deletions(-)
Index: linux-2.6/drivers/ide/pci/piix.c
===
--- linux-2.6.orig/drivers/ide/pci/piix.c
+++ linux-2.6/drivers/ide/pci/piix.c
@@ -1,10 +1,10 @@
/*
- * linux/drivers/ide/pci/piix.c Version 0.46December 3, 2006
+ * linux/drivers/ide/pci/piix.c Version 0.47February 5, 2007
*
* Copyright (C) 1998-1999 Andrzej Krzysztofowicz, Author and Maintainer
* Copyright (C) 1998-2000 Andre Hedrick <[EMAIL PROTECTED]>
* Copyright (C) 2003 Red Hat Inc <[EMAIL PROTECTED]>
- * Copyright (C) 2006 MontaVista Software, Inc. <[EMAIL PROTECTED]>
+ * Copyright (C) 2006-2007 MontaVista Software, Inc. <[EMAIL PROTECTED]>
*
* May be copied or modified under the terms of the GNU General Public License
*
@@ -205,14 +205,13 @@ static u8 piix_dma_2_pio (u8 xfer_rate)
}
/**
- * piix_tune_drive - tune a drive attached to a PIIX
+ * piix_tune_pio - tune PIIX for PIO mode
* @drive: drive to tune
* @pio: desired PIO mode
*
- * Set the interface PIO mode based upon the settings done by AMI BIOS
- * (might be useful if drive is not registered in CMOS for any reason).
+ * Set the interface PIO mode based upon the settings done by AMI BIOS.
*/
-static void piix_tune_drive (ide_drive_t *drive, u8 pio)
+static void piix_tune_pio (ide_drive_t *drive, u8 pio)
{
ide_hwif_t *hwif= HWIF(drive);
struct pci_dev *dev = hwif->pci_dev;
@@ -233,8 +232,6 @@ static void piix_tune_drive (ide_drive_t
{ 2, 1 },
{ 2, 3 }, };
- pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
-
/*
* Master vs slave is synchronized above us but the slave register is
* shared by the two hwifs so the corner case of two slave timeouts in
@@ -253,19 +250,20 @@ static void piix_tune_drive (ide_drive_t
master_data |= 0x4000;
master_data &= ~0x0070;
if (pio > 1) {
- /* enable PPE, IE and TIME */
- master_data = master_data | (control << 4);
+ /* Set PPE, IE and TIME */
+ master_data |= control << 4;
}
pci_read_config_byte(dev, slave_port, &slave_data);
- slave_data = slave_data & (hwif->channel ? 0x0f : 0xf0);
- slave_data = slave_data | (((timings[pio][0] << 2) |
timings[pio][1]) << (hwif->channel ? 4 : 0));
+ slave_data &= hwif->channel ? 0x0f : 0xf0;
+ slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) <<
+ (hwif->channel ? 4 : 0);
} else {
master_data &= ~0x3307;
if (pio > 1) {
/* enable PPE, IE and TIME */
- master_data = master_data | control;
+ master_data |= control;
}
- master_data = master_data | (timings[pio][0] << 12) |
(timings[pio][1] << 8);
+ master_data |= (timings[pio][0] << 12) | (timings[pio][1] << 8);
}
pci_write_config_word(dev, master_port, master_data);
if (is_slave)
@@ -274,6 +272,21 @@ static void piix_tune_drive (ide_drive_t
}
/**
+ * piix_tune_drive - tune a drive attached to PIIX
+ * @drive: drive to tune
+ * @pio: desired PIO mode
+ *
+ * Set the drive's PIO mode (might be useful if drive is not registered
+ * in CMOS for any reason).
+ */
+static void piix_tune_drive (ide_drive_t *drive, u8 pio)
+{
+ pio = ide_get_best_pio_mode(drive, pio, 4, NULL);
+ piix_tune_pio(drive, pio);
+ (void) ide_config_drive_speed(drive, XFER_PIO_0 + pio);
+}
+
+/**
* piix_tune_chipset - tune a PIIX interface
* @drive: IDE drive to tune
* @xferspeed: speed to configure
@@ -348,8 +361,8 @@ static int piix_tune_chipset (ide_drive_
pci_write_config_byte(dev, 0x55, (u8) reg55 & ~w_flag);
}
- piix_tune_drive(drive, piix_dma_2_pio(speed));
- return (ide_config_drive_speed(drive, sp
Re: [PATCH] (2.6.20-rc7) cmd64x: fix PIO mode setup (take 2)
Hello. Bartlomiej Zolnierkiewicz wrote: Index: linux-2.6/drivers/ide/pci/cmd64x.c === --- linux-2.6.orig/drivers/ide/pci/cmd64x.c +++ linux-2.6/drivers/ide/pci/cmd64x.c While this was always incorrectly setting PIO4, the PIO4 is "the usual" case and for this driver we need to program PIO explicitly even when using DMA. Hm, why it's *so* special, i.e. why almost all the other drivers can get away without it (the majority seems to have autotune set *only* if hwif->dma_base is seen as 0 in the init_hwif() method? :-/ The core code doesn't program PIO mode unless told to (->autotune flag == 1) so after the above change PIO mode won't be programmed et all. I think that we now need to set ->autotune unconditionally in init_hwif_cmd64x(). Don't think we *need* to. Look at the code at the end of init_chipset() method. Those values it writes to ARTTIM/DRWTIM registers already matches PIO4! That's another question what this code is doing there, being both duplicate and misplaced. :-) For me it looks like a bunch of hacks to get things working on BIOS/firmware-less and non-x86 systems. :) [ This code pre-dates PIO4 forcing in config_cmd64x_chipset_for_pio(). ] The "#ifdef __i386__" part looked the most mysterous: why anyone would want to program less *IDE* address setup on x86 than on non-x86? :-O I was going to remove all that crap as well but decided to defer it, mostly because of that part... Bart MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Impact of no_lba48{_dma} = 1 ?
Hello. Alan wrote: Only relevant for DMA really So rqsize is only needed for DMA accesses? Not really, it affects both PIO and DMA. PIO transfers are sector at a time, or multi-sector up to a usual limit of about 16 sectors. You're clearly mixing 2 things here. PIO transfers are 1 sectors _per interrupt_ or N (usually 16) sectors in the multiple mode. The ANSI specified transfer limit is 256 for LBA28 (which fails on some drives), and 65536 with LBA48 _per command_. MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Impact of no_lba48{_dma} = 1 ?
Hello.
Steven Scholz wrote:
So rqsize is only needed for DMA accesses?
PIO transfers are sector at a time, or multi-sector up to a usual limit
of about 16 sectors.
So again: rqsize does not matter for PIO transfer?
Looking at the lines
if (hwif->no_lba48_dma && lba48 && dma) {
if (block + rq->nr_sectors > 1ULL << 28)
dma = 0;
else
lba48 = 0;
}
in drivers/ide/ide-disk.c. Does that mean that 48bit LBA adressing using the
task_ioreg_t stuff is only used for sector numbers larger 1 << 28?
If you're asking in general, no, that does not follow from that code.
It only takes care of the no_lba48_dma case.
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
