Re: Problem setting dma using pata_it821x driver in vanilla 2.6.23.12 and gentoo-sources 2.6.23-r3
On Sun, 13 Jan 2008, Tejun Heo wrote: Does this change anything? And, yeah, I really wanna see the hdparm --Istdout too. I applied the patch to libata-core.c. A scsi device, /dev/sdb, was recognized, but partition /dev/sdb1 (where all the data resides) is not seen. Boot log messages and hdparm output follow: Linux version 2.6.23.12 ([EMAIL PROTECTED]) (gcc version 4.1.1 (Gentoo 4.1.1-r3)) #5 SMP Sun Jan 13 01:07:07 EST 2008 BIOS-provided physical RAM map: BIOS-e820: - 0009f800 (usable) BIOS-e820: 0009f800 - 000a (reserved) BIOS-e820: 000f - 0010 (reserved) BIOS-e820: 0010 - 5fff (usable) BIOS-e820: 5fff - 5fff3000 (ACPI NVS) BIOS-e820: 5fff3000 - 6000 (ACPI data) BIOS-e820: fec0 - 0001 (reserved) 639MB HIGHMEM available. 896MB LOWMEM available. found SMP MP-table at 000f5320 Entering add_active_range(0, 0, 393200) 0 entries of 256 used Zone PFN ranges: DMA 0 -> 4096 Normal 4096 -> 229376 HighMem229376 -> 393200 Movable zone start PFN for each node early_node_map[1] active PFN ranges 0:0 -> 393200 On node 0 totalpages: 393200 DMA zone: 32 pages used for memmap DMA zone: 0 pages reserved DMA zone: 4064 pages, LIFO batch:0 Normal zone: 1760 pages used for memmap Normal zone: 223520 pages, LIFO batch:31 HighMem zone: 1279 pages used for memmap HighMem zone: 162545 pages, LIFO batch:31 Movable zone: 0 pages used for memmap DMI 2.3 present. Intel MultiProcessor Specification v1.4 Virtual Wire compatibility mode. OEM ID: OEM0 Product ID: PROD APIC at: 0xFEE0 Processor #0 15:2 APIC version 17 I/O APIC #2 Version 17 at 0xFEC0. Enabling APIC mode: Flat. Using 1 I/O APICs Processors: 1 Allocating PCI resources starting at 7000 (gap: 6000:9ec0) Built 1 zonelists in Zone order. Total pages: 390129 Kernel command line: BOOT_IMAGE=p1ata2.6.23 ro root=307 ide0=ata66 ide2=ata66 ide_setup: ide0=ata66 -- OBSOLETE OPTION, WILL BE REMOVED SOON! ide_setup: ide2=ata66 -- OBSOLETE OPTION, WILL BE REMOVED SOON! mapped APIC to b000 (fee0) mapped IOAPIC to a000 (fec0) Enabling fast FPU save and restore... done. Enabling unmasked SIMD FPU exception support... done. Initializing CPU#0 PID hash table entries: 4096 (order: 12, 16384 bytes) Detected 2813.563 MHz processor. Console: colour VGA+ 80x25 console [tty0] enabled Dentry cache hash table entries: 131072 (order: 7, 524288 bytes) Inode-cache hash table entries: 65536 (order: 6, 262144 bytes) Memory: 1552764k/1572800k available (3124k kernel code, 18908k reserved, 1707k data, 256k init, 655296k highmem) virtual kernel memory layout: fixmap : 0xffe1a000 - 0xf000 (1940 kB) pkmap : 0xff80 - 0xffc0 (4096 kB) vmalloc : 0xf880 - 0xff7fe000 ( 111 MB) lowmem : 0xc000 - 0xf800 ( 896 MB) .init : 0xc05bf000 - 0xc05ff000 ( 256 kB) .data : 0xc040d037 - 0xc05b7ebc (1707 kB) .text : 0xc010 - 0xc040d037 (3124 kB) Checking if this processor honours the WP bit even in supervisor mode... Ok. Calibrating delay using timer specific routine.. 5631.42 BogoMIPS (lpj=11262850) Mount-cache hash table entries: 512 CPU: After generic identify, caps: bfebfbff 4400 CPU: Trace cache: 12K uops, L1 D cache: 8K CPU: L2 cache: 512K CPU: Physical Processor ID: 0 CPU: After all inits, caps: bfebfbff b080 4400 Intel machine check architecture supported. Intel machine check reporting enabled on CPU#0. CPU0: Intel P4/Xeon Extended MCE MSRs (12) available CPU0: Thermal monitoring enabled Compat vDSO mapped to e000. Checking 'hlt' instruction... OK. SMP alternatives: switching to UP code Freeing SMP alternatives: 17k freed CPU0: Intel(R) Pentium(R) 4 CPU 2.80GHz stepping 09 Total of 1 processors activated (5631.42 BogoMIPS). ExtINT not setup in hardware but reported by MP table ENABLING IO-APIC IRQs ..TIMER: vector=0x31 apic1=0 pin1=2 apic2=0 pin2=0 Brought up 1 CPUs NET: Registered protocol family 16 PCI: PCI BIOS revision 2.10 entry at 0xfb640, last bus=3 PCI: Using configuration type 1 Setting up standard PCI resources SCSI subsystem initialized libata version 2.21 loaded. usbcore: registered new interface driver usbfs usbcore: registered new interface driver hub usbcore: registered new device driver usb PCI: Probing PCI hardware PCI: Probing PCI hardware (bus 00) PCI quirk: region 1000-107f claimed by ICH4 ACPI/GPIO/TCO PCI quirk: region 1080-10bf claimed by ICH4 GPIO PCI: Transparent bridge - :00:1e.0 PCI: Using IRQ router PIIX/ICH [8086/24d0] at :00:1f.0 PCI->APIC IRQ transform: :00:1d.0[A] -> IRQ 16 PCI->APIC IRQ transform: :00:1d.1[B] -> IRQ 19 PCI->APIC IRQ transform: :00:1d.2[C] -> IRQ 18 PCI->APIC IRQ transfo
Re: 40-wire cable detected when directly connected
Tobias Müller wrote: > Hi > >> Please apply the attached patch and specify libata.force_cbl=80 as >> kernel boot parameter. If you load libata from initrd or after boot you >> need to pass 'force_cbl=80' as module parameter. How you do it depends >> on your distro. Ah.. right. I'm brewing more complete debug helper patch. I'll take the above into consideration. -- tejun - 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: Problem setting dma using pata_it821x driver in vanilla 2.6.23.12 and gentoo-sources 2.6.23-r3
Marc Howard Zuckman wrote: > On Sat, 12 Jan 2008, Bartlomiej Zolnierkiewicz wrote: > >> >> In "smart mode" controller takes care of mode programming. >> >>> Cc'ing Alan and Bartlomiej. Guys, is this the smart mode problem? Do >>> ide and libata have this fixed in 2.6.24-rc? >> >> [ added Alan and myself ;) to cc: ] >> >> Marc, please also send output of 'hdparm --Istdout /dev/hde' command. >> > This is the output from hdparm with my non-working vanilla 2.6.23.12. > > > /dev/hde: > > 2008 1301 0501 4721 0101 0005 > > 496e 7465 6772 6174 6564 > 2054 6563 686e 6f6c 6f67 7920 4578 7072 > 6573 7320 496e 6320 2020 2020 2020 > > 0fff 0007 > > > > 003f > 9eae 12a1 > > > > 0001 0003 3133 2121 9111 0091 3266 > 0021 0021 0001 0505 0005 0500 0005 > 0005 0005 0040 > 0001 > > > > > > > > > > > > Hmm... this is horrifying. $ hdparm --Istdin < hdparm.out ATA device, with non-removable media Model Number: Integrated Technology Express Inc Serial Number: G! Standards: Likely used: 1 Configuration: Logical max current cylinders 0 0 heads 0 0 sectors/track 0 0 -- device size with M = 1024*1024: 0 MBytes device size with M = 1000*1000: 0 MBytes Capabilities: IORDY not likely Cannot perform double-word IO R/W multiple sector transfer: not supported DMA: not supported PIO: pio0 The only thing which matches the number of sectors is at word 100 which is in reserved area. What's going on here? -- tejun - 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: Problem setting dma using pata_it821x driver in vanilla 2.6.23.12 and gentoo-sources 2.6.23-r3
On Sun, 13 Jan 2008, Tejun Heo wrote: Does this change anything? And, yeah, I really wanna see the hdparm --Istdout too. I will apply the patch and test it as you have requested. To be clear, the output of the hpdarm -Istdout /dev/hde command that I reported a few minutes ago was performed with a vanilla 2.6.23.12 kernel that, as you pointed out, was utilizing the it821x driver rather than the pata_it821x driver. That may be obvious to you, but I thought I should be explicit about it. Now, with reference to the 2.6.23.12 kernel with the unpatched version of the libata code, I don't think I can do much with hdparm since the raid array does not seem to be recognized as a device. If my understanding is correct based on the kernel boot messages, it seems to me that pata_it821x is going to treat the array as a scsi block device, which on my system will configure as /dev/sdb if the array is recognized. Marc H. Zuckman - 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: Problem setting dma using pata_it821x driver in vanilla 2.6.23.12 and gentoo-sources 2.6.23-r3
On Sat, 12 Jan 2008, Bartlomiej Zolnierkiewicz wrote:
In "smart mode" controller takes care of mode programming.
Cc'ing Alan and Bartlomiej. Guys, is this the smart mode problem? Do
ide and libata have this fixed in 2.6.24-rc?
[ added Alan and myself ;) to cc: ]
Marc, please also send output of 'hdparm --Istdout /dev/hde' command.
This is the output from hdparm with my non-working vanilla 2.6.23.12.
/dev/hde:
2008 1301 0501 4721 0101 0005
496e 7465 6772 6174 6564
2054 6563 686e 6f6c 6f67 7920 4578 7072
6573 7320 496e 6320 2020 2020 2020
0fff 0007
003f
9eae 12a1
0001 0003 3133 2121 9111 0091 3266
0021 0021 0001 0505 0005 0500 0005
0005 0005 0040
0001
The output from this command with the it821x supported raid array in my 2.6.20
kernel version is identical EXCEPT for the hex codes delimited by the *.
With 2.6.20, those hex codes are 0925.
Marc H. Zuckman
-
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: Problem setting dma using pata_it821x driver in vanilla 2.6.23.12 and gentoo-sources 2.6.23-r3
Marc Howard Zuckman wrote:
> On Fri, 11 Jan 2008, Tejun Heo, replying to Marc Zuckman wrote:
>
>>> Zuckman:
>
>>> Is it possible that both drivers are present in the kernel and conflict
>>> with one another in some fashion?
>> Heo:
>
>> The don't conflict. If both are in the kernel, IDE drivers have
>> priority. Can you please try the libata driver and see if anything is
>> different?
>
> It's different, but the drive array does not function.
>
> These are the boot messages with libata and pata_it821x configured.
> Note that I left the old ide drivers in the kernel for the ICH5
> controller.
Does this change anything? And, yeah, I really wanna see the hdparm
--Istdout too.
Thanks.
--
tejun
---
drivers/ata/libata-core.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: tree0/drivers/ata/libata-core.c
===
--- tree0.orig/drivers/ata/libata-core.c
+++ tree0/drivers/ata/libata-core.c
@@ -1745,7 +1745,7 @@ int ata_dev_read_id(struct ata_device *d
* anything else..
* Some drives were very specific about that exact sequence.
*/
- if (ata_id_major_version(id) < 4 || !ata_id_has_lba(id)) {
+ if (0 && (ata_id_major_version(id) < 4 || !ata_id_has_lba(id))) {
err_mask = ata_dev_init_params(dev, id[3], id[6]);
if (err_mask) {
rc = -EIO;
Re: Problem setting dma using pata_it821x driver in vanilla 2.6.23.12 and gentoo-sources 2.6.23-r3
On Fri, 11 Jan 2008, Tejun Heo, replying to Marc Zuckman wrote: Zuckman: Is it possible that both drivers are present in the kernel and conflict with one another in some fashion? Heo: The don't conflict. If both are in the kernel, IDE drivers have priority. Can you please try the libata driver and see if anything is different? It's different, but the drive array does not function. These are the boot messages with libata and pata_it821x configured. Note that I left the old ide drivers in the kernel for the ICH5 controller. Linux version 2.6.23.12 ([EMAIL PROTECTED]) (gcc version 4.1.1 (Gentoo 4.1.1-r3)) #4 SMP Sat Jan 12 08:43:11 EST 2008 BIOS-provided physical RAM map: BIOS-e820: - 0009f800 (usable) BIOS-e820: 0009f800 - 000a (reserved) BIOS-e820: 000f - 0010 (reserved) BIOS-e820: 0010 - 5fff (usable) BIOS-e820: 5fff - 5fff3000 (ACPI NVS) BIOS-e820: 5fff3000 - 6000 (ACPI data) BIOS-e820: fec0 - 0001 (reserved) 639MB HIGHMEM available. 896MB LOWMEM available. found SMP MP-table at 000f5320 Entering add_active_range(0, 0, 393200) 0 entries of 256 used Zone PFN ranges: DMA 0 -> 4096 Normal 4096 -> 229376 HighMem229376 -> 393200 Movable zone start PFN for each node early_node_map[1] active PFN ranges 0:0 -> 393200 On node 0 totalpages: 393200 DMA zone: 32 pages used for memmap DMA zone: 0 pages reserved DMA zone: 4064 pages, LIFO batch:0 Normal zone: 1760 pages used for memmap Normal zone: 223520 pages, LIFO batch:31 HighMem zone: 1279 pages used for memmap HighMem zone: 162545 pages, LIFO batch:31 Movable zone: 0 pages used for memmap DMI 2.3 present. Intel MultiProcessor Specification v1.4 Virtual Wire compatibility mode. OEM ID: OEM0 Product ID: PROD APIC at: 0xFEE0 Processor #0 15:2 APIC version 17 I/O APIC #2 Version 17 at 0xFEC0. Enabling APIC mode: Flat. Using 1 I/O APICs Processors: 1 Allocating PCI resources starting at 7000 (gap: 6000:9ec0) Built 1 zonelists in Zone order. Total pages: 390129 Kernel command line: BOOT_IMAGE=ata2.6.23 ro root=307 ide0=ata66 ide2=ata66 ide_setup: ide0=ata66 -- OBSOLETE OPTION, WILL BE REMOVED SOON! ide_setup: ide2=ata66 -- OBSOLETE OPTION, WILL BE REMOVED SOON! mapped APIC to b000 (fee0) mapped IOAPIC to a000 (fec0) Enabling fast FPU save and restore... done. Enabling unmasked SIMD FPU exception support... done. Initializing CPU#0 PID hash table entries: 4096 (order: 12, 16384 bytes) Detected 2813.721 MHz processor. Console: colour VGA+ 80x25 console [tty0] enabled Dentry cache hash table entries: 131072 (order: 7, 524288 bytes) Inode-cache hash table entries: 65536 (order: 6, 262144 bytes) Memory: 1552756k/1572800k available (3124k kernel code, 18916k reserved, 1711k data, 256k init, 655296k highmem) virtual kernel memory layout: fixmap : 0xffe1a000 - 0xf000 (1940 kB) pkmap : 0xff80 - 0xffc0 (4096 kB) vmalloc : 0xf880 - 0xff7fe000 ( 111 MB) lowmem : 0xc000 - 0xf800 ( 896 MB) .init : 0xc05c1000 - 0xc0601000 ( 256 kB) .data : 0xc040d15f - 0xc05b8ebc (1711 kB) .text : 0xc010 - 0xc040d15f (3124 kB) Checking if this processor honours the WP bit even in supervisor mode... Ok. Calibrating delay using timer specific routine.. 5631.43 BogoMIPS (lpj=11262875) Mount-cache hash table entries: 512 CPU: After generic identify, caps: bfebfbff 4400 CPU: Trace cache: 12K uops, L1 D cache: 8K CPU: L2 cache: 512K CPU: Physical Processor ID: 0 CPU: After all inits, caps: bfebfbff b080 4400 Intel machine check architecture supported. Intel machine check reporting enabled on CPU#0. CPU0: Intel P4/Xeon Extended MCE MSRs (12) available CPU0: Thermal monitoring enabled Compat vDSO mapped to e000. Checking 'hlt' instruction... OK. SMP alternatives: switching to UP code Freeing SMP alternatives: 17k freed CPU0: Intel(R) Pentium(R) 4 CPU 2.80GHz stepping 09 Total of 1 processors activated (5631.43 BogoMIPS). ExtINT not setup in hardware but reported by MP table ENABLING IO-APIC IRQs ..TIMER: vector=0x31 apic1=0 pin1=2 apic2=0 pin2=0 Brought up 1 CPUs NET: Registered protocol family 16 PCI: PCI BIOS revision 2.10 entry at 0xfb640, last bus=3 PCI: Using configuration type 1 Setting up standard PCI resources SCSI subsystem initialized libata version 2.21 loaded. usbcore: registered new interface driver usbfs usbcore: registered new interface driver hub usbcore: registered new device driver usb PCI: Probing PCI hardware PCI: Probing PCI hardware (bus 00) PCI quirk: region 1000-107f claimed by ICH4 ACPI/GPIO/TCO PCI quirk: region 1080-10bf claimed by ICH4 GPIO PCI: Transparent bridge - :0
Re: Sata 4726 Testing
Andrew Ryder wrote: > Tejun, > > No problem. I should be thanking you and your team for all the hard work > writing the drivers for the hardware.. > > I think this patch might have solved the issue. I did a quick test: warm > rebooted and turned the 4726 on and off 3 times and all disks were > successfully detected each time. I'll do a more thorough test tomorrow > or Sunday. Great. I forwarded the patch for #upstream-fixes as the patch is in the right direction whether it fully fixes your problem or not. Please lemme know the test result. Gaston, for the initial detection failure messages when connected to ICH AHCI, I don't really have much idea what's going on or how to fix it. I suspect it's related to initial FISes from drives arriving asynchronously after hard resets but can't tell much without bus analyzer. Anyways, as EH can handle the case quickly enough, I guess we can leave it at that for now. If you have ideas on what's going on or how to fix it, please lemme know. Thanks. -- tejun - 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 #upstream-fixes] sata_sil24: freeze on non-dev errors reported via CERR
CERR reports errors detected during executing a command. This doesn't
mean the error is tied to the command and can be recovered by just
issuing it again. Many of the errors are fatal port-wide connditions
including HSM violation, host bus error and ATA bus error and require
freezing and port reset.
The freezing part wasn't implemented previously. This used to be okay
because port resets were scheduled anyway and EH eventually resets and
recovers the port. With PMP support added, this is no longer true.
The error condition and recover actions are attributed to the fan-out
port and the host port condition isn't properly recovered leading to
EH failures.
This patch makes CERR errors which require resets to freeze the port.
This will force host port reset and proper recovery.
Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Cc: Andrew Ryder <[EMAIL PROTECTED]>
---
drivers/ata/sata_sil24.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index b4c674d..d84561d 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -1094,10 +1094,13 @@ static void sil24_error_intr(struct ata_port *ap)
if (ci && ci->desc) {
err_mask |= ci->err_mask;
action |= ci->action;
+ if (action & ATA_EH_RESET_MASK)
+ freeze = 1;
ata_ehi_push_desc(ehi, "%s", ci->desc);
} else {
err_mask |= AC_ERR_OTHER;
action |= ATA_EH_SOFTRESET;
+ freeze = 1;
ata_ehi_push_desc(ehi, "unknown command error %d",
cerr);
}
-
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 #upstream-fixes] sata_sil24: fix stupid typo
Fix stupid typo.
Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
drivers/ata/sata_sil24.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index b4c674d..d9c8b32 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -301,7 +301,7 @@ static struct sil24_cerr_info {
[PORT_CERR_PKT_PROT]= { AC_ERR_HSM, ATA_EH_SOFTRESET,
"invalid data directon for ATAPI CDB" },
[PORT_CERR_SGT_BOUNDARY] = { AC_ERR_SYSTEM, ATA_EH_SOFTRESET,
-"SGT no on qword boundary" },
+"SGT not on qword boundary" },
[PORT_CERR_SGT_TGTABRT] = { AC_ERR_HOST_BUS, ATA_EH_SOFTRESET,
"PCI target abort while fetching SGT" },
[PORT_CERR_SGT_MSTABRT] = { AC_ERR_HOST_BUS, ATA_EH_SOFTRESET,
-
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
SATA timeouts on CD DVD RW
I keep getting the following error messages during bootup with kernel 2.6.23.12 : scsi 4:0:0:0: CD-ROMTSSTcorp CDDVDW TS-L633A UO00 PQ: 0 ANSI: 5 ata3.00: qc timeout (cmd 0xa0) ata3.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata3.00: (irq_stat 0x4001) ata3.00: cmd a0/00:00:00:00:20/00:00:00:00:00/a0 tag 0 cdb 0x0 data 0 res 51/60:03:00:00:00/00:00:00:00:00/a0 Emask 0x5 (timeout) ata3: soft resetting port ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300) ata3.00: configured for PIO4 ata3: EH complete The error repeats many times and it takes about 10 minutes for the box to boot. If I unplug the DVD drive the problem naturally disappears. The machine is an Intel SR1530SH. Below is the output from lspci : 00:00.0 Host bridge: Intel Corporation Server DRAM Controller 00:01.0 PCI bridge: Intel Corporation Server Host-Primary PCI Express Bridge 00:19.0 Ethernet controller: Intel Corporation 82566DM-2 Gigabit Network Connection (rev 02) 00:1c.0 PCI bridge: Intel Corporation PCI Express Port 1 (rev 02) 00:1c.4 PCI bridge: Intel Corporation PCI Express Port 5 (rev 02) 00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev 92) 00:1f.0 ISA bridge: Intel Corporation LPC Interface Controller (rev 02) 00:1f.2 SATA controller: Intel Corporation 6 port SATA AHCI Controller (rev 02) 00:1f.3 SMBus: Intel Corporation SMBus Controller (rev 02) 01:00.0 Ethernet controller: Intel Corporation 82571EB Gigabit Ethernet Controller (rev 06) 01:00.1 Ethernet controller: Intel Corporation 82571EB Gigabit Ethernet Controller (rev 06) 03:00.0 VGA compatible controller: Matrox Graphics, Inc. MGA G200e [Pilot] ServerEngines (SEP1) (rev 02) 04:02.0 Ethernet controller: Intel Corporation 82541GI Gigabit Ethernet Controller (rev 05) I've tried switching acpi off, using irqpoll, switching from AHCI to RAID in the BIOS but it makes no difference at all. I also tried kernel 2.6.21.5 (Slackware 12.0) and the problem is still there. Any ideas? Thanks Paul - 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: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM
On Sat, 2008-01-12 at 19:38 -0600, Robert Hancock wrote: > James Bottomley wrote: > > On Sat, 2008-01-12 at 17:04 -0600, Robert Hancock wrote: > >> I don't think the problem is that there's some buffer which is getting > >> allocated above 4GB and never bounced, since the problem goes away if > >> ADMA is disabled entirely and the DMA mask remains 32-bit always. My > >> guess is something is basing its decision on whether to bounce or not on > >> the device DMA mask. That can't possibly work properly for sata_nv since > >> the same PCI device has 2 ports, one of which can be in ADMA mode and > >> 64-bit capable and the other can be in legacy mode and only 32-bit capable. > > > > Erm, well, you can't decouple them. Having a differing blk queue bounce > > mask and device mask is going to cause huge trouble. The reason is this > > insidious nasty called swiotlb. Basically, with it enabled (and again, > > it can be on ia64 or x86_64), the kernel can bypass the bounce limit > > safe in the knowledge that swiotlb will fix up behind in the dma_map_ > > Unfortunately, if the device mask doesn't match the queue mask then > > swiotlb will never kick in and you'll end up with mapped pages beyond > > the 4GB limit. > > Yuck.. All the IOMMU DMA mapping code checks against the device DMA > mask, so it looks like if we get to the point of doing the DMA mapping > on >4GB addresses in libata we're screwed with this approach. > > The key problem is that both ata_ports share the same struct device with > one DMA mask which really doesn't match what this controller wants. I > wonder if we could do a different struct device for each port? > > Other than that, I guess the solutions would be to just set a 32-bit > mask on the device if either port has an ATAPI device connected (which > is fairly ugly, considering that you could do things like hotplug an > ATAPI device when the other port was in use, for example), or do > something to prevent requests from reaching this point with >4GB > addresses in the first place.. Well ... assuming this is the problem (and perhaps we'd better get the traces to confirm) there are at least three possible solutions: 1. As you say, just take the pci device mask down to 32 bits. 2. Find the problematic allocations and add GFP_DMA32 3. set the mask on the actual SCSI device rather than the PCI device and pass that into dma_map_ (this approach would have to get signoff from the arch people; I know it will work on parisc and x86 but I'm not sure about any other arch). > >> Tejun, I believe you had a patch that was printing warnings when libata > >> tried to program a legacy PRD with an address over 4GB. Could we change > >> that to WARN_ON and get someone experiencing this to try it and > >> see what the stack trace points to? > > > > Unfortunately, the stack trace probably won't help, since the command > > likely gets issued from the block request function, so the trace won't > > go back to the culpable initiator; that's why the command would be > > helpful. > > Well, dumping the ATA command surely isn't helpful, as I'm sure it will > be PACKET. I guess we'd have to dump out the actual CDB.. Sorry, when a SCSI person says dump the command, they mean the CDB. James - 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: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM
James Bottomley wrote: On Sat, 2008-01-12 at 17:04 -0600, Robert Hancock wrote: I don't think the problem is that there's some buffer which is getting allocated above 4GB and never bounced, since the problem goes away if ADMA is disabled entirely and the DMA mask remains 32-bit always. My guess is something is basing its decision on whether to bounce or not on the device DMA mask. That can't possibly work properly for sata_nv since the same PCI device has 2 ports, one of which can be in ADMA mode and 64-bit capable and the other can be in legacy mode and only 32-bit capable. Erm, well, you can't decouple them. Having a differing blk queue bounce mask and device mask is going to cause huge trouble. The reason is this insidious nasty called swiotlb. Basically, with it enabled (and again, it can be on ia64 or x86_64), the kernel can bypass the bounce limit safe in the knowledge that swiotlb will fix up behind in the dma_map_ Unfortunately, if the device mask doesn't match the queue mask then swiotlb will never kick in and you'll end up with mapped pages beyond the 4GB limit. Yuck.. All the IOMMU DMA mapping code checks against the device DMA mask, so it looks like if we get to the point of doing the DMA mapping on >4GB addresses in libata we're screwed with this approach. The key problem is that both ata_ports share the same struct device with one DMA mask which really doesn't match what this controller wants. I wonder if we could do a different struct device for each port? Other than that, I guess the solutions would be to just set a 32-bit mask on the device if either port has an ATAPI device connected (which is fairly ugly, considering that you could do things like hotplug an ATAPI device when the other port was in use, for example), or do something to prevent requests from reaching this point with >4GB addresses in the first place.. Tejun, I believe you had a patch that was printing warnings when libata tried to program a legacy PRD with an address over 4GB. Could we change that to WARN_ON and get someone experiencing this to try it and see what the stack trace points to? Unfortunately, the stack trace probably won't help, since the command likely gets issued from the block request function, so the trace won't go back to the culpable initiator; that's why the command would be helpful. Well, dumping the ATA command surely isn't helpful, as I'm sure it will be PACKET. I guess we'd have to dump out the actual CDB.. - 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
SATA timeouts on two disks
Hi, Recently I'm experiencing strange sata errors on my desktop system. The system was recently equipped with three 250 GB SATA drives from three different manufacturers and I'm having an identical problem on two of them. The drives are connected to two on-board controllers on an Asus A8V board, which were both running with Linux for more than two years with older SATA disks without problems. A hardware failure seems unlikely to me as the same error occurrs on two brand new disks from two different manufacturers. I'm running a vanilla 2.6.23.12 kernel. Errror on sdc happened about 10 times tonight, each time I could hear the disk spin down and up again, while the system was frozen for several seconds: ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x18 action 0x2 frozen ata2.00: cmd ea/00:00:00:00:00/00:00:00:00:00/a0 tag 0 cdb 0x0 data 0 res 40/00:00:00:00:40/00:00:00:00:00/00 Emask 0x4 (timeout) ata2: soft resetting port ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300) ata2.00: configured for UDMA/133 ata2: EH complete sd 1:0:0:0: [sdb] 488397168 512-byte hardware sectors (250059 MB) sd 1:0:0:0: [sdb] Write Protect is off sd 1:0:0:0: [sdb] Mode Sense: 00 3a 00 00 sd 1:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA In the log I also found several identical errors on one other drive: ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata5.00: cmd 25/00:08:b7:f2:11/00:00:13:00:00/e0 tag 0 cdb 0x0 data 4096 in res 40/00:01:00:4f:c2/00:00:00:00:00/00 Emask 0x4 (timeout) ata5: soft resetting port ata5.00: configured for UDMA/33 ata5: EH complete sd 4:0:0:0: [sdc] 488397168 512-byte hardware sectors (250059 MB) sd 4:0:0:0: [sdc] Write Protect is off sd 4:0:0:0: [sdc] Mode Sense: 00 3a 00 00 sd 4:0:0:0: [sdc] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA Can this be the result of a hardware failure? I've seen several drives being added to an NCQ blacklist during the last weeks. Is it possible that my drives need to be added here, too? Or have I just two failing drives? Thanks a lot for any clues, Jim System boot log extract: sata_promise :00:08.0: version 2.10 ACPI: PCI Interrupt :00:08.0[A] -> GSI 18 (level, low) -> IRQ 18 scsi0 : sata_promise scsi1 : sata_promise scsi2 : sata_promise ata1: SATA max UDMA/133 cmd 0xf882e200 ctl 0xf882e238 bmdma 0x irq 18 ata2: SATA max UDMA/133 cmd 0xf882e280 ctl 0xf882e2b8 bmdma 0x irq 18 ata3: PATA max UDMA/133 cmd 0xf882e300 ctl 0xf882e338 bmdma 0x irq 18 ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) ata1.00: ATA-8: SAMSUNG HD252KJ, CM100-12, max UDMA7 ata1.00: 488397168 sectors, multi 0: LBA48 NCQ (depth 0/32) ata1.00: configured for UDMA/133 ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300) ata2.00: ATA-7: WDC WD2500JS-55NCB1, 10.02E01, max UDMA/133 ata2.00: 488397168 sectors, multi 0: LBA48 NCQ (depth 0/32) ata2.00: configured for UDMA/133 scsi 0:0:0:0: Direct-Access ATA SAMSUNG HD252KJ CM10 PQ: 0 ANSI: 5 sd 0:0:0:0: [sda] 488397168 512-byte hardware sectors (250059 MB) sd 0:0:0:0: [sda] Write Protect is off sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00 sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sd 0:0:0:0: [sda] 488397168 512-byte hardware sectors (250059 MB) sd 0:0:0:0: [sda] Write Protect is off sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00 sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sda: sda2 sda3 sd 0:0:0:0: [sda] Attached SCSI disk scsi 1:0:0:0: Direct-Access ATA WDC WD2500JS-55N 10.0 PQ: 0 ANSI: 5 sd 1:0:0:0: [sdb] 488397168 512-byte hardware sectors (250059 MB) sd 1:0:0:0: [sdb] Write Protect is off sd 1:0:0:0: [sdb] Mode Sense: 00 3a 00 00 sd 1:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sd 1:0:0:0: [sdb] 488397168 512-byte hardware sectors (250059 MB) sd 1:0:0:0: [sdb] Write Protect is off sd 1:0:0:0: [sdb] Mode Sense: 00 3a 00 00 sd 1:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sdb: sdb2 sdb3 sd 1:0:0:0: [sdb] Attached SCSI disk sata_via :00:0f.0: version 2.3 ACPI: PCI Interrupt :00:0f.0[B] -> GSI 20 (level, low) -> IRQ 17 sata_via :00:0f.0: routed to hard irq line 10 scsi3 : sata_via scsi4 : sata_via ata4: SATA max UDMA/133 cmd 0x0001d000 ctl 0x0001c802 bmdma 0x0001b800 irq 17 ata5: SATA max UDMA/133 cmd 0x0001c400 ctl 0x0001c002 bmdma 0x0001b808 irq 17 ata4: SATA link down 1.5 Gbps (SStatus 0 SControl 300) ata5: SATA link up 1.5 Gbps (SStatus 113 SControl 300) ata5.00: ATA-7: MAXTOR STM3250820AS, 3.AAE, max UDMA/133 ata5.00: 488397168 sectors, multi 16: LBA48 NCQ (depth 0/32) ata5.00: configured for UDMA/133 scsi 4:0:0:0: Direct-Access ATA MAXTOR STM325082 3.AA PQ: 0 ANSI: 5 sd 4:0:0:0: [sdc] 488397168 512-byte hardware sectors (250059 MB) sd 4:0:0:0: [sdc] Write Protect is off sd 4:0:0:0: [sdc] Mode Sense:
Re: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM
On Sat, 2008-01-12 at 17:04 -0600, Robert Hancock wrote: > I don't think the problem is that there's some buffer which is getting > allocated above 4GB and never bounced, since the problem goes away if > ADMA is disabled entirely and the DMA mask remains 32-bit always. My > guess is something is basing its decision on whether to bounce or not on > the device DMA mask. That can't possibly work properly for sata_nv since > the same PCI device has 2 ports, one of which can be in ADMA mode and > 64-bit capable and the other can be in legacy mode and only 32-bit capable. Erm, well, you can't decouple them. Having a differing blk queue bounce mask and device mask is going to cause huge trouble. The reason is this insidious nasty called swiotlb. Basically, with it enabled (and again, it can be on ia64 or x86_64), the kernel can bypass the bounce limit safe in the knowledge that swiotlb will fix up behind in the dma_map_ Unfortunately, if the device mask doesn't match the queue mask then swiotlb will never kick in and you'll end up with mapped pages beyond the 4GB limit. > Tejun, I believe you had a patch that was printing warnings when libata > tried to program a legacy PRD with an address over 4GB. Could we change > that to WARN_ON and get someone experiencing this to try it and > see what the stack trace points to? Unfortunately, the stack trace probably won't help, since the command likely gets issued from the block request function, so the trace won't go back to the culpable initiator; that's why the command would be helpful. James - 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: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM
James Bottomley wrote: With mem<=4098M or sata_nv.adma=0 it still mounts and works ok. As I wrote, it would appear that somehow the blk_queue_bounce_limit setting that the driver has made is not being respected and the block layer is still trying to feed it addresses over 4GB. Any ideas anyone? Actually, I'd be very sceptical that the blk_queue_bounce_limit isn't working as advertised; there'd be a large number of failures if it were not. However, the "as advertised" part doesn't seem to be generally well understood. The point being that block commands are only bounced if they come from the filesystem or the user, not if they're generated directly inside the kernel. Since the fault occurs before mount, it's suggestive of the latter. A long time ago, GFP_KERNEL allocations meant that the memory allocated was physically under 4GB. Then x86_64 (and before it ia64) wanted to break this. So they introduced a new flag: GFP_DMA32 that behaved like the old GFP_KERNEL and then changed GFP_KERNEL on their architectures to return memory from anywhere. I'd strongly suggest some piece of kernel memory was allocated for a transfer buffer without GFP_DMA32 and then passed in to the driver. Unfortunately, that could be anywhere inside cdrom or sr. Knowing what the actual command is might help ... some of the distinctive MMC media ones only have a single source. Just to give some background on what sata_nv is trying to do: There are two sets of static DMA memory allocations that sata_nv does, the legacy ATA PRD and padding buffer which need to be in the lower 4GB, and the ADMA APRD and CPB areas which can be anywhere in 64-bit memory. With this patch, this is done by setting a 32-bit DMA mask before allocating the legacy areas and setting a 64-bit DMA mask before allocating the ADMA areas. Previously the driver just set a 64-bit mask before the legacy PRD got allocated so it could end up above 4GB, in which case legacy DMA couldn't possibly work. That part of the problem appears to be successfully fixed by the patch in question. There's a further problem with runtime DMA mapping, however. Normally when ADMA is enabled the controller can reach anywhere in 64-bit memory. However, if an ATAPI device is connected, since ADMA doesn't work with ATAPI commands we have to switch it off on that port and use legacy DMA, which is limited to 32-bit. This is where the blk_queue_bounce_limit call comes in, it's trying to make the block layer bounce requests above 4GB when legacy DMA is in use. I don't think the problem is that there's some buffer which is getting allocated above 4GB and never bounced, since the problem goes away if ADMA is disabled entirely and the DMA mask remains 32-bit always. My guess is something is basing its decision on whether to bounce or not on the device DMA mask. That can't possibly work properly for sata_nv since the same PCI device has 2 ports, one of which can be in ADMA mode and 64-bit capable and the other can be in legacy mode and only 32-bit capable. Tejun, I believe you had a patch that was printing warnings when libata tried to program a legacy PRD with an address over 4GB. Could we change that to WARN_ON and get someone experiencing this to try it and see what the stack trace points to? - 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 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page
On Saturday 12 January 2008, Borislav Petkov wrote: [...] > > > set_disk_ro(floppy->disk, floppy->wp); > > > - page = (idefloppy_flexible_disk_page_t *) (header + 1); > > > - > > > - page->transfer_rate = be16_to_cpu(page->transfer_rate); > > > - page->sector_size = be16_to_cpu(page->sector_size); > > > - page->cyls = be16_to_cpu(page->cyls); > > > - page->rpm = be16_to_cpu(page->rpm); > > > - capacity = page->cyls * page->heads * page->sectors * page->sector_size; > > > - if (memcmp (page, &floppy->flexible_disk_page, sizeof > > > (idefloppy_flexible_disk_page_t))) > > > + > > > + transfer_rate = be16_to_cpu(*(u16 *)&pc.buffer[8 + 2]); > > > + sector_size = be16_to_cpu(*(u16 *)&pc.buffer[8 + 6]); > > > + cyls = be16_to_cpu(*(u16 *)&pc.buffer[8 + 8]); > > > + rpm = be16_to_cpu(*(u16 *)&pc.buffer[8 + 28]); > > > + heads = pc.buffer[8 + 4]; > > > + sectors = pc.buffer[8 + 5]; > > > + > > > + capacity = cyls * heads * sectors * sector_size; > > > + > > > + if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags) > > > > IDEFLOPPY_MEDIA_CHANGED is set when block device is opened for the first > > time (please check idefloppy_open() for details) so I don't think it is > > the right change. 'Flexible Disk Page' is only 32 bytes so we are better > > off with leaving 'u8 flexible_disk_page[32]' in idefloppy_floppy_t and > > doing things the old way. > > > > Besides please do not intermix real changes like the above one with purely > > cleanup ones like idefloppy_flexible_disk_page_t removal. This is bad from > > maintainability point of view. If some patch causes problems it is easier > > to narrow it down by heaving purely cleanup changes separated out + if we > > would need to revert the real change we would have to make a separate patch > > doing it instead of just reverting the guilty commit (given that we don't > > want cleanup changes to be reverted as well). > > How about we get rid of that chunk altogether? floppy->flexible_disk_page is > used only here for the purpose of printk-ing to the syslog and has no "real" > purpose otherwise. Do we need that info spewed into the syslog at all? Well, it has some debugging value since drive's capabilities are given in 'Flexible Disk Page' but fine with me given that this change is separated out from idefloppy_flexible_disk_page_t removal and pushed at the end of patch series. Thanks, Bart - 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 18/21] ide-floppy: fix error handling in idefloppy_probe()
On Sat, Jan 12, 2008 at 09:18:01PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Friday 11 January 2008, Borislav Petkov wrote:
> > Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> > ---
> > drivers/ide/ide-floppy.c |3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> > index 89b26ea..0729df5 100644
> > --- a/drivers/ide/ide-floppy.c
> > +++ b/drivers/ide/ide-floppy.c
> > @@ -1737,7 +1737,8 @@ static int ide_floppy_probe(ide_drive_t *drive)
> > " emulation.\n", drive->name);
> > goto failed;
> > }
> > - if ((floppy = kzalloc(sizeof (idefloppy_floppy_t), GFP_KERNEL)) ==
> > NULL) {
> > + floppy = kzalloc(sizeof(idefloppy_floppy_t), GFP_KERNEL);
> > + if (!floppy) {
> > printk(KERN_ERR "ide-floppy: %s: Can't allocate a floppy"
> > " structure\n", drive->name);
> > goto failed;
>
> I'm unable to see any problem with error handling here?
I changed it simply because checkpatch.pl complains so:
ERROR: do not use assignment in if condition (+ if ((floppy =
kzalloc(sizeof(idefloppy_floppy_t), GFP_KERNEL)) == NULL))
#1740: FILE: home/boris/tmp/ide-floppy.c:1740:
+ if ((floppy = kzalloc(sizeof (idefloppy_floppy_t), GFP_KERNEL)) == NULL)
{
> This change should be combined with the rest of checkpatch.pl fixes.
ok.
--
Regards/Gruß,
Boris.
-
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 00/21] ide-floppy redux v2
On Sat, Jan 12, 2008 at 09:14:39PM +0100, Bartlomiej Zolnierkiewicz wrote: [...] > PS1 Please rebase the patches still needing polishing on top of updated > IDE quilt tree, recast them and respin the patch series (no need to post > already merged patches). sure, will do. > PS2 what happend to "fix DMA error reporting" patch? (#13 is missing, hmm?) Yeah, this is strange. It seems git-send-email missed some of the patches. I had to send #16,#17 manually but didn't notice #13 was also missing. Will send with the next batch. -- Regards/Gruß, Boris. - 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 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page
[...] > This is not an equivalent transformation: > > header->wp is 0 or 1 > pc.buffer[3] & 0x80 is 0 or 0x80 > > It seems to work fine for ->wp (because it is needlessly defined as 'int') > but may seriously confuse set_disk_ro() and thus bdev_read_only() users. > > Should be fixed to '(pc.buffer[3] & 0x80) ? 1 : 0' (or something similar). upps, sorry, that was silly. I changed it to: floppy->wp = !!(pc.buffer[3] & 0x80); > > set_disk_ro(floppy->disk, floppy->wp); > > - page = (idefloppy_flexible_disk_page_t *) (header + 1); > > - > > - page->transfer_rate = be16_to_cpu(page->transfer_rate); > > - page->sector_size = be16_to_cpu(page->sector_size); > > - page->cyls = be16_to_cpu(page->cyls); > > - page->rpm = be16_to_cpu(page->rpm); > > - capacity = page->cyls * page->heads * page->sectors * page->sector_size; > > - if (memcmp (page, &floppy->flexible_disk_page, sizeof > > (idefloppy_flexible_disk_page_t))) > > + > > + transfer_rate = be16_to_cpu(*(u16 *)&pc.buffer[8 + 2]); > > + sector_size = be16_to_cpu(*(u16 *)&pc.buffer[8 + 6]); > > + cyls = be16_to_cpu(*(u16 *)&pc.buffer[8 + 8]); > > + rpm = be16_to_cpu(*(u16 *)&pc.buffer[8 + 28]); > > + heads = pc.buffer[8 + 4]; > > + sectors = pc.buffer[8 + 5]; > > + > > + capacity = cyls * heads * sectors * sector_size; > > + > > + if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags) > > IDEFLOPPY_MEDIA_CHANGED is set when block device is opened for the first > time (please check idefloppy_open() for details) so I don't think it is > the right change. 'Flexible Disk Page' is only 32 bytes so we are better > off with leaving 'u8 flexible_disk_page[32]' in idefloppy_floppy_t and > doing things the old way. > > Besides please do not intermix real changes like the above one with purely > cleanup ones like idefloppy_flexible_disk_page_t removal. This is bad from > maintainability point of view. If some patch causes problems it is easier > to narrow it down by heaving purely cleanup changes separated out + if we > would need to revert the real change we would have to make a separate patch > doing it instead of just reverting the guilty commit (given that we don't > want cleanup changes to be reverted as well). How about we get rid of that chunk altogether? floppy->flexible_disk_page is used only here for the purpose of printk-ing to the syslog and has no "real" purpose otherwise. Do we need that info spewed into the syslog at all? -- Regards/Gruß, Boris. - 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: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM
On Sat, 2008-01-12 at 13:25 -0600, Robert Hancock wrote: > Alexander wrote: > > Robert Hancock wrote: > >> There's this patch which was intended to fix it: > >> > >> http://lkml.org/lkml/2007/11/22/148 > > > > I applied this patch to 2.6.24-rc7. Now at boot time my DVD-RW is > > normaly detected as: > > > > sr0: scsi3-mmc drive: 48x/48x writer dvd-ram cd/rw xa/form2 cdda tray > > > > But I cannot mount it. All my attempts failed with > > > > ISOFS: Unable to identify CD-ROM format. > > > > With mem<=4098M or sata_nv.adma=0 it still mounts and works ok. > > As I wrote, it would appear that somehow the blk_queue_bounce_limit > setting that the driver has made is not being respected and the block > layer is still trying to feed it addresses over 4GB. Any ideas anyone? Actually, I'd be very sceptical that the blk_queue_bounce_limit isn't working as advertised; there'd be a large number of failures if it were not. However, the "as advertised" part doesn't seem to be generally well understood. The point being that block commands are only bounced if they come from the filesystem or the user, not if they're generated directly inside the kernel. Since the fault occurs before mount, it's suggestive of the latter. A long time ago, GFP_KERNEL allocations meant that the memory allocated was physically under 4GB. Then x86_64 (and before it ia64) wanted to break this. So they introduced a new flag: GFP_DMA32 that behaved like the old GFP_KERNEL and then changed GFP_KERNEL on their architectures to return memory from anywhere. I'd strongly suggest some piece of kernel memory was allocated for a transfer buffer without GFP_DMA32 and then passed in to the driver. Unfortunately, that could be anywhere inside cdrom or sr. Knowing what the actual command is might help ... some of the distinctive MMC media ones only have a single source. James - 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 18/21] ide-floppy: fix error handling in idefloppy_probe()
On Friday 11 January 2008, Borislav Petkov wrote:
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
> drivers/ide/ide-floppy.c |3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 89b26ea..0729df5 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -1737,7 +1737,8 @@ static int ide_floppy_probe(ide_drive_t *drive)
> " emulation.\n", drive->name);
> goto failed;
> }
> - if ((floppy = kzalloc(sizeof (idefloppy_floppy_t), GFP_KERNEL)) ==
> NULL) {
> + floppy = kzalloc(sizeof(idefloppy_floppy_t), GFP_KERNEL);
> + if (!floppy) {
> printk(KERN_ERR "ide-floppy: %s: Can't allocate a floppy"
> " structure\n", drive->name);
> goto failed;
I'm unable to see any problem with error handling here?
This change should be combined with the rest of checkpatch.pl fixes.
-
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 19/21] ide-floppy: fix most of the remaining checkpatch.pl issues
On Friday 11 January 2008, Borislav Petkov wrote:
> i.e.,
> ERROR: switch and case should be at the same indent
> ERROR: need spaces around that '=' (ctx:VxV)
> ERROR: trailing statements should be on next line
> WARNING: no space between function name and open parenthesis '('
> WARNING: printk() should include KERN_ facility level
> ERROR: That open brace { should be on the previous line
> ERROR: use tabs not spaces
> ERROR: do not use assignment in if condition
> WARNING: braces {} are not necessary for single statement blocks
> ERROR: need space after that ',' (ctx:VxV)
> WARNING: line over 80 characters
> ERROR: do not use assignment in if condition
> ...
This should be the very last patch in the series
(and combined with patch #11).
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
> drivers/ide/ide-floppy.c | 147 +++--
> 1 files changed, 75 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 0729df5..3d9b1e5 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -47,13 +47,13 @@
> #define IDEFLOPPY_DEBUG_INFO 0
> #define IDEFLOPPY_DEBUG_BUGS 1
>
> -#define IDEFLOPPY_DEBUG( fmt, args... )
> +#define IDEFLOPPY_DEBUG(fmt, args...)
>
> #if IDEFLOPPY_DEBUG_LOG
> #define debug_log(fmt, args...) \
> printk(KERN_INFO "ide-floppy: " fmt, ## args)
> #else
> -#define debug_log(fmt, args... ) do {} while(0)
> +#define debug_log(fmt, args...) do {} while (0)
> #endif
Hmmm, these could have been dealt with in patch #4...
[...]
> @@ -1314,34 +1314,34 @@ static int idefloppy_identify_device (ide_drive_t
> *drive,struct hd_driveid *id)
> #if IDEFLOPPY_DEBUG_INFO
> printk(KERN_INFO "Dumping ATAPI Identify Device floppy parameters\n");
> switch (gcw.protocol) {
> - case 0: case 1: sprintf(buffer, "ATA");break;
> - case 2: sprintf(buffer, "ATAPI");break;
> - case 3: sprintf(buffer, "Reserved (Unknown to
> ide-floppy)");break;
> + case 0: case 1: sprintf(buffer, "ATA"); break;
> + case 2: sprintf(buffer, "ATAPI"); break;
> + case 3: sprintf(buffer, "Reserved (Unknown to ide-floppy)"); break;
> }
> printk(KERN_INFO "Protocol Type: %s\n", buffer);
> switch (gcw.device_type) {
> - case 0: sprintf(buffer, "Direct-access Device");break;
> - case 1: sprintf(buffer, "Streaming Tape Device");break;
> - case 2: case 3: case 4: sprintf (buffer, "Reserved");break;
> - case 5: sprintf(buffer, "CD-ROM Device");break;
> - case 6: sprintf(buffer, "Reserved");
> - case 7: sprintf(buffer, "Optical memory Device");break;
> - case 0x1f: sprintf(buffer, "Unknown or no Device type");break;
> - default: sprintf(buffer, "Reserved");
> + case 0: sprintf(buffer, "Direct-access Device"); break;
> + case 1: sprintf(buffer, "Streaming Tape Device"); break;
> + case 2: case 3: case 4: sprintf(buffer, "Reserved"); break;
> + case 5: sprintf(buffer, "CD-ROM Device"); break;
> + case 6: sprintf(buffer, "Reserved");
> + case 7: sprintf(buffer, "Optical memory Device"); break;
> + case 0x1f: sprintf(buffer, "Unknown or no Device type"); break;
> + default: sprintf(buffer, "Reserved");
> }
> printk(KERN_INFO "Device Type: %x - %s\n", gcw.device_type, buffer);
> printk(KERN_INFO "Removable: %s\n", gcw.removable ? "Yes":"No");
> switch (gcw.drq_type) {
> - case 0: sprintf(buffer, "Microprocessor DRQ");break;
> - case 1: sprintf(buffer, "Interrupt DRQ");break;
> - case 2: sprintf(buffer, "Accelerated DRQ");break;
> - case 3: sprintf(buffer, "Reserved");break;
> + case 0: sprintf(buffer, "Microprocessor DRQ"); break;
> + case 1: sprintf(buffer, "Interrupt DRQ"); break;
> + case 2: sprintf(buffer, "Accelerated DRQ"); break;
> + case 3: sprintf(buffer, "Reserved"); break;
> }
> printk(KERN_INFO "Command Packet DRQ Type: %s\n", buffer);
> switch (gcw.packet_size) {
> - case 0: sprintf(buffer, "12 bytes");break;
> - case 1: sprintf(buffer, "16 bytes");break;
> - default: sprintf(buffer, "Reserved");break;
> + case 0: sprintf(buffer, "12 bytes"); break;
> + case 1: sprintf(buffer, "16 bytes"); break;
> + default: sprintf(buffer, "Reserved"); break;
> }
> printk(KERN_INFO "Command Packet Size: %s\n", buffer);
> #endif /* IDEFLOPPY_DEBUG_INFO */
> @@ -1349,13 +1349,16 @@ static int idefloppy_identify_device (ide_drive_t
> *drive,struct hd_driveid *id)
> if (gcw.protocol != 2)
> printk(KERN_ERR "ide-floppy: Protocol is not ATAPI\n");
> else if (gcw.device_type != 0)
> - printk(KERN_ERR "ide-floppy: Device type is not set to
> floppy\n");
> + printk(KERN_ERR "ide-floppy: Device typ
Re: [PATCH 14/21] ide-floppy: mv idefloppy_{should_,}report_error
On Friday 11 January 2008, Borislav Petkov wrote:
> In addition to shortening the function name, move the printk-call into the
> function thereby saving some code lines. Also, make the function out_of_line
> since it is not on a performance critical path.
>
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
> drivers/ide/ide-floppy.c | 37 ++---
> 1 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 49d83a1..b718615 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -707,16 +707,18 @@ static ide_startstop_t
> idefloppy_transfer_pc1(ide_drive_t *drive)
> return ide_started;
> }
>
> -/*
> - * Suppresses error messages resulting from Medium not present.
> - */
> -static inline int idefloppy_should_report_error(idefloppy_floppy_t *floppy)
> +static void idefloppy_report_error(idefloppy_floppy_t *floppy,
> + idefloppy_pc_t *pc)
> {
-> Would make a sense to move the comment here instead of removing it
(it is useful unless you remeber all ->{sense_key,asc,ascq} value).
> if (floppy->sense_key == 0x02 &&
> floppy->asc == 0x3a &&
> floppy->ascq == 0x00)
> - return 0;
> - return 1;
> + return;
Otherwise the patch is fine.
-
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 21/21] ide-floppy: remove atomic test_*bit macros
On Friday 11 January 2008, Borislav Petkov wrote:
> This change is temporary and after unification of the IDE subsystem proper
> bit setting and testing macros will be introduced.
>
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
> drivers/ide/ide-floppy.c | 82
> +-
> 1 files changed, 45 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 4106eb4..29c1983 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -479,12 +479,12 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t
> *drive)
>
> debug_log("Reached %s interrupt handler\n", __FUNCTION__);
>
> - if (test_bit(PC_DMA_IN_PROGRESS, &pc->flags)) {
> + if ((1UL << PC_DMA_IN_PROGRESS) & pc->flags) {
How's about introducing new defines i.e.
enum {
IDE_FLOPPY_FLAG_PC_ABORT= (1 << 0),
IDE_FLOPPY_FLAG_PC_DMA_RECOMMENDED = (1 << 1),
IDE_FLOPPY_FLAG_PC_DMA_IN_PROGRESS = (1 << 2),
...
}
instead of open-coding the bit-shifts?
> dma_error = HWIF(drive)->ide_dma_end(drive);
> if (dma_error) {
> printk(KERN_ERR "%s: DMA %s error\n", drive->name,
> write ? "write" : "read");
> - set_bit(PC_DMA_ERROR, &pc->flags);
> + pc->flags |= (1UL << PC_DMA_ERROR);
> } else {
> pc->actually_transferred = pc->request_transfer;
> idefloppy_update_buffers(drive, pc);
> @@ -499,11 +499,11 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t
> *drive)
> /* No more interrupts */
> debug_log("Packet command completed, %d bytes transferred\n",
> pc->actually_transferred);
> - clear_bit(PC_DMA_IN_PROGRESS, &pc->flags);
> + pc->flags &= ((1UL << PC_DMA_IN_PROGRESS) ^ ~0UL);
Same can be achieved with:
pc->flags &= ~(1 << PC_DMA_IN_PROGRESS);
-
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 12/21] ide-floppy: factor out ioctl handlers from idefloppy_ioctl()
On Friday 11 January 2008, Borislav Petkov wrote:
> By passing idefloppy_floppy_t *floppy to the factored out functions, we get
> rid of (almost) all local vars so stack usage should be at minimum here. Also,
> we merge idefloppy_begin_format() into idefloppy_format_start() since it is
> its
> only user.
>
> Also,
> - remove unneeded scsi ioctl chunk
They are _needed_, despite the name these ioctls are _not_ limited
to SCSI subsystem.
[...]
> + int prevent = (arg) ? 1 : 0;
parentheses are unnecessary
[...]
> +static int idefloppy_format_unit(idefloppy_floppy_t *floppy, unsigned long
> arg)
__user tag was dropped from 'arg'
(I bet that this would make sparse checking unhappy)
> +{
> + int blocks, length, flags, err = 0;
> + int __user *argp = (int __user *)arg;
wouldn't be needed if the 'arg' was of 'int __user' type and the casting was
done in the caller function
[...]
> + if (idefloppy_queue_pc_tail(floppy->drive, &pc)) {
> + err = -EIO;
> + goto out;
'goto out' is unnecessary
> + }
> +
> +out:
> + if (err)
> + clear_bit(IDEFLOPPY_FORMAT_IN_PROGRESS, &floppy->flags);
> + return err;
> +}
[...]
> - /*
> - * skip SCSI_IOCTL_SEND_COMMAND (deprecated)
> - * and CDROM_SEND_PACKET (legacy) ioctls
> - */
> - if (cmd != CDROM_SEND_PACKET && cmd != SCSI_IOCTL_SEND_COMMAND)
> - err = scsi_cmd_ioctl(file, bdev->bd_disk->queue,
> - bdev->bd_disk, cmd, argp);
> - else
> - err = -ENOTTY;
> -
> - if (err == -ENOTTY)
> - err = generic_ide_ioctl(drive, file, bdev, cmd, arg);
> -
> - return err;
> + return generic_ide_ioctl(drive, file, bdev, cmd, arg);
this whole chunk needs to be reverted
Please recast/resubmit.
-
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 20/21] ide-floppy: merge idefloppy_{input,output}_buffers
On Friday 11 January 2008, Borislav Petkov wrote:
> We merge idefloppy_{input,output}_buffers() into idefloppy_io_buffers() by
> introducing a 4th arg. called direction. According to its value
> we atapi_input_bytes() or atapi_output_bytes(). Also, simplify the interrupt
This change is fine but ...
> handler by removing multiple calls testing the data direction and using a
> local
> variable instead.
... the patch replaces 'test_bit(PC_WRITING, &pc->flags)' check with
'rq_data_dir(rq) == WRITE' one. While this may look as "trivial" change it
is not such. It should be done only after auditing the driver and making
sure that we are not introducing subtle regressions (=> I see that some
commands are setting PC_WRITING but are not setting REQ_RW bit), especially
given that these changes were not tested with the real hardware.
Please separate this change to another (post-)patch.
PS It would also be nice to remove IDEFLOPPY_DEBUG_BUGS define in a pre-patch.
-
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 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page
On Saturday 12 January 2008, Bartlomiej Zolnierkiewicz wrote: [...] > > - header = (idefloppy_mode_parameter_header_t *) pc.buffer; > > - floppy->wp = header->wp; > > + floppy->wp = pc.buffer[3] & 0x80; > > This is not an equivalent transformation: > > header->wp is 0 or 1 > pc.buffer[3] & 0x80 is 0 or 0x80 > > It seems to work fine for ->wp (because it is needlessly defined as 'int') > but may seriously confuse set_disk_ro() and thus bdev_read_only() users. > > Should be fixed to '(pc.buffer[3] & 0x80) ? 1 : 0' (or something similar). Update: this change belongs to patch #10 (+ the need for such change in patch #6 is a hint that #10 should be before #6) - 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/21] ide-floppy: fix comments formatting
On Friday 11 January 2008, Borislav Petkov wrote: > That is, > - remove unnecessary comments > - shorten comments > - shorten lines longer 80 columns > - cleanup whitespace > - add a missing loglevel KERN_ to a printk-call > - fix misc checkpatch warnings Majority of this patch consists of checkpatch.pl fixes so it should be merged with patch #20. Also a lot of code beautified here is _heavily_ modified in later patches so some of fixups below could be moved there (which would also decrease the size of this patch significantly). > Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]> > --- > drivers/ide/ide-floppy.c | 402 > +- > 1 files changed, 181 insertions(+), 221 deletions(-) [...] > +#define PC_ABORT0 /* Set when an error is > considered\ > +normal - We won't retry */ '\' shouldn't be there and /* ... */ #define PC_ABORT0 would be probably more readable > +#define PC_DMA_RECOMMENDED 2 /* DMA use preferred, if possible */ please make it match the other flags while at it (space -> tab) [...] > -#define IDEFLOPPY_USE_READ12 2 /* Use READ12/WRITE12 or > READ10/WRITE10 */ > +#define IDEFLOPPY_USE_READ12 2 /* READ(10|12)/WRITE(10|12) */ The original comment was way more informative. Moreover this particular flag is never set so it could be just removed (together with some dead code for handling it) in a separate (pre-)patch. > #define IDEFLOPPY_FORMAT_IN_PROGRESS3 /* Format in progress */ please make it match the other flags while at it (tab -> space) > -#define IDEFLOPPY_CLIK_DRIVE 4 /* Avoid commands not supported > in Clik drive */ > -#define IDEFLOPPY_ZIP_DRIVE 5 /* Requires BH algorithm for > packets */ > +#define IDEFLOPPY_CLIK_DRIVE 4 /* Avoid commands not supported\ > +in Clik drive */ > +#define IDEFLOPPY_ZIP_DRIVE 5 /* Requires BH algorithm for\ > +packets */ no need for '\' characters [...] > -static void idefloppy_queue_pc_head (ide_drive_t *drive,idefloppy_pc_t > *pc,struct request *rq) > +static void idefloppy_queue_pc_head(ide_drive_t *drive, idefloppy_pc_t *pc, > + struct request *rq) minor CodingStyle nitpick: static void idefloppy_queue_pc_head(ide_drive_t *drive, idefloppy_pc_t *pc, struct request *rq) would be more readable IMO but it is a matter of personal taste [ same applies for other similar modifications done by this patch series ] - 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 00/21] ide-floppy redux v2
Hi, On Saturday 12 January 2008, Bartlomiej Zolnierkiewicz wrote: > On Friday 11 January 2008, Borislav Petkov wrote: > > > > Hi Bart, > > > >here's the second version of the ide-floppy refactoring trail. All the > > patches are based on the version of your quilt tree from the 05.01. Also, > > you've > > already applied patch 5 in this series but i'm submitting it still for the > > sake > > of completeness. > > > > > > drivers/ide/ide-cd.c |2 - > > drivers/ide/ide-floppy.c | 1403 > > ++ > > include/linux/cdrom.h|1 + > > include/linux/ide.h |3 + > > 4 files changed, 558 insertions(+), 851 deletions(-) > > applied patches #1-4, #8 > (#4 with some changes) > > #5 was already applied so I just re-ordered it into the right spot > > I have some comments for #6-7 applied #9, #15 and #17 some comments for #11-12, #14 and #18-21 in separate mails #10 and #16 are fine but because they depend on unmerged patches I'm unable to apply them currently Overall: good job! 300 LOC removed from the driver, code size savings and a lot of preparations for the future ATAPI handling unification. :) Thanks, Bart PS1 Please rebase the patches still needing polishing on top of updated IDE quilt tree, recast them and respin the patch series (no need to post already merged patches). PS2 what happend to "fix DMA error reporting" patch? (#13 is missing, hmm?) - 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/8] ide: add ->cable_detect method to ide_hwif_t
Hello.
Bartlomiej Zolnierkiewicz wrote:
* Add ->cable_detect method to ide_hwif_t.
* Call the new method in ide_init_port() if:
- the host supports UDMA modes > UDMA2 ('hwif->ultra_mask & 78')
- DMA initialization was successful (if hwif->dma_base is not set
ide_init_port() sets hwif->ultra_mask to zero)
- "idex=ata66" is not used ('hwif->cbl != ATA_CBL_PATA40_SHORT')
* Convert PCI host drivers to use ->cable_detect method.
While at it:
* Factor out cable detection to separate functions (if not already done).
* hpt366.c/it8213.c/slc90e66.c:
- don't check cable type if "idex=ata66" is used
* pdc202xx_new.c:
- add __devinit tag to pdcnew_cable_detect()
* pdc202xx_old.c:
- rename pdc202xx_old_cable_detect() to pdc2026x_old_cable_detect()
- add __devinit tag to pdc2026x_old_cable_detect()
Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]>
Index: b/drivers/ide/ide-probe.c
===
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1343,6 +1343,11 @@ static void ide_init_port(ide_hwif_t *hw
/* call chipset specific routine for each enabled port */
if (d->init_hwif)
d->init_hwif(hwif);
+
+ if (hwif->cable_detect && (hwif->ultra_mask & 0x78)) {
+ if (hwif->cbl != ATA_CBL_PATA40_SHORT)
+ hwif->cbl = hwif->cable_detect(hwif);
+ }
Could be collapsed to a single *if* statement...
Index: b/drivers/ide/pci/alim15x3.c
===
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -666,13 +666,12 @@ static void __devinit init_hwif_common_a
hwif->set_dma_mode = &ali_set_dma_mode;
hwif->udma_filter = &ali_udma_filter;
+ hwif->cable_detect = ata66_ali15x3;
Why not give that function a "standard" name while at it?
static const struct ide_port_info atiixp_pci_info[] __devinitdata = {
Index: b/drivers/ide/pci/cmd64x.c
===
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -393,6 +393,8 @@ static void __devinit init_hwif_cmd64x(i
hwif->set_pio_mode = &cmd64x_set_pio_mode;
hwif->set_dma_mode = &cmd64x_set_dma_mode;
+ hwif->cable_detect = ata66_cmd64x;
+
Same question...
Index: b/drivers/ide/pci/hpt366.c
===
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -1279,12 +1279,55 @@ static unsigned int __devinit init_chips
return dev->irq;
}
+static u8 __devinit hpt3xx_cable_detect(ide_hwif_t *hwif)
+{
+ struct pci_dev *dev= to_pci_dev(hwif->dev);
+ struct hpt_info *info = pci_get_drvdata(dev);
+ u8 chip_type= info->chip_type;
+ u8 scr1 = 0, ata66 = hwif->channel ? 0x01 : 0x02;
The 'ata66' is pretty bad name for this variable (reversed sense), let's
go with 'mask'...
+
+ /*
+* The HPT37x uses the CBLID pins as outputs for MA15/MA16
+* address lines to access an external EEPROM. To read valid
+* cable detect state the pins must be enabled as inputs.
+*/
+ if (chip_type == HPT374 && (PCI_FUNC(dev->devfn) & 1)) {
+ /*
+* HPT374 PCI function 1
+* - set bit 15 of reg 0x52 to enable TCBLID as input
+* - set bit 15 of reg 0x56 to enable FCBLID as input
+*/
+ u8 mcr_addr = hwif->select_data + 2;
+ u16 mcr;
+
+ pci_read_config_word(dev, mcr_addr, &mcr);
+ pci_write_config_word(dev, mcr_addr, (mcr | 0x8000));
+ /* now read cable id register */
+ pci_read_config_byte(dev, 0x5a, &scr1);
+ pci_write_config_word(dev, mcr_addr, mcr);
+ } else if (chip_type >= HPT370) {
+ /*
+* HPT370/372 and 374 pcifn 0
+* - clear bit 0 of reg 0x5b to enable P/SCBLID as inputs
+*/
+ u8 scr2 = 0;
+
+ pci_read_config_byte(dev, 0x5b, &scr2);
+ pci_write_config_byte(dev, 0x5b, (scr2 & ~1));
+ /* now read cable id register */
+ pci_read_config_byte(dev, 0x5a, &scr1);
+ pci_write_config_byte(dev, 0x5b, scr2);
Sigh, my pretty formatting is gone... at least don't leave double spaces
and needless parens. :-)
+ } else
+ pci_read_config_byte(dev, 0x5a, &scr1);
+
+ return (scr1 & ata66) ? ATA_CBL_PATA40 : ATA_CBL_PATA80;
+}
+
[...]
--- a/drivers/ide/pci/it8213.c
+++ b/drivers/ide/pci/it8213.c
Index: b/drivers/ide/pci/it821x.c
===
--- a/drivers/ide/pci/it821x.c
+++ b/drivers/ide/pci/it821x.c
@@ -579,14 +579,13 @@ static void __devinit init_hwif_it821x(i
Re: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM
Alexander wrote: Robert Hancock wrote: There's this patch which was intended to fix it: http://lkml.org/lkml/2007/11/22/148 I applied this patch to 2.6.24-rc7. Now at boot time my DVD-RW is normaly detected as: sr0: scsi3-mmc drive: 48x/48x writer dvd-ram cd/rw xa/form2 cdda tray But I cannot mount it. All my attempts failed with ISOFS: Unable to identify CD-ROM format. With mem<=4098M or sata_nv.adma=0 it still mounts and works ok. As I wrote, it would appear that somehow the blk_queue_bounce_limit setting that the driver has made is not being respected and the block layer is still trying to feed it addresses over 4GB. Any ideas anyone? - 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
SATA port multiplier(Sil 3132) and JMicron 361-363
My SII 3132 port multiplier works with the JMB363 controller on my motherboard using kernel-2.6.24-rc3 but not with the JMB361 esata controller which i have on a second motherboard. I get messages: ata5.15: failed to read PMP GSCR[1] (Emask=0x4) ahci :00:12.0: controller can't do PMP, turning off CAP_PMP The motherboard maker (ECS) and JMicron data claim that JMB361 has a port multiplier. Is there any chance this controller can be made to work in linux? I am willing to test patches. - 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: 40-wire cable detected when directly connected
Hi
Bartlomiej Zolnierkiewicz schrieb:
As a workaround you can try using IDE subsystem siimage driver and pass
"idex=ata66" option or modify Tejun's patch to also override device side
cable detection by replacing ATA_CBL_PATA80 with ATA_CBL_PATA40_SHORT.
I changed some code in libata=core.c in ata_dev_xfermask (see patch2),
that xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA) isn't set, when forcing 80c.
Everything seems to work now. I have a about 41 MB/s while reading.
Regards
Tobias
dmesg says
[ 40.854101] ata7: PATA max UDMA/133 irq 16
[ 40.854162] ata8: PATA max UDMA/133 irq 16
[ 41.008881] ata7: forcing 80c
[ 41.008949] ata7.00: ATA-4: SanDisk SDCFX4-8192, HDX 4.20, max UDMA/66
[ 41.009014] ata7.00: 16007040 sectors, multi 0: LBA
[ 41.009751] ata7.00: configured for UDMA/66
[ 41.168606] ata8: forcing 80c
[ 41.168673] ata8.00: ATA-4: SanDisk SDCFX3-2048, HDX 4.08, max MWDMA2
[ 41.168738] ata8.00: 4001760 sectors, multi 0: LBA
[ 41.188267] ata8.00: configured for MWDMA2
[ 41.188418] scsi 6:0:0:0: Direct-Access ATA SanDisk SDCFX4-8
HDX PQ: 0 ANSI: 5
[ 41.188622] sd 6:0:0:0: [sdc] 16007040 512-byte hardware sectors
(8196 MB)
[ 41.188696] sd 6:0:0:0: [sdc] Write Protect is off
[ 41.188759] sd 6:0:0:0: [sdc] Mode Sense: 00 3a 00 00
[ 41.188776] sd 6:0:0:0: [sdc] Write cache: disabled, read cache:
enabled, doesn't support DPO or FUA
[ 41.188918] sd 6:0:0:0: [sdc] 16007040 512-byte hardware sectors
(8196 MB)
[ 41.188990] sd 6:0:0:0: [sdc] Write Protect is off
[ 41.189054] sd 6:0:0:0: [sdc] Mode Sense: 00 3a 00 00
[ 41.189070] sd 6:0:0:0: [sdc] Write cache: disabled, read cache:
enabled, doesn't support DPO or FUA
[ 41.189167] sdc: sdc1 sdc2
[ 41.189880] sd 6:0:0:0: [sdc] Attached SCSI disk
[ 41.190092] scsi 7:0:0:0: Direct-Access ATA SanDisk SDCFX3-2
HDX PQ: 0 ANSI: 5
[ 41.190283] sd 7:0:0:0: [sdd] 4001760 512-byte hardware sectors (2049 MB)
[ 41.190357] sd 7:0:0:0: [sdd] Write Protect is off
[ 41.190419] sd 7:0:0:0: [sdd] Mode Sense: 00 3a 00 00
[ 41.190436] sd 7:0:0:0: [sdd] Write cache: disabled, read cache:
enabled, doesn't support DPO or FUA
[ 41.190567] sd 7:0:0:0: [sdd] 4001760 512-byte hardware sectors (2049 MB)
[ 41.190640] sd 7:0:0:0: [sdd] Write Protect is off
[ 41.190702] sd 7:0:0:0: [sdd] Mode Sense: 00 3a 00 00
[ 41.190719] sd 7:0:0:0: [sdd] Write cache: disabled, read cache:
enabled, doesn't support DPO or FUA
[ 41.190815] sdd: sdd1
[ 41.191733] sd 7:0:0:0: [sdd] Attached SCSI disk
hdparm -I /dev/sda
[...]
Capabilities:
LBA, IORDY(may be)(cannot be disabled)
Standby timer values: spec'd by Vendor
R/W multiple sector transfer: Max = 4 Current = 0
DMA: mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 *udma4
Cycle time: min=120ns recommended=120ns
PIO: pio0 pio1 pio2 pio3 pio4
Cycle time: no flow control=120ns IORDY flow control=120ns
[...]
dd if=/dev/sdc of=/dev/null bs=1M count=16 iflag=direct
16+0 Datensätze ein
16+0 Datensätze aus
16777216 Bytes (17 MB) kopiert, 0,402367 s, 41,7 MB/s
diff --git a/linux-2.6.24-rc7/drivers/ata/libata-core.c
b/linux-2.6.24-rc7-twam/drivers/ata/libata-core.c
index 4753a18..ac5a0d4 100644
--- a/linux-2.6.24-rc7/drivers/ata/libata-core.c
+++ b/linux-2.6.24-rc7-twam/drivers/ata/libata-core.c
@@ -119,6 +119,10 @@ int libata_noacpi = 0;
module_param_named(noacpi, libata_noacpi, int, 0444);
MODULE_PARM_DESC(noacpi, "Disables the use of ACPI in probe/suspend/resume
when set");
+int libata_force_cbl = 0;
+module_param_named(force_cbl, libata_force_cbl, int, 0644);
+MODULE_PARM_DESC(force_cbl, "force PATA cable type (0=keep, 40=40c, 80=80c)");
+
MODULE_AUTHOR("Jeff Garzik");
MODULE_DESCRIPTION("Library module for ATA devices");
MODULE_LICENSE("GPL");
@@ -4318,16 +4322,29 @@ static void ata_dev_xfermask(struct ata_device *dev)
* drive side as well. Cases where we know a 40wire cable
* is used safely for 80 are not checked here.
*/
- if (xfer_mask & (0xF8 << ATA_SHIFT_UDMA))
- /* UDMA/44 or higher would be available */
- if ((ap->cbl == ATA_CBL_PATA40) ||
- (ata_is_40wire(dev) &&
- (ap->cbl == ATA_CBL_PATA_UNK ||
-ap->cbl == ATA_CBL_PATA80))) {
- ata_dev_printk(dev, KERN_WARNING,
-"limited to UDMA/33 due to 40-wire cable\n");
- xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);
+ if (xfer_mask & (0xF8 << ATA_SHIFT_UDMA)) {
+switch (libata_force_cbl) {
+case 40:
+ /* limit to UDMA/33 */
+ata_dev_printk(dev, KERN_INFO, "forcing
40c\n");
+ xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);
+ break;
+case 80:
+ /*
Re: 40-wire cable detected when directly connected
Hi Bartlomiej Zolnierkiewicz schrieb: As a workaround you can try using IDE subsystem siimage driver and pass "idex=ata66" option or modify Tejun's patch to also override device side cable detection by replacing ATA_CBL_PATA80 with ATA_CBL_PATA40_SHORT. I'll try this. Could you send the output of 'lspci -vvv -xxx' command? 03:04.0 RAID bus controller: Silicon Image, Inc. PCI0680 Ultra ATA-133 Host Controller (rev 02) Subsystem: Silicon Image, Inc. Winic W-680 (Silicon Image 680 based) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 64, Cache Line Size: 4 bytes Interrupt: pin A routed to IRQ 16 Region 0: I/O ports at df00 [size=8] Region 1: I/O ports at de00 [size=4] Region 2: I/O ports at dd00 [size=8] Region 3: I/O ports at dc00 [size=4] Region 4: I/O ports at db00 [size=16] Region 5: Memory at fdcff000 (32-bit, non-prefetchable) [size=256] [virtual] Expansion ROM at fdb0 [disabled] [size=512K] Capabilities: [60] Power Management version 2 Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-) Status: D0 PME-Enable- DSel=0 DScale=2 PME- Kernel driver in use: pata_sil680 00: 95 10 80 06 07 00 90 02 02 00 04 01 01 40 00 00 10: 01 df 00 00 01 de 00 00 01 dd 00 00 01 dc 00 00 20: 01 db 00 00 00 f0 cf fd 00 00 00 00 95 10 80 36 30: 00 00 00 00 60 00 00 00 00 00 00 00 0f 01 00 00 40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 60: 01 00 22 06 00 40 00 64 00 00 00 00 00 00 00 00 70: 08 00 00 00 00 60 1f 7c 08 00 00 00 00 30 1f 7c 80: 03 00 00 00 02 00 00 00 00 00 11 00 09 19 22 51 90: 00 fe 00 0d ff ff ff 3b 00 00 00 19 00 00 00 00 a0: 01 62 c1 10 c1 10 8a 32 c1 10 92 43 07 40 09 40 b0: 01 62 c1 10 c1 10 8a 32 c1 10 92 43 00 40 09 40 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 Regards Tobias smime.p7s Description: S/MIME Cryptographic Signature
Re: 40-wire cable detected when directly connected
On Saturday 12 January 2008, Bartlomiej Zolnierkiewicz wrote: [...] > I've re-read the whole thread and it seems that the possible solution for > Addonics card would be to detect it by PCI Subsystem Vendor/Device IDs. It seems I wasn't paying enough attention, Tejun already thought of this but unfortunately Addonics didn't set custom Subsystem IDs. > Could you send the output of 'lspci -vvv -xxx' command? Still may be useful. Bart - 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: 40-wire cable detected when directly connected
On Saturday 12 January 2008, Tobias Müller wrote: > Bartlomiej Zolnierkiewicz schrieb: > > Please send 'hdparm --Istdout /dev/hdc' output. > > /dev/sdc: [...] Thanks, device has no cable detection (no surprise here, it is a CF card) so over-riding only host side cable detection won't work. As a workaround you can try using IDE subsystem siimage driver and pass "idex=ata66" option or modify Tejun's patch to also override device side cable detection by replacing ATA_CBL_PATA80 with ATA_CBL_PATA40_SHORT. I've re-read the whole thread and it seems that the possible solution for Addonics card would be to detect it by PCI Subsystem Vendor/Device IDs. Could you send the output of 'lspci -vvv -xxx' command? Bart - 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: Problem setting dma using pata_it821x driver in vanilla 2.6.23.12 and gentoo-sources 2.6.23-r3
On Thursday 10 January 2008, Tejun Heo wrote: > Marc Howard Zuckman wrote: > > With apologies if this was supposed to be an attachment: > > Inline is fine. > > > IT8212: IDE controller at PCI slot :03:0c.0 > > IT8212: chipset revision 17 > > it821x: controller in smart mode. > > IT8212: 100% native mode on irq 21 > > ide2: BM-DMA at 0xb800-0xb807, BIOS settings: hde:DMA, hdf:pio > > ide3: BM-DMA at 0xb808-0xb80f, BIOS settings: hdg:pio, hdh:pio > > Probing IDE interface ide2... > > hde: Integrated Technology Express Inc, ATA DISK drive > > hde: IT8212 RAID 1 volume. > > hde: selected mode 0x0 > > ide2 at 0xa810-0xa817,0xac02 on irq 21 > > Probing IDE interface ide3... > > Probing IDE interface ide3... > > Probing IDE interface ide4... > > Probing IDE interface ide5... > > hda: max request size: 128KiB > > hda: Host Protected Area detected. > > current capacity is 195369455 sectors (100029 MB) > > native capacity is 195371568 sectors (100030 MB) > > hda: Host Protected Area disabled. > > hda: 195371568 sectors (100030 MB) w/2048KiB Cache, CHS=65535/16/63, > > UDMA(100) > > hda: cache flushes not supported > > hda: hda1 hda2 hda3 < hda5 hda6 hda7 > > > Oh... I thought you were using libata drivers because you mentioned > pata_it821x on the subject. The IDE driver supports 'hdparm -d 1' fine. > I wonder why that's not working. Also, the driver is configuring your > disk to PIO0. In "smart mode" controller takes care of mode programming. > Cc'ing Alan and Bartlomiej. Guys, is this the smart mode problem? Do > ide and libata have this fixed in 2.6.24-rc? [ added Alan and myself ;) to cc: ] Possibly. Marc, please also send output of 'hdparm --Istdout /dev/hde' command. Thanks, Bart - 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: 40-wire cable detected when directly connected
Bartlomiej Zolnierkiewicz schrieb:
Please send 'hdparm --Istdout /dev/hdc' output.
/dev/sdc:
044a 3e08 0010 0240 003f 00f4
3f80 2020 2020 3031 3036 3131 4532
3239 3753 3035 3130 0002 0002 0004 4844
5820 342e 3230 5361 6e44 6973 6b20 5344
4346 5834 2d38 3139 3220 2020 2020 2020
2020 2020 2020 2020 2020 2020 2020 0004
0300 0200 0007 3e08 0010
003f 3f80 00f4 0100 3f80 00f4 0007
0003 0078 0078 0078 0078
0010 0020 4004 4000 0004 4000
041f
0082
smime.p7s
Description: S/MIME Cryptographic Signature
Re: 40-wire cable detected when directly connected
On Friday 11 January 2008, Tobias Müller wrote: > Tejun Heo schrieb: > > I don't know very well about CF but does it even fill UDMA/33? What > > does 'dd if=/dev/sdc of=/dev/null bs=1M count=16 iflag=direct' say? You > > can increase count for more reliable result. > > 16+0 Datensätze ein > 16+0 Datensätze aus > 16777216 Bytes (17 MB) kopiert, 0,561688 s, 29,9 MB/s > > And hdparm -I /dev/sdc says it should be compatible to udma5. [...] Please send 'hdparm --Istdout /dev/hdc' output. Thanks, Bart - 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 07/21] ide-floppy: remove struct idefloppy_capacity_descriptor
On Friday 11 January 2008, Borislav Petkov wrote:
> We test here for updated capacity descriptors by checking whether the media
> has changed instead of memcmp-ing with a cached copy of the capacity
> descriptors.
>
> Also:
>
> - remove one of 2 consecutive if (!i)-tests.
> - start loop at 1 in idefloppy_get_format_capacities() since userspace doesn't
> need the current/maximum capacity descriptor. i.e the first one.
> - fix comments formatting
>
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
> drivers/ide/ide-floppy.c | 132 +
> 1 files changed, 50 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 679d48e..5c85833 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -119,29 +119,7 @@ typedef struct idefloppy_packet_command_s {
>
> #define PC_SUPPRESS_ERROR 6 /* Suppress error
> reporting */
>
> -/*
> - * Format capacity
> - */
> -typedef struct {
> - u8 reserved[3];
> - u8 length; /* Length of the following
> descriptors in bytes */
> -} idefloppy_capacity_header_t;
minor nitpick:
idefloppy_capacity_header_t removal wasn't mentioned in the patch description
[...]
> @@ -1229,17 +1205,16 @@ static int idefloppy_get_sfrp_bit(ide_drive_t *drive)
> }
>
> /*
> - * Determine if a media is present in the floppy drive, and if so,
> - * its LBA capacity.
> + * Determine if a media is present in the floppy drive, and if so, its LBA
> + * capacity.
> */
> -static int idefloppy_get_capacity (ide_drive_t *drive)
> +static int idefloppy_get_capacity(ide_drive_t *drive)
Care to rename it to ide_floppy_get_capacity() while at it
(it has only two users)?
> {
> idefloppy_floppy_t *floppy = drive->driver_data;
> idefloppy_pc_t pc;
> - idefloppy_capacity_header_t *header;
> - idefloppy_capacity_descriptor_t *descriptor;
> - int i, descriptors, rc = 1, blocks, length;
> -
> + int i, desc_cnt, rc = 1, blocks, length;
> + u8 header_len;
desc_cnt (== header_len / 8) can be u8 as well
> drive->bios_cyl = 0;
> drive->bios_head = drive->bios_sect = 0;
> floppy->blocks = 0;
> @@ -1251,17 +1226,17 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
> printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
> return 1;
> }
> - header = (idefloppy_capacity_header_t *) pc.buffer;
> - descriptors = header->length / sizeof(idefloppy_capacity_descriptor_t);
> - descriptor = (idefloppy_capacity_descriptor_t *) (header + 1);
> + header_len = pc.buffer[3];
> + desc_cnt = header_len / 8; /* capacity descriptor of 8 bytes */
>
> - for (i = 0; i < descriptors; i++, descriptor++) {
> - blocks = descriptor->blocks = be32_to_cpu(descriptor->blocks);
> - length = descriptor->length = be16_to_cpu(descriptor->length);
> + for (i = 0; i < desc_cnt; i++) {
> + unsigned int desc_start = 4 + i*8;
missing newline
> + blocks = be32_to_cpu(*(u32 *)&pc.buffer[desc_start]);
> + length = be16_to_cpu(*(u16 *)&pc.buffer[desc_start + 6]);
if the last debug_log() call in the loop will be moved here
> - if (!i)
> + if (!i)
> {
the 'if (!i)' can be replaced with:
if (i)
continue;
> - switch (descriptor->dc) {
> + switch (pc.buffer[desc_start + 4] & 0x03) {
> /* Clik! drive returns this instead of CAPACITY_CURRENT */
> case CAPACITY_UNFORMATTED:
> if (!test_bit(IDEFLOPPY_CLIK_DRIVE, &floppy->flags))
> @@ -1272,11 +1247,11 @@ static int idefloppy_get_capacity (ide_drive_t *drive)
> break;
> case CAPACITY_CURRENT:
> /* Normal Zip/LS-120 disks */
> - if (memcmp(descriptor, &floppy->capacity, sizeof
> (idefloppy_capacity_descriptor_t)))
> + if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags)
Same issue as with similar change in patch #6:
[ Please note that idefloppy_get_capacity() is called from idefloppy_setup()
and IDEFLOPPY_MEDIA_CHANGED flag is first set in idefloppy_open(). ]
IMO it is the best to leave floppy->capacity alone for now.
> printk(KERN_INFO "%s: %dkB, %d blocks, %d "
> "sector size\n", drive->name,
> blocks * length / 1024, blocks, length);
> - floppy->capacity = *descriptor;
> +
> if (!length || length % 512) {
> printk(KERN_NOTICE "%s: %d bytes block size "
> "not supported\n", drive->name, length);
> @@ -1303,9 +1278,8 @@ static int ideflop
Re: [PATCH 00/21] ide-floppy redux v2
On Friday 11 January 2008, Borislav Petkov wrote: > > Hi Bart, > >here's the second version of the ide-floppy refactoring trail. All the > patches are based on the version of your quilt tree from the 05.01. Also, > you've > already applied patch 5 in this series but i'm submitting it still for the > sake > of completeness. > > > drivers/ide/ide-cd.c |2 - > drivers/ide/ide-floppy.c | 1403 > ++ > include/linux/cdrom.h|1 + > include/linux/ide.h |3 + > 4 files changed, 558 insertions(+), 851 deletions(-) applied patches #1-4, #8 (#4 with some changes) #5 was already applied so I just re-ordered it into the right spot I have some comments for #6-7 - 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 06/21] ide-floppy: remove struct idefloppy_flexible_disk_page
On Friday 11 January 2008, Borislav Petkov wrote:
> The driver used to test whether the flexible disk page has changed by
> memcmp-ing
> it with a cached copy of a previous version of the page from a different remo-
> vable medium. Since, according to the SFF-8070i spec, the flexible disk page
> "specifies parameters relating to the currently installed medium type," this
> comparison is now done by simply checking whether the medium has changed.
>
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
> drivers/ide/ide-floppy.c | 89 -
> 1 files changed, 32 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 2b9885f..679d48e 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
[...]
> @@ -1188,50 +1159,54 @@ static int idefloppy_queue_pc_tail (ide_drive_t
> *drive,idefloppy_pc_t *pc)
> }
>
> /*
> - * Look at the flexible disk page parameters. We will ignore the CHS
> - * capacity parameters and use the LBA parameters instead.
> + * Look at the flexible disk page parameters. We will ignore the CHS capacity
> + * parameters and use the LBA parameters instead.
> */
> -static int idefloppy_get_flexible_disk_page (ide_drive_t *drive)
> +static int idefloppy_get_flexible_disk_page(ide_drive_t *drive)
Care to rename it to ide_floppy_get_flexible_disk_page() while at it
(it has only one user)?
> {
> idefloppy_floppy_t *floppy = drive->driver_data;
> idefloppy_pc_t pc;
> - idefloppy_mode_parameter_header_t *header;
> - idefloppy_flexible_disk_page_t *page;
> int capacity, lba_capacity;
> + u8 heads, sectors;
> + u16 transfer_rate, sector_size, cyls, rpm;
some CodingStyle nitpicks (not really that important, rather personal taste):
u16 transfer_rate, sector_size, cyls, rpm;
u8 heads, sectors;
> - idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE,
> MODE_SENSE_CURRENT);
> - if (idefloppy_queue_pc_tail(drive,&pc)) {
> - printk(KERN_ERR "ide-floppy: Can't get flexible disk "
> - "page parameters\n");
> + idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE,
> + MODE_SENSE_CURRENT);
idefloppy_create_mode_sense_cmd(&pc, IDEFLOPPY_FLEXIBLE_DISK_PAGE,
MODE_SENSE_CURRENT);
> + if (idefloppy_queue_pc_tail(drive, &pc)) {
> + printk(KERN_ERR "ide-floppy: Can't get flexible disk page"
> + " parameters\n");
> return 1;
> }
> - header = (idefloppy_mode_parameter_header_t *) pc.buffer;
> - floppy->wp = header->wp;
> + floppy->wp = pc.buffer[3] & 0x80;
This is not an equivalent transformation:
header->wp is 0 or 1
pc.buffer[3] & 0x80 is 0 or 0x80
It seems to work fine for ->wp (because it is needlessly defined as 'int')
but may seriously confuse set_disk_ro() and thus bdev_read_only() users.
Should be fixed to '(pc.buffer[3] & 0x80) ? 1 : 0' (or something similar).
> set_disk_ro(floppy->disk, floppy->wp);
> - page = (idefloppy_flexible_disk_page_t *) (header + 1);
> -
> - page->transfer_rate = be16_to_cpu(page->transfer_rate);
> - page->sector_size = be16_to_cpu(page->sector_size);
> - page->cyls = be16_to_cpu(page->cyls);
> - page->rpm = be16_to_cpu(page->rpm);
> - capacity = page->cyls * page->heads * page->sectors * page->sector_size;
> - if (memcmp (page, &floppy->flexible_disk_page, sizeof
> (idefloppy_flexible_disk_page_t)))
> +
> + transfer_rate = be16_to_cpu(*(u16 *)&pc.buffer[8 + 2]);
> + sector_size = be16_to_cpu(*(u16 *)&pc.buffer[8 + 6]);
> + cyls = be16_to_cpu(*(u16 *)&pc.buffer[8 + 8]);
> + rpm = be16_to_cpu(*(u16 *)&pc.buffer[8 + 28]);
> + heads = pc.buffer[8 + 4];
> + sectors = pc.buffer[8 + 5];
> +
> + capacity = cyls * heads * sectors * sector_size;
> +
> + if ((1UL << IDEFLOPPY_MEDIA_CHANGED) & floppy->flags)
IDEFLOPPY_MEDIA_CHANGED is set when block device is opened for the first
time (please check idefloppy_open() for details) so I don't think it is
the right change. 'Flexible Disk Page' is only 32 bytes so we are better
off with leaving 'u8 flexible_disk_page[32]' in idefloppy_floppy_t and
doing things the old way.
Besides please do not intermix real changes like the above one with purely
cleanup ones like idefloppy_flexible_disk_page_t removal. This is bad from
maintainability point of view. If some patch causes problems it is easier
to narrow it down by heaving purely cleanup changes separated out + if we
would need to revert the real change we would have to make a separate patch
doing it instead of just reverting the guilty commit (given that we don't
want cleanup changes to be reverted as well).
> printk(KERN_INFO "%s: %dkB, %d/%d/%d CHS, %d kBps, "
>
Re: [PATCH 04/21] ide-floppy: cleanup and unify debugging macro calls
On Friday 11 January 2008, Borislav Petkov wrote:
> * some debug_log() calls were not using "ide-floppy: " prefix
>
> * a few used printk levels different than KERN_INFO (KERN_NOTICE
> and KERN_ERR, which is the default one if no level is given)
>
> There should be no functional change resulting from this patch.
Hmm, but there are functional changes as noted above, I removed this line
from the patch description.
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
with IDEFLOPPY_DEBUG_LOG set to 1:
drivers/ide/ide-floppy.c: In function ‘idefloppy_pc_intr’:
drivers/ide/ide-floppy.c:704: warning: too many arguments for format
drivers/ide/ide-floppy.c: In function ‘idefloppy_do_request’:
drivers/ide/ide-floppy.c:1126: error: ‘struct request’ has no member named
‘flags’
make[1]: *** [drivers/ide/ide-floppy.o] Error 1
make: *** [drivers/ide/ide-floppy.o] Error 2
which translate to:
[...]
> if ((stat & ERR_STAT) || test_bit(PC_DMA_ERROR, &pc->flags)) {
> /* Error detected */
> - debug_log(KERN_INFO "ide-floppy: %s: I/O error\n",
> - drive->name);
> + debug_log("I/O error\n", drive->name);
"%s: I/O error\n"
[...]
> - debug_log(KERN_INFO "dev: %s, flags: %lx, errors: %d\n",
> + debug_log("dev: %s, flags: %lx, errors: %d\n",
> rq->rq_disk ? rq->rq_disk->disk_name : "?",
> rq->flags, rq->errors);
This was broken before this patch by rq->flags -> rq->cmd_type change
(so your patch is not to blame for breaking IDEFLOPPY_DEBUG_LOG ;).
I fixed the above issues while merging the patch.
-
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: 40-wire cable detected when directly connected
Hi Please apply the attached patch and specify libata.force_cbl=80 as kernel boot parameter. If you load libata from initrd or after boot you need to pass 'force_cbl=80' as module parameter. How you do it depends on your distro. [ 41.116289] ata7: forcing 80c [ 41.116356] ata7.00: ATA-4: SanDisk SDCFX4-8192, HDX 4.20, max UDMA/66 [ 41.116420] ata7.00: 16007040 sectors, multi 0: LBA [ 41.116489] ata7.00: limited to UDMA/33 due to 40-wire cable [ 41.117219] ata7.00: configured for UDMA/33 Speed is still the same and hdparm -I /dev/sdc sill says [...] Capabilities: LBA, IORDY(may be)(cannot be disabled) Standby timer values: spec'd by Vendor R/W multiple sector transfer: Max = 4 Current = 0 DMA: mdma0 mdma1 mdma2 udma0 udma1 *udma2 udma3 udma4 Cycle time: min=120ns recommended=120ns PIO: pio0 pio1 pio2 pio3 pio4 Cycle time: no flow control=120ns IORDY flow control=120ns [...] Regards Tobias smime.p7s Description: S/MIME Cryptographic Signature
Updated shutdown in Debian Lenny/Sid
Hello! The issue mentioned on [0] happened on my system a few weeks ago. But now it seems that Debian Testing (Lenny) and Unstable (Sid) corrected this problem with sysvinit (2.86.ds1-47)[1]. Perhaps you can add the distro to your list of fixed distros with updated shutdown. Regards Stephan Windmüller [0] http://linux-ata.org/shutdown.html [1] http://packages.debian.org/lenny/sysvinit signature.asc Description: Digital signature
Re: PROBLEM REMAINS: [sata_nv ADMA breaks ATAPI] Crash on accessing DVD-RAM
Robert Hancock wrote: > There's this patch which was intended to fix it: > > http://lkml.org/lkml/2007/11/22/148 I applied this patch to 2.6.24-rc7. Now at boot time my DVD-RW is normaly detected as: sr0: scsi3-mmc drive: 48x/48x writer dvd-ram cd/rw xa/form2 cdda tray But I cannot mount it. All my attempts failed with ISOFS: Unable to identify CD-ROM format. With mem<=4098M or sata_nv.adma=0 it still mounts and works ok. - 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
